-
Notifications
You must be signed in to change notification settings - Fork 246
Added helper endpoint to generate image of the progress of a task #1754
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 | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,225 @@ | ||||||||||||||||||
| <?php | ||||||||||||||||||
|
|
||||||||||||||||||
| use DBA\Chunk; | ||||||||||||||||||
| use JetBrains\PhpStorm\NoReturn; | ||||||||||||||||||
| use Psr\Http\Message\ResponseInterface as Response; | ||||||||||||||||||
| use Psr\Http\Message\ServerRequestInterface as Request; | ||||||||||||||||||
| use DBA\Factory; | ||||||||||||||||||
| use DBA\OrderFilter; | ||||||||||||||||||
| use DBA\QueryFilter; | ||||||||||||||||||
| use DBA\Task; | ||||||||||||||||||
| use Middlewares\Utils\HttpErrorException; | ||||||||||||||||||
| use Slim\Exception\HttpNotFoundException; | ||||||||||||||||||
|
|
||||||||||||||||||
| require_once(dirname(__FILE__) . "/../common/AbstractHelperAPI.class.php"); | ||||||||||||||||||
|
|
||||||||||||||||||
| class GetTaskProgressImageHelperAPI extends AbstractHelperAPI { | ||||||||||||||||||
| public static function getBaseUri(): string { | ||||||||||||||||||
| return "/api/v2/helper/getTaskProgressImage"; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| public static function getAvailableMethods(): array { | ||||||||||||||||||
| return ['GET']; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| public function getRequiredPermissions(string $method): array { | ||||||||||||||||||
| return [Task::PERM_READ]; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * getfile is different because it returns actual binary data. | ||||||||||||||||||
| */ | ||||||||||||||||||
| public static function getResponse(): null { | ||||||||||||||||||
| return null; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| #[NoReturn] public function actionPost(array $data): object|array|null { | ||||||||||||||||||
| assert(False, "GetTaskProgressImage has no POST"); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Description of get params for swagger. | ||||||||||||||||||
| */ | ||||||||||||||||||
| public function getParamsSwagger(): array { | ||||||||||||||||||
| return [ | ||||||||||||||||||
| [ | ||||||||||||||||||
| "in" => "query", | ||||||||||||||||||
| "name" => "supertask", | ||||||||||||||||||
| "schema" => [ | ||||||||||||||||||
| "type" => "integer", | ||||||||||||||||||
| "format" => "int32" | ||||||||||||||||||
| ], | ||||||||||||||||||
| "required" => false, | ||||||||||||||||||
| "example" => 1, | ||||||||||||||||||
| "description" => "The ID of the supertask where you want to create the progress image of." | ||||||||||||||||||
| ], | ||||||||||||||||||
| [ | ||||||||||||||||||
| "in" => "query", | ||||||||||||||||||
| "name" => "task", | ||||||||||||||||||
| "schema" => [ | ||||||||||||||||||
| "type" => "integer", | ||||||||||||||||||
| "format" => "int32" | ||||||||||||||||||
| ], | ||||||||||||||||||
| "required" => false, | ||||||||||||||||||
| "example" => 1, | ||||||||||||||||||
| "description" => "The ID of the task where you want to create the progress image of." | ||||||||||||||||||
| ] | ||||||||||||||||||
| ]; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Endpoint to download files | ||||||||||||||||||
| * @param Request $request | ||||||||||||||||||
| * @param Response $response | ||||||||||||||||||
| * @return Response | ||||||||||||||||||
| * @throws HTException | ||||||||||||||||||
| * @throws HttpErrorException | ||||||||||||||||||
| * @throws HttpForbidden | ||||||||||||||||||
| */ | ||||||||||||||||||
| public function handleGet(Request $request, Response $response): Response { | ||||||||||||||||||
| $this->preCommon($request); | ||||||||||||||||||
| $task_id = $request->getQueryParams()['task']; | ||||||||||||||||||
| $supertask_id = $request->getQueryParams()['supertask']; | ||||||||||||||||||
|
Comment on lines
+82
to
+83
|
||||||||||||||||||
| $task_id = $request->getQueryParams()['task']; | |
| $supertask_id = $request->getQueryParams()['supertask']; | |
| $queryParams = $request->getQueryParams(); | |
| if (!isset($queryParams['task']) && !isset($queryParams['supertask'])) { | |
| throw new HttpError("No task or super task has been provided"); | |
| } | |
| $task_id = isset($queryParams['task']) ? $queryParams['task'] : null; | |
| $supertask_id = isset($queryParams['supertask']) ? $queryParams['supertask'] : null; |
Copilot
AI
Nov 12, 2025
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.
Inconsistent variable usage in conditional check. Line 122 checks isset($supertask_id) but line 96 uses else if ($supertask_id) which will be truthy even if $supertask_id is 0 or empty string. The logic should be consistent - either use isset() in both places or check for truthiness in both. Additionally, the variable may not be set if neither parameter is provided (which is now only caught by the final else on line 101).
Copilot
AI
Nov 12, 2025
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.
Duplicate code: Lines 129-131 and 135-137 perform identical database queries to fetch chunks. The query on lines 135-137 is redundant and should be removed. The $chunks variable from lines 129-131 should be reused.
| $qF = new QueryFilter(Chunk::TASK_ID, $tasks[$i]->getId(), "="); | |
| $chunks = Factory::getChunkFactory()->filter([Factory::FILTER => $qF]); |
Copilot
AI
Nov 12, 2025
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.
Potential undefined variable usage. If the $supertask_id path is taken (lines 96-100), the variable $task is never defined, but it's used in line 156 within the else block. This will cause an error when accessing a supertask but falling into the non-supertask rendering path. The conditional logic on line 122 should prevent this, but the code structure is fragile and could lead to bugs.
| else { | |
| else { | |
| if (!isset($task)) { | |
| throw new HttpError("Task is not defined for non-supertask rendering path."); | |
| } |
Copilot
AI
Nov 12, 2025
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.
Wrong QueryFilter field. This should use Chunk::TASK_ID instead of Task::TASK_ID since we're filtering chunks by task ID, not tasks. The Chunk model has a TASK_ID field that should be used here.
| $qF = new QueryFilter(Task::TASK_ID, $task->getId(), "="); | |
| $qF = new QueryFilter(Chunk::TASK_ID, $task->getId(), "="); |
Copilot
AI
Nov 12, 2025
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.
Missing resource cleanup. The image resource created by imagecreatetruecolor() on line 108 is never explicitly destroyed with imagedestroy($image). While PHP will clean this up eventually, it's best practice to explicitly free image resources after use, especially for endpoints that may be called frequently.
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.
Python-style boolean
Falseused in PHP assertion. PHP uses lowercasefalse. This will cause a fatal error if the code path is ever reached and assertions are enabled, asFalseis an undefined constant in PHP (unless defined elsewhere).