search

opensafely-core / airlock

Other
0 stars 0 forks source link

Add a Returned state for release requests #383

Closed rebkwok closed 1 month ago

rebkwok commented 1 month ago

Fixes #326. Currently there's no way to return a request from the UI, buttons to do that are left for a later PR (#388).

After a request is submitted, at least 2 reviewers must review each file. At that point, it can be approved/released if all the reviews are approvals. Otherwise, it can be returned. (Note: currently a request will all approved files can also be returned - tbc if we want to allow that.) Once returned, the request moves into RETURNED state. The researcher can update files (or not) and re-submit it. This allows for a researcher to re-submit without making any changes. The assumption is that they could have answered questions to provide clarification, but not feel that the file required updating.

Any updated files are actually new RequestFile objects, so have no reviews and automatically require review when the request is re-submitted.

Any unchanged files that are approved do not need re-review, and their review status remains unchanged. Any unchanged files that were rejected have their status set to UNDECIDED for that reviewer. This is done by Airlock at the point that the request is re-submitted.

rebkwok commented 1 month ago

We now have methods to reset a file review (i.e. reviewer changes their mind after either approving or rejecting) and to mark a file as undecided (done by Airlock when a request is resubmitted, for files that were rejected in the previous round). I'm not sure "undecided" is a good name - it seemed fine at the time, but I wrote this PR before the reset-review was done, but I'm not sure what's a better one.

rebkwok commented 1 month ago

I did initially consider a different state for approving, but (a) Peter said that was not part of this ticket and (b) it means that when you re-submit, there's no way to tell that a reviewer has made a new decision. I.e. if they mark a file as "CLARIFY" (or whatever), and it stays as "CLARIFY" when resubmitted, and "CLARIFY" is a good enough decision for returning, there's no way for a reviewer to say that they still need clarification. I think it would still require the approval status to be reset in some way on re-submitting so that the next round requires an explicit decision on all non-approved files.

bloodearnest commented 1 month ago

I did initially consider a different state for approving, but (a) Peter said that was not part of this ticket and (b) it means that when you re-submit, there's no way to tell that a reviewer has made a new decision. I.e. if they mark a file as "CLARIFY" (or whatever), and it stays as "CLARIFY" when resubmitted, and "CLARIFY" is a good enough decision for returning, there's no way for a reviewer to say that they still need clarification. I think it would still require the approval status to be reset in some way on re-submitting so that the next round requires an explicit decision on all non-approved files.

Good point, we will a status reset of some sort either way, and deferring until we implement the new vote state makes sense to me.