pkp/pkp-lib#8887 REST API for dashboard views #9469
Conversation
Vitaliy-1
force-pushed
the
i8887_views_counts
branch
2 times, most recently
from
b9cda09
to
ae0fbf2
Compare
| } | ||
|
|
||
| /** @copydoc DAO::get() */ | ||
| public function get(int $id): ?ReviewAssignment |
There was a problem hiding this comment.
Please add a second (optional) $submisisonId parameter; the SchemaDAO::get() function supports the concept of a parent ID, and this can be used to ensure that calling code verifies that the ID belongs to the expected submission. (I'll tag a couple of examples with a link to this comment in a sec.)
Likewise for Repository::exists.
| $errors = $this->schemaService->formatValidationErrors($validator->errors()); | ||
| } | ||
|
|
||
| Hook::call('Category::validate', [&$errors, $object, $props, $allowedLocales, $primaryLocale]); |
There was a problem hiding this comment.
Also, for new hooks, please use the ::run conventions (#8089). (Here and elsewhere)
There was a problem hiding this comment.
Sorry, (hopefully) one more request related to hooks :) Can you document the hooks that got removed in the issue, along with anything else that should go into the Release Notebook, and then drop a link to that comment into #9276 where we track these things?
There was a problem hiding this comment.
One more :) When this code gets linted, you should see the pre-commit hook come in to add hook self-documentation in the function self-docs for the new hooks. If that doesn't happen, can you check whether your pre-commit hooks are running as expected?
| * Update the status of the review round an assignment is attached to. This | ||
| * should be fired whenever a reviewer assignment is modified. | ||
| */ | ||
| protected function updateReviewRoundStatus(ReviewAssignment $reviewAssignment): bool |
There was a problem hiding this comment.
I suspect functions like this would be better placed in the DAO, but since it's protected, no biggie if we decide to rearrange things later.
| /** | ||
| * Filter by review round ids | ||
| */ | ||
| public function filterByReviewRoundIds(?array $reviewRoundIds): self |
There was a problem hiding this comment.
Better to type-hint a static return type, rather than self, I think: https://wiki.php.net/rfc/static_return_type
| @@ -0,0 +1,298 @@ | |||
| <?php | |||
| /** | |||
| * @file classes/submission/reviewAssignment/Collector.php | |||
There was a problem hiding this comment.
This is very nice and clean and demonstrates very well the latest features we want to add throughout the codebase -- thanks!
There was a problem hiding this comment.
I just made a few comments, @Vitaliy-1; it's excellent to start to see this stuff coming into the newer coding conventions, and will lead to many downstream improvements!
Vitaliy-1
force-pushed
the
i8887_views_counts
branch
2 times, most recently
from
80367df
to
83b6b67
Compare
Vitaliy-1
force-pushed
the
i8887_views_counts
branch
from
83b6b67
to
bb532c7
Compare
No description provided.