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

Feat/reviewqueue #451

Merged
merged 12 commits into from
Feb 25, 2025
Merged

Feat/reviewqueue #451

merged 12 commits into from
Feb 25, 2025

Conversation

PGijsbers
Copy link
Collaborator

@PGijsbers PGijsbers commented Feb 19, 2025

Change

Adds convenience functions for reviewers through a review router.
Reviewers can use its endpoints to either get a list of pending submission, or get a specific one.

In a follow up PR we will add conveniences for the user (getting review status for assets, let them read review notes)

How to Test

If your keycloak instance lacks a reviewer account, make sure to recreate your kc:

./scripts/down.sh
./scripts/clean.sh
./scripts/up.sh

Keycloak then has both the regular user, as well as a reviewer user (username: reviewer, password: password).
You can submit a new asset for review as a regular user, and check, approve or reject the submission as a reviewer, and use the new endpoints to get insight into the pending/completed reviews.

Checklist

  • Tests have been added or updated to reflect the changes, or their absence is explicitly explained.
  • Documentation has been added or updated to reflect the changes, or their absence is explicitly explained.
  • A self-review has been conducted checking:
    • No unintended changes have been committed.
    • The changes in isolation seem reasonable.
    • Anything that may be odd or unintuitive is provided with a GitHub comment explaining it (but consider if this should not be a code comment or in the documentation instead).
  • All CI checks pass before pinging a reviewer, or provide an explanation if they do not.

Related Issues

@@ -30,6 +30,7 @@ class Review(SQLModel, table=True): # type: ignore [call-arg]

reviewer_identifier: str = Field(
foreign_key="user.subject_identifier",
exclude=True,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indicates that the field will not be serialized to JSON, which is to say it will not be shown to end users.

@PGijsbers PGijsbers requested a review from Taniya-Das February 24, 2025 14:49
@PGijsbers PGijsbers marked this pull request as ready for review February 24, 2025 14:50
@PGijsbers PGijsbers added the enhancement New feature or request label Feb 24, 2025
Copy link
Collaborator

@Taniya-Das Taniya-Das left a comment

Choose a reason for hiding this comment

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

Looks good.
Does it make sense to add the review decision and comment in the submission list, so that reviewer can see the details of the completed reviews?

@PGijsbers
Copy link
Collaborator Author

Good idea! Turned it into a separate issue so we can follow up separately :)

@PGijsbers PGijsbers merged commit 925423d into develop Feb 25, 2025
1 check passed
@PGijsbers PGijsbers deleted the feat/reviewqueue branch February 25, 2025 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants