Merge branch '3830-update-status-internal-review' into develop

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

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 = {})

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 <strong>waiting</strong> for the internal review")
         else
           _("I'm waiting for an <strong>internal review</strong> 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 <strong>internal review</strong>")
         else
           # To match what would happen if this method didn't exist, because

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)

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 <strong>waiting</strong> 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 <strong>internal review</strong>"
+          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(

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