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

Hotfix/to conclude review api view qs bug #629

Merged
merged 6 commits into from
Mar 21, 2024

Conversation

EdoStorm96
Copy link
Contributor

Fixes #628. The is_committee_review check was not properly implemented yet, which led to some stray supervisor reviews, appearing in certain API views. See the issue for more info. This should fix the problems.

@EdoStorm96 EdoStorm96 requested review from tymees and miggol March 21, 2024 10:30
@EdoStorm96 EdoStorm96 linked an issue Mar 21, 2024 that may be closed by this pull request
7 tasks
Copy link
Member

@tymees tymees left a comment

Choose a reason for hiding this comment

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

Changes look good. One small comment, but that is not an actual issue AFAIK.

However, the Request Changes is related to the branch. Please go through the acceptation branch first. You do not have to do an acceptation deploy (alhtough, you may if you want), this is purely a git-thing.

(We've had... Issues with backmerging master into acc and then into dev).

Copy link
Member

Choose a reason for hiding this comment

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

While not an issue, OpenSupervisorDecisionApiView's QS is still written with the old behaviour in mind. In this specific case, it works out. Still, probably a refactor candidate at some point.

Again, no fix is needed for this hotfix

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah, good spot

@miggol miggol changed the base branch from master to acceptation March 21, 2024 10:53
@miggol miggol merged commit b2d6ecf into acceptation Mar 21, 2024
4 checks passed
miggol added a commit that referenced this pull request Mar 21, 2024
* fix: add is_committee_review check to ToConcludeReviewApiView qs

* fix: add is_committee_review check to other API views

* formatting

---------

Co-authored-by: Edo Storm <[email protected]>
Co-authored-by: Ty Mees <[email protected]>
miggol added a commit that referenced this pull request Mar 27, 2024
* Hotfix/to conclude review api view qs bug (#629)

* fix: add is_committee_review check to ToConcludeReviewApiView qs

* fix: add is_committee_review check to other API views

* formatting

---------

Co-authored-by: Ty Mees <[email protected]>

* fix: apply is_committee_review to filter in get_committee_decisions()

* formatting

* fix: Save reviews after closure in update_go

* fix: hacky changes to working of end_date in qs

* formatting

* fix: remove explicit protocol prefix from file links (#637)

* fix: remove explicit protocol prefix from file links

Also adding quotations around the URL

* feat: Add protocol prefix to BASE_URL for consistency

---------

Co-authored-by: Edo Storm <[email protected]>
Co-authored-by: Ty Mees <[email protected]>
Co-authored-by: Edo Storm <[email protected]>
miggol added a commit that referenced this pull request Apr 3, 2024
* fix: correct 'start of proposal' redirect for pre_approved

* fix: improve buttons on proposal_confirmation.html

* fix: diff bugfix

* fix: buttons on task and session delete

* fix: formatting

* Revert 'confirmation sent' text on button back to past tense

* Hotfix/to conclude review api view qs bug (#629)

* fix: add is_committee_review check to ToConcludeReviewApiView qs

* fix: add is_committee_review check to other API views

* formatting

---------

Co-authored-by: Ty Mees <[email protected]>

* feat: is_final_decision method on Decision model

* feat: Warning when submitting a final decision

* fix: A capital letter

* fix: Show decide review action when already decided

* feat: translations

* fix: Some fuzzy or missing translations

* feat: Turn final deicision warning into alert box

* fix: Same translation somehow needed fixing again...

* feat: Alternate action description for feedback editing

---------

Co-authored-by: Edo Storm <[email protected]>
Co-authored-by: Edo Storm <[email protected]>
Co-authored-by: Ty Mees <[email protected]>
@EdoStorm96 EdoStorm96 deleted the hotfix/ToConcludeReviewApiView-qs-bug branch April 4, 2024 08:50
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.

"Applications waiting for conclusion" shows supervisor reviews
3 participants