From 61f4cded75d76def9433c4fbbe8c1a4af467b8ec Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 18 Nov 2022 15:26:31 +0100 Subject: [PATCH 01/24] poc(batch_operation): just the model and some specs and a poc for archive them without async --- app/models/batch_operation.rb | 31 +++++++++++++ app/models/dossier.rb | 3 +- app/models/instructeur.rb | 2 +- ...1118133711_create_table_batch_operation.rb | 14 ++++++ ...4105_add_batch_operation_id_to_dossiers.rb | 5 ++ ...6_add_foreign_key_to_batch_operation_id.rb | 5 ++ ...idate_foreigh_key_to_batch_operation_id.rb | 5 ++ ...eign_key_to_batch_operation_instructeur.rb | 5 ++ ...ate_key_to_batch_boperation_instructeur.rb | 5 ++ ...dd_index_to_dossiers_batch_operation_id.rb | 7 +++ db/schema.rb | 17 ++++++- spec/factories/batch_operation.rb | 4 ++ spec/models/batch_operation_spec.rb | 46 +++++++++++++++++++ spec/models/dossier_spec.rb | 5 ++ spec/models/instructeur_spec.rb | 1 + 15 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 app/models/batch_operation.rb create mode 100644 db/migrate/20221118133711_create_table_batch_operation.rb create mode 100644 db/migrate/20221118134105_add_batch_operation_id_to_dossiers.rb create mode 100644 db/migrate/20221118134156_add_foreign_key_to_batch_operation_id.rb create mode 100644 db/migrate/20221118134231_validate_foreigh_key_to_batch_operation_id.rb create mode 100644 db/migrate/20221119050905_add_foreign_key_to_batch_operation_instructeur.rb create mode 100644 db/migrate/20221119050928_add_validate_key_to_batch_boperation_instructeur.rb create mode 100644 db/migrate/20221122123721_add_index_to_dossiers_batch_operation_id.rb create mode 100644 spec/factories/batch_operation.rb create mode 100644 spec/models/batch_operation_spec.rb diff --git a/app/models/batch_operation.rb b/app/models/batch_operation.rb new file mode 100644 index 000000000..dfb977ae8 --- /dev/null +++ b/app/models/batch_operation.rb @@ -0,0 +1,31 @@ +# == Schema Information +# +# Table name: batch_operations +# +# id :bigint not null, primary key +# failed_dossier_ids :bigint default([]), not null, is an Array +# finished_at :datetime +# operation :string not null +# payload :jsonb not null +# run_at :datetime +# success_dossier_ids :bigint default([]), not null, is an Array +# created_at :datetime not null +# updated_at :datetime not null +# instructeur_id :bigint not null +# +class BatchOperation < ApplicationRecord + enum operation: { + archiver: 'archiver' + } + + has_many :dossiers, dependent: :nullify + belongs_to :instructeur + validates :operation, presence: true + + def process + case operation + when BatchOperation.operations.fetch(:archiver) + dossiers.map { |dossier| dossier.archiver!(instructeur) } + end + end +end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index bb2690aa5..95117be0f 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -35,6 +35,7 @@ # termine_close_to_expiration_notice_sent_at :datetime # created_at :datetime # updated_at :datetime +# batch_operation_id :bigint # dossier_transfer_id :bigint # groupe_instructeur_id :bigint # parent_dossier_id :bigint @@ -134,7 +135,7 @@ class Dossier < ApplicationRecord belongs_to :revision, class_name: 'ProcedureRevision', optional: false belongs_to :user, optional: true belongs_to :parent_dossier, class_name: 'Dossier', optional: true - + belongs_to :batch_operation, optional: true has_one :france_connect_information, through: :user has_one :procedure, through: :revision diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index f1490852c..170df6da7 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -18,7 +18,7 @@ class Instructeur < ApplicationRecord has_many :groupe_instructeurs, -> { order(:label) }, through: :assign_to has_many :unordered_groupe_instructeurs, through: :assign_to, source: :groupe_instructeur has_many :procedures, -> { distinct }, through: :unordered_groupe_instructeurs - + has_many :batch_operations, dependent: :nullify has_many :assign_to_with_email_notifications, -> { with_email_notifications }, class_name: 'AssignTo', inverse_of: :instructeur has_many :groupe_instructeur_with_email_notifications, through: :assign_to_with_email_notifications, source: :groupe_instructeur diff --git a/db/migrate/20221118133711_create_table_batch_operation.rb b/db/migrate/20221118133711_create_table_batch_operation.rb new file mode 100644 index 000000000..0d1b7a910 --- /dev/null +++ b/db/migrate/20221118133711_create_table_batch_operation.rb @@ -0,0 +1,14 @@ +class CreateTableBatchOperation < ActiveRecord::Migration[6.1] + def change + create_table :batch_operations do |t| + t.bigint :instructeur_id, null: false + t.string :operation, null: false + t.jsonb :payload, default: {}, null: false + t.bigint :failed_dossier_ids, array: true, default: [], null: false + t.bigint :success_dossier_ids, array: true, default: [], null: false + t.datetime :run_at + t.datetime :finished_at + t.timestamps + end + end +end diff --git a/db/migrate/20221118134105_add_batch_operation_id_to_dossiers.rb b/db/migrate/20221118134105_add_batch_operation_id_to_dossiers.rb new file mode 100644 index 000000000..c98f52c50 --- /dev/null +++ b/db/migrate/20221118134105_add_batch_operation_id_to_dossiers.rb @@ -0,0 +1,5 @@ +class AddBatchOperationIdToDossiers < ActiveRecord::Migration[6.1] + def change + add_column :dossiers, :batch_operation_id, :bigint + end +end diff --git a/db/migrate/20221118134156_add_foreign_key_to_batch_operation_id.rb b/db/migrate/20221118134156_add_foreign_key_to_batch_operation_id.rb new file mode 100644 index 000000000..7671f840e --- /dev/null +++ b/db/migrate/20221118134156_add_foreign_key_to_batch_operation_id.rb @@ -0,0 +1,5 @@ +class AddForeignKeyToBatchOperationId < ActiveRecord::Migration[6.1] + def change + add_foreign_key "dossiers", "batch_operations", validate: false + end +end diff --git a/db/migrate/20221118134231_validate_foreigh_key_to_batch_operation_id.rb b/db/migrate/20221118134231_validate_foreigh_key_to_batch_operation_id.rb new file mode 100644 index 000000000..0f7585a5e --- /dev/null +++ b/db/migrate/20221118134231_validate_foreigh_key_to_batch_operation_id.rb @@ -0,0 +1,5 @@ +class ValidateForeighKeyToBatchOperationId < ActiveRecord::Migration[6.1] + def change + validate_foreign_key "dossiers", "batch_operations" + end +end diff --git a/db/migrate/20221119050905_add_foreign_key_to_batch_operation_instructeur.rb b/db/migrate/20221119050905_add_foreign_key_to_batch_operation_instructeur.rb new file mode 100644 index 000000000..dd220dad4 --- /dev/null +++ b/db/migrate/20221119050905_add_foreign_key_to_batch_operation_instructeur.rb @@ -0,0 +1,5 @@ +class AddForeignKeyToBatchOperationInstructeur < ActiveRecord::Migration[6.1] + def change + add_foreign_key "batch_operations", "instructeurs", validate: false + end +end diff --git a/db/migrate/20221119050928_add_validate_key_to_batch_boperation_instructeur.rb b/db/migrate/20221119050928_add_validate_key_to_batch_boperation_instructeur.rb new file mode 100644 index 000000000..308a0f5b2 --- /dev/null +++ b/db/migrate/20221119050928_add_validate_key_to_batch_boperation_instructeur.rb @@ -0,0 +1,5 @@ +class AddValidateKeyToBatchBoperationInstructeur < ActiveRecord::Migration[6.1] + def change + validate_foreign_key "batch_operations", "instructeurs" + end +end diff --git a/db/migrate/20221122123721_add_index_to_dossiers_batch_operation_id.rb b/db/migrate/20221122123721_add_index_to_dossiers_batch_operation_id.rb new file mode 100644 index 000000000..bc8ca426e --- /dev/null +++ b/db/migrate/20221122123721_add_index_to_dossiers_batch_operation_id.rb @@ -0,0 +1,7 @@ +class AddIndexToDossiersBatchOperationId < ActiveRecord::Migration[6.1] + include Database::MigrationHelpers + disable_ddl_transaction! + def up + add_concurrent_index :dossiers, [:batch_operation_id] + end +end diff --git a/db/schema.rb b/db/schema.rb index 4a468c3b6..341be4d18 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,6 @@ # It's strongly recommended that you check this file into your version control system. ActiveRecord::Schema.define(version: 2022_11_30_113745) do - # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -158,6 +157,18 @@ ActiveRecord::Schema.define(version: 2022_11_30_113745) do t.index ["experts_procedure_id"], name: "index_avis_on_experts_procedure_id" end + create_table "batch_operations", force: :cascade do |t| + t.datetime "created_at", precision: 6, null: false + t.bigint "failed_dossier_ids", default: [], null: false, array: true + t.datetime "finished_at" + t.bigint "instructeur_id", null: false + t.string "operation", null: false + t.jsonb "payload", default: {}, null: false + t.datetime "run_at" + t.bigint "success_dossier_ids", default: [], null: false, array: true + t.datetime "updated_at", precision: 6, null: false + end + create_table "bill_signatures", force: :cascade do |t| t.datetime "created_at", null: false t.string "digest" @@ -307,6 +318,7 @@ ActiveRecord::Schema.define(version: 2022_11_30_113745) do t.datetime "archived_at" t.string "archived_by" t.boolean "autorisation_donnees" + t.bigint "batch_operation_id" t.datetime "brouillon_close_to_expiration_notice_sent_at" t.interval "conservation_extension", default: "PT0S" t.datetime "created_at" @@ -340,6 +352,7 @@ ActiveRecord::Schema.define(version: 2022_11_30_113745) do t.datetime "updated_at" t.integer "user_id" t.index ["archived"], name: "index_dossiers_on_archived" + t.index ["batch_operation_id"], name: "index_dossiers_on_batch_operation_id" t.index ["dossier_transfer_id"], name: "index_dossiers_on_dossier_transfer_id" t.index ["groupe_instructeur_id"], name: "index_dossiers_on_groupe_instructeur_id" t.index ["hidden_at"], name: "index_dossiers_on_hidden_at" @@ -913,6 +926,7 @@ ActiveRecord::Schema.define(version: 2022_11_30_113745) do add_foreign_key "attestations", "dossiers" add_foreign_key "avis", "dossiers" add_foreign_key "avis", "experts_procedures" + add_foreign_key "batch_operations", "instructeurs" add_foreign_key "bulk_messages_groupe_instructeurs", "bulk_messages" add_foreign_key "bulk_messages_groupe_instructeurs", "groupe_instructeurs" add_foreign_key "champs", "champs", column: "parent_id" @@ -925,6 +939,7 @@ ActiveRecord::Schema.define(version: 2022_11_30_113745) do add_foreign_key "commentaires", "instructeurs" add_foreign_key "dossier_operation_logs", "bill_signatures" add_foreign_key "dossier_transfer_logs", "dossiers" + add_foreign_key "dossiers", "batch_operations" add_foreign_key "dossiers", "dossier_transfers" add_foreign_key "dossiers", "dossiers", column: "parent_dossier_id" add_foreign_key "dossiers", "groupe_instructeurs" diff --git a/spec/factories/batch_operation.rb b/spec/factories/batch_operation.rb new file mode 100644 index 000000000..b749a6a0d --- /dev/null +++ b/spec/factories/batch_operation.rb @@ -0,0 +1,4 @@ +FactoryBot.define do + factory :batch_operation do + end +end diff --git a/spec/models/batch_operation_spec.rb b/spec/models/batch_operation_spec.rb new file mode 100644 index 000000000..48ef67537 --- /dev/null +++ b/spec/models/batch_operation_spec.rb @@ -0,0 +1,46 @@ +describe BatchOperation, type: :model do + describe 'association' do + it { is_expected.to have_many(:dossiers) } + it { is_expected.to belong_to(:instructeur) } + end + + describe 'attributes' do + subject { BatchOperation.new } + it { expect(subject.payload).to eq({}) } + it { expect(subject.failed_dossier_ids).to eq([]) } + it { expect(subject.success_dossier_ids).to eq([]) } + it { expect(subject.run_at).to eq(nil) } + it { expect(subject.finished_at).to eq(nil) } + it { expect(subject.operation).to eq(nil) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:operation) } + end + + describe 'process' do + let(:procedure) { create(:procedure, :with_instructeur) } + + subject do + create(:batch_operation, instructeur: procedure.instructeurs.first, + operation: operation, + dossiers: dossiers) + end + + context 'archive' do + + let(:operation) { BatchOperation.operations.fetch(:archiver) } + let(:dossier_accepte) { create(:dossier, :accepte, procedure: procedure) } + let(:dossier_refuse) { create(:dossier, :refuse, procedure: procedure) } + let(:dossier_classe_sans_suite) { create(:dossier, :sans_suite, procedure: procedure) } + let(:dossiers) { [dossier_accepte, dossier_refuse, dossier_classe_sans_suite] } + + it 'works' do + expect { subject.process() } + .to change { dossiers.map(&:reload).map(&:archived) } + .from(dossiers.map { false }) + .to(dossiers.map { true }) + end + end + end +end diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 1e1cff1a5..b038424ed 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -1855,6 +1855,11 @@ describe Dossier do end end + describe 'BatchOperation' do + subject { build(:dossier) } + it { is_expected.to belong_to(:batch_operation).optional } + end + private def count_for_month(processed_by_month, month) diff --git a/spec/models/instructeur_spec.rb b/spec/models/instructeur_spec.rb index c32d2d0e7..a587027e2 100644 --- a/spec/models/instructeur_spec.rb +++ b/spec/models/instructeur_spec.rb @@ -14,6 +14,7 @@ describe Instructeur, type: :model do describe 'associations' do it { is_expected.to have_and_belong_to_many(:administrateurs) } + it { is_expected.to have_many(:batch_operations) } end describe 'follow' do From b11dbafc59d13a0c3861cab961f581f0b23d863b Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 18 Nov 2022 16:59:46 +0100 Subject: [PATCH 02/24] poc(batch_operation_job): implement archive dossiers with async behaviour, lock, run_at, finished_at, failed_dossier_ids, success_dossier_ids --- app/jobs/batch_operation_job.rb | 25 +++++++++ app/models/batch_operation.rb | 21 ++++++- spec/factories/batch_operation.rb | 10 ++++ spec/jobs/batch_operation_job_spec.rb | 79 +++++++++++++++++++++++++++ spec/models/batch_operation_spec.rb | 37 +++++++------ 5 files changed, 152 insertions(+), 20 deletions(-) create mode 100644 app/jobs/batch_operation_job.rb create mode 100644 spec/jobs/batch_operation_job_spec.rb diff --git a/app/jobs/batch_operation_job.rb b/app/jobs/batch_operation_job.rb new file mode 100644 index 000000000..2f534a575 --- /dev/null +++ b/app/jobs/batch_operation_job.rb @@ -0,0 +1,25 @@ +class BatchOperationJob < ApplicationJob + # what about wrapping all of that in a transaction + # but, what about nested transaction because batch_operation.process_one(dossier) can run transaction + def perform(batch_operation, dossier) + success = true + begin + batch_operation.process_one(dossier) + dossier.update(batch_operation: nil) + rescue => error + success = false + raise error + ensure + batch_operation.reload # reload before deciding if it has been finished + batch_operation.run_at = Time.zone.now if batch_operation.called_for_first_time? + batch_operation.finished_at = Time.zone.now if batch_operation.called_for_last_time? + if success # beware to this one, will be refactored for stronger atomicity + batch_operation.success_dossier_ids.push(dossier.id) + batch_operation.failed_dossier_ids = batch_operation.failed_dossier_ids.reject { |d| d.dossier.id } + else + batch_operation.failed_dossier_ids.push(dossier.id) + end + batch_operation.save! + end + end +end diff --git a/app/models/batch_operation.rb b/app/models/batch_operation.rb index dfb977ae8..8d0dd7a22 100644 --- a/app/models/batch_operation.rb +++ b/app/models/batch_operation.rb @@ -13,6 +13,7 @@ # updated_at :datetime not null # instructeur_id :bigint not null # + class BatchOperation < ApplicationRecord enum operation: { archiver: 'archiver' @@ -22,10 +23,26 @@ class BatchOperation < ApplicationRecord belongs_to :instructeur validates :operation, presence: true - def process + def enqueue_all + Dossier.joins(:procedure) + .where(procedure: { id: instructeur.procedures.ids }) + .where(id: dossiers.ids) + .map { |dossier| BatchOperationProcessOneJob.perform_later(self, dossier) } + end + + def process_one(dossier) case operation when BatchOperation.operations.fetch(:archiver) - dossiers.map { |dossier| dossier.archiver!(instructeur) } + dossier.archiver!(instructeur) end + true + end + + def called_for_first_time? + run_at.nil? + end + + def called_for_last_time? # beware, must be reloaded first + dossiers.count.zero? end end diff --git a/spec/factories/batch_operation.rb b/spec/factories/batch_operation.rb index b749a6a0d..12e4a312f 100644 --- a/spec/factories/batch_operation.rb +++ b/spec/factories/batch_operation.rb @@ -1,4 +1,14 @@ FactoryBot.define do factory :batch_operation do + trait :archiver do + operation { BatchOperation.operations.fetch(:archiver) } + dossiers do + [ + association(:dossier, :accepte), + association(:dossier, :refuse), + association(:dossier, :sans_suite) + ] + end + end end end diff --git a/spec/jobs/batch_operation_job_spec.rb b/spec/jobs/batch_operation_job_spec.rb new file mode 100644 index 000000000..a2a2a6dd8 --- /dev/null +++ b/spec/jobs/batch_operation_job_spec.rb @@ -0,0 +1,79 @@ +describe BatchOperationJob, type: :job do + describe 'perform' do + let(:batch_operation) do + create(:batch_operation, :archiver, + options.merge(instructeur: create(:instructeur))) + end + let(:dossier_job) { batch_operation.dossiers.first } + subject { BatchOperationJob.new(batch_operation, dossier_job) } + let(:options) { {} } + + it 'just call the process one' do + expect { subject.perform_now } + .to change { dossier_job.reload.archived } + .from(false) + .to(true) + end + + it 'unlock the dossier' do + expect { subject.perform_now } + .to change { dossier_job.reload.batch_operation } + .from(batch_operation) + .to(nil) + end + + context 'when it succeed' do + it 'pushes dossier_job id to batch_operation.success_dossier_ids' do + expect { subject.perform_now } + .to change { batch_operation.success_dossier_ids } + .from([]) + .to([dossier_job.id]) + end + end + + context 'when it fails' do + it 'pushes dossier_job id to batch_operation.failed_dossier_ids' do + expect(batch_operation).to receive(:process_one).with(dossier_job).and_raise("KO") + expect { subject.perform_now }.to raise_error("KO") + expect(batch_operation.reload.failed_dossier_ids).to eq([dossier_job.id]) + end + end + + context 'when it is the first job' do + it 'sets run_at at first' do + run_at = 2.minutes.ago + Timecop.freeze(run_at) do + expect { subject.perform_now } + .to change { batch_operation.run_at } + .from(nil) + .to(run_at) + end + end + end + + context 'when it is the second job (meaning run_at was already set) but not the last' do + let(:preview_run_at) { 2.days.ago } + let(:options) { { run_at: preview_run_at } } + it 'does not change run_at' do + expect { subject.perform_now }.not_to change { batch_operation.reload.run_at } + end + end + + context 'when it is the last job' do + before do + batch_operation.dossiers + .where.not(id: dossier_job.id) + .update_all(batch_operation_id: nil) + end + it 'sets finished_at' do + finished_at = Time.zone.now + Timecop.freeze(finished_at) do + expect { subject.perform_now } + .to change { batch_operation.reload.finished_at } + .from(nil) + .to(finished_at) + end + end + end + end +end diff --git a/spec/models/batch_operation_spec.rb b/spec/models/batch_operation_spec.rb index 48ef67537..bbc64c1c3 100644 --- a/spec/models/batch_operation_spec.rb +++ b/spec/models/batch_operation_spec.rb @@ -18,28 +18,29 @@ describe BatchOperation, type: :model do it { is_expected.to validate_presence_of(:operation) } end - describe 'process' do - let(:procedure) { create(:procedure, :with_instructeur) } + describe 'enqueue_all' do + context 'given dossier_ids not in instructeur procedures' do + subject do + create(:batch_operation, :archiver, instructeur: create(:instructeur), invalid_instructeur: create(:instructeur)) + end - subject do - create(:batch_operation, instructeur: procedure.instructeurs.first, - operation: operation, - dossiers: dossiers) + it 'does not enqueues any BatchOperationProcessOneJob' do + expect { subject.enqueue_all() } + .not_to have_enqueued_job(BatchOperationProcessOneJob) + end end - context 'archive' do + context 'given dossier_ids in instructeur procedures' do + subject do + create(:batch_operation, :archiver, instructeur: create(:instructeur)) + end - let(:operation) { BatchOperation.operations.fetch(:archiver) } - let(:dossier_accepte) { create(:dossier, :accepte, procedure: procedure) } - let(:dossier_refuse) { create(:dossier, :refuse, procedure: procedure) } - let(:dossier_classe_sans_suite) { create(:dossier, :sans_suite, procedure: procedure) } - let(:dossiers) { [dossier_accepte, dossier_refuse, dossier_classe_sans_suite] } - - it 'works' do - expect { subject.process() } - .to change { dossiers.map(&:reload).map(&:archived) } - .from(dossiers.map { false }) - .to(dossiers.map { true }) + it 'enqueues as many BatchOperationProcessOneJob as dossiers_ids' do + expect { subject.enqueue_all() } + .to have_enqueued_job(BatchOperationProcessOneJob) + .with(subject, subject.dossiers.first) + .with(subject, subject.dossiers.second) + .with(subject, subject.dossiers.third) end end end From 7df86c50fbc5efac3f58ca52fd6f1a7ce4337249 Mon Sep 17 00:00:00 2001 From: Martin Date: Sat, 19 Nov 2022 06:03:59 +0100 Subject: [PATCH 03/24] poc(batch_operations_controller): implement simple [not yet with procedure_presentation] action to create a batch operation --- .../batch_operations_controller.rb | 34 ++++++++++++++ app/jobs/batch_operation_enqueue_all_job.rb | 6 +++ ....rb => batch_operation_process_one_job.rb} | 2 +- app/models/batch_operation.rb | 1 + config/routes.rb | 2 + .../batch_operations_controller_spec.rb | 44 +++++++++++++++++++ spec/factories/batch_operation.rb | 14 +++--- ...> batch_operation_process_one_job_spec.rb} | 4 +- 8 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 app/controllers/instructeurs/batch_operations_controller.rb create mode 100644 app/jobs/batch_operation_enqueue_all_job.rb rename app/jobs/{batch_operation_job.rb => batch_operation_process_one_job.rb} (95%) create mode 100644 spec/controllers/instructeurs/batch_operations_controller_spec.rb rename spec/jobs/{batch_operation_job_spec.rb => batch_operation_process_one_job_spec.rb} (94%) diff --git a/app/controllers/instructeurs/batch_operations_controller.rb b/app/controllers/instructeurs/batch_operations_controller.rb new file mode 100644 index 000000000..722bdacdb --- /dev/null +++ b/app/controllers/instructeurs/batch_operations_controller.rb @@ -0,0 +1,34 @@ +module Instructeurs + class BatchOperationsController < ApplicationController + before_action :set_procedure + before_action :ensure_ownership! + + def create + ActiveRecord::Base.transaction do + batch_operation = BatchOperation.create!(batch_operation_params.merge(instructeur: current_instructeur)) + BatchOperationEnqueueAllJob.perform_later(batch_operation) + end + redirect_back(fallback_location: instructeur_procedure_url(@procedure.id)) + end + + private + + def batch_operation_params + params.require(:batch_operation) + .permit(:operation, dossier_ids: []).tap do |params| + # TODO: filter dossiers_ids out of instructeurs.dossiers.ids + end + end + + def set_procedure + @procedure = Procedure.find(params[:procedure_id]) + end + + def ensure_ownership! + if !current_instructeur.procedures.exists?(@procedure.id) + flash[:alert] = "Vous n’avez pas accès à cette démarche" + redirect_to root_path + end + end + end +end diff --git a/app/jobs/batch_operation_enqueue_all_job.rb b/app/jobs/batch_operation_enqueue_all_job.rb new file mode 100644 index 000000000..2edc6d62b --- /dev/null +++ b/app/jobs/batch_operation_enqueue_all_job.rb @@ -0,0 +1,6 @@ +class BatchOperationEnqueueAllJob < ApplicationJob + def perform(batch_operation) + batch_operation.enqueue_all + end +end + diff --git a/app/jobs/batch_operation_job.rb b/app/jobs/batch_operation_process_one_job.rb similarity index 95% rename from app/jobs/batch_operation_job.rb rename to app/jobs/batch_operation_process_one_job.rb index 2f534a575..dd6e3abdd 100644 --- a/app/jobs/batch_operation_job.rb +++ b/app/jobs/batch_operation_process_one_job.rb @@ -1,4 +1,4 @@ -class BatchOperationJob < ApplicationJob +class BatchOperationProcessOneJob < ApplicationJob # what about wrapping all of that in a transaction # but, what about nested transaction because batch_operation.process_one(dossier) can run transaction def perform(batch_operation, dossier) diff --git a/app/models/batch_operation.rb b/app/models/batch_operation.rb index 8d0dd7a22..11eb1e32a 100644 --- a/app/models/batch_operation.rb +++ b/app/models/batch_operation.rb @@ -21,6 +21,7 @@ class BatchOperation < ApplicationRecord has_many :dossiers, dependent: :nullify belongs_to :instructeur + validates :operation, presence: true def enqueue_all diff --git a/config/routes.rb b/config/routes.rb index d83c877e3..d745204a6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -414,6 +414,8 @@ Rails.application.routes.draw do get 'telecharger_pjs' => 'dossiers#telecharger_pjs' end end + + resources :batch_operations, only: [:create] end end end diff --git a/spec/controllers/instructeurs/batch_operations_controller_spec.rb b/spec/controllers/instructeurs/batch_operations_controller_spec.rb new file mode 100644 index 000000000..b21fabb27 --- /dev/null +++ b/spec/controllers/instructeurs/batch_operations_controller_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +describe Instructeurs::BatchOperationsController, type: :controller do + let(:instructeur) { create(:instructeur) } + let(:procedure) { create(:procedure, :published, :for_individual, instructeurs: [instructeur]) } + let!(:dossier) { create(:dossier, :en_construction, :with_individual, procedure: procedure) } + + describe '#POST create' do + before { sign_in(instructeur.user) } + + context 'ACL' do + subject { post :create, params: { procedure_id: create(:procedure).id } } + before { sign_in(instructeur.user) } + it 'fails when procedure does not belongs to instructeur' do + expect(subject).to have_http_status(302) + end + end + + context 'success' do + let(:params) do + { + procedure_id: procedure.id, + batch_operation: { + operation: BatchOperation.operations.fetch(:archiver), + dossier_ids: [ dossier.id ] + } + } + end + subject { post :create, params: params } + before { sign_in(instructeur.user) } + it 'creates a batch operation for our signed in instructeur' do + expect { subject }.to change { instructeur.batch_operations.count }.by(1) + expect(BatchOperation.first.dossiers).to include(dossier) + end + it 'created a batch operation contains dossiers' do + subject + expect(BatchOperation.first.dossiers).to include(dossier) + end + it 'enqueues a BatchOperationJob' do + expect {subject}.to have_enqueued_job(BatchOperationEnqueueAllJob).with(BatchOperation.last) + end + end + end +end diff --git a/spec/factories/batch_operation.rb b/spec/factories/batch_operation.rb index 12e4a312f..3c5bcb068 100644 --- a/spec/factories/batch_operation.rb +++ b/spec/factories/batch_operation.rb @@ -1,12 +1,16 @@ FactoryBot.define do factory :batch_operation do + transient do + invalid_instructeur { nil } + end trait :archiver do operation { BatchOperation.operations.fetch(:archiver) } - dossiers do - [ - association(:dossier, :accepte), - association(:dossier, :refuse), - association(:dossier, :sans_suite) + after(:build) do |batch_operation, _evaluator| + procedure = create(:procedure, instructeurs: [_evaluator.invalid_instructeur.presence || batch_operation.instructeur]) + batch_operation.dossiers = [ + build(:dossier, :accepte, procedure: procedure), + build(:dossier, :refuse, procedure: procedure), + build(:dossier, :sans_suite, procedure: procedure) ] end end diff --git a/spec/jobs/batch_operation_job_spec.rb b/spec/jobs/batch_operation_process_one_job_spec.rb similarity index 94% rename from spec/jobs/batch_operation_job_spec.rb rename to spec/jobs/batch_operation_process_one_job_spec.rb index a2a2a6dd8..8e59a9857 100644 --- a/spec/jobs/batch_operation_job_spec.rb +++ b/spec/jobs/batch_operation_process_one_job_spec.rb @@ -1,11 +1,11 @@ -describe BatchOperationJob, type: :job do +describe BatchOperationProcessOneJob, type: :job do describe 'perform' do let(:batch_operation) do create(:batch_operation, :archiver, options.merge(instructeur: create(:instructeur))) end let(:dossier_job) { batch_operation.dossiers.first } - subject { BatchOperationJob.new(batch_operation, dossier_job) } + subject { BatchOperationProcessOneJob.new(batch_operation, dossier_job) } let(:options) { {} } it 'just call the process one' do From beb39027d0969970217f6b27ce359b97eb14dc53 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 21 Nov 2022 16:32:17 +0100 Subject: [PATCH 04/24] poc(batch_operation.ui): implement simple ui to trigger a batch of current page --- app/assets/stylesheets/table.scss | 3 + .../dossiers/batch_operation_component.rb | 23 +++ .../batch_operation_component.en.yml | 3 + .../batch_operation_component.fr.yml | 3 + .../batch_operation_component.html.haml | 5 + .../controllers/batch_operation_controller.ts | 48 +++++ app/javascript/entrypoints/main.css | 1 + app/jobs/batch_operation_enqueue_all_job.rb | 1 - app/models/batch_operation.rb | 16 ++ .../procedures/_dossier_actions.html.haml | 2 +- .../instructeurs/procedures/show.html.haml | 165 ++++++++++-------- .../batch_operation_component_spec.rb | 25 +++ .../batch_operations_controller_spec.rb | 4 +- 13 files changed, 223 insertions(+), 76 deletions(-) create mode 100644 app/components/dossiers/batch_operation_component.rb create mode 100644 app/components/dossiers/batch_operation_component/batch_operation_component.en.yml create mode 100644 app/components/dossiers/batch_operation_component/batch_operation_component.fr.yml create mode 100644 app/components/dossiers/batch_operation_component/batch_operation_component.html.haml create mode 100644 app/javascript/controllers/batch_operation_controller.ts create mode 100644 spec/components/dossiers/batch_operation_component_spec.rb diff --git a/app/assets/stylesheets/table.scss b/app/assets/stylesheets/table.scss index 6c955a29e..8f835d163 100644 --- a/app/assets/stylesheets/table.scss +++ b/app/assets/stylesheets/table.scss @@ -51,3 +51,6 @@ } } } +.force-table-100{ + width: calc(100vw); +} diff --git a/app/components/dossiers/batch_operation_component.rb b/app/components/dossiers/batch_operation_component.rb new file mode 100644 index 000000000..37944a08d --- /dev/null +++ b/app/components/dossiers/batch_operation_component.rb @@ -0,0 +1,23 @@ +class Dossiers::BatchOperationComponent < ApplicationComponent + attr_reader :statut, :procedure + + def initialize(statut:, procedure:) + @statut = statut + @procedure = procedure + end + + def render? + @statut == 'traites' + end + + def available_operations + options = [] + case @statut + when 'traites' then + options.push [t(".operations.archiver"), BatchOperation.operations.fetch(:archiver)] + else + end + + options + end +end diff --git a/app/components/dossiers/batch_operation_component/batch_operation_component.en.yml b/app/components/dossiers/batch_operation_component/batch_operation_component.en.yml new file mode 100644 index 000000000..c4c095f35 --- /dev/null +++ b/app/components/dossiers/batch_operation_component/batch_operation_component.en.yml @@ -0,0 +1,3 @@ +fr: + operations: + archiver: 'Archive the selection' diff --git a/app/components/dossiers/batch_operation_component/batch_operation_component.fr.yml b/app/components/dossiers/batch_operation_component/batch_operation_component.fr.yml new file mode 100644 index 000000000..1605608d1 --- /dev/null +++ b/app/components/dossiers/batch_operation_component/batch_operation_component.fr.yml @@ -0,0 +1,3 @@ +fr: + operations: + archiver: 'Archiver la sélection' diff --git a/app/components/dossiers/batch_operation_component/batch_operation_component.html.haml b/app/components/dossiers/batch_operation_component/batch_operation_component.html.haml new file mode 100644 index 000000000..5e156162d --- /dev/null +++ b/app/components/dossiers/batch_operation_component/batch_operation_component.html.haml @@ -0,0 +1,5 @@ += form_for(BatchOperation.new, url: Rails.application.routes.url_helpers.instructeur_batch_operations_path(procedure_id: procedure.id), method: :post, id: dom_id(BatchOperation.new), html: { class: 'flex justify-end' }, data: { "batch-operation-target" => "form"}) do |form| + .flex.align-center + - available_operations.each do |opt| + = form.submit opt[0], class: "fr-btn", disabled: :disabled, name: "#{form.object_name}[operation]", data: { "batch-operation-target" => "submit", "submitter-operation" => opt[1]} + diff --git a/app/javascript/controllers/batch_operation_controller.ts b/app/javascript/controllers/batch_operation_controller.ts new file mode 100644 index 000000000..d1b10fe69 --- /dev/null +++ b/app/javascript/controllers/batch_operation_controller.ts @@ -0,0 +1,48 @@ +import { ApplicationController } from './application_controller'; + +export class BatchOperationController extends ApplicationController { + static targets = ['input', 'all', 'submit', 'form']; + + declare readonly submit: HTMLFormElement; + declare readonly submit: HTMLInputElement; + declare readonly allTarget: HTMLInputElement; + declare readonly inputTargets: HTMLInputElement[]; + + connect() { + this.formTarget.addEventListener( + 'submit', + this.interceptFormSubmit.bind(this) + ); + } + + // DSFR recommends a or