Merge pull request #2201 from UKGovernmentBEIS/2948-2946-add-is_oda-t…

…o-reports

(2948)(2946) Add `is_oda` to reports and report uniqueness validation

CHANGELOG.md

@@ -5,6 +5,8 @@
 [Full changelog][unreleased]
 
 - Remove ispf_fund_in_stealth_mode that hides ISPF funds from users
+- Add `is_oda` attribute to reports and populate it to `false` for all existing ISPF reports
+- Amend report validation to permit two editable reports per fund and organisation, as long as they have different ODA types
 
 ## Release 138 - 2023-10-27
 

app/models/report.rb

@@ -91,7 +91,8 @@ def no_unapproved_reports_per_series
 
     unless Report.where(
       fund: fund,
-      organisation: organisation
+      organisation: organisation,
+      is_oda: is_oda
     ).all?(&:approved?)
       errors.add(:base, I18n.t("activerecord.errors.models.report.unapproved_reports_html"))
     end
@@ -136,7 +137,7 @@ def financial_quarter_is_not_in_the_future?
   def no_prexisting_later_report?
     return false unless financial_quarter && financial_year
 
-    latest_report = organisation.reports.in_historical_order.where(fund_id: fund_id).first
+    latest_report = organisation.reports.in_historical_order.where(fund_id: fund_id, is_oda: is_oda).first
     if latest_report && latest_report.financial_period.last > financial_period.last
       errors.add(:financial_period, "A report already exists for a later period.")
     end

db/migrate/20231107133740_add_is_oda_to_reports.rb

@@ -0,0 +1,12 @@
+class AddIsOdaToReports < ActiveRecord::Migration[6.1]
+  def up
+    add_column :reports, :is_oda, :boolean
+
+    ispf = Activity.by_roda_identifier("ISPF")
+    Report.where(fund_id: ispf.id).update_all(is_oda: false) if ispf
+  end
+
+  def down
+    remove_column :reports, :is_oda
+  end
+end

db/migrate/20231107141900_enforce_one_editable_report_per_series_by_oda_type.rb

@@ -0,0 +1,13 @@
+class EnforceOneEditableReportPerSeriesByOdaType < ActiveRecord::Migration[6.1]
+  def change
+    remove_index :reports, [:fund_id, :organisation_id],
+      where: "state <> 'approved'",
+      unique: true,
+      name: "enforce_one_editable_report_per_series"
+
+    add_index :reports, [:fund_id, :organisation_id, :is_oda],
+      where: "state <> 'approved'",
+      unique: true,
+      name: "enforce_one_editable_report_per_series"
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2023_03_07_102933) do
+ActiveRecord::Schema.define(version: 2023_11_07_141900) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
@@ -310,7 +310,8 @@
     t.integer "financial_year"
     t.string "export_filename"
     t.datetime "approved_at"
-    t.index ["fund_id", "organisation_id"], name: "enforce_one_editable_report_per_series", unique: true, where: "((state)::text <> 'approved'::text)"
+    t.boolean "is_oda"
+    t.index ["fund_id", "organisation_id", "is_oda"], name: "enforce_one_editable_report_per_series", unique: true, where: "((state)::text <> 'approved'::text)"
     t.index ["fund_id", "organisation_id"], name: "enforce_one_historic_report_per_series", unique: true, where: "((financial_quarter IS NULL) OR (financial_year IS NULL))"
     t.index ["fund_id"], name: "index_reports_on_fund_id"
     t.index ["organisation_id"], name: "index_reports_on_organisation_id"

spec/models/report_spec.rb

@@ -12,16 +12,33 @@
     end
 
     context "in the :new validation context" do
-      it "validates there are no unapproved reports for the organisation and fund" do
-        organisation = create(:partner_organisation)
-        existing_approved_report = create(:report, :approved, organisation: organisation)
-        existing_unapproved_report = create(:report, state: "awaiting_changes", organisation: organisation)
+      context "for an ODA-only fund" do
+        it "validates there are no unapproved reports for the organisation and fund" do
+          organisation = create(:partner_organisation)
+          existing_approved_report = create(:report, :approved, organisation: organisation)
+          existing_unapproved_report = create(:report, state: "awaiting_changes", organisation: organisation)
 
-        new_valid_report = build(:report, fund: existing_approved_report.fund, organisation: organisation)
-        new_invalid_report = build(:report, fund: existing_unapproved_report.fund, organisation: organisation)
+          new_valid_report = build(:report, fund: existing_approved_report.fund, organisation: organisation)
+          new_invalid_report = build(:report, fund: existing_unapproved_report.fund, organisation: organisation)
 
-        expect(new_invalid_report.valid?(:new)).to be(false)
-        expect(new_valid_report.valid?).to be(true)
+          expect(new_invalid_report.valid?(:new)).to be(false)
+          expect(new_valid_report.valid?(:new)).to be(true)
+        end
+      end
+
+      context "for a hybrid ODA and non-ODA fund such as ISPF" do
+        it "validates there are no unapproved reports for the organisation, fund, and ODA type" do
+          organisation = create(:partner_organisation)
+          _existing_approved_oda_report = create(:report, :for_ispf, :approved, is_oda: true, organisation: organisation)
+          _existing_approved_non_oda_report = create(:report, :for_ispf, :approved, is_oda: false, organisation: organisation)
+          _existing_unapproved_oda_report = create(:report, :for_ispf, is_oda: true, state: "awaiting_changes", organisation: organisation)
+
+          new_valid_report = build(:report, :for_ispf, is_oda: false, organisation: organisation)
+          new_invalid_report = build(:report, :for_ispf, is_oda: true, organisation: organisation)
+
+          expect(new_invalid_report.valid?(:new)).to be(false)
+          expect(new_valid_report.valid?(:new)).to be(true)
+        end
       end
 
       it "validates the presence of financial_quarter and financial_year" do
@@ -32,13 +49,13 @@
       end
 
       context "validates that the financial quarter is previous to the current quarter" do
-        it "when creating a report for the next finanical quarter in the same financial year" do
+        it "when creating a report for the next financial quarter in the same financial year" do
           travel_to(Date.parse("01-04-2020")) do
             new_report = build(:report, financial_quarter: 2, financial_year: 2020)
             expect(new_report.valid?(:new)).to be(false)
           end
         end
-        it "when creating a report for the next finanical quarter in the next financial year" do
+        it "when creating a report for the next financial quarter in the next financial year" do
           travel_to(Date.parse("01-02-2020")) do
             new_report = build(:report, financial_quarter: 1, financial_year: 2020)
             expect(new_report.valid?(:new)).to be(false)
@@ -61,6 +78,7 @@
       describe "Ensuring that a new report does not attempt to rewrite history" do
         let(:organisation) { create(:partner_organisation) }
         let(:fund) { create(:fund_activity) }
+
         context "where a report already exists for a period later than that of the new report" do
           it "is not valid" do
             create(:report, :approved, organisation: organisation, fund: fund, financial_quarter: 4, financial_year: 2018)
@@ -69,7 +87,18 @@
               expect(new_report.valid?(:new)).to be(false)
             end
           end
+
+          context "and the existing report has a different ODA type than the new report" do
+            it "is valid" do
+              create(:report, :for_ispf, :approved, is_oda: false, organisation: organisation, financial_quarter: 4, financial_year: 2018)
+              travel_to(Date.parse("01-04-2020")) do
+                new_report = build(:report, :for_ispf, is_oda: true, organisation: organisation, financial_quarter: 3, financial_year: 2018)
+                expect(new_report.valid?(:new)).to be(true)
+              end
+            end
+          end
         end
+
         context "where a report does not exist for a period later than that of the new report" do
           it "is valid" do
             create(:report, :approved, organisation: organisation, fund: fund, financial_quarter: 4, financial_year: 2018)