Besides #717 I've discovered more issues with the discontinue functionality.
First, both the ReviewAction and ReviewDiscontinueView make the same logic mistake when choosing if a review can be discontinued. They check review.continuation. However, that's a non-nullable field which defaults to Continuations.GO.
Unintentionally both the action and view decide that a review can not be discontinued based on seeing review.continuation == Continuations.GO, but really they should first check if the review / proposal has been decided on at all before drawing conclusions from the continuation.
Second, selecting discontinuation as a conclusion explicitly in ReviewCloseView does not do anything or call the discontinue function. I don't believe discontinue was intended to be an option here. It just got added by default as a choice. Closing the review as discontinued from this page is completely undefined and could cause a mess.
Besides #717 I've discovered more issues with the discontinue functionality.
First, both the
ReviewAction
andReviewDiscontinueView
make the same logic mistake when choosing if a review can be discontinued. They checkreview.continuation
. However, that's a non-nullable field which defaults toContinuations.GO
.Unintentionally both the action and view decide that a review can not be discontinued based on seeing
review.continuation == Continuations.GO
, but really they should first check if the review / proposal has been decided on at all before drawing conclusions from the continuation.Second, selecting discontinuation as a conclusion explicitly in
ReviewCloseView
does not do anything or call the discontinue function. I don't believe discontinue was intended to be an option here. It just got added by default as a choice. Closing the review as discontinued from this page is completely undefined and could cause a mess.