Skip to content
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

API for emailLogEntry #10455

Merged
merged 3 commits into from
Oct 2, 2024
Merged

API for emailLogEntry #10455

merged 3 commits into from
Oct 2, 2024

Conversation

taslangraham
Copy link
Contributor

For #10412

@taslangraham taslangraham changed the title I10412 main pkp/pkp-lib#10412 Sep 23, 2024
@taslangraham taslangraham changed the title pkp/pkp-lib#10412 API for emailLogEntry Sep 24, 2024
@taslangraham taslangraham marked this pull request as ready for review September 25, 2024 16:49

foreach ($illuminateRequest->query->keys() as $queryParam) {
if (!in_array($queryParam, $allowedQueryParams)) {
return response()->json([
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand where this is coming from but I don't think we should respond with an error of an invalid query parameter is included (this is the case elsewhere as well). For example, I'm using an XDEBUG_SESSION query parameter to debug API requests and this will fail because of that. I think it's okay to have the correct query parameters documented and silently ignore others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted. I'll update accordingly

protected function getMany(Request $illuminateRequest): JsonResponse
{
$allowedQueryParams = ['submissionId', 'eventType'];
$authorEmailLogEventTypes = [ SubmissionEmailLogEventType::EDITOR_NOTIFY_AUTHOR, SubmissionEmailLogEventType::COPYEDIT_NOTIFY_AUTHOR,SubmissionEmailLogEventType::PROOFREAD_NOTIFY_AUTHOR];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some spacing issues in the array.

api/v1/reviews/PKPReviewController.php Show resolved Hide resolved
@taslangraham taslangraham force-pushed the i10412-main branch 3 times, most recently from 69914ed to e9a1eaf Compare September 27, 2024 22:35
- rename endpoint to indicate that it is for authors
- implement additional validation checks
@taslangraham taslangraham merged commit 09388ce into pkp:main Oct 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants