pkp/pkp-lib#11885 Implemented edit, delete, and search endpoints for task templates and fixed stageId on add #11968

Hafsa-Naeem · 2025-10-23T09:59:06Z

Vitaliy-1

Thanks, I left a couple of comments. Can you also do a rebase, this would also require additional modifications because of a recent merge.

Vitaliy-1 · 2025-10-23T13:16:03Z

api/v1/editTaskTemplates/PKPEditTaskTemplateController.php

+        $id = (int) $request->route('templateId');
+
+        $template = Template::query()
+            ->where('context_id', $contextId)


It's better to use scope here as we avoid using column names besides the Model itself

Vitaliy-1 · 2025-10-23T13:17:48Z

api/v1/editTaskTemplates/PKPEditTaskTemplateController.php

+
+        $template = Template::query()
+            ->where('context_id', $contextId)
+            ->findOrFail($id);


We would need the explicit response error code (404) and message

Vitaliy-1 · 2025-10-23T13:19:51Z

api/v1/editTaskTemplates/PKPEditTaskTemplateController.php

+            if (array_key_exists('stageId', $data)) $updates['stage_id'] = (int) $data['stageId'];
+            if (array_key_exists('title', $data)) $updates['title'] = $data['title'];
+            if (array_key_exists('include', $data)) $updates['include'] = (bool) $data['include'];
+            if (array_key_exists('emailTemplateKey', $data)) $updates['email_template_key'] = $data['emailTemplateKey'];


Just removed the email template key association. The current idea is to hard code the message into the template using description attribute

Vitaliy-1 · 2025-10-23T13:21:03Z

api/v1/editTaskTemplates/PKPEditTaskTemplateController.php

+
+        DB::transaction(function () use ($template, $data) {
+            $updates = [];
+            if (array_key_exists('stageId', $data)) $updates['stage_id'] = (int) $data['stageId'];


I think we don't need these checks and go directly to the update

Vitaliy-1 · 2025-10-23T13:24:29Z

api/v1/editTaskTemplates/PKPEditTaskTemplateController.php

+            if ($updates) {
+                $template->update($updates);
+            }
+            if (array_key_exists('userGroupIds', $data)) {


Maybe something like the following would be better:

$userGroupIds = Arr::pull($data, 'userGroupIds'); $template->update($data); $template->userGroups()->sync($userGroupIds);

Vitaliy-1 · 2025-10-23T13:26:05Z

api/v1/editTaskTemplates/formRequests/AddTaskTemplate.php

+        $stages = Application::getApplicationStages();
+        $stageIds = array_values(array_unique(array_filter(
+            array_map('intval', array_merge(array_keys((array)$stages), array_values((array)$stages))),
+            fn ($id) => $id > 0


probably we don't need this check

Vitaliy-1 · 2025-10-23T13:27:20Z

api/v1/editTaskTemplates/formRequests/AddTaskTemplate.php

            'title' => ['required', 'string', 'max:255'],
            'include' => ['boolean'],
            'emailTemplateKey' => ['sometimes', 'nullable', 'string', 'max:255', Rule::in($emailKeys)],
+            'type' => ['required','integer','in:1,2'],


I think we can re-use our EditorialTaskType enum here instead of plain numbers

Vitaliy-1 · 2025-10-23T13:30:28Z

api/v1/editTaskTemplates/formRequests/UpdateTaskTemplate.php

+        $stages = Application::getApplicationStages();
+        $stageIds = array_values(array_unique(array_filter(
+            array_map('intval', array_merge(array_keys((array)$stages), array_values((array)$stages))),
+            fn ($id) => $id > 0


Probably this check isn't needed

Vitaliy-1 · 2025-10-23T13:30:56Z

api/v1/editTaskTemplates/formRequests/UpdateTaskTemplate.php

+        )));
+
+        // Context-scoped email keys
+        $emailKeys = \APP\facades\Repo::emailTemplate()


This can be removed as we don't rely on email templates anymore

Vitaliy-1 · 2025-10-23T13:32:29Z

classes/editorialTask/Template.php


    public $timestamps = true;

+    public const TYPE_DISCUSSION = 1;


We can just re-use EdirotialTaskType enum

Vitaliy-1

Thanks! I left a couple of comments

Vitaliy-1 · 2025-10-28T14:07:12Z

api/v1/editTaskTemplates/PKPEditTaskTemplateController.php

+
+        DB::transaction(function () use ($template, $data) {
+            $userGroupIds = Arr::pull($data, 'userGroupIds');
+            $updates = collect($data)


Not anything critical but this should already be done during validation

Vitaliy-1 · 2025-10-28T14:11:13Z

api/v1/editTaskTemplates/formRequests/UpdateTaskTemplate.php

+use PKP\editorialTask\enums\EditorialTaskDueInterval;
+use PKP\editorialTask\enums\EditorialTaskType;
+
+class UpdateTaskTemplate extends FormRequest


I think this class and AddTaskTemplate could have a common parent to avoid code duplication. Or AddTaskTemplate could extend this class. Or have a common trait...

Vitaliy-1 · 2025-10-28T14:16:10Z

api/v1/editTaskTemplates/formRequests/UpdateTaskTemplate.php

+
+class UpdateTaskTemplate extends FormRequest
+{
+    public function authorize(): bool


This won't work as we haven't yet integrated/adopted Laravel auth policies (it uses Laravel gates and policies according to the documentation: https://laravel.com/docs/12.x/validation#authorizing-form-requests)

Vitaliy-1 · 2025-10-28T14:37:37Z

classes/editorialTask/Template.php

+        }
+
+        $tokens = preg_split('/\s+/', $phrase) ?: [];
+        $tokens = array_values(array_filter($tokens, fn ($t) => $t !== ''));


We also should do a special treatment for the task or discussion keyword within search phrase. E.g., the search phrase

task ethics

The code should perform the search by the keyword ethics within the tasks only.

Vitaliy-1 · 2025-10-28T14:38:35Z

classes/editorialTask/Template.php

+            return $query;
+        }
+
+        $tokens = preg_split('/\s+/', $phrase) ?: [];


Not sure but maybe we already got a helper to break the phrase onto tokens somewhere in our codebase.

@Vitaliy-1 I checked the PKP codebase for existing helpers around tokenizing search phrases, but didn't find a reusable helper for this. Other search-related code appears to split phrases inline in a similar way.
For now I've kept the simple preg_split('/\s+/', ...) implementation here to stay consistent. If we later introduce a shared helper for free-text search tokens, I can switch this scope to use it.

Vitaliy-1 · 2025-10-29T12:21:01Z

@bozana, could you take a look at the search implementation for task templates? I'm not much familiar with our patterns and best practices, e.g., breaking the string into tokens/keywords. Also, should we use vocabulary for this? The idea is to search matches in the title and description/headnote. Ideally grouping and showing matches in the title first.

bozana

Hi @Hafsa-Naeem and @Vitaliy-1,
I went through the changes and left some comments.
Thanks!

api/v1/editTaskTemplates/formRequests/AddTaskTemplate.php

api/v1/editTaskTemplates/formRequests/UpdateTaskTemplate.php

api/v1/editTaskTemplates/resources/TaskTemplateResource.php

api/v1/editTaskTemplates/PKPEditTaskTemplateController.php

classes/editorialTask/Template.php

bozana · 2025-11-04T10:11:48Z

classes/editorialTask/Template.php

+                $outer->where(function (Builder $q) use ($like, $settingsTable, $pk, $selfTable) {
+                    $q->whereRaw('LOWER(title) LIKE ?', [$like])
+                    ->orWhereRaw('LOWER(description) LIKE ?', [$like])
+                    ->orWhereExists(function ($sub) use ($like, $settingsTable, $pk, $selfTable) {


Builder $sub -- type Builder?

bozana · 2025-11-05T07:17:39Z

Hi @Vitaliy-1, oh I first now see your message above.
I think we have never used vocabularies, so I think this is not needed.
Regarding "The idea is to search matches in the title and description/headnote. Ideally grouping and showing matches in the title first.": hmmm... I am not sure how to implement ranking, would need to think about it...

api/v1/editTaskTemplates/formRequests/TaskTemplateRequestTrait.php

classes/editorialTask/Template.php

Vitaliy-1

One small comment but not a showstopper

Vitaliy-1 · 2025-11-17T16:56:35Z

api/v1/editTaskTemplates/formRequests/TaskTemplateRequestTrait.php

+    protected function getStageIds(): array
+    {
+        $stages = Application::getApplicationStages();
+        return array_map('intval', array_values($stages));


I think that there already was a comment regarding this, do we need array_values and array_map here. I mean Application::getApplicationStages() already returns non-associative array

@Vitaliy-1 Yes! I'll update this accordingly.

…plates and fixed stageId on add

…ding in template search

Hafsa-Naeem force-pushed the i11885-main-fix branch from 8b2614a to ae0f9f1 Compare October 23, 2025 10:02

Hafsa-Naeem mentioned this pull request Oct 23, 2025

Implement Edit, Delete and Search endpoints #11885

Open

Vitaliy-1 requested changes Oct 23, 2025

View reviewed changes

Hafsa-Naeem force-pushed the i11885-main-fix branch 3 times, most recently from 6b7fc8b to 754ce24 Compare October 28, 2025 09:21

Vitaliy-1 requested changes Oct 28, 2025

View reviewed changes

Vitaliy-1 requested a review from bozana October 29, 2025 12:13

bozana reviewed Nov 4, 2025

View reviewed changes

Hafsa-Naeem force-pushed the i11885-main-fix branch 2 times, most recently from edc8132 to bc76e6f Compare November 10, 2025 16:10

bozana reviewed Nov 12, 2025

View reviewed changes

api/v1/editTaskTemplates/formRequests/TaskTemplateRequestTrait.php Outdated Show resolved Hide resolved

bozana reviewed Nov 12, 2025

View reviewed changes

classes/editorialTask/Template.php Outdated Show resolved Hide resolved

bozana reviewed Nov 12, 2025

View reviewed changes

classes/editorialTask/Template.php Outdated Show resolved Hide resolved

bozana reviewed Nov 12, 2025

View reviewed changes

classes/editorialTask/Template.php Show resolved Hide resolved

Hafsa-Naeem force-pushed the i11885-main-fix branch from bc76e6f to 3e43e9a Compare November 17, 2025 16:00

Vitaliy-1 approved these changes Nov 17, 2025

View reviewed changes

Hafsa-Naeem force-pushed the i11885-main-fix branch from 3e43e9a to 65de0c3 Compare November 18, 2025 15:18

Hafsa-Naeem added 5 commits November 20, 2025 00:40

pkp#11885 Implemented edit, delete, and search endpoints for task tem…

e01e218

…plates and fixed stageId on add

pkp#11885 Refactor edit task templates API and validation rules

1d3d708

pkp#11885 Refactor editTaskTemplate requests and search scope

6652bc6

Improve edit task template search, validation, and controller logic

059d50c

pkp#11885 Simplify stage casting logic and ensure consistent case-fol…

2ccae7e

…ding in template search

Hafsa-Naeem force-pushed the i11885-main-fix branch from 65de0c3 to 2ccae7e Compare November 19, 2025 19:41

pkp/pkp-lib#11885 Implemented edit, delete, and search endpoints for task templates and fixed stageId on add #11968

Are you sure you want to change the base?

pkp/pkp-lib#11885 Implemented edit, delete, and search endpoints for task templates and fixed stageId on add #11968

Conversation

Hafsa-Naeem commented Oct 23, 2025

Uh oh!

Vitaliy-1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vitaliy-1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vitaliy-1 commented Oct 29, 2025

Uh oh!

bozana left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bozana commented Nov 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Vitaliy-1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!