From 4a2d2009ea2d35bb9aef7cd73b606f9b3d381ba7 Mon Sep 17 00:00:00 2001 From: Cristina <579522+CristinaRO@users.noreply.github.com> Date: Tue, 23 Jan 2024 15:45:09 +0000 Subject: [PATCH 01/10] Change the service owner's IATI reference As a consequence of BEIS having become DSIT, the organisation has received a new IATI reference. In this commit we change the organisation details for BEIs/DSIT and for BEIS Finance/DSIT Finance, through a data migration. The data has already been changed in staging and development, but we had forgotten to change the alternate names of the service owner org, so we have written this data migration in such a way as to be able to find the service owner org by either of its past or present identifiers. The data migration will need to be run via the application console, with `rails runner db/data/20240123152048_change_beis_to_dsit.rb`. The change to the `SERVICE_OWNER_IATI_REFERENCE` will not cause any issues in the interim, because we have coded the application to be able to find the service owner by either ID. --- CHANGELOG.md | 2 ++ app/models/organisation.rb | 2 +- db/data/20240123152048_change_beis_to_dsit.rb | 28 +++++++++++++++++++ db/seeds/organisations.yml | 2 +- ..._create_a_programme_level_activity_spec.rb | 12 ++++---- 5 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 db/data/20240123152048_change_beis_to_dsit.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index e549c992a..b00c7633f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ [Full changelog][unreleased] +- Change the transparency identifier and names for the DSIT organisations (DSIT and DSIT Finance) + ## Release 143 - 2024-01-23 [Full changelog][143] diff --git a/app/models/organisation.rb b/app/models/organisation.rb index af5786f41..7ddd7cb3b 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -1,5 +1,5 @@ class Organisation < ApplicationRecord - SERVICE_OWNER_IATI_REFERENCE = "GB-GOV-13" + SERVICE_OWNER_IATI_REFERENCE = "GB-GOV-26" strip_attributes only: [:iati_reference] has_many :users diff --git a/db/data/20240123152048_change_beis_to_dsit.rb b/db/data/20240123152048_change_beis_to_dsit.rb new file mode 100644 index 000000000..bb5ded636 --- /dev/null +++ b/db/data/20240123152048_change_beis_to_dsit.rb @@ -0,0 +1,28 @@ +# Run me with `rails runner db/data/20240123152048_change_beis_to_dsit.rb` + +service_owner = Organisation.where(iati_reference: ["GB-GOV-13", "GB-GOV-26"]).first +if service_owner + service_owner.iati_reference = "GB-GOV-26" + service_owner.name = "DEPARTMENT FOR SCIENCE, INNOVATION AND TECHNOLOGY" + service_owner.beis_organisation_reference = "DSIT" + service_owner.alternate_names = [ + "DEPARTMENT FOR SCIENCE, INNOVATION & TECHNOLOGY", + "Department for Science, Innovation and Technology", + "Department for Science, Innovation & Technology" + ] + + unless service_owner.save + puts "Failed to save the changes to #{service_owner.name}: #{service_owner.errors.messages.inspect}" + end +end + +finance = Organisation.where(iati_reference: "GB-GOV-13-OPERATIONS").first +if finance + finance.iati_reference = "GB-GOV-26-OPERATIONS" + finance.name = "DSIT FINANCE" + finance.beis_organisation_reference = "DF" + + unless finance.save + puts "Failed to save the changes to #{finance.name}: #{finance.errors.messages.inspect}" + end +end diff --git a/db/seeds/organisations.yml b/db/seeds/organisations.yml index 4ae0db2bd..58ac2ad72 100644 --- a/db/seeds/organisations.yml +++ b/db/seeds/organisations.yml @@ -1,6 +1,6 @@ - name: Department for Business, Energy and Industrial Strategy short_name: BEIS - reference: GB-GOV-13 + reference: GB-GOV-26 type: 10 role: service_owner - name: UK Space Agency diff --git a/spec/features/beis_users_can_create_a_programme_level_activity_spec.rb b/spec/features/beis_users_can_create_a_programme_level_activity_spec.rb index a9596a62a..dfb03eee7 100644 --- a/spec/features/beis_users_can_create_a_programme_level_activity_spec.rb +++ b/spec/features/beis_users_can_create_a_programme_level_activity_spec.rb @@ -58,10 +58,10 @@ expect(created_activity.oda_eligibility).to eq(activity.oda_eligibility) expect(created_activity.accountable_organisation_name).to eq("Department for Business, Energy and Industrial Strategy") - expect(created_activity.accountable_organisation_reference).to eq("GB-GOV-13") + expect(created_activity.accountable_organisation_reference).to eq("GB-GOV-26") expect(created_activity.accountable_organisation_type).to eq("10") - expect(created_activity.transparency_identifier).to eql("GB-GOV-13-#{created_activity.roda_identifier}") + expect(created_activity.transparency_identifier).to eql("GB-GOV-26-#{created_activity.roda_identifier}") expect_implementing_organisation_to_be_the_partner_organisation( activity: created_activity, @@ -116,10 +116,10 @@ expect(created_activity.oda_eligibility).to eq(activity.oda_eligibility) expect(created_activity.accountable_organisation_name).to eq("Department for Business, Energy and Industrial Strategy") - expect(created_activity.accountable_organisation_reference).to eq("GB-GOV-13") + expect(created_activity.accountable_organisation_reference).to eq("GB-GOV-26") expect(created_activity.accountable_organisation_type).to eq("10") - expect(created_activity.transparency_identifier).to eql("GB-GOV-13-#{created_activity.roda_identifier}") + expect(created_activity.transparency_identifier).to eql("GB-GOV-26-#{created_activity.roda_identifier}") expect(created_activity.commitment.value).to eq(activity.commitment.value) expect(created_activity.publish_to_iati).to be(true) @@ -167,10 +167,10 @@ expect(created_activity.oda_eligibility).to eq(activity.oda_eligibility) expect(created_activity.accountable_organisation_name).to eq("Department for Business, Energy and Industrial Strategy") - expect(created_activity.accountable_organisation_reference).to eq("GB-GOV-13") + expect(created_activity.accountable_organisation_reference).to eq("GB-GOV-26") expect(created_activity.accountable_organisation_type).to eq("10") - expect(created_activity.transparency_identifier).to eql("GB-GOV-13-#{created_activity.roda_identifier}") + expect(created_activity.transparency_identifier).to eql("GB-GOV-26-#{created_activity.roda_identifier}") expect(created_activity.commitment.value).to eq(activity.commitment.value) expect(created_activity.publish_to_iati).to be(true) From 58eade2d2b8f25db864a281ba474c5b272397e09 Mon Sep 17 00:00:00 2001 From: Cristina <579522+CristinaRO@users.noreply.github.com> Date: Thu, 11 Jan 2024 14:31:53 +0000 Subject: [PATCH 02/10] Update documentation and hints to use new ID --- config/locales/models/actual.en.yml | 2 +- config/locales/models/organisation.en.yml | 2 +- doc/activity-identifiers.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/locales/models/actual.en.yml b/config/locales/models/actual.en.yml index 0a7d8d9e2..26d81ceb7 100644 --- a/config/locales/models/actual.en.yml +++ b/config/locales/models/actual.en.yml @@ -46,7 +46,7 @@ en: description: For example, 2020 quarter one spend on the Early Career Research Network project. disbursement_channel: The channel through which the funds will flow for this actual. providing_organisation: The organisation where this actual is coming from. - providing_organisation_reference_html: For example, GB-GOV-13. To lookup codes or for more information see the organisation finder service (Opens in new window). + providing_organisation_reference_html: For example, GB-GOV-26. To lookup codes or for more information see the organisation finder service (Opens in new window). receiving_organisation: The organisation receiving the money from this actual spend. receiving_organisation_reference_html: For example, GB-COH-12345. To lookup codes or for more information see the organisation finder service (Opens in new window). table: diff --git a/config/locales/models/organisation.en.yml b/config/locales/models/organisation.en.yml index 6c82408b6..61e365d6c 100644 --- a/config/locales/models/organisation.en.yml +++ b/config/locales/models/organisation.en.yml @@ -123,7 +123,7 @@ en: organisation: attributes: iati_reference: - format: Identifiers must start with a country code and company type separated by a dash, eg. GB-GOV-13 + format: Identifiers must start with a country code and company type separated by a dash, eg. GB-GOV-26 blank: Enter an IATI reference default_currency: blank: Enter a default currency diff --git a/doc/activity-identifiers.md b/doc/activity-identifiers.md index 2b742e4b6..67b017c69 100644 --- a/doc/activity-identifiers.md +++ b/doc/activity-identifiers.md @@ -112,7 +112,7 @@ end users. This identifier is a transformed version of the RODA Identifier that's compatible with the IATI rules. Any string of characters in the RODA Identifier that are not letters, digits, or `-` are replaced with `-`, and the -organisational prefix `GB-GOV-13-` is prepended to the result. +organisational prefix `GB-GOV-26-` is prepended to the result. This identifier is not set by the ingest process _or_ assigned directly by end users; it is derived from the RODA Identifier when that is set. From 9fad55037439309212d37dd06a9871b818af5c31 Mon Sep 17 00:00:00 2001 From: Cristina <579522+CristinaRO@users.noreply.github.com> Date: Thu, 11 Jan 2024 14:41:10 +0000 Subject: [PATCH 03/10] Change spec to use a non-reserved IATI reference The spec is correct with regards to format, but it can give the wrong impression about using that specific identifier for a non-service owner organisation. --- spec/models/organisation_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index 09a52a57e..1425853d7 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -44,7 +44,7 @@ describe "#iati_reference" do it "returns true if it does matches a known structure XX-XXX-" do - organisation = build(:partner_organisation, iati_reference: "GB-GOV-13") + organisation = build(:partner_organisation, iati_reference: "GB-GOV-44") result = organisation.valid? expect(result).to be(true) end From a486e4b79f12bf83e955a85ed2943d0941783bb4 Mon Sep 17 00:00:00 2001 From: Cristina <579522+CristinaRO@users.noreply.github.com> Date: Mon, 22 Jan 2024 19:32:56 +0000 Subject: [PATCH 04/10] Change transp id of activities, actuals, forecasts For activities that will be continuing under DSIT: - change their transparency identifier to replace the string "GB-GOV-13" with "GB-GOV-26"; e.g. `GB-GOV-13-ISPF-UKSA-AH2TS7K-SPB9U87` becomes `GB-GOV-26-ISPF-UKSA-AH2TS7K-SPB9U87` - change the `providing_organisation_reference` for all their actuals, from "GB-GOV-13" to "GB-GOV-26" - change the `providing_organisation_reference` for all their forecasts, from "GB-GOV-13" to "GB-GOV-26" The rake task can be run with: ``` bundle exec rake activities:continuing_activities ``` By default, it will perform a dry run. Only invoking the task with `DRY_RUN=false` will effect any changes: ``` DRY_RUN=false bundle exec rake activities:continuing_activities ``` It can also be invoked with an environment variable to skip validations, if we determine that DSIT and us are OK with that: ``` SKIP_VALIDATION=true DRY_RUN=false bundle exec rake activities:continuing_activities ``` --- CHANGELOG.md | 1 + ...uing_activities_actuals_and_forecasts.rake | 78 +++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 lib/tasks/continuing_activities_actuals_and_forecasts.rake diff --git a/CHANGELOG.md b/CHANGELOG.md index b00c7633f..54163ac30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ [Full changelog][unreleased] - Change the transparency identifier and names for the DSIT organisations (DSIT and DSIT Finance) +- Add a rake task to change the transparency identifier for activities that will continue under DSIT, and the providing org reference for their actual spend and forecasts ## Release 143 - 2024-01-23 diff --git a/lib/tasks/continuing_activities_actuals_and_forecasts.rake b/lib/tasks/continuing_activities_actuals_and_forecasts.rake new file mode 100644 index 000000000..31dc8d654 --- /dev/null +++ b/lib/tasks/continuing_activities_actuals_and_forecasts.rake @@ -0,0 +1,78 @@ +namespace :activities do + task continuing_activities: :environment do + dry_run = ENV.fetch("DRY_RUN", "true").downcase + unless dry_run == "false" + puts "Performing a dry run of the script. No activities, actuals, or forecasts will actually be changed. Pass DRY_RUN=false to the task to do any changes." + end + + skip_validation = ENV.fetch("SKIP_VALIDATION", "").downcase + if skip_validation == "true" + puts "Validations will be skipped. Activities, actuals, and forecasts will be changed in place." + end + + continuing_activities = Export::ContinuingActivities.new.activities + continuing_activities.each do |activity| + puts "\n Activity #{activity.roda_identifier}" + + original_transparency_identifier = activity.transparency_identifier.to_s + updated_previous_identifier = original_transparency_identifier + updated_transparency_identifier = original_transparency_identifier.sub(/\AGB-GOV-13/, "GB-GOV-26") + + if original_transparency_identifier == updated_transparency_identifier + puts "NO CHANGE: transparency_identifier=#{original_transparency_identifier}" + elsif dry_run == "false" + if skip_validation == "true" + activity.update_columns( + previous_identifier: updated_previous_identifier, + transparency_identifier: updated_transparency_identifier, + updated_at: Time.current + ) + elsif activity.update( + previous_identifier: updated_previous_identifier, + transparency_identifier: updated_transparency_identifier + ) + activity.reload + puts "UPDATED: transparency_identifier=#{activity.transparency_identifier} previous_identifier=#{activity.previous_identifier}" + else + puts "ERROR: #{activity.errors.messages.inspect}" + end + end + + activity_actuals = activity.actuals.where(providing_organisation_reference: "GB-GOV-13") + puts "Eligible actuals: #{activity_actuals.count}" + + if dry_run == "false" + activity_actuals.each do |actual| + if skip_validation == "true" + actual.update_columns( + providing_organisation_reference: "GB-GOV-26", + updated_at: Time.current + ) + elsif actual.update(providing_organisation_reference: "GB-GOV-26") + puts "Actual #{actual.id} UPDATED" + else + puts "Actual #{actual.id} ERROR: #{actual.errors.messages.inspect}" + end + end + end + + activity_forecasts = Forecast.unscoped.where(parent_activity_id: activity.id, providing_organisation_reference: "GB-GOV-13") + puts "Eligible forecasts: #{activity_forecasts.count}" + + if dry_run == "false" + activity_forecasts.each do |forecast| + if skip_validation == "true" + forecast.update_columns( + providing_organisation_reference: "GB-GOV-26", + updated_at: Time.current + ) + elsif forecast.update(providing_organisation_reference: "GB-GOV-26") + puts "Forecast #{forecast.id} UPDATED" + else + puts "Forecast #{forecast.id} ERROR: #{forecast.errors.messages.inspect}" + end + end + end + end + end +end From af406a5bccda84d62d9966a773161a7162744ba6 Mon Sep 17 00:00:00 2001 From: meyric Date: Wed, 17 Jan 2024 16:33:07 +0000 Subject: [PATCH 05/10] Model a imported row We want to help users by showing any rows that were skipped during a csv import. To facilitate this we want a simple object that stores the row number the imported item came from as well as the imported object itself. This is the same thing we do for rows that contain errors with the Import::Error object. We chose to namespace this to `csv` as the row number implies import from at least tabulated data. --- CHANGELOG.md | 2 ++ app/models/import/csv/imported_row.rb | 8 ++++++++ spec/models/import/csv/imported_row_spec.rb | 20 ++++++++++++++++++++ 3 files changed, 30 insertions(+) create mode 100644 app/models/import/csv/imported_row.rb create mode 100644 spec/models/import/csv/imported_row_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 54163ac30..0859de4ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - Change the transparency identifier and names for the DSIT organisations (DSIT and DSIT Finance) - Add a rake task to change the transparency identifier for activities that will continue under DSIT, and the providing org reference for their actual spend and forecasts +- model a simple imported row so that users can see which row in the csv import + were skipped ## Release 143 - 2024-01-23 diff --git a/app/models/import/csv/imported_row.rb b/app/models/import/csv/imported_row.rb new file mode 100644 index 000000000..97ad0dd26 --- /dev/null +++ b/app/models/import/csv/imported_row.rb @@ -0,0 +1,8 @@ +class Import::Csv::ImportedRow + attr_reader :csv_row_number, :object + + def initialize(index, object) + @csv_row_number = index + 2 + @object = object + end +end diff --git a/spec/models/import/csv/imported_row_spec.rb b/spec/models/import/csv/imported_row_spec.rb new file mode 100644 index 000000000..5aee455f2 --- /dev/null +++ b/spec/models/import/csv/imported_row_spec.rb @@ -0,0 +1,20 @@ +require "rails_helper" + +RSpec.describe Import::Csv::ImportedRow do + describe "#csv_row_number" do + it "returns the row number of the imported item from the source file" do + result = described_class.new(0, nil) + + expect(result.csv_row_number).to be 2 + end + end + + describe "#object" do + it "returns the object that the imported row result in" do + imported_object = double("Imported Object") + result = described_class.new(0, imported_object) + + expect(result.object).to be imported_object + end + end +end From 38a3fc023db35981ba077ea219916658ea7c27df Mon Sep 17 00:00:00 2001 From: meyric Date: Wed, 17 Jan 2024 16:36:59 +0000 Subject: [PATCH 06/10] Update the success view for the new import The new Activity actual, refund and comment import breaks the imported rows in to meaningful types that we want to show to the user. Here we add four new tables for showing the imported objects, we choose to add new ones as the existing shared partials are for a different view and do not have the appropriate content and we prefer the extra partials closer to their use than make more complicated shared partials. We then update the controller to provide the data to the view, this uses the new Import::Csv::ImportedRow object so that, when required, we can provide the row number to the user. Along the way we have to update some specs to account for the new `Import::Csv::ImportedRow` objects being returned by the `FileService`. --- CHANGELOG.md | 2 ++ app/controllers/actuals/uploads_controller.rb | 15 ++++---- .../file_service.rb | 16 ++++----- app/views/actuals/uploads/_actuals.html.haml | 36 +++++++++++++++++++ app/views/actuals/uploads/_comments.html.haml | 19 ++++++++++ app/views/actuals/uploads/_refunds.html.haml | 35 ++++++++++++++++++ app/views/actuals/uploads/_skipped.html.haml | 15 ++++++++ app/views/actuals/uploads/update.html.haml | 24 +++++++++++-- .../actuals/uploads_controller_spec.rb | 8 +++-- ...als_refunds_and_activity_comments_spec.rb} | 25 ++++++++++--- .../file_service_spec.rb | 8 ++--- 11 files changed, 175 insertions(+), 28 deletions(-) create mode 100644 app/views/actuals/uploads/_actuals.html.haml create mode 100644 app/views/actuals/uploads/_comments.html.haml create mode 100644 app/views/actuals/uploads/_refunds.html.haml create mode 100644 app/views/actuals/uploads/_skipped.html.haml rename spec/features/import/{users_can_upload_actuals_refunds_and_actvitiy_comments_spec.rb => users_can_upload_actuals_refunds_and_activity_comments_spec.rb} (84%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0859de4ef..53dc49375 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ - Add a rake task to change the transparency identifier for activities that will continue under DSIT, and the providing org reference for their actual spend and forecasts - model a simple imported row so that users can see which row in the csv import were skipped +- the Activity actual, refund and comment upload success view now shows the + imported actuals, refunds, activity comments and skipped rows ## Release 143 - 2024-01-23 diff --git a/app/controllers/actuals/uploads_controller.rb b/app/controllers/actuals/uploads_controller.rb index 9ccdae00d..d553b00ac 100644 --- a/app/controllers/actuals/uploads_controller.rb +++ b/app/controllers/actuals/uploads_controller.rb @@ -47,13 +47,16 @@ def update @errors = importer.errors if import_result - # the old import and the UI combine Actuals and Refunds, so we have to do the same - # once we have tested the import, we will come back and make the UI improvements - # to make the most of the new importer - imported_actuals_and_refunds = importer.imported_actuals + importer.imported_refunds + @total_actuals = total_transactions(importer.imported_actuals) + @grouped_actuals = grouped_transactions(importer.imported_actuals) + + @total_refunds = total_transactions(importer.imported_refunds) + @grouped_refunds = grouped_transactions(importer.imported_refunds) + + @imported_activity_comments = importer.imported_comments + + @skipped_rows = importer.skipped_rows - @total_actuals = total_transactions(imported_actuals_and_refunds) - @grouped_actuals = grouped_transactions(imported_actuals_and_refunds) @success = true flash.now[:notice] = t("action.actual.upload.success") diff --git a/app/services/import/csv/activity_actual_refund_comment/file_service.rb b/app/services/import/csv/activity_actual_refund_comment/file_service.rb index 56d3fa0df..7e8990a27 100644 --- a/app/services/import/csv/activity_actual_refund_comment/file_service.rb +++ b/app/services/import/csv/activity_actual_refund_comment/file_service.rb @@ -29,7 +29,7 @@ def import! collate_errors_from_row_importer(index, row_importer) if row_importer.errors.any? - imported_row + Import::Csv::ImportedRow.new(index, imported_row) end unless @errors.empty? @@ -41,26 +41,26 @@ def import! end def imported_actuals - @imported_rows.select do |row| - row.is_a?(Actual) + @imported_rows.filter_map do |row| + row.object if row.object.is_a?(Actual) end end def imported_refunds - @imported_rows.select do |row| - row.is_a?(Refund) + @imported_rows.filter_map do |row| + row.object if row.object.is_a?(Refund) end end def imported_comments - @imported_rows.select do |row| - row.is_a?(Comment) + @imported_rows.filter_map do |row| + row.object if row.object.is_a?(Comment) end end def skipped_rows @imported_rows.select do |row| - row.is_a?(Import::Csv::ActivityActualRefundComment::SkippedRow) + row.object.is_a?(Import::Csv::ActivityActualRefundComment::SkippedRow) end end diff --git a/app/views/actuals/uploads/_actuals.html.haml b/app/views/actuals/uploads/_actuals.html.haml new file mode 100644 index 000000000..732e7a0bc --- /dev/null +++ b/app/views/actuals/uploads/_actuals.html.haml @@ -0,0 +1,36 @@ +%table.govuk-table#actuals + %caption.govuk-table__caption.govuk-table__caption--m + Actuals + %thead.govuk-table__head + %tr.govuk-table__row + %th.govuk-table__header{class: "govuk-!-width-one-third", scope: "col"} Activity + %th.govuk-table__header{class: "govuk-!-width-one-third", scope: "col"} RODA Identifier + %th.govuk-table__header{class: "govuk-!-width-one-third", scope: "col"} Value + %tbody.govuk-table__body + + - @grouped_actuals.each do |activity, actuals| + + %tr.govuk-table__row{ id: "activity_actuals_#{activity.id}" } + %td.govuk-table__cell{class: "govuk-!-width-one-third"} + = activity.title + %td.govuk-table__cell{class: "govuk-!-width-one-third"} + = activity.roda_identifier + %td.govuk-table__cell{class: "govuk-!-width-one-third"} + %table.govuk-table.actuals + - actuals.each do |actual| + %tr.govuk-table__row + %td.govuk-table__cell--numeric{class: "govuk-!-width-one-half", scope: "col"} + = actual.financial_quarter_and_year + %td.govuk-table__cell--numeric{class: "govuk-!-width-one-half", scope: "col"} + = actual.value + - if actual.comment + %tr.govuk-table__row + %td.govuk-table__cell--numeric{class: "govuk-!-width-full", scope: "col", colspan: 2} + = actual.comment.body + + + %tr.govuk-table__row.totals + %td.govuk-table__cell Total + %td.govuk-table__cell + %td.govuk-table__cell.govuk-table__cell--numeric + = @total_actuals diff --git a/app/views/actuals/uploads/_comments.html.haml b/app/views/actuals/uploads/_comments.html.haml new file mode 100644 index 000000000..46d94e76e --- /dev/null +++ b/app/views/actuals/uploads/_comments.html.haml @@ -0,0 +1,19 @@ +%table.govuk-table#comments + %caption.govuk-table__caption.govuk-table__caption--m + Activity Comments + %thead.govuk-table__head + %tr.govuk-table__row + %th.govuk-table__header{class: "govuk-!-width-one-third", scope: "col"} Activity + %th.govuk-table__header{class: "govuk-!-width-one-third", scope: "col"} RODA Identifier + %th.govuk-table__header{class: "govuk-!-width-one-third", scope: "col"} Comment + %tbody.govuk-table__body + + - @imported_activity_comments.each do |comment| + + %tr.govuk-table__row{ id: "activity_comment_#{comment.id}" } + %td.govuk-table__cell{class: "govuk-!-width-one-third"} + = comment.associated_activity.title + %td.govuk-table__cell{class: "govuk-!-width-one-third"} + = comment.associated_activity.roda_identifier + %td.govuk-table__cell{class: "govuk-!-width-one-third"} + = comment.body diff --git a/app/views/actuals/uploads/_refunds.html.haml b/app/views/actuals/uploads/_refunds.html.haml new file mode 100644 index 000000000..267889626 --- /dev/null +++ b/app/views/actuals/uploads/_refunds.html.haml @@ -0,0 +1,35 @@ +%table.govuk-table#refunds + %caption.govuk-table__caption.govuk-table__caption--m + Refunds + %thead.govuk-table__head + %tr.govuk-table__row + %th.govuk-table__header{class: "govuk-!-width-one-third", scope: "col"} Activity + %th.govuk-table__header{class: "govuk-!-width-one-third", scope: "col"} RODA Identifier + %th.govuk-table__header{class: "govuk-!-width-one-third", scope: "col"} Value + %tbody.govuk-table__body + + - @grouped_refunds.each do |activity, refunds| + + %tr.govuk-table__row{ id: "activity_actuals_#{activity.id}" } + %td.govuk-table__cell{class: "govuk-!-width-one-third"} + = activity.title + %td.govuk-table__cell{class: "govuk-!-width-one-third"} + = activity.roda_identifier + %td.govuk-table__cell{class: "govuk-!-width-one-third"} + %table.govuk-table.actuals + - refunds.each do |refund| + %tr.govuk-table__row + %td.govuk-table__cell--numeric{class: "govuk-!-width-one-half", scope: "col"} + = refund.financial_quarter_and_year + %td.govuk-table__cell--numeric{class: "govuk-!-width-one-half", scope: "col"} + = refund.value + - if refund.comment + %tr.govuk-table__row + %td.govuk-table__cell--numeric{class: "govuk-!-width-full", scope: "col", colspan: 2} + = refund.comment.body + + %tr.govuk-table__row.totals + %td.govuk-table__cell Total + %td.govuk-table__cell + %td.govuk-table__cell.govuk-table__cell--numeric + = @total_refunds diff --git a/app/views/actuals/uploads/_skipped.html.haml b/app/views/actuals/uploads/_skipped.html.haml new file mode 100644 index 000000000..fe6cbe06e --- /dev/null +++ b/app/views/actuals/uploads/_skipped.html.haml @@ -0,0 +1,15 @@ +%table.govuk-table#skipped + %caption.govuk-table__caption.govuk-table__caption--m + Skipped rows + %thead.govuk-table__head + %tr.govuk-table__row + %th.govuk-table__header{class: "govuk-!-width-one-third", scope: "col"} Row number + %th.govuk-table__header{class: "govuk-!-width-two-thirds", scope: "col"} RODA Identifier + %tbody.govuk-table__body + - @skipped_rows.each do |row| + + %tr.govuk-table__row{ id: "skipped-row-#{row.csv_row_number}" } + %td.govuk-table__cell{class: "govuk-!-width-one-third"} + = row.csv_row_number + %td.govuk-table__cell{class: "govuk-!-width-two-thirds"} + = row.object.roda_identifier diff --git a/app/views/actuals/uploads/update.html.haml b/app/views/actuals/uploads/update.html.haml index d3caf5a20..63257b0f4 100644 --- a/app/views/actuals/uploads/update.html.haml +++ b/app/views/actuals/uploads/update.html.haml @@ -26,9 +26,27 @@ %h1.govuk-heading-xl = t("page_title.actual.upload_success") - .govuk-grid-row - .govuk-grid-column-full - = render partial: "shared/actuals/actuals_by_activity" + - if ROLLOUT.active?(:use_new_activity_actual_refund_comment_importer) + .govuk-grid-row + .govuk-grid-column-full + = render partial: "actuals" + + .govuk-grid-row + .govuk-grid-column-full + = render partial: "refunds" + + .govuk-grid-row + .govuk-grid-column-full + = render partial: "comments" + + .govuk-grid-row + .govuk-grid-column-full + = render partial: "skipped" + + - else + .govuk-grid-row + .govuk-grid-column-full + = render partial: "shared/actuals/actuals_by_activity" .govuk-grid-row .govuk-grid-column-two-thirds diff --git a/spec/controllers/actuals/uploads_controller_spec.rb b/spec/controllers/actuals/uploads_controller_spec.rb index 288933d63..597c46663 100644 --- a/spec/controllers/actuals/uploads_controller_spec.rb +++ b/spec/controllers/actuals/uploads_controller_spec.rb @@ -128,7 +128,8 @@ allow(fake_import_file).to receive(:valid?).and_return(true) allow(CsvFileUpload).to receive(:new).and_return(fake_import_file) - importer = instance_double(Actual::Import, import: true) + importer = instance_double(Actual::Import) + allow(importer).to receive(:import).and_return(true) allow(importer).to receive(:errors).and_return([]) allow(importer).to receive(:imported_actuals).and_return([]) allow(importer).to receive(:invalid_with_comment).and_return(false) @@ -152,10 +153,13 @@ allow(fake_import_file).to receive(:valid?).and_return(true) allow(CsvFileUpload).to receive(:new).and_return(fake_import_file) - importer = instance_double(Import::Csv::ActivityActualRefundComment::FileService, import!: true) + importer = instance_double(Import::Csv::ActivityActualRefundComment::FileService) + allow(importer).to receive(:import!).and_return(true) allow(importer).to receive(:errors).and_return([]) allow(importer).to receive(:imported_actuals).and_return([]) allow(importer).to receive(:imported_refunds).and_return([]) + allow(importer).to receive(:imported_comments).and_return([]) + allow(importer).to receive(:skipped_rows).and_return([]) allow(Import::Csv::ActivityActualRefundComment::FileService).to receive(:new).and_return(importer) allow(Actual::Import).to receive(:new) diff --git a/spec/features/import/users_can_upload_actuals_refunds_and_actvitiy_comments_spec.rb b/spec/features/import/users_can_upload_actuals_refunds_and_activity_comments_spec.rb similarity index 84% rename from spec/features/import/users_can_upload_actuals_refunds_and_actvitiy_comments_spec.rb rename to spec/features/import/users_can_upload_actuals_refunds_and_activity_comments_spec.rb index 8c1549636..833898b5c 100644 --- a/spec/features/import/users_can_upload_actuals_refunds_and_actvitiy_comments_spec.rb +++ b/spec/features/import/users_can_upload_actuals_refunds_and_activity_comments_spec.rb @@ -23,6 +23,7 @@ #{project.roda_identifier} | 1 | 2020 | 20000 | 0 | #{another_project.roda_identifier} | 1 | 2020 | 0 | 30000 | Refund comment #{yet_another_project.roda_identifier} | 1 | 2020 | 0 | 0 | Activity comment + #{project.roda_identifier} | 1 | 2020 | 0 | 0 | CSV upload_csv(csv_data) end @@ -36,11 +37,25 @@ expect(page).to have_text(t("action.actual.upload.success")) - expect(page).to have_content(project.roda_identifier) - expect(page).to have_content("£20,000.00") - - expect(page).to have_content(another_project.roda_identifier) - expect(page).to have_content("-£30,000.00") + within("#actuals") do + expect(page).to have_content(project.roda_identifier) + expect(page).to have_content("£20,000.00") + end + + within("#refunds") do + expect(page).to have_content(another_project.roda_identifier) + expect(page).to have_content("-£30,000.00") + end + + within("#comments") do + expect(page).to have_content(yet_another_project.roda_identifier) + expect(page).to have_content("Activity comment") + end + + within("#skipped") do + expect(page).to have_content("5") + expect(page).to have_content(project.roda_identifier) + end end it "allows the user to see the activity comment" do diff --git a/spec/services/import/csv/activity_actual_refund_comment/file_service_spec.rb b/spec/services/import/csv/activity_actual_refund_comment/file_service_spec.rb index a6bcb080c..ee741d35d 100644 --- a/spec/services/import/csv/activity_actual_refund_comment/file_service_spec.rb +++ b/spec/services/import/csv/activity_actual_refund_comment/file_service_spec.rb @@ -33,7 +33,7 @@ it "creates the Actual and returns true" do result = subject.import! - actual = subject.imported_rows.first + actual = subject.imported_rows.first.object expect(actual.parent_activity).to be activity expect(actual.value).to eql BigDecimal("10000") @@ -73,7 +73,7 @@ it "creates the Refund and returns true" do result = subject.import! - refund = subject.imported_rows.first + refund = subject.imported_rows.first.object expect(refund.parent_activity).to eql activity expect(refund.comment.body).to eql "This is a refund comment." @@ -112,7 +112,7 @@ it "creates the Comment and returns true" do result = subject.import! - imported_comment = subject.imported_rows.first + imported_comment = subject.imported_rows.first.object expect(result).to be true @@ -157,7 +157,7 @@ it "records the details of the skipped row" do subject.import! - skipped = subject.skipped_rows.first + skipped = subject.skipped_rows.first.object expect(skipped.roda_identifier).to eql "RODA-ID" expect(skipped.financial_quarter).to eql "2" From 26573a3053fc7f30385a7c78596653c3c625c476 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 24 Jan 2024 11:06:05 +0000 Subject: [PATCH 07/10] Bump shoulda-matchers from 6.0.0 to 6.1.0 Bumps [shoulda-matchers](https://github.com/thoughtbot/shoulda-matchers) from 6.0.0 to 6.1.0. - [Release notes](https://github.com/thoughtbot/shoulda-matchers/releases) - [Changelog](https://github.com/thoughtbot/shoulda-matchers/blob/main/CHANGELOG.md) - [Commits](https://github.com/thoughtbot/shoulda-matchers/compare/v6.0.0...v6.1.0) --- updated-dependencies: - dependency-name: shoulda-matchers dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 795d5c463..e9e9b107c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -259,7 +259,7 @@ GEM mini_portile2 (2.8.5) mini_racer (0.8.0) libv8-node (~> 18.16.0.0) - minitest (5.21.1) + minitest (5.21.2) monetize (1.12.0) money (~> 6.12) money (6.16.0) @@ -433,7 +433,7 @@ GEM rubyzip (>= 1.2.2, < 3.0) websocket (~> 1.0) sexp_processor (4.16.1) - shoulda-matchers (6.0.0) + shoulda-matchers (6.1.0) activesupport (>= 5.2.0) sidekiq (6.5.12) connection_pool (>= 2.2.5, < 3) From 75f1f05a4eb4acd82bb9c03f8fc2427f11292741 Mon Sep 17 00:00:00 2001 From: Cristina <579522+CristinaRO@users.noreply.github.com> Date: Wed, 17 Jan 2024 18:04:43 +0000 Subject: [PATCH 08/10] Use transparency identifier as iati-identifier Use an activity's `transparency_identifier` as the `iati-identifier` in XML exports. If the activity has a `previous_identifier`, include it in the XML as `other-identifier`. This is the opposite of what the app was doing previously, but this change has been specifically requested by DSIT (Dan S). --- CHANGELOG.md | 1 + app/views/shared/xml/_activity.xml.haml | 4 ++-- .../beis_users_can_view_an_activity_as_xml_spec.rb | 10 ++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53dc49375..1be0b0596 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ were skipped - the Activity actual, refund and comment upload success view now shows the imported actuals, refunds, activity comments and skipped rows +- Use an activity's `transparency_identifier` as `iati-identifier` in XML exports, and the `previous_identifier`, if it exists, as `other-identifier` ## Release 143 - 2024-01-23 diff --git a/app/views/shared/xml/_activity.xml.haml b/app/views/shared/xml/_activity.xml.haml index 7ff86087c..f30d1c746 100644 --- a/app/views/shared/xml/_activity.xml.haml +++ b/app/views/shared/xml/_activity.xml.haml @@ -1,5 +1,5 @@ %iati-activity{"default-currency" => activity.default_currency, "xml:lang" => activity.organisation.language_code } - %iati-identifier= activity.iati_identifier + %iati-identifier= activity.transparency_identifier %reporting-org{"type" => reporting_organisation.organisation_type, "ref" => reporting_organisation.iati_reference } %narrative= reporting_organisation.name %title @@ -22,7 +22,7 @@ %participating-org{"ref" => organisation.iati_reference, "type" => organisation.organisation_type, "role" => 4} %narrative= organisation.name - if activity.previous_identifier? - %other-identifier{"ref" => activity.transparency_identifier, type: "A1"} + %other-identifier{"ref" => activity.previous_identifier, type: "A1"} %activity-status{"code" => activity.iati_status}/ - if activity.planned_start_date? %activity-date{"iso-date" => activity.planned_start_date, type: "1"}/ diff --git a/spec/features/beis_users_can_view_an_activity_as_xml_spec.rb b/spec/features/beis_users_can_view_an_activity_as_xml_spec.rb index 0ecf8b716..72346995c 100644 --- a/spec/features/beis_users_can_view_an_activity_as_xml_spec.rb +++ b/spec/features/beis_users_can_view_an_activity_as_xml_spec.rb @@ -24,18 +24,16 @@ ) end - it "shows the previous identifier as the activity identifier" do + it "shows the activity transparency identifier as the iati identifier" do visit organisation_activity_path(organisation, activity, format: :xml) - expect(xml.at("iati-activity/iati-identifier").text).to eq(activity.previous_identifier) + expect(xml.at("iati-activity/iati-identifier").text).to eq(activity.transparency_identifier) end - it "shows the activity transparency identifier as the other identifier" do - iati_identifier = activity.transparency_identifier - + it "shows the previous identifier as the other identifier" do visit organisation_activity_path(organisation, activity, format: :xml) - expect(xml.at("iati-activity/other-identifier/@ref").text).to eq(iati_identifier) + expect(xml.at("iati-activity/other-identifier/@ref").text).to eq(activity.previous_identifier) end end From 1c781b45d70571cce368da5d6ea414e79b6c7ba8 Mon Sep 17 00:00:00 2001 From: Cristina <579522+CristinaRO@users.noreply.github.com> Date: Wed, 17 Jan 2024 18:06:39 +0000 Subject: [PATCH 09/10] Remove unused code The `iati_identifier` method was only used to prioritise an activity's `previous_identifier`, where it existed, for inclusion in XML exports. The change in requirements means that this method is now unused, and can be removed. --- app/models/activity.rb | 8 -------- spec/models/activity_spec.rb | 12 ------------ 2 files changed, 20 deletions(-) diff --git a/app/models/activity.rb b/app/models/activity.rb index dfcdfd4db..2863b7a6a 100644 --- a/app/models/activity.rb +++ b/app/models/activity.rb @@ -511,14 +511,6 @@ def child_level end end - def iati_identifier - if previous_identifier.present? - previous_identifier - else - transparency_identifier - end - end - def iati_scope Iati::ActivityScopeService.new(benefitting_countries).call end diff --git a/spec/models/activity_spec.rb b/spec/models/activity_spec.rb index 53a8a77ff..cfdbaf5cf 100644 --- a/spec/models/activity_spec.rb +++ b/spec/models/activity_spec.rb @@ -1338,18 +1338,6 @@ end end - describe "#iati_identifier" do - it "returns the previous_identifier if it exists" do - activity = create(:project_activity, previous_identifier: "previous-id", transparency_identifier: "transparency-id") - expect(activity.iati_identifier).to eq("previous-id") - end - - it "returns the transparency_identifier if previous_identifier is not set" do - activity = create(:project_activity, previous_identifier: nil, transparency_identifier: "transparency-id") - expect(activity.iati_identifier).to eq("transparency-id") - end - end - describe "#actual_total_for_report_financial_quarter" do let(:project) { create(:project_activity) } From ad7fd9c4eef69d7505f6937aa00b9d4d14cc62af Mon Sep 17 00:00:00 2001 From: meyric Date: Wed, 24 Jan 2024 12:45:41 +0000 Subject: [PATCH 10/10] Release 144 - Change the transparency identifier and names for the DSIT organisations (DSIT and DSIT Finance) - Add a rake task to change the transparency identifier for activities that will continue under DSIT, and the providing org reference for their actual spend and forecasts - model a simple imported row so that users can see which row in the csv import were skipped - the Activity actual, refund and comment upload success view now shows the imported actuals, refunds, activity comments and skipped rows - Use an activity's `transparency_identifier` as `iati-identifier` in XML exports, and the `previous_identifier`, if it exists, as `other-identifier` --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1be0b0596..f0f6e2e98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ [Full changelog][unreleased] +## Release 144 - 2024-01-24 + +[Full changelog][144] + - Change the transparency identifier and names for the DSIT organisations (DSIT and DSIT Finance) - Add a rake task to change the transparency identifier for activities that will continue under DSIT, and the providing org reference for their actual spend and forecasts - model a simple imported row so that users can see which row in the csv import @@ -1657,7 +1661,8 @@ - Planned start and end dates are mandatory - Actual start and end dates must not be in the future -[unreleased]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-143...HEAD +[unreleased]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-144...HEAD +[144]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-143...release-144 [143]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-142...release-143 [142]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-141...release-142 [141]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-140...release-141