[#3830] Update request base calculated status

[gbp]

Relevant issue(s)

Fixes #3830

What does this do?

Update request base calculated status

Why was this needed?

Once a requester has asked for a internal review their request's base status shouldn't ever be waiting_classification as this limits the possible classification statuses.

Returning internal_review allows the requester to classify the request as I'm still waiting for the internal review after they receive an acknowledgement of their review.

Implementation notes

Screenshots

Notes to reviewer

[garethrees]

(For future PRs), it's really helpful to add before/after screenshots to demonstrate the before/after so that it's clear that we have reproduced the problem behaviour and that the commit fixes it, and that the reviewer understands what they're looking for when assessing the fix.

---

Once a requester has asked for a internal review their request's base status shouldn't ever be waiting_classification

This logic doesn't sound right. Waiting classification is where the user must update the state, which they could well be asked to do after an internal review. For example:

1. some correspondence has happened and the user is not happy
2. they request an internal review
3. the authority sends an auto-acknowledgement of their correspondence

At step 3 the request would need classification. Obviously the fix has rendered the correct option, but I'm not sure I understand why.

---

The original issue (#3830 (comment)) noted:

I think we could resolve this by switching it to use @info_request.described_state

This implementation obviously adds the option, but would be good for this PR to note why we didn't just switch to #described_state

---

This returns the "internal review" variation of the form if the request has ever had an internal review requested. I think that's okay? What happens if we get a chain like:

• normal outgoing
• internal review
• normal outgoing
• normal outgoing

Should it always be treated as internal review in those later messages?

[gbp]

Rebased without doing any of the changes required to move this forward.

[gbp]

Once a requester has asked for a internal review their request's base status shouldn't ever be waiting_classification

This logic doesn't sound right. Waiting classification is where the user must update the state, which they could well be asked to do after an internal review.

It's been awhile but I think what I was getting on here was the internal_review state is equivalent to waiting_classification once a review has been requested, but yes, I agree that sounds wrong.

Looking back on it now, it seems like I was thinking this because it was an easy way to get the correct options displayed on the describe state form. I probably was thinking short of adding new states (like internal_review_waiting_classification/internal_review_done) there is no real way to track if an internal review had been request or completed.

[gbp]

The original issue (#3830 (comment)) noted:

I think we could resolve this by switching it to use @info_request.described_state

This implementation obviously adds the option, but would be good for this PR to note why we didn't just switch to #described_state

I have looked at this method but using it causes other issues, for example the overdue and very overdue phases aren't returned any more. There is extra logic in calculate_status to handle these states. I'm happy that sticking with calculate_status is the easier option.

[gbp]

This returns the "internal review" variation of the form if the request has ever had an internal review requested. I think that's okay? ...

There is a way we can still have both variations of the form. I have pushed a fixup which reverts the changes to calculate_status, instead now using the internal_review_requested? conditional in the state calculator to show the internal_review option (but bumped to the top of the list to be more prominent), we already show this option on the main (non-internal review variation) form if the user has selected 'Update the status of this request' from the action popup. I think this is all that is needed to fixed this issue/PR.

There are really 4 variations; screenshots of each are below:

[garethrees]

Nice – seems to look good! 🙏