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