-
Notifications
You must be signed in to change notification settings - Fork 1
fix(tasks): исправлен тип параметра dealership_id в фильтре #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,10 +29,10 @@ public function __construct( | |
| public function index(Request $request): JsonResponse | ||
| { | ||
| $perPage = (int) $request->query('per_page', '15'); | ||
| $dealershipId = $request->query('dealership_id'); | ||
| $dealershipId = $request->query('dealership_id') !== null && $request->query('dealership_id') !== '' ? (int) $request->query('dealership_id') : null; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Casting to int still leaves $dealershipId as 0, but the later Prompt for AI agents |
||
| $status = $request->query('status'); | ||
| $date = $request->query('date'); | ||
| $userId = $request->query('user_id'); | ||
| $userId = $request->query('user_id') !== null && $request->query('user_id') !== '' ? (int) $request->query('user_id') : null; | ||
|
|
||
| $query = Shift::with(['user', 'dealership', 'replacement.replacingUser', 'replacement.replacedUser']); | ||
|
|
||
|
|
@@ -263,7 +263,7 @@ public function destroy(int $id): JsonResponse | |
| */ | ||
| public function current(Request $request): JsonResponse | ||
| { | ||
| $dealershipId = $request->query('dealership_id'); | ||
| $dealershipId = $request->query('dealership_id') !== null && $request->query('dealership_id') !== '' ? (int) $request->query('dealership_id') : null; | ||
| $currentShifts = $this->shiftService->getCurrentShifts($dealershipId); | ||
|
|
||
| return response()->json([ | ||
|
|
@@ -279,7 +279,7 @@ public function current(Request $request): JsonResponse | |
| */ | ||
| public function statistics(Request $request): JsonResponse | ||
| { | ||
| $dealershipId = $request->query('dealership_id'); | ||
| $dealershipId = $request->query('dealership_id') !== null && $request->query('dealership_id') !== '' ? (int) $request->query('dealership_id') : null; | ||
| $startDate = $request->query('start_date') | ||
| ? Carbon::parse($request->query('start_date')) | ||
| : Carbon::now()->subDays(7); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,11 +17,11 @@ public function index(Request $request) | |
| $perPage = (int) $request->query('per_page', '15'); | ||
|
|
||
| // Получаем все параметры фильтрации | ||
| $dealershipId = $request->query('dealership_id'); | ||
| $dealershipId = $request->query('dealership_id') !== null && $request->query('dealership_id') !== '' ? (int) $request->query('dealership_id') : null; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here too, the conversion to int leaves Prompt for AI agents |
||
| $taskType = $request->query('task_type'); | ||
| $isActive = $request->query('is_active'); | ||
| $tags = $request->query('tags'); | ||
| $creatorId = $request->query('creator_id'); | ||
| $creatorId = $request->query('creator_id') !== null && $request->query('creator_id') !== '' ? (int) $request->query('creator_id') : null; | ||
| $responseType = $request->query('response_type'); | ||
| $deadlineFrom = $request->query('deadline_from'); | ||
| $deadlineTo = $request->query('deadline_to'); | ||
|
|
@@ -384,7 +384,7 @@ public function destroy($id) | |
|
|
||
| public function postponed(Request $request) | ||
| { | ||
| $dealershipId = $request->query('dealership_id'); | ||
| $dealershipId = $request->query('dealership_id') !== null && $request->query('dealership_id') !== '' ? (int) $request->query('dealership_id') : null; | ||
|
|
||
| $query = Task::with(['creator', 'dealership', 'responses']) | ||
| ->where('postpone_count', '>', 0) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,126 @@ | ||||||
| <?php | ||||||
|
|
||||||
| /** | ||||||
| * Debug script to understand dealership_id filtering behavior | ||||||
| * Tests how query parameters are processed and filtered | ||||||
| */ | ||||||
|
|
||||||
| require __DIR__ . '/../vendor/autoload.php'; | ||||||
|
|
||||||
| $app = require_once __DIR__ . '/../bootstrap/app.php'; | ||||||
| $app->make(Illuminate\Contracts\Console\Kernel::class)->bootstrap(); | ||||||
|
|
||||||
| use App\Models\Task; | ||||||
| use App\Models\AutoDealership; | ||||||
|
|
||||||
| echo "=== Debug Dealership Filtering ===\n\n"; | ||||||
|
|
||||||
| // First, let's see what dealerships exist | ||||||
| echo "Available dealerships:\n"; | ||||||
| $dealerships = AutoDealership::all(['id', 'name']); | ||||||
| foreach ($dealerships as $d) { | ||||||
| echo " ID: {$d->id}, Name: {$d->name}\n"; | ||||||
| } | ||||||
| echo "\n"; | ||||||
|
|
||||||
| // Check tasks for each dealership | ||||||
| echo "Tasks per dealership:\n"; | ||||||
| foreach ($dealerships as $d) { | ||||||
| $count = Task::where('dealership_id', $d->id)->count(); | ||||||
| echo " Dealership {$d->id} ({$d->name}): {$count} tasks\n"; | ||||||
| } | ||||||
| echo "\n"; | ||||||
|
|
||||||
| // Now let's simulate what happens with different query parameter values | ||||||
| echo "=== Simulating query parameter processing ===\n\n"; | ||||||
|
|
||||||
| $testCases = [ | ||||||
| ['dealership_id' => '1'], | ||||||
| ['dealership_id' => '0'], | ||||||
| ['dealership_id' => null], | ||||||
| [], // no parameter | ||||||
| ]; | ||||||
|
|
||||||
| foreach ($testCases as $index => $params) { | ||||||
| echo "Test case " . ($index + 1) . ": "; | ||||||
|
|
||||||
| if (!isset($params['dealership_id'])) { | ||||||
| echo "No dealership_id parameter\n"; | ||||||
| $dealershipId = null; | ||||||
| } else { | ||||||
| echo "dealership_id = " . var_export($params['dealership_id'], true) . "\n"; | ||||||
|
|
||||||
| // Simulate the current code logic | ||||||
| $dealershipId = isset($params['dealership_id']) && $params['dealership_id'] !== null | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The guard should treat empty strings like nulls before casting; otherwise Prompt for AI agents |
||||||
| ? (int) $params['dealership_id'] | ||||||
| : null; | ||||||
| } | ||||||
|
|
||||||
| echo " After processing: dealershipId = " . var_export($dealershipId, true) . "\n"; | ||||||
| echo " Truthiness check: if (\$dealershipId) = " . ($dealershipId ? 'true' : 'false') . "\n"; | ||||||
|
|
||||||
| // Execute the query | ||||||
| $query = Task::query(); | ||||||
| if ($dealershipId) { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using Prompt for AI agents
Suggested change
|
||||||
| $query->where('dealership_id', $dealershipId); | ||||||
| echo " Filter applied: WHERE dealership_id = {$dealershipId}\n"; | ||||||
| } else { | ||||||
| echo " Filter NOT applied (showing all tasks)\n"; | ||||||
| } | ||||||
|
|
||||||
| $count = $query->count(); | ||||||
| echo " Result: {$count} tasks\n"; | ||||||
|
|
||||||
| // Show first few task IDs | ||||||
| if ($count > 0) { | ||||||
| $taskIds = $query->limit(5)->pluck('id')->toArray(); | ||||||
| echo " Sample task IDs: " . implode(', ', $taskIds) . "\n"; | ||||||
| } | ||||||
|
|
||||||
| echo "\n"; | ||||||
| } | ||||||
|
|
||||||
| // Now let's check what the actual HTTP request would look like | ||||||
| echo "=== HTTP Request Simulation ===\n\n"; | ||||||
|
|
||||||
| // Simulate Illuminate\Http\Request | ||||||
| use Illuminate\Http\Request; | ||||||
|
|
||||||
| $httpCases = [ | ||||||
| ['dealership_id=1'], | ||||||
| ['dealership_id=0'], | ||||||
| [''], // no query string | ||||||
| ]; | ||||||
|
|
||||||
| foreach ($httpCases as $index => $queryString) { | ||||||
| echo "HTTP case " . ($index + 1) . ": Query string = " . ($queryString ? "?{$queryString}" : "(empty)") . "\n"; | ||||||
|
|
||||||
| // Create a fake request | ||||||
| $request = Request::create('/api/v1/tasks' . ($queryString ? "?{$queryString}" : ''), 'GET'); | ||||||
|
|
||||||
| $dealershipId = $request->query('dealership_id') !== null | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Align this guard with the production fix by also checking for empty strings before casting, so empty parameters stay null. Prompt for AI agents |
||||||
| ? (int) $request->query('dealership_id') | ||||||
| : null; | ||||||
|
|
||||||
| echo " \$request->query('dealership_id') = " . var_export($request->query('dealership_id'), true) . "\n"; | ||||||
| echo " After !== null check and cast: " . var_export($dealershipId, true) . "\n"; | ||||||
| echo " Truthiness: if (\$dealershipId) = " . ($dealershipId ? 'true' : 'false') . "\n"; | ||||||
|
|
||||||
| $query = Task::query(); | ||||||
| if ($dealershipId) { | ||||||
| $query->where('dealership_id', $dealershipId); | ||||||
| echo " Filter WOULD be applied\n"; | ||||||
| } else { | ||||||
| echo " Filter WOULD NOT be applied\n"; | ||||||
| } | ||||||
|
|
||||||
| echo "\n"; | ||||||
| } | ||||||
|
|
||||||
| echo "=== Analysis ===\n"; | ||||||
| echo "If dealership_id=1 is not filtering correctly, possible causes:\n"; | ||||||
| echo "1. The query string is not being parsed correctly\n"; | ||||||
| echo "2. The database has no tasks with dealership_id=1\n"; | ||||||
| echo "3. There's a type mismatch in the WHERE clause\n"; | ||||||
| echo "4. The frontend is not sending the parameter correctly\n"; | ||||||
| echo "\nRun this script to see actual behavior!\n"; | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,132 @@ | ||||||
| <?php | ||||||
|
|
||||||
| /** | ||||||
| * Test script to verify dealership_id filtering with value 0 | ||||||
| * | ||||||
| * This script tests the fix for the issue where dealership_id=0 was | ||||||
| * incorrectly converted to null due to PHP's falsy behavior. | ||||||
| */ | ||||||
|
|
||||||
| echo "Testing dealership_id filtering logic...\n\n"; | ||||||
|
|
||||||
| // Simulate the OLD BUGGY behavior | ||||||
| function oldBuggyLogic($queryValue) { | ||||||
| return $queryValue ? (int) $queryValue : null; | ||||||
| } | ||||||
|
|
||||||
| // Simulate the INTERMEDIATE behavior (from previous commit) | ||||||
| function intermediateLogic($queryValue) { | ||||||
| return $queryValue !== null ? (int) $queryValue : null; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blank query parameters come through as empty strings, so this condition passes and Prompt for AI agents
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| // Simulate the NEW FULLY FIXED behavior | ||||||
| function newFixedLogic($queryValue) { | ||||||
| return ($queryValue !== null && $queryValue !== '') ? (int) $queryValue : null; | ||||||
| } | ||||||
|
|
||||||
| // Test cases | ||||||
| $testCases = [ | ||||||
| ['value' => '0', 'description' => 'String "0" (first dealership)'], | ||||||
| ['value' => 0, 'description' => 'Integer 0'], | ||||||
| ['value' => '1', 'description' => 'String "1"'], | ||||||
| ['value' => 1, 'description' => 'Integer 1'], | ||||||
| ['value' => null, 'description' => 'null (no filter)'], | ||||||
| ['value' => '', 'description' => 'Empty string'], | ||||||
| ]; | ||||||
|
|
||||||
| echo "=== OLD BUGGY LOGIC ===\n"; | ||||||
| foreach ($testCases as $test) { | ||||||
| $result = oldBuggyLogic($test['value']); | ||||||
| $displayValue = var_export($test['value'], true); | ||||||
| $displayResult = var_export($result, true); | ||||||
| echo sprintf("Input: %-20s => Result: %-10s\n", $displayValue, $displayResult); | ||||||
| } | ||||||
|
|
||||||
| echo "\n=== INTERMEDIATE LOGIC (still buggy with empty strings) ===\n"; | ||||||
| foreach ($testCases as $test) { | ||||||
| $result = intermediateLogic($test['value']); | ||||||
| $displayValue = var_export($test['value'], true); | ||||||
| $displayResult = var_export($result, true); | ||||||
| $warning = ($test['value'] === '' && $result === 0) ? ' ⚠️ ISSUE: Empty string -> 0' : ''; | ||||||
| echo sprintf("Input: %-20s => Result: %-10s%s\n", $displayValue, $displayResult, $warning); | ||||||
| } | ||||||
|
|
||||||
| echo "\n=== NEW FULLY FIXED LOGIC ===\n"; | ||||||
| foreach ($testCases as $test) { | ||||||
| $result = newFixedLogic($test['value']); | ||||||
| $displayValue = var_export($test['value'], true); | ||||||
| $displayResult = var_export($result, true); | ||||||
| echo sprintf("Input: %-20s => Result: %-10s\n", $displayValue, $displayResult); | ||||||
| } | ||||||
|
|
||||||
| echo "\n=== ANALYSIS ===\n"; | ||||||
| echo "FIRST BUG: '0' (string) or 0 (integer) evaluated as falsy in PHP,\n"; | ||||||
| echo "so the ternary operator `condition ? value : null` returned null instead of 0.\n\n"; | ||||||
| echo "SECOND BUG (discovered after user feedback): Empty string '' is cast to 0,\n"; | ||||||
| echo "then the 'if (0)' check fails, causing filter not to be applied.\n\n"; | ||||||
| echo "The complete fix checks both `!== null` AND `!== ''`, which correctly handles:\n"; | ||||||
| echo " - 0 (valid dealership ID) => returns 0 ✓\n"; | ||||||
| echo " - 1 (valid dealership ID) => returns 1 ✓\n"; | ||||||
| echo " - '' (empty string) => returns null (no filter) ✓\n"; | ||||||
| echo " - null (no filter applied) => returns null ✓\n\n"; | ||||||
|
|
||||||
| // Test query parameter simulation | ||||||
| echo "=== SIMULATING REQUEST QUERY ===\n"; | ||||||
|
|
||||||
| class FakeRequest { | ||||||
| private $params; | ||||||
|
|
||||||
| public function __construct($params) { | ||||||
| $this->params = $params; | ||||||
| } | ||||||
|
|
||||||
| public function query($key, $default = null) { | ||||||
| return $this->params[$key] ?? $default; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Test with dealership_id=0 | ||||||
| $request1 = new FakeRequest(['dealership_id' => '0']); | ||||||
| $oldResult1 = $request1->query('dealership_id') ? (int) $request1->query('dealership_id') : null; | ||||||
| $intermediateResult1 = $request1->query('dealership_id') !== null ? (int) $request1->query('dealership_id') : null; | ||||||
| $newResult1 = ($request1->query('dealership_id') !== null && $request1->query('dealership_id') !== '') ? (int) $request1->query('dealership_id') : null; | ||||||
|
|
||||||
| echo "Query param: dealership_id=0\n"; | ||||||
| echo " Old logic: " . var_export($oldResult1, true) . " (BUG: should be 0!)\n"; | ||||||
| echo " Intermediate: " . var_export($intermediateResult1, true) . " (CORRECT!)\n"; | ||||||
| echo " New logic: " . var_export($newResult1, true) . " (CORRECT!)\n\n"; | ||||||
|
|
||||||
| // Test with dealership_id=1 | ||||||
| $request2 = new FakeRequest(['dealership_id' => '1']); | ||||||
| $oldResult2 = $request2->query('dealership_id') ? (int) $request2->query('dealership_id') : null; | ||||||
| $intermediateResult2 = $request2->query('dealership_id') !== null ? (int) $request2->query('dealership_id') : null; | ||||||
| $newResult2 = ($request2->query('dealership_id') !== null && $request2->query('dealership_id') !== '') ? (int) $request2->query('dealership_id') : null; | ||||||
|
|
||||||
| echo "Query param: dealership_id=1\n"; | ||||||
| echo " Old logic: " . var_export($oldResult2, true) . " (correct)\n"; | ||||||
| echo " Intermediate: " . var_export($intermediateResult2, true) . " (correct)\n"; | ||||||
| echo " New logic: " . var_export($newResult2, true) . " (correct)\n\n"; | ||||||
|
|
||||||
| // Test with dealership_id= (empty string) | ||||||
| $request3 = new FakeRequest(['dealership_id' => '']); | ||||||
| $oldResult3 = $request3->query('dealership_id') ? (int) $request3->query('dealership_id') : null; | ||||||
| $intermediateResult3 = $request3->query('dealership_id') !== null ? (int) $request3->query('dealership_id') : null; | ||||||
| $newResult3 = ($request3->query('dealership_id') !== null && $request3->query('dealership_id') !== '') ? (int) $request3->query('dealership_id') : null; | ||||||
|
|
||||||
| echo "Query param: dealership_id= (empty string)\n"; | ||||||
| echo " Old logic: " . var_export($oldResult3, true) . " (returns null, which is correct)\n"; | ||||||
| echo " Intermediate: " . var_export($intermediateResult3, true) . " (BUG: '' cast to 0!)\n"; | ||||||
| echo " New logic: " . var_export($newResult3, true) . " (CORRECT: returns null)\n\n"; | ||||||
|
|
||||||
| // Test with no dealership_id | ||||||
| $request4 = new FakeRequest([]); | ||||||
| $oldResult4 = $request4->query('dealership_id') ? (int) $request4->query('dealership_id') : null; | ||||||
| $intermediateResult4 = $request4->query('dealership_id') !== null ? (int) $request4->query('dealership_id') : null; | ||||||
| $newResult4 = ($request4->query('dealership_id') !== null && $request4->query('dealership_id') !== '') ? (int) $request4->query('dealership_id') : null; | ||||||
|
|
||||||
| echo "Query param: (no dealership_id)\n"; | ||||||
| echo " Old logic: " . var_export($oldResult4, true) . " (correct)\n"; | ||||||
| echo " Intermediate: " . var_export($intermediateResult4, true) . " (correct)\n"; | ||||||
| echo " New logic: " . var_export($newResult4, true) . " (correct)\n\n"; | ||||||
|
|
||||||
| echo "✓ The new logic correctly handles all edge cases!\n"; | ||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Casting the dealership query parameter to int leaves
$dealershipIdat0fordealership_id=0, but downstream we still checkif ($dealershipId), so the filter is dropped and dealership 0 queries continue to return unfiltered results. We need to keep the value nullable but update the conditional logic (e.g. check!== null) so dealership 0 is respected.Prompt for AI agents