Peer review: draft workflow, closing, invitation expirations #667

sgfost · 2023-08-18T21:57:20Z

Major changes

Force the peer review process to take place on unpublished releases by either creating a new copy draft release or converting an existing draft release into an under_review release
- (resolves comses/planning#72)
add closed to peer reviews so they can be marked as inactive by the submitter or editor
- (resolves comses/planning#32)

other bugfixes/refactors:

fix and refactor pr dashboard filtering
- (resolves comses/planning#172)
refresh expiration date when re-sending an invite
- (resolves comses/planning#141)
use the 'editing' editor when logging feedback actions instead of the editor stored on the invitation
- (resolves comses/planning#176)

remove draft/live flags in favor of a status field with under_review as a choice to indicate editable, private releases for peer reviews * missing tests and frontend work

redirect if there is an active review for a codebase, this shouldn't be necessary now additionally, set the "under review" release to automatically publish once a peer review is complete, if that codebase is marked live (releases have been published previously), this logic is somewhat arbitrary..

- allow a choice of converting a draft or making a new release when requesting from a draft - update peer review messaging/guidance to explain the new workflow - minor misc frontend fixes

sgfost · 2023-08-24T23:33:24Z

slightly underestimated the amt of consideration this needed. Tests are being worked on and will need to be fairly thoroughly tested on staging as well

squashme [no ci]

this led to the permissions being handled in the wrong place for certain requests - fix remaining test issues

sgfost · 2023-09-01T17:41:23Z

shouldn't provide an option for creating a new release when requesting from a draft -- only convert to 'under review'

last_published_on will never differ from first_published_at on a release unless it was unpublished and republished by admin. I believe we want to show last modified instead which indicates the last time anything in the release (metadata/contributors/files/etc.) was changed

keep platform and langs always but clear release notes and output url when we are creating a new draft - clean up create_release() and callers - fix false negative in request peer review test - fix draft/live flag -> status migration to avoid updating last_modified when reverting

when contributor name comes from a user rather than being explicitly set on the contributor, codebase.authors_list would ignore these - adjust wording on 'notify reviewers' button

…ivate alternatively we could just always publish once a model passes peer review

sgfost · 2023-09-20T22:52:48Z

this seems all be working correctly now, just need to decide on what to do with a release after peer review completes.

Current strategy is to automatically publish if the codebase has live == True (codebase has at least 1 other release that is published), otherwise set to unpublished which is neither a draft nor live and prevents editing files but remains private until published

alee · 2023-09-20T23:33:17Z

I think the strategy of keep it unpublished if unpublished, publish if there's already a live one is a reasonable one. It might be safest though to make all steps explicit, so we keep it unpublished until they explicitly publish it (e.g., the "congratulations, peer review passed" email might have a direct link to publish the model)

sgfost · 2023-09-21T00:53:41Z

Yeah good idea, that reminds me, I should add a link to edit the release in the email that gets sent to the submitter after changes are requested

"review_complete" release status keeps the release unpublished but does not allowing modifying files - added a quick link to the publish modal which is given in the email after a review completes and on the detail page if the release is publishable + editable - added link to edit release in the revisions requested email

resolves comses/planning#172

resolves comses/planning#176

analogous to closing on github serves two purposes: 1. de-cluttering the dashboard, particularly in the case of legacy reviews on public releases that stall when changes are requested and a new release has to be made (resolves comses/planning#32) 2. allowing an author/requester to close inactive reviews since we now only allow one active review per codebase open at one time also includes some small refactors/bugfixes (resolves comses/planning#172)

Codebase.author_list was skipping authors even if they had an associated User because of order of operations / precedence refactor logic in the Contributor class to make `Contributor.has_name` return a truthy value if there's an associated User

- use f strings instead of format for simple interpolation - adjust peer review success email, we really will get it done by Q2 2024... really - remove dead imports

sgfost · 2023-10-19T16:48:30Z

frontend/src/components/releaseEditor/ReviewModal.vue

@@ -13,6 +13,7 @@
    >
      <template #body>
        <div class="alert alert-danger mb-4" role="alert">
+          <!-- FIXME: is there a way to deduplicate this + reminders.jinja -->


easiest way would be mounting a vue component in place of the template. Feels silly doing so for static markup but may beat duplicating it

I suspected that might be the only way forward since we regularly embed frontend things into the backend but not vice versa easily.. I think it's not a blocker for this PR moving forward. I'm OK with having two sets of guidance, let's make an issue out of it maybe for a student dev to take on

- to remove duplication of content across ReviewModal and reminders.jinja could create a Vue component and mount it wherever reminders are needed but seems a bit much - fix test call site, should look into refactoring the fs module(s) at some point + add more test coverage

alee · 2023-10-19T20:14:46Z

django/library/models.py

@@ -1014,17 +1085,33 @@ class CodebaseRelease(index.Indexed, ClusterableModel):
    * git repository in /repository/<codebase_identifier>/
    """

+    class Status(models.TextChoices):


good work unifying liveness, draft, and peer review status here

alee · 2023-10-19T20:44:01Z

frontend/src/stores/releaseEditor.ts

@@ -90,7 +91,7 @@ export const useReleaseEditorStore = defineStore("releaseEditor", () => {
    isInitialized.value = false;
    await fetchCodebaseRelease(identifier, versionNumber);
    await fetchMediaFiles();
-    if (!release.value.live) {
+    if (release.value.canEditOriginals) {


feat: refresh expiration date when resending pr invites

162436e

resolves comses/planning#141

sgfost added the enhancement label Aug 18, 2023

sgfost added 3 commits August 23, 2023 15:59

feat(WIP): create draft-like release copies for models under review

0b405bd

remove draft/live flags in favor of a status field with under_review as a choice to indicate editable, private releases for peer reviews * missing tests and frontend work

feat: add msging/options on the client for the new pr flow

6cc2b4c

- allow a choice of converting a draft or making a new release when requesting from a draft - update peer review messaging/guidance to explain the new workflow - minor misc frontend fixes

sgfost added 2 commits August 24, 2023 16:35

style: run prettier

85c3a7e

squashme [no ci]

fix: resolve permissions relying on an actual 'live' field

a1d67ea

this led to the permissions being handled in the wrong place for certain requests - fix remaining test issues

sgfost added 2 commits September 6, 2023 18:04

fix: always convert draft->under review, reformat request modal

f803c69

test: add coverage for release creation + peer review requests

26e13ef

sgfost marked this pull request as ready for review September 7, 2023 21:56

sgfost added 5 commits September 19, 2023 14:37

fix: do not convert release to review draft after creating a new one

a8d4ddf

fix: citation text missing some author names

43cb41b

when contributor name comes from a user rather than being explicitly set on the contributor, codebase.authors_list would ignore these - adjust wording on 'notify reviewers' button

fix: give 'unpublished' status to release that passes pr but stays pr…

1106bad

…ivate alternatively we could just always publish once a model passes peer review

fix: wrong character

6760457

sgfost added 5 commits September 22, 2023 13:39

fix: simplify review dashboard filtering

6cc2f2f

resolves comses/planning#172

fix: use the actual editor for peer review accept/revise logs

57ae229

resolves comses/planning#176

style: run prettier

37985c7

sgfost changed the title ~~Peer review: draft workflow and invitation expirations~~ Peer review: draft workflow, closing, invitation expirations Sep 29, 2023

alee added 3 commits October 16, 2023 12:48

style: black

c231f24

refactor: minor hygiene

a3990ca

- use f strings instead of format for simple interpolation - adjust peer review success email, we really will get it done by Q2 2024... really - remove dead imports

sgfost commented Oct 19, 2023

View reviewed changes

alee force-pushed the peer-review-improvements-1 branch from bbfc9d3 to 424cea6 Compare October 19, 2023 16:51

alee force-pushed the peer-review-improvements-1 branch from 424cea6 to 2308e91 Compare October 19, 2023 20:08

alee approved these changes Oct 19, 2023

View reviewed changes

alee merged commit c32d2e1 into comses:main Oct 19, 2023
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Peer review: draft workflow, closing, invitation expirations #667

Peer review: draft workflow, closing, invitation expirations #667

sgfost commented Aug 18, 2023 •

edited

Loading

sgfost commented Aug 24, 2023 •

edited

Loading

sgfost commented Sep 1, 2023

sgfost commented Sep 20, 2023

alee commented Sep 20, 2023

sgfost commented Sep 21, 2023

sgfost Oct 19, 2023

alee Oct 19, 2023 •

edited

Loading

alee Oct 19, 2023

alee Oct 19, 2023

Peer review: draft workflow, closing, invitation expirations #667

Peer review: draft workflow, closing, invitation expirations #667

Conversation

sgfost commented Aug 18, 2023 • edited Loading

Major changes

other bugfixes/refactors:

sgfost commented Aug 24, 2023 • edited Loading

sgfost commented Sep 1, 2023

sgfost commented Sep 20, 2023

alee commented Sep 20, 2023

sgfost commented Sep 21, 2023

sgfost Oct 19, 2023

Choose a reason for hiding this comment

alee Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

alee Oct 19, 2023

Choose a reason for hiding this comment

alee Oct 19, 2023

Choose a reason for hiding this comment

sgfost commented Aug 18, 2023 •

edited

Loading

sgfost commented Aug 24, 2023 •

edited

Loading

alee Oct 19, 2023 •

edited

Loading