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

Share common definitions in Review and ReviewCreate #462

Merged
merged 2 commits into from
Feb 28, 2025

Conversation

PGijsbers
Copy link
Collaborator

@PGijsbers PGijsbers commented Feb 27, 2025

Change

Refactor class hierarchy to allow Review and ReviewCreate share common definitions, s.t. we only need to define examples or limits once.
Also limit the maximum length of review to LONG (1800 chars).
I think that should be enough for most asset reviews, and it's easier to expand the limit than to shrink it.

How to Test

Changes are under unit test. Affected endpoints is RESOURCE/review/v1/ if you want to manually test or inspect updated documentation.

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

@PGijsbers PGijsbers added the maint Maintenance work, e.g., refactoring, testing, automation. label Feb 27, 2025
@PGijsbers PGijsbers requested a review from Taniya-Das February 27, 2025 10:12
@Taniya-Das
Copy link
Collaborator

I can see the comment field in `/submissions/v1/{identifier}, but no way for the user to submit a comment when sending an asset for review.

@PGijsbers
Copy link
Collaborator Author

Ah, I accidentally left the comment for submissions in there.. this was meant to be for the reviewer comment only. I'll update the PR to simply also include the submission comment. Thanks!

@PGijsbers PGijsbers force-pushed the enh/share-review-fields branch from 3ba0711 to 4dd607b Compare February 28, 2025 09:58
@PGijsbers
Copy link
Collaborator Author

I updated the PR to allow users to also submit a comment (in the body). Might move the identifier also to the body, but that will be a next PR -- I want to move the submission endpoint for all entities to the review router instead of making them entitiy-specific.

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 to me!

@PGijsbers PGijsbers merged commit 6aa9a2e into develop Feb 28, 2025
1 check passed
@PGijsbers PGijsbers deleted the enh/share-review-fields branch February 28, 2025 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maint Maintenance work, e.g., refactoring, testing, automation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants