Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CPDNPQ-2270] port rake file and service from ECF (bulk script for revert to pending) #2025

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions app/services/one_off/bulk_change_applications_to_pending.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# frozen_string_literal: true

module OneOff
class BulkChangeApplicationsToPending
attr_reader :application_ecf_ids

def initialize(application_ecf_ids:)
@application_ecf_ids = application_ecf_ids
end

def run!(dry_run: true)
result = {}
ActiveRecord::Base.transaction do
result = application_ecf_ids.each_with_object({}) do |application_ecf_id, hash|
application = Application.find_by(ecf_id: application_ecf_id)
revert_to_pending = Applications::RevertToPending.new(application:, change_status_to_pending: "yes")
success = application && revert_to_pending.revert
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super minor but RevertToPending already validates presence of application ?

hash[application_ecf_id] = outcome(success, application, revert_to_pending.errors)
end

raise ActiveRecord::Rollback if dry_run
end

result
end

private

def outcome(success, application, errors)
return "Not found" if application.nil?
return "Changed to pending" if success

errors.map(&:message).join(", ")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errors.full_messages.to_sentence ?

end
end
end
35 changes: 35 additions & 0 deletions lib/tasks/bulk_change_applications_to_pending.rake
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# frozen_string_literal: true

require "rake"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be needed?


namespace :one_off do
# Bulk change NPQ applications to pending.
# Accepts a CSV of application (ecf) IDs (without a header row) and a dry run
# flag. If the dry run flag is true, or not set, the changes will not be performed
# but the changes that would be made will be logged. Set dry run to false
# to commit the changes.
#
# Example usage (dry run):
# bundle exec rake 'one_off:bulk_change_to_pending[applications.csv]'
#
# Example usage (perform change):
# bundle exec rake 'one_off:bulk_change_to_pending[applications.csv,false]'
desc "Change NPQ applications to pending"
task :bulk_change_to_pending, %i[file dry_run] => :environment do |_task, args|
logger = Logger.new($stdout)
csv_file_path = args[:file]
dry_run = args[:dry_run] != "false"
unless File.exist?(csv_file_path)
logger.error "File not found: #{csv_file_path}"
return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exit(1) ?

end

application_ecf_ids = CSV.read(csv_file_path, headers: false).flatten

logger.info "Bulk changing #{application_ecf_ids.size} applications to pending#{' (dry run)' if dry_run}..."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sleep(10) - gives the developer time to think 'oops' and press ctrl-c ? Might be belt and braces though, there's already a dry run feature? thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one might be overkill.
I'd expect the developer will already run this with dry-run first, and the effort in creating/uploading the CSV file in the first place means it shouldn't be a trivial thing to run accidentally.

result = OneOff::BulkChangeApplicationsToPending.new(application_ecf_ids:).run!(dry_run:)

logger.info JSON.pretty_generate(result)
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
require "rails_helper"

Rails.application.load_tasks

RSpec.describe "one_off:bulk_change_to_pending" do
let(:csv_file) { Tempfile.new }

before do
create(:application, :accepted)
create(:application, :accepted)
csv_file.write(Application.all.pluck(:ecf_id).join("\n"))
csv_file.rewind
end

after do
Rake::Task["one_off:bulk_change_to_pending"].reenable
end

context "when dry run not specified" do
subject { Rake::Task["one_off:bulk_change_to_pending"].invoke(csv_file.path) }

it "does not update any applications" do
subject
expect(Application.all.pluck(:lead_provider_approval_status)).to eq %w[accepted accepted]
end
end

context "when dry run true" do
subject { Rake::Task["one_off:bulk_change_to_pending"].invoke(csv_file.path, "true") }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subject(:run_task)


it "does not update any applications" do
subject
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_task ? Clarifies what subject is doing ?

expect(Application.all.pluck(:lead_provider_approval_status)).to eq %w[accepted accepted]
end
end

context "when dry run false" do
subject { Rake::Task["one_off:bulk_change_to_pending"].invoke(csv_file.path, "false") }

it "updates applications to pending" do
subject
expect(Application.all.pluck(:lead_provider_approval_status)).to eq %w[pending pending]
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
require "rails_helper"

RSpec.describe OneOff::BulkChangeApplicationsToPending do
let(:application_ecf_ids) { [application.ecf_id] }
let(:instance) { described_class.new(application_ecf_ids:) }

describe "#run!" do
let(:dry_run) { false }

subject(:run) { instance.run!(dry_run:) }

RSpec.shared_examples "changes to pending" do |initial_state|
it { expect { run }.to(change { application.reload.lead_provider_approval_status }.from(initial_state).to("pending")) }
it { expect(run[application.ecf_id]).to eq("Changed to pending") }
end

RSpec.shared_examples "does not change to pending" do |result|
it { expect { run }.not_to(change { application.reload.lead_provider_approval_status }) }
it { expect(run[application.ecf_id]).to match(result) }
end

context "when there is an accepted application" do
let(:application) { create(:application, :accepted) }

it_behaves_like "changes to pending", "accepted"
end

context "when there is a rejected application" do
let(:application) { create(:application, :rejected) }

it_behaves_like "changes to pending", "rejected"
end

%i[submitted voided ineligible].each do |state|
context "when the application has #{state} declarations" do
let(:declaration) { create(:declaration, state) }

context "when the application is accepted" do
let(:application) { declaration.application }

it_behaves_like "changes to pending", "accepted"
end

context "when the application is rejected" do
let(:application) do
declaration.application.tap do |application|
application.update!(lead_provider_approval_status: "rejected")
end
end

it_behaves_like "changes to pending", "rejected"
end
end
end

context "when the application is already pending" do
let(:application) { create(:application, :pending) }

it_behaves_like "does not change to pending", /lead provider approval status is not Accepted/
end

context "when the application doesn't exist" do
let(:application_ecf_id) { SecureRandom.uuid }
let(:application_ecf_ids) { [application_ecf_id] }

it { expect(run[application_ecf_id]).to eq("Not found") }
end

Declaration.states.keys.excluding("submitted", "voided", "ineligible").each do |state|
context "when the application has #{state} declarations" do
let(:declaration) { create(:declaration, state) }
let(:application) { declaration.application }

it_behaves_like "does not change to pending", /There are already declarations for this participant/
end
end

context "when dry_run is true" do
let(:dry_run) { true }

let(:application) { create(:application, :accepted) }

it { expect { run }.not_to(change { application.reload.lead_provider_approval_status }) }
it { expect(run[application.ecf_id]).to eq("Changed to pending") }
end
end
end
Loading