diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb
index c0fc40092..77628492b 100644
--- a/app/controllers/merge_requests_controller.rb
+++ b/app/controllers/merge_requests_controller.rb
@@ -64,7 +64,7 @@ class MergeRequestsController < ApplicationController
def start_merge
if @merge_request.status == "ready_to_merge"
- @merge_request.update!(status: :processing)
+ @merge_request.update!(processing: true)
ProcessMergeRequestJob.perform_later(merge_request: @merge_request)
end
diff --git a/app/jobs/process_merge_request_job.rb b/app/jobs/process_merge_request_job.rb
index b706d9a81..cdec20814 100644
--- a/app/jobs/process_merge_request_job.rb
+++ b/app/jobs/process_merge_request_job.rb
@@ -7,7 +7,7 @@ class ProcessMergeRequestJob < ApplicationJob
merge_date = merge_request.merge_date
Merge::MergeOrganisationsService.new(absorbing_organisation_id:, merging_organisation_ids:, merge_date:).call
- merge_request.update!(status: "request_merged", last_failed_attempt: nil)
+ merge_request.update!(request_merged: true, last_failed_attempt: nil)
rescue StandardError
merge_request.update!(last_failed_attempt: Time.zone.now)
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 078399c2a..91db085c5 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -4,27 +4,24 @@ class MergeRequest < ApplicationRecord
belongs_to :absorbing_organisation, class_name: "Organisation", optional: true
has_many :merging_organisations, through: :merge_request_organisations, source: :merging_organisation
belongs_to :requester, class_name: "User", optional: true
- before_save :update_status!
STATUS = {
- "merge_issues" => 0,
- "incomplete" => 1,
- "ready_to_merge" => 2,
- "processing" => 3,
- "request_merged" => 4,
- "deleted" => 5,
+ merge_issues: "merge_issues",
+ incomplete: "incomplete",
+ ready_to_merge: "ready_to_merge",
+ processing: "processing",
+ request_merged: "request_merged",
+ deleted: "deleted",
}.freeze
enum status: STATUS
- scope :not_merged, -> { where.not(status: "request_merged") }
- scope :merged, -> { where(status: "request_merged") }
+ scope :not_merged, -> { where(request_merged: [false, nil]) }
+ scope :merged, -> { where(request_merged: true) }
scope :visible, lambda {
open_collection_period_start_date = FormHandler.instance.start_date_of_earliest_open_collection_period
merged.where("merge_requests.merge_date >= ?", open_collection_period_start_date).or(not_merged).where(discarded_at: nil)
}
- attr_accessor :skip_update_status
-
def absorbing_organisation_name
absorbing_organisation&.name || ""
end
@@ -37,20 +34,14 @@ class MergeRequest < ApplicationRecord
update!(discarded_at: Time.zone.now)
end
- def update_status!
- return if skip_update_status
-
- self.status = calculate_status
- end
-
- def calculate_status
- return "deleted" if discarded_at.present?
- return "request_merged" if status == "request_merged"
- return "processing" if status == "processing"
- return "incomplete" unless required_questions_answered?
- return "ready_to_merge" if absorbing_organisation_signed_dsa?
+ def status
+ return STATUS[:deleted] if discarded_at.present?
+ return STATUS[:request_merged] if request_merged
+ return STATUS[:processing] if processing
+ return STATUS[:incomplete] unless required_questions_answered?
+ return STATUS[:ready_to_merge] if absorbing_organisation_signed_dsa?
- "merge_issues"
+ STATUS[:merge_issues]
end
def required_questions_answered?
diff --git a/app/models/merge_request_organisation.rb b/app/models/merge_request_organisation.rb
index 18a77d93a..6dda8b35e 100644
--- a/app/models/merge_request_organisation.rb
+++ b/app/models/merge_request_organisation.rb
@@ -5,22 +5,15 @@ class MergeRequestOrganisation < ApplicationRecord
validates :merging_organisation, presence: { message: I18n.t("validations.merge_request.merging_organisation_id.blank") }
validate :validate_merging_organisations
- scope :merged, -> { joins(:merge_request).where(merge_requests: { status: "request_merged" }) }
+ scope :merged, -> { joins(:merge_request).where(merge_requests: { request_merged: true }) }
scope :with_merging_organisation, ->(merging_organisation) { where(merging_organisation:) }
- after_save :update_merge_request_status
-
has_paper_trail
def merging_organisation_name
merging_organisation.name || ""
end
- def update_merge_request_status
- merge_request.update_status!
- merge_request.save!
- end
-
private
def validate_merging_organisations
diff --git a/app/views/merge_requests/_merge_request_list.html.erb b/app/views/merge_requests/_merge_request_list.html.erb
index e048472a7..c133aabc1 100644
--- a/app/views/merge_requests/_merge_request_list.html.erb
+++ b/app/views/merge_requests/_merge_request_list.html.erb
@@ -4,7 +4,7 @@
<% else %>
<%= govuk_table do |table| %>
<%= table.with_caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do %>
- <%= @merge_requests.where.not(status: 4).count %> unresolved merge requests
+ <%= @merge_requests.not_merged.count %> unresolved merge requests
<% end %>
<%= table.with_head do |head| %>
<%= head.with_row do |row| %>
diff --git a/db/migrate/20240819100411_update_merge_request_fields_for_status.rb b/db/migrate/20240819100411_update_merge_request_fields_for_status.rb
new file mode 100644
index 000000000..6613d27a4
--- /dev/null
+++ b/db/migrate/20240819100411_update_merge_request_fields_for_status.rb
@@ -0,0 +1,17 @@
+class UpdateMergeRequestFieldsForStatus < ActiveRecord::Migration[7.0]
+ def up
+ change_table :merge_requests, bulk: true do |t|
+ t.column :request_merged, :boolean
+ t.column :processing, :boolean
+ t.remove :status
+ end
+ end
+
+ def down
+ change_table :merge_requests, bulk: true do |t|
+ t.remove :request_merged
+ t.remove :processing
+ t.column :status, :string
+ end
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 9f46a7ae4..57df3eb3f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema[7.0].define(version: 2024_08_14_083017) do
+ActiveRecord::Schema[7.0].define(version: 2024_08_19_100411) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -419,7 +419,6 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_14_083017) do
t.integer "requesting_organisation_id"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
- t.integer "status"
t.integer "absorbing_organisation_id"
t.datetime "merge_date"
t.integer "requester_id"
@@ -433,6 +432,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_14_083017) do
t.boolean "signed_dsa", default: false
t.datetime "discarded_at"
t.datetime "last_failed_attempt"
+ t.boolean "request_merged"
+ t.boolean "processing"
end
create_table "notifications", force: :cascade do |t|
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 6ada68985..1f52dd640 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -3,9 +3,9 @@ require "rails_helper"
RSpec.describe MergeRequest, type: :model do
describe ".visible" do
let(:open_collection_period_start_date) { 1.year.ago }
- let!(:merged_recent) { create(:merge_request, status: "request_merged", merge_date: 3.months.ago) }
- let!(:merged_old) { create(:merge_request, status: "request_merged", merge_date: 18.months.ago) }
- let!(:not_merged) { create(:merge_request, status: "incomplete") }
+ let!(:merged_recent) { create(:merge_request, request_merged: true, merge_date: 3.months.ago) }
+ let!(:merged_old) { create(:merge_request, request_merged: true, merge_date: 18.months.ago) }
+ let!(:not_merged) { create(:merge_request, request_merged: false) }
before do
allow(FormHandler.instance).to receive(:start_date_of_earliest_open_collection_period).and_return(open_collection_period_start_date)
@@ -43,50 +43,50 @@ RSpec.describe MergeRequest, type: :model do
end
end
- describe "#calculate_status" do
+ describe "#status" do
it "returns the correct status for deleted merge request" do
merge_request = build(:merge_request, id: 1, discarded_at: Time.zone.today)
- expect(merge_request.calculate_status).to eq "deleted"
+ expect(merge_request.status).to eq MergeRequest::STATUS[:deleted]
end
it "returns the correct status for a merged request" do
- merge_request = build(:merge_request, id: 1, status: "request_merged")
- expect(merge_request.calculate_status).to eq "request_merged"
+ merge_request = build(:merge_request, id: 1, request_merged: true)
+ expect(merge_request.status).to eq MergeRequest::STATUS[:request_merged]
end
it "returns the correct status for a ready to merge request" do
merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation), merge_date: Time.zone.today)
create(:merge_request_organisation, merge_request:)
- expect(merge_request.calculate_status).to eq "ready_to_merge"
+ expect(merge_request.status).to eq MergeRequest::STATUS[:ready_to_merge]
end
it "returns the merge issues if dsa is not signed for absorbing organisation" do
merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation, with_dsa: false), merge_date: Time.zone.today)
create(:merge_request_organisation, merge_request:)
- expect(merge_request.calculate_status).to eq "merge_issues"
+ expect(merge_request.status).to eq MergeRequest::STATUS[:merge_issues]
end
it "returns the incomplete if absorbing organisation is missing" do
merge_request = build(:merge_request, id: 1, absorbing_organisation: nil, merge_date: Time.zone.today)
create(:merge_request_organisation, merge_request:)
- expect(merge_request.calculate_status).to eq "incomplete"
+ expect(merge_request.status).to eq MergeRequest::STATUS[:incomplete]
end
it "returns the incomplete if merge requests organisation is missing" do
merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation), merge_date: Time.zone.today)
- expect(merge_request.calculate_status).to eq "incomplete"
+ expect(merge_request.status).to eq MergeRequest::STATUS[:incomplete]
end
it "returns the incomplete if merge date is missing" do
merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation))
create(:merge_request_organisation, merge_request:)
- expect(merge_request.calculate_status).to eq "incomplete"
+ expect(merge_request.status).to eq MergeRequest::STATUS[:incomplete]
end
it "returns processing if merge is processing" do
- merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation), status: "processing")
+ merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation), processing: true)
create(:merge_request_organisation, merge_request:)
- expect(merge_request.calculate_status).to eq "processing"
+ expect(merge_request.status).to eq MergeRequest::STATUS[:processing]
end
end
end
diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb
index 8b7a577a7..8e406ea5f 100644
--- a/spec/requests/merge_requests_controller_spec.rb
+++ b/spec/requests/merge_requests_controller_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe MergeRequestsController, type: :request do
end
context "when creating a new merge request" do
- let(:params) { { merge_request: { requesting_organisation_id: support_user.organisation_id, status: "incomplete" } } }
+ let(:params) { { merge_request: { requesting_organisation_id: support_user.organisation_id } } }
before do
post "/merge-request", headers:, params:
@@ -30,7 +30,7 @@ RSpec.describe MergeRequestsController, type: :request do
end
context "when passing a different requesting organisation id" do
- let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "incomplete" } } }
+ let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id } } }
it "creates merge request with current user organisation" do
follow_redirect!
@@ -74,7 +74,7 @@ RSpec.describe MergeRequestsController, type: :request do
let(:params) { { merge_request: { merging_organisation: other_organisation.id, new_merging_org_ids: [] } } }
before do
- MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "request_merged")
+ MergeRequest.create!(requesting_organisation_id: other_organisation.id, request_merged: true)
patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
@@ -89,7 +89,7 @@ RSpec.describe MergeRequestsController, type: :request do
let(:params) { { merge_request: { merging_organisation: another_organisation.id, new_merging_org_ids: [] } } }
before do
- existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "request_merged")
+ existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, request_merged: true)
MergeRequestOrganisation.create!(merge_request_id: existing_merge_request.id, merging_organisation_id: another_organisation.id)
patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
@@ -107,7 +107,7 @@ RSpec.describe MergeRequestsController, type: :request do
let(:params) { { merge_request: { merging_organisation: another_organisation.id, new_merging_org_ids: [] } } }
before do
- existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "incomplete")
+ existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id)
MergeRequestOrganisation.create!(merge_request_id: existing_merge_request.id, merging_organisation_id: another_organisation.id)
patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params:
end
@@ -428,7 +428,7 @@ RSpec.describe MergeRequestsController, type: :request do
end
describe "#merging_organisations" do
- let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "incomplete" } } }
+ let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id } } }
context "when creating a new merge request" do
before do
diff --git a/spec/views/merge_requests/show.html.erb_spec.rb b/spec/views/merge_requests/show.html.erb_spec.rb
index b6e6a3ce2..75fa4197d 100644
--- a/spec/views/merge_requests/show.html.erb_spec.rb
+++ b/spec/views/merge_requests/show.html.erb_spec.rb
@@ -3,7 +3,7 @@ require "rails_helper"
RSpec.describe "merge_requests/show.html.erb", type: :view do
let(:absorbing_organisation) { create(:organisation, name: "Absorbing Org", with_dsa: false) }
let(:dpo_user) { create(:user, name: "DPO User", is_dpo: true, organisation: absorbing_organisation) }
- let(:merge_request) { create(:merge_request, absorbing_organisation_id: absorbing_organisation.id, signed_dsa: false, status: 1) }
+ let(:merge_request) { create(:merge_request, absorbing_organisation_id: absorbing_organisation.id, signed_dsa: false) }
before do
assign(:merge_request, merge_request)
@@ -53,7 +53,7 @@ RSpec.describe "merge_requests/show.html.erb", type: :view do
context "when the merge request is complete" do
before do
- merge_request.update!(status: 4, signed_dsa: true, total_users: 10, total_schemes: 5, total_lettings_logs: 20, total_sales_logs: 30, total_stock_owners: 40, total_managing_agents: 50)
+ merge_request.update!(request_merged: true, signed_dsa: true, total_users: 10, total_schemes: 5, total_lettings_logs: 20, total_sales_logs: 30, total_stock_owners: 40, total_managing_agents: 50)
assign(:merge_request, merge_request)
render
end