diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 7bb920109b..e3e799b6f0 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -758,6 +758,10 @@ def must_be_internal_or_external end end + def internal_review_requested? + outgoing_messages.where(what_doing: 'internal_review').any? + end + def is_external? external_url.nil? ? false : true end diff --git a/app/models/info_request/state/calculator.rb b/app/models/info_request/state/calculator.rb index b4109f120d..cc1bdc92a9 100644 --- a/app/models/info_request/state/calculator.rb +++ b/app/models/info_request/state/calculator.rb @@ -93,7 +93,9 @@ def transitions(opts = {}) other: {} } end - opts.merge!(in_internal_review: state == 'internal_review') + opts[:in_internal_review] = state == 'internal_review' + opts[:internal_review_requested] = + @info_request.internal_review_requested? build_transitions_hash(opts) end @@ -132,7 +134,11 @@ def pending_states(opts) if opts.fetch(:in_internal_review, false) states = %w[internal_review gone_postal] else - states = %w[ + states = [] + if opts.fetch(:internal_review_requested, false) + states += ['internal_review'] + end + states += %w[ waiting_response waiting_clarification gone_postal @@ -141,7 +147,7 @@ def pending_states(opts) states += ['internal_review'] end end - states + states.uniq end def complete_states(_opts = {}) diff --git a/app/models/info_request/state/transitions.rb b/app/models/info_request/state/transitions.rb index 149719910a..a1eb1782d0 100644 --- a/app/models/info_request/state/transitions.rb +++ b/app/models/info_request/state/transitions.rb @@ -86,7 +86,8 @@ def self.owner_gone_postal_transition_label(_opts = {}) end def self.owner_internal_review_transition_label(opts = {}) - if opts.fetch(:in_internal_review, false) + if opts.fetch(:in_internal_review, false) || + opts.fetch(:internal_review_requested, false) _("I'm still waiting for the internal review") else _("I'm waiting for an internal review response") @@ -139,7 +140,8 @@ def self.other_user_gone_postal_transition_label(_opts = {}) end def self.other_user_internal_review_transition_label(opts = {}) - if opts.fetch(:in_internal_review, false) + if opts.fetch(:in_internal_review, false) || + opts.fetch(:internal_review_requested, false) _("Still awaiting an internal review") else # To match what would happen if this method didn't exist, because diff --git a/doc/CHANGES.md b/doc/CHANGES.md index 09f8827d7d..e8aedd21f4 100644 --- a/doc/CHANGES.md +++ b/doc/CHANGES.md @@ -2,6 +2,7 @@ ## Highlighted Features +* Update request base calculated status for internal reviews (Graeme Porteous) * Automatically apply `not_many_requests` tag to bodies who don't have many public requests so that they can be found in a public list or have tag-based notes applied (Gareth Rees) diff --git a/spec/models/info_request/state/calculator_spec.rb b/spec/models/info_request/state/calculator_spec.rb index 7b109ebca8..4a43fe6ad1 100644 --- a/spec/models/info_request/state/calculator_spec.rb +++ b/spec/models/info_request/state/calculator_spec.rb @@ -237,6 +237,64 @@ end end + context "when the request received a response after being in internal_review" do + let(:info_request) do + FactoryBot.create( + :info_request, :with_internal_review_request, + described_state: 'waiting_response' + ) + end + + before do + mail = Mail.new + mail.to info_request.incoming_email + mail.body 'Internal review reply' + info_request.receive(mail, mail.to_s) + end + + context "and the user is the owner" do + it "returns only all pending states with internal_review first" do + transitions = calculator.transitions( + is_owning_user: true, + user_asked_to_update_status: false + ) + expected = %w[internal_review waiting_response waiting_clarification + gone_postal] + expect(transitions[:pending].keys).to eq(expected) + end + + it "returns a different label for the internal_review status" do + transitions = calculator.transitions( + is_owning_user: true, + user_asked_to_update_status: false + ) + expected = "I'm still waiting for the internal review" + expect(transitions[:pending]["internal_review"]).to eq expected + end + end + + context "and the user is some other user" do + it "returns only all pending states with internal_review first" do + transitions = calculator.transitions( + is_owning_user: false, + user_asked_to_update_status: false + ) + expected = %w[internal_review waiting_response waiting_clarification + gone_postal] + expect(transitions[:pending].keys).to eq(expected) + end + + it "returns a different label for the internal_review status" do + transitions = calculator.transitions( + is_owning_user: false, + user_asked_to_update_status: false + ) + expected = "Still awaiting an internal review" + expect(transitions[:pending]["internal_review"]).to eq expected + end + end + end + context "when the request is in an 'other' state" do context "and the user is the owner" do it_behaves_like( diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index b5f1ea4b40..711dc22586 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -1631,6 +1631,23 @@ end end + describe '#internal_review_requested?' do + subject { info_request.internal_review_requested? } + + context 'when internal review has been sent' do + let(:info_request) do + FactoryBot.create(:info_request, :with_internal_review_request) + end + + it { is_expected.to eq true } + end + + context 'when internal review has not been sent' do + let(:info_request) { FactoryBot.create(:info_request) } + it { is_expected.to eq false } + end + end + describe '#is_external?' do it 'returns true if there is an external url' do