From 76ac52317a173d956a84d05cb75bc56b322822d7 Mon Sep 17 00:00:00 2001 From: phil-l-brockwell Date: Tue, 26 Nov 2024 15:42:50 +0000 Subject: [PATCH 1/5] Remove Add Collaborators form from Admin::FormAnswers::CompanyDetails --- .../form_answers/collaborators_controller.rb | 51 ------------ app/policies/form_answer_policy.rb | 4 - .../collaborators/_form.html.slim | 27 ------- .../collaborators/_search_results.html.slim | 13 --- .../form_answers/collaborators/create.js.slim | 6 -- .../form_answers/collaborators/search.js.slim | 6 -- .../company_details/_user_accounts.html.slim | 4 - app/views/admin/users/_form.html.slim | 10 --- config/routes.rb | 3 - .../admin/form_answers/collaborators_spec.rb | 81 ------------------- .../company_details_fulfilling_spec.rb | 2 +- 11 files changed, 1 insertion(+), 206 deletions(-) delete mode 100644 app/controllers/admin/form_answers/collaborators_controller.rb delete mode 100644 app/views/admin/form_answers/collaborators/_form.html.slim delete mode 100644 app/views/admin/form_answers/collaborators/_search_results.html.slim delete mode 100644 app/views/admin/form_answers/collaborators/create.js.slim delete mode 100644 app/views/admin/form_answers/collaborators/search.js.slim delete mode 100644 spec/features/admin/form_answers/collaborators_spec.rb diff --git a/app/controllers/admin/form_answers/collaborators_controller.rb b/app/controllers/admin/form_answers/collaborators_controller.rb deleted file mode 100644 index e78bf2536..000000000 --- a/app/controllers/admin/form_answers/collaborators_controller.rb +++ /dev/null @@ -1,51 +0,0 @@ -class Admin::FormAnswers::CollaboratorsController < Admin::BaseController - expose(:form_answer) do - FormAnswer.find(params[:form_answer_id]) - end - - expose(:user) do - User.find(params[:user_id]) - end - - expose(:search_users) do - AdminActions::SearchCollaboratorCandidates.new( - form_answer, - search_params, - ) - end - - expose(:add_collaborator_interactor) do - AdminActions::AddCollaborator.new( - form_answer, - user, - ) - end - - expose(:candidates) do - search_users.candidates - end - - def search - authorize form_answer, :can_add_collaborators_to_application? - - if search_users.valid? - search_users.run - end - end - - def create - authorize form_answer, :can_add_collaborators_to_application? - - add_collaborator_interactor.run - end - - private - - def search_params - if params[:search].present? - params.require(:search).permit( - :query, - ) - end - end -end diff --git a/app/policies/form_answer_policy.rb b/app/policies/form_answer_policy.rb index d64ea16ea..4786e0e8f 100644 --- a/app/policies/form_answer_policy.rb +++ b/app/policies/form_answer_policy.rb @@ -135,10 +135,6 @@ def can_download_original_pdf_of_application_before_deadline? record.pdf_version.present? end - def can_add_collaborators_to_application? - admin? - end - private def audit_certificate_available? diff --git a/app/views/admin/form_answers/collaborators/_form.html.slim b/app/views/admin/form_answers/collaborators/_form.html.slim deleted file mode 100644 index 3d9bd8727..000000000 --- a/app/views/admin/form_answers/collaborators/_form.html.slim +++ /dev/null @@ -1,27 +0,0 @@ -= simple_form_for :search, - url: search_admin_form_answer_collaborators_url(resource), - remote: true, - method: :get, - as: nil, - html: { class: "admin-search-collaborators-form" } do |f| - - .form-container - label.form-label for="admin-search-collaborators-query" Add collaborator - - .alert.alert-danger.hidden.js-admin-search-collaborators-error-box role="alert" - - ul.list-unstyled.list-actions.hidden.js-admin-search-collaborators-results-box - - .form-block - .row - .col-md-12 - = f.input :query, - as: :string, - label: false, - input_html: { class: "form-control", id: "admin-search-collaborators-query" }, - wrapper_html: { class: 'pull-left col-md-10 admin-search-collaborators-query' }, - placeholder: "Type part of email, first name or last name" - - .text-right - = f.submit "Search", class: "btn btn-primary pull-right" - .clear diff --git a/app/views/admin/form_answers/collaborators/_search_results.html.slim b/app/views/admin/form_answers/collaborators/_search_results.html.slim deleted file mode 100644 index f3e35fdf5..000000000 --- a/app/views/admin/form_answers/collaborators/_search_results.html.slim +++ /dev/null @@ -1,13 +0,0 @@ -- if candidates.present? - - candidates.each do |user| - li id="collaborator_candidate_#{user.id}" - = link_to "#{user.full_name} (#{user.email})", edit_admin_user_path(user) - - if user.can_be_added_to_collaborators_to_another_account? - = link_to "Add", admin_form_answer_collaborators_url(form_answer_id: form_answer.id, user_id: user), - remote: true, - method: :post, - class: "pull-right btn btn-default" - - else - br - i.cant_add_to_collaborators_message - | can not be added as linked with another account! diff --git a/app/views/admin/form_answers/collaborators/create.js.slim b/app/views/admin/form_answers/collaborators/create.js.slim deleted file mode 100644 index e236acd38..000000000 --- a/app/views/admin/form_answers/collaborators/create.js.slim +++ /dev/null @@ -1,6 +0,0 @@ -- if add_collaborator_interactor.success? - | $('.js-collaborators-list').replaceWith("#{j render('list', resource: form_answer)}"); - | $(".js-admin-search-collaborators-error-box").addClass("hidden"); - | $("#collaborator_candidate_#{user.id}").remove(); -- else - | $(".js-admin-search-collaborators-error-box").text("#{add_collaborator_interactor.errors}").removeClass("hidden"); diff --git a/app/views/admin/form_answers/collaborators/search.js.slim b/app/views/admin/form_answers/collaborators/search.js.slim deleted file mode 100644 index 267e91b09..000000000 --- a/app/views/admin/form_answers/collaborators/search.js.slim +++ /dev/null @@ -1,6 +0,0 @@ -- if search_users.valid? - | $('.js-admin-search-collaborators-results-box').html("#{j render("search_results")}").removeClass("hidden"); - | $(".js-admin-search-collaborators-error-box").addClass("hidden"); -- else - | $('.js-admin-search-collaborators-results-box').addClass("hidden"); - | $(".js-admin-search-collaborators-error-box").text("#{search_users.error.html_safe}").removeClass("hidden"); diff --git a/app/views/admin/form_answers/company_details/_user_accounts.html.slim b/app/views/admin/form_answers/company_details/_user_accounts.html.slim index bfd565ab0..84a4de18c 100644 --- a/app/views/admin/form_answers/company_details/_user_accounts.html.slim +++ b/app/views/admin/form_answers/company_details/_user_accounts.html.slim @@ -1,8 +1,4 @@ .form-group .form-container label.form-label User accounts - = render "admin/form_answers/collaborators/list" - - - if policy(resource).can_add_collaborators_to_application? - = render "admin/form_answers/collaborators/form" diff --git a/app/views/admin/users/_form.html.slim b/app/views/admin/users/_form.html.slim index d527262e3..78d3ec1dc 100644 --- a/app/views/admin/users/_form.html.slim +++ b/app/views/admin/users/_form.html.slim @@ -28,16 +28,6 @@ .panel-body = render "fields_contact_preferences", f: f - - unless action_name == "new" - .panel.panel-default[data-controller="element-focus"] - .panel-heading id="section-collaborators-header" - h2.panel-title - a.collapsed data-toggle="collapse" data-parent="#user-form-panel" href="#section-collaborators" aria-expanded="false" aria-controls="section-collaborators" data-element-focus-target="reveal" - ' Collaborators - #section-collaborators.section-collaborators.panel-collapse.collapse[aria-labelledby="section-collaborators-header" data-element-scroll-target="accordion"] - .panel-body - = render "fields_collaborators", f: f, resource: resource - br .clearfix .pull-right diff --git a/config/routes.rb b/config/routes.rb index dee13d7da..644fbbbf6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -351,9 +351,6 @@ resources :case_summaries, only: [:index] resources :draft_notes, only: [:create, :update] resources :review_corp_responsibility, only: [:create] - resources :collaborators, only: [:create], module: "form_answers" do - get :search, on: :collection - end end resource :settings, only: [:show] do diff --git a/spec/features/admin/form_answers/collaborators_spec.rb b/spec/features/admin/form_answers/collaborators_spec.rb deleted file mode 100644 index e4a2e5952..000000000 --- a/spec/features/admin/form_answers/collaborators_spec.rb +++ /dev/null @@ -1,81 +0,0 @@ -require "rails_helper" - -describe "Collaborators", ' -As a an Admin -I want to be able to add collaborators to any account -So that they can collaborate applications -' do - include ActiveJob::TestHelper - - let!(:admin) { create(:admin) } - - let!(:form_answer) do - create :form_answer, - :innovation, - :submitted - end - - let!(:account) do - form_answer.account - end - - before do - login_admin admin - visit admin_form_answer_path(form_answer) - end - - describe "Add new Collaborator" do - describe "Invalid Attempts", js: true do - describe "Attempt to add person, which is already associated with another account which has application" do - let!(:user_associated_with_another_account) do - create :user, - :completed_profile, - first_name: "Applicant with account", - role: "account_admin" - end - - let!(:another_form_answer) do - create :form_answer, - :innovation, - :submitted, - user: user_associated_with_another_account - end - - it "can't add" do - find("a[aria-controls='section-company-details']").click - - within(".admin-search-collaborators-form") do - fill_in "search[query]", with: "plicant with acc" - first("input[type='submit']").click - - within(".js-admin-search-collaborators-results-box") do - expect_to_see(user_associated_with_another_account.first_name) - expect_to_see("can not be added as linked with another account!") - expect(page).to have_no_link("Add") - end - end - end - end - end - - describe "Success Add to Collaborators", js: true do - let(:email) { generate(:email) } - let!(:user) { create(:user, email: email) } - - it "should add user to collaborators with regular role" do - find("a[aria-controls='section-company-details']").click - - within(".admin-search-collaborators-form") do - fill_in "search[query]", with: email.to_s[2..-2] - first("input[type='submit']").click - - within(".js-admin-search-collaborators-results-box") do - expect_to_see(user.email) - expect_to_see_no("can not be added as linked with another account!") - expect(page).to have_link("Add") - end - end - end - end - end -end diff --git a/spec/features/admin/form_answers/company_details_fulfilling_spec.rb b/spec/features/admin/form_answers/company_details_fulfilling_spec.rb index 42de7cfff..0b5027257 100644 --- a/spec/features/admin/form_answers/company_details_fulfilling_spec.rb +++ b/spec/features/admin/form_answers/company_details_fulfilling_spec.rb @@ -32,7 +32,7 @@ it "can see the edit buttons" do within ".company-details-forms" do - expect(page).to have_selector("input[type='submit']", count: 13) + expect(page).to have_selector("input[type='submit']", count: 12) end end end From 9a806a01528b718cb94bf4854098797826ddcb43 Mon Sep 17 00:00:00 2001 From: phil-l-brockwell Date: Wed, 27 Nov 2024 16:54:39 +0000 Subject: [PATCH 2/5] Refactor existing Interactors --- .../admin_actions/add_collaborator.rb | 102 +++++++-- .../search_collaborator_candidates.rb | 15 +- app/models/form_answer.rb | 8 + app/models/user.rb | 8 - .../admin_actions/add_collaborator_spec.rb | 197 ++++++++++++++++++ 5 files changed, 295 insertions(+), 35 deletions(-) create mode 100644 spec/interactors/admin_actions/add_collaborator_spec.rb diff --git a/app/interactors/admin_actions/add_collaborator.rb b/app/interactors/admin_actions/add_collaborator.rb index 99a6d3773..ac1a044f0 100644 --- a/app/interactors/admin_actions/add_collaborator.rb +++ b/app/interactors/admin_actions/add_collaborator.rb @@ -1,20 +1,24 @@ module AdminActions class AddCollaborator - attr_reader :form_answer, - :user, - :success, - :errors + attr_reader :account, :role, :transfer_form_answers, :new_owner_id, :collaborator, :success, :errors - def initialize(form_answer, user) - @form_answer = form_answer - @user = user + def initialize(account:, collaborator:, params:, existing_account: collaborator.account) + @account = account + @collaborator = collaborator + @existing_account = existing_account + @role = params.fetch(:role, "regular") + @transfer_form_answers = params.fetch(:transfer_form_answers, false) + @new_owner_id = params.fetch(:new_owner_id, nil) + @errors = [] end def run - if user.can_be_added_to_collaborators_to_another_account? - persist! + if account_will_be_orphaned? + @errors << "User account has active users, ownership of the account must be transferred" + elsif progressed_form_answers_will_be_orphaned? + @errors << "User has applications in progress, and there are no other users on the account to transfer them to" else - @errors = "can't be added as linked with another account!" + persist! end self @@ -24,17 +28,83 @@ def success? @success.present? end + def error_messages + @errors.join(", ") + end + private def persist! - user.role = "regular" - user.account = form_answer.account - - if user.save + ActiveRecord::Base.transaction do + transfer_collaborator! + transfer_ownership! + handle_form_answers! + delete_existing_account! @success = true - else - @errors = user.errors.full_messages.join(", ") end + rescue ActiveRecord::RecordInvalid => e + @errors << e.message + end + + def existing_account_has_other_collaborators? + return unless @existing_account + + @existing_account.collaborators_without(collaborator).any? + end + + def account_will_be_orphaned? + collaborator_is_owner? && existing_account_has_other_collaborators? && new_owner_id.blank? + end + + def collaborator_is_owner? + return unless @existing_account + + @existing_account.owner == collaborator + end + + def progressed_form_answers_will_be_orphaned? + keep_form_answers_on_original_account? && progressed_form_answers? && !existing_account_has_other_collaborators? + end + + def progressed_form_answers? + collaborator.form_answers.any? && collaborator.form_answers.any?(&:any_progress?) + end + + def transfer_form_answers? + transfer_form_answers + end + + def keep_form_answers_on_original_account? + !transfer_form_answers? + end + + def handle_form_answers! + return unless @collaborator.form_answers.any? + + if transfer_form_answers? + @collaborator.form_answers.each { |f| f.update!(account: @account) } + elsif existing_account_has_other_collaborators? + @collaborator.form_answers.each { |f| f.update!(user: @existing_account.owner) } + elsif !progressed_form_answers? + @collaborator.form_answers.each(&:destroy!) + end + end + + def transfer_ownership! + return unless @new_owner_id + + @existing_account.update!(owner_id: @new_owner_id) + end + + def transfer_collaborator! + collaborator.update!(role: role, account: account) + end + + def delete_existing_account! + return unless @existing_account + return if existing_account_has_other_collaborators? + + @existing_account.destroy! end end end diff --git a/app/interactors/admin_actions/search_collaborator_candidates.rb b/app/interactors/admin_actions/search_collaborator_candidates.rb index 766adb27c..4a41816b1 100644 --- a/app/interactors/admin_actions/search_collaborator_candidates.rb +++ b/app/interactors/admin_actions/search_collaborator_candidates.rb @@ -1,17 +1,10 @@ module AdminActions class SearchCollaboratorCandidates - attr_accessor :form_answer, - :account, - :existing_collaborators, - :candidates, - :query, - :error + attr_accessor :account, :existing_collaborators, :candidates, :query, :error - def initialize(form_answer, query = nil) - @query = query[:query] - @form_answer = form_answer - @account = form_answer.account - @existing_collaborators = account.users + def initialize(existing_collaborators:, params: {}) + @query = params[:query] + @existing_collaborators = existing_collaborators end def run diff --git a/app/models/form_answer.rb b/app/models/form_answer.rb index 7347dfd5e..cf62e80a9 100644 --- a/app/models/form_answer.rb +++ b/app/models/form_answer.rb @@ -34,6 +34,8 @@ class FormAnswer < ApplicationRecord }, } + ZERO_PROGRESS = 0.06666666666666667 + POSSIBLE_AWARDS = [ "trade", # International Trade Award "innovation", # Innovation Award @@ -341,6 +343,12 @@ def fill_progress_in_percents ((fill_progress || 0) * 100).floor.to_s + "%" end + def any_progress? + return unless fill_progress + + fill_progress > ZERO_PROGRESS + end + def performance_years case award_type when "innovation" diff --git a/app/models/user.rb b/app/models/user.rb index 2e946b64d..80919aec7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -142,14 +142,6 @@ def reset_password_period_valid? end end - def can_be_added_to_collaborators_to_another_account? - account.blank? || ( - account.present? && - form_answers.blank? && - account.form_answers.blank? - ) - end - def new_member? created_at > 3.days.ago end diff --git a/spec/interactors/admin_actions/add_collaborator_spec.rb b/spec/interactors/admin_actions/add_collaborator_spec.rb new file mode 100644 index 000000000..dba2ec71f --- /dev/null +++ b/spec/interactors/admin_actions/add_collaborator_spec.rb @@ -0,0 +1,197 @@ +require "rails_helper" + +describe AdminActions::AddCollaborator do + describe "#run" do + let(:result) { described_class.new(account:, collaborator:, params:, existing_account:).run } + let(:account) { create(:account) } + let(:collaborator) { create(:user, form_answers: form_answers, account: collaborator_account, role: "regular") } + let(:existing_account) { collaborator.account } + let(:collaborator_account) { create(:account) } + let(:collaborator_account_other_users) { [] } + let(:form_answers) { [] } + let(:params) { { role: role, transfer_form_answers: transfer_form_answers, new_owner_id: new_owner_id } } + let(:role) { "account_admin" } + let(:new_owner_id) { nil } + + context "when transfer_form_answers is true" do + let(:transfer_form_answers) { true } + + context "when the collaborator has form_answers" do + let(:form_answers) { create_list(:form_answer, 3) } + + context "when the collaborator_account has no other users" do + it "transfers the collaborator and form_answers, and the collaborator account is deleted" do + expect(result).to be_success + expect(collaborator.account).to eq(account) + expect(collaborator.role).to eq(role) + expect { collaborator_account.reload }.to raise_error(ActiveRecord::RecordNotFound) + form_answers.each { |f| expect(f.account).to eq(account) } + end + end + + context "when the collaborator_account has other users" do + let!(:collaborator_account_other_users) { create_list(:user, 2, account: collaborator_account) } + let(:new_owner_id) { collaborator_account_other_users.first.id } + + it "transfers the collaborator and form_answers, and ownership of the collaborator account is transferred" do + expect(result).to be_success + expect(collaborator.account).to eq(account) + expect(collaborator.role).to eq(role) + expect(collaborator_account.owner).to eq(collaborator_account_other_users.first) + expect(collaborator_account.users).not_to include(collaborator) + form_answers.each { |f| expect(f.account).to eq(account) } + end + end + end + end + + context "when transfer_form_answers is false" do + let(:transfer_form_answers) { false } + + context "when the existing_account is nil" do + let(:existing_account) { nil } + + it "transfers the collaborator" do + expect(result).to be_success + expect(collaborator.account).to eq(account) + expect(collaborator.role).to eq(role) + end + end + + context "when the collaborator does not have form_answers" do + let(:form_answers) { [] } + + it "transfers the collaborator and the collaborator account is deleted" do + expect(result).to be_success + expect(collaborator.account).to eq(account) + expect(collaborator.role).to eq(role) + expect { collaborator_account.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + context "when the collaborator is the owner of the account" do + before do + collaborator_account.owner = collaborator + collaborator_account.save + end + + it "transfers the collaborator and the collaborator account is deleted" do + expect(result).to be_success + expect(collaborator.account).to eq(account) + expect(collaborator.role).to eq(role) + expect { collaborator_account.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context "when the collaborator_account has other users" do + let!(:collaborator_account_other_users) { create_list(:user, 2, account: collaborator_account) } + + it "transfers the collaborator and the collaborator account is not deleted" do + expect(result).to be_success + expect(collaborator.account).to eq(account) + expect(collaborator.role).to eq(role) + expect(collaborator_account.users).to eq(collaborator_account_other_users) + end + + context "when the collaborator is the owner of the account" do + let(:new_owner_id) { collaborator_account_other_users.first.id } + + before do + collaborator_account.owner = collaborator + collaborator_account.save + end + + it "transfers the collaborator and ownership of the collaborator_account" do + expect(result).to be_success + expect(collaborator.account).to eq(account) + expect(collaborator.role).to eq(role) + expect(collaborator_account.owner).to eq(collaborator_account_other_users.first) + expect(collaborator_account.users).not_to include(collaborator) + end + + context "when the new_owner_id is not specified" do + let(:new_owner_id) { nil } + + it "is not a success" do + expect(result).not_to be_success + expect(result.error_messages) + .to eq("User account has active users, ownership of the account must be transferred") + expect(collaborator.account).to eq(collaborator_account) + end + end + + context "when an invalid new_owner_id is specified" do + let(:new_owner_id) { 1000 } + + it "is not a success" do + expect(result).not_to be_success + expect(result.error_messages) + .to eq("Validation failed: Owner Owner is empty - it is a required field and must be filled in") + expect(collaborator.reload.account).to eq(collaborator_account) + end + end + end + end + end + context "when the collaborator has form_answers" do + let(:form_answers) { create_list(:form_answer, 3) } + + context "when the collaborator_account has no other users" do + it "transfers the collaborator, and the collaborator account and form_answers are deleted" do + expect(result).to be_success + expect(collaborator.account).to eq(account) + expect(collaborator.role).to eq(role) + expect { collaborator_account.reload }.to raise_error(ActiveRecord::RecordNotFound) + form_answers.each { |f| expect { f.reload }.to raise_error(ActiveRecord::RecordNotFound) } + end + end + + context "when the form_answers are in progress" do + let(:form_answers) { create_list(:form_answer, 3, :development) } + + context "when the collaborator_account has other users" do + let!(:collaborator_account_other_users) { create_list(:user, 2, account: collaborator_account) } + + context "when the new_owner_id is specified" do + let(:new_owner_id) { collaborator_account_other_users.first.id } + + it "transfers the collaborator and ownership of the form_answers to the new owner" do + expect(result).to be_success + expect(collaborator.account).to eq(account) + expect(collaborator.role).to eq(role) + expect(collaborator_account.owner).to eq(collaborator_account_other_users.first) + form_answers.each { |f| expect(f.user).to eq(collaborator_account_other_users.first) } + expect(collaborator_account.users).not_to include(collaborator) + end + end + + context "when the new_owner_id is not specified" do + let(:new_owner_id) { nil } + + before do + collaborator_account.owner = collaborator_account_other_users.first + collaborator_account.save + end + + it "transfers the collaborator and ownership of the form_answers to the existing owner" do + expect(result).to be_success + expect(collaborator.account).to eq(account) + expect(collaborator.role).to eq(role) + expect(collaborator_account.users).not_to include(collaborator) + form_answers.each { |f| expect(f.user).to eq(collaborator_account_other_users.first) } + end + end + end + + context "when the collaborator_account has no other users" do + it "is not a success" do + expect(result).not_to be_success + expect(result.error_messages) + .to eq("User has applications in progress, and there are no other users on the account to transfer them to") + expect(collaborator.account).to eq(collaborator_account) + end + end + end + end + end + end +end From 4cdb008120a5d51c3db73eff89232dc7029e359b Mon Sep 17 00:00:00 2001 From: phil-l-brockwell Date: Mon, 9 Dec 2024 10:40:22 +0000 Subject: [PATCH 3/5] Add collaborator actions to Admin::Users#edit --- app/controllers/admin/admins_controller.rb | 5 + app/controllers/admin/assessors_controller.rb | 4 + app/controllers/admin/judges_controller.rb | 4 + .../admin/users/collaborators_controller.rb | 52 ++++++ app/controllers/admin/users_controller.rb | 4 + app/models/form_answer.rb | 9 +- app/policies/user_policy.rb | 2 +- .../users/_fields_collaborators.html.slim | 5 +- .../collaborators/_search_form.html.slim | 27 +++ .../collaborators/_search_results.html.slim | 9 + .../collaborators/_transfer_form.html.slim | 29 ++++ .../admin/users/collaborators/search.js.slim | 6 + app/views/admin/users/edit.html.slim | 10 ++ config/routes.rb | 4 + .../admin/users/collaborators_spec.rb | 155 ++++++++++++++++++ 15 files changed, 318 insertions(+), 7 deletions(-) create mode 100644 app/controllers/admin/users/collaborators_controller.rb create mode 100644 app/views/admin/users/collaborators/_search_form.html.slim create mode 100644 app/views/admin/users/collaborators/_search_results.html.slim create mode 100644 app/views/admin/users/collaborators/_transfer_form.html.slim create mode 100644 app/views/admin/users/collaborators/search.js.slim create mode 100644 spec/features/admin/users/collaborators_spec.rb diff --git a/app/controllers/admin/admins_controller.rb b/app/controllers/admin/admins_controller.rb index 10cadf06f..0120049cf 100644 --- a/app/controllers/admin/admins_controller.rb +++ b/app/controllers/admin/admins_controller.rb @@ -1,5 +1,10 @@ class Admin::AdminsController < Admin::UsersController before_action :find_resource, except: [:index, :new, :create, :login_as_assessor, :login_as_user] + + expose(:collaborators) do + nil + end + def index params[:search] ||= AdminSearch::DEFAULT_SEARCH params[:search].permit! diff --git a/app/controllers/admin/assessors_controller.rb b/app/controllers/admin/assessors_controller.rb index b232f3ddf..35720c3f2 100644 --- a/app/controllers/admin/assessors_controller.rb +++ b/app/controllers/admin/assessors_controller.rb @@ -15,6 +15,10 @@ class Admin::AssessorsController < Admin::UsersController :bulk_deactivate_dt, ] + expose(:collaborators) do + nil + end + def index params[:search] ||= AssessorSearch::DEFAULT_SEARCH params[:search].permit! diff --git a/app/controllers/admin/judges_controller.rb b/app/controllers/admin/judges_controller.rb index 558eeba3a..d0c1df729 100644 --- a/app/controllers/admin/judges_controller.rb +++ b/app/controllers/admin/judges_controller.rb @@ -1,4 +1,8 @@ class Admin::JudgesController < Admin::UsersController + expose(:collaborators) do + nil + end + def index params[:search] ||= JudgeSearch::DEFAULT_SEARCH params[:search].permit! diff --git a/app/controllers/admin/users/collaborators_controller.rb b/app/controllers/admin/users/collaborators_controller.rb new file mode 100644 index 000000000..90226c958 --- /dev/null +++ b/app/controllers/admin/users/collaborators_controller.rb @@ -0,0 +1,52 @@ +class Admin::Users::CollaboratorsController < Admin::BaseController + expose(:user) do + User.find(params[:user_id]) + end + + expose(:collaborator) do + User.find(params[:collaborator_id]) + end + + expose(:search_users) do + AdminActions::SearchCollaboratorCandidates.new(existing_collaborators: user.account.users, params: search_params) + end + + expose(:add_collaborator_interactor) do + AdminActions::AddCollaborator.new(account: user.account, collaborator:, params: create_params) + end + + expose(:candidates) do + search_users.candidates + end + + def search + authorize user, :can_add_collaborators_to_account? + search_users.run if search_users.valid? + end + + def create + authorize user, :can_add_collaborators_to_account? + + add_collaborator_interactor.run.tap do |result| + if result.success? + redirect_to edit_admin_user_path(user), notice: "#{collaborator.email} successfully added to Collaborators!" + else + redirect_to edit_admin_user_path(user), notice: "#{collaborator.email} could not be added to Collaborators: #{result.error_messages}" + end + end + end + + private + + def create_params + params.require(:user).permit(:transfer_form_answers, :new_owner_id, :role).tap do |p| + p[:transfer_form_answers] = ActiveModel::Type::Boolean.new.cast(p[:transfer_form_answers]) + end + end + + def search_params + return if params[:search].blank? + + params.require(:search).permit(:query) + end +end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 699a28d68..2ececa26b 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -1,6 +1,10 @@ class Admin::UsersController < Admin::BaseController before_action :find_resource, except: [:index, :new, :create] + expose(:collaborators) do + @resource.account.collaborators_without(@resource) + end + def index params[:search] ||= UserSearch::DEFAULT_SEARCH params[:search].permit! diff --git a/app/models/form_answer.rb b/app/models/form_answer.rb index cf62e80a9..7698a9b20 100644 --- a/app/models/form_answer.rb +++ b/app/models/form_answer.rb @@ -34,8 +34,6 @@ class FormAnswer < ApplicationRecord }, } - ZERO_PROGRESS = 0.06666666666666667 - POSSIBLE_AWARDS = [ "trade", # International Trade Award "innovation", # Innovation Award @@ -328,13 +326,16 @@ def head_of_business end def company_or_nominee_from_document - comp_attr = promotion? ? "organization_name" : "company_name" name = document[comp_attr] name = nominee_full_name_from_document if promotion? && name.blank? name = name.try(:strip) name.presence end + def comp_attr + promotion? ? "organization_name" : "company_name" + end + def nominee_full_name_from_document "#{document["nominee_info_first_name"]} #{document["nominee_info_last_name"]}".strip end @@ -346,7 +347,7 @@ def fill_progress_in_percents def any_progress? return unless fill_progress - fill_progress > ZERO_PROGRESS + document.except(comp_attr).any? || eligibility&.answers&.any? end def performance_years diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 447b9822f..d95c723ff 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -1,5 +1,5 @@ class UserPolicy < ApplicationPolicy - %w[index? update? create? show? new?].each do |method| + %w[index? update? create? show? new? can_add_collaborators_to_account?].each do |method| define_method method do admin? end diff --git a/app/views/admin/users/_fields_collaborators.html.slim b/app/views/admin/users/_fields_collaborators.html.slim index 3816ed319..96d172afd 100644 --- a/app/views/admin/users/_fields_collaborators.html.slim +++ b/app/views/admin/users/_fields_collaborators.html.slim @@ -1,6 +1,4 @@ -/ TODO collaborators new/delete, also the collaborators variable - if action_name == "edit" - - collaborators = resource.account.collaborators_without(resource) - if collaborators.any? = render "admin/collaborators/list", collaborators: collaborators - else @@ -11,3 +9,6 @@ br p.p-empty This user has not added any collaborators. br + + - if policy(resource).can_add_collaborators_to_account? + = render "admin/users/collaborators/search_form", resource: resource diff --git a/app/views/admin/users/collaborators/_search_form.html.slim b/app/views/admin/users/collaborators/_search_form.html.slim new file mode 100644 index 000000000..61b926fae --- /dev/null +++ b/app/views/admin/users/collaborators/_search_form.html.slim @@ -0,0 +1,27 @@ += simple_form_for :search, + url: search_admin_user_collaborators_url(resource), + remote: true, + method: :get, + as: nil, + html: { class: "admin-search-collaborators-form" } do |f| + + .form-container + label.form-label for="admin-search-collaborators-query" Add collaborator + + .alert.alert-danger.hidden.js-admin-search-collaborators-error-box role="alert" + + .form-block + .row + .col-md-12 + = f.input :query, + as: :string, + label: false, + input_html: { class: "form-control", id: "admin-search-collaborators-query" }, + wrapper_html: { class: 'pull-left col-md-10 admin-search-collaborators-query' }, + placeholder: "Type part of email, first name or last name" + + .text-right + = f.submit "Search", class: "btn btn-primary pull-right" + .clear + +ul.list-unstyled.list-actions.hidden.js-admin-search-collaborators-results-box diff --git a/app/views/admin/users/collaborators/_search_results.html.slim b/app/views/admin/users/collaborators/_search_results.html.slim new file mode 100644 index 000000000..d052be58a --- /dev/null +++ b/app/views/admin/users/collaborators/_search_results.html.slim @@ -0,0 +1,9 @@ +- if candidates.present? + - candidates.each do |candidate| + li.list-group-item id="user_#{candidate.id}" + p.pull-right + = link_to "#{candidate.full_name} (#{candidate.email})", edit_admin_user_path(candidate) + + br + + p= render "admin/users/collaborators/transfer_form", collaborator: candidate diff --git a/app/views/admin/users/collaborators/_transfer_form.html.slim b/app/views/admin/users/collaborators/_transfer_form.html.slim new file mode 100644 index 000000000..76fafddf8 --- /dev/null +++ b/app/views/admin/users/collaborators/_transfer_form.html.slim @@ -0,0 +1,29 @@ += simple_form_for :user, url: admin_user_collaborators_url(collaborator_id: collaborator.id), method: :post do |f| + - if collaborator&.account.collaborators_without(collaborator).any? && collaborator.account.owner == collaborator + .question-group + h3= f.label :new_owner_id, { label: "Owner", for: "new_owner_id_#{collaborator.id}" } + .row + .col-sm-6 + = f.select :new_owner_id, + collaborator.account.collaborators_without(collaborator).map { |u| [u.email, u.id] }, + { include_blank: 'Select New Owner…' }, + { class: "form-control", id: "new_owner_id_#{collaborator.id}" } + + .question-group + h3= f.label :role, { label: "Role", for: "role_#{collaborator.id}" } + .row + .col-sm-4 + = f.select :role, + User::POSSIBLE_ROLES.map { |r| [r.humanize, r] }, + { selected: collaborator.role }, + { class: "form-control", id: "role_#{collaborator.id}" } + + - if collaborator.form_answers.any? + .question-group + h3= f.label :transfer_form_answers, { label: "Transfer existing Application/s", for: "transfer_form_answers_#{collaborator.id}" } + .row + .col-sm-2 + = f.check_box :transfer_form_answers, value: true, id: "transfer_form_answers_#{collaborator.id}" + + br + = f.submit "Transfer", class: "btn btn-secondary" diff --git a/app/views/admin/users/collaborators/search.js.slim b/app/views/admin/users/collaborators/search.js.slim new file mode 100644 index 000000000..267e91b09 --- /dev/null +++ b/app/views/admin/users/collaborators/search.js.slim @@ -0,0 +1,6 @@ +- if search_users.valid? + | $('.js-admin-search-collaborators-results-box').html("#{j render("search_results")}").removeClass("hidden"); + | $(".js-admin-search-collaborators-error-box").addClass("hidden"); +- else + | $('.js-admin-search-collaborators-results-box').addClass("hidden"); + | $(".js-admin-search-collaborators-error-box").text("#{search_users.error.html_safe}").removeClass("hidden"); diff --git a/app/views/admin/users/edit.html.slim b/app/views/admin/users/edit.html.slim index fb5c4e9ad..ff353e103 100644 --- a/app/views/admin/users/edit.html.slim +++ b/app/views/admin/users/edit.html.slim @@ -3,3 +3,13 @@ = "Edit #{controller_name == "users" ? "applicant" : controller_name.singularize}" = render 'form', resource: @resource + +- if collaborators + .panel.panel-default[data-controller="element-focus"] + .panel-heading id="section-collaborators-header" + h2.panel-title + a.collapsed data-toggle="collapse" data-parent="#user-form-panel" href="#section-collaborators" aria-expanded="false" aria-controls="section-collaborators" data-element-focus-target="reveal" + ' Collaborators + #section-collaborators.section-collaborators.panel-collapse.collapse[aria-labelledby="section-collaborators-header" data-element-scroll-target="accordion"] + .panel-body + = render "fields_collaborators", resource: @resource diff --git a/config/routes.rb b/config/routes.rb index 644fbbbf6..6e93fb78e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -263,6 +263,10 @@ patch :unlock post :scan_via_debounce_api end + + resources :collaborators, only: [:create], module: :users do + get :search, on: :collection + end end resources :collaborator_deletion, only: [:destroy] diff --git a/spec/features/admin/users/collaborators_spec.rb b/spec/features/admin/users/collaborators_spec.rb new file mode 100644 index 000000000..a65bfd762 --- /dev/null +++ b/spec/features/admin/users/collaborators_spec.rb @@ -0,0 +1,155 @@ +require "rails_helper" + +describe "Collaborators", ' +As a an Admin +I want to be able to add collaborators to any account +So that they can collaborate applications +', js: true do + include ActiveJob::TestHelper + + let!(:admin) { create(:admin) } + let(:existing_user) { create(:user, :completed_profile) } + + before do + login_admin admin + visit edit_admin_user_path(existing_user) + end + + describe "Add Collaborator" do + let(:email) { generate(:email) } + let!(:user) { create(:user, email: email) } + let(:form_answers) { [] } + + context "with the Account Admin role" do + let(:role) { "Account admin" } + + before do + search_for_user(email) + transfer_user(role) + end + + it "should transfer the user with the correct role" do + expect(page).to have_content("#{email} successfully added to Collaborators!") + expect(page).to have_content("#{email} successfully added to Collaborators!") + login_as existing_user + click_link "Collaborators" + expect_to_see_user(user, role: "Admin and collaborator") + end + end + + context "with the regular role" do + let(:role) { "Regular" } + + before do + search_for_user(email) + transfer_user(role) + end + + it "should transfer the user with the correct role" do + expect(page).to have_content("#{email} successfully added to Collaborators!") + expect(page).to have_content("#{email} successfully added to Collaborators!") + login_as existing_user + click_link "Collaborators" + expect_to_see_user(user, role: "Collaborator only") + end + end + + context "with form_answers" do + let!(:form_answers) { create_list(:form_answer, 3, user: user, account: user.account) } + let(:role) { "Account admin" } + context "when transferring form_answers" do + before do + search_for_user(email) + transfer_user(role, transfer_form_answers: true) + end + + it "should transfer the form_answers and user with the correct role" do + expect(page).to have_content("#{email} successfully added to Collaborators!") + login_as existing_user, scope: :user + click_link "Collaborators" + expect_to_see_user(user, role: "Admin and collaborator") + expect(existing_user.account.reload.form_answers).to eq(form_answers) + expect(user.reload.form_answers).to eq(form_answers) + end + end + + context "when not transferring form_answers" do + context "with no other users" do + before do + search_for_user(email) + transfer_user(role, transfer_form_answers: false) + end + + it "should transfer the user with the correct role" do + expect(page).to have_content("#{email} successfully added to Collaborators!") + login_as existing_user + click_link "Collaborators" + expect_to_see_user(user, role: "Admin and collaborator") + expect(existing_user.account.reload.form_answers).to eq([]) + expect(user.reload.form_answers).to eq([]) + end + end + + context "with other users" do + let!(:other_user) { create(:user, :completed_profile, account: user.account) } + + before do + search_for_user(email) + transfer_user(role, transfer_form_answers: false, new_owner: other_user) + end + + it "should transfer the user with the correct role" do + expect(page).to have_content("#{email} successfully added to Collaborators!") + login_as existing_user + click_link "Collaborators" + expect_to_see_user(user, role: "Admin and collaborator") + expect(existing_user.account.reload.form_answers).to eq([]) + expect(user.reload.form_answers).to eq([]) + expect(other_user.reload.form_answers).to eq(form_answers) + end + end + + context "when the form_answers are in progress" do + let!(:form_answers) { create_list(:form_answer, 3, :development, user: user, account: user.account) } + + before do + search_for_user(email) + transfer_user(role, transfer_form_answers: false) + end + + it "should display an error" do + expect(page).to have_content( + "#{user.email} could not be added to Collaborators: User has applications in progress, and there are no other users on the account to transfer them to", + ) + end + end + end + end + end + + def expect_to_see_user(user, role:) + within "#user_#{user.id}" do + expect(page).to have_content(user.full_name) + expect(page).to have_content(role) + end + end + + def transfer_user(role, transfer_form_answers: false, new_owner: nil) + within(".js-admin-search-collaborators-results-box") do + select role, from: "user[role]" + select new_owner.email, from: "user[new_owner_id]" if new_owner + check "user[transfer_form_answers]" if transfer_form_answers + + click_button "Transfer" + end + end + + def search_for_user(email) + find("a[aria-controls='section-collaborators']").click + + within(".admin-search-collaborators-form") do + fill_in "search[query]", with: email.to_s[2..-2] + first("input[type='submit']").click + end + end +end From 34609443c7a63eb9cc55866a068e682d30b86628 Mon Sep 17 00:00:00 2001 From: phil-l-brockwell Date: Tue, 10 Dec 2024 17:20:05 +0000 Subject: [PATCH 4/5] Add FormAnswer.touched scope for checking progressed applications --- .../admin_actions/add_collaborator.rb | 2 +- app/models/form_answer.rb | 12 +++++------ spec/models/form_answer_spec.rb | 21 +++++++++++++++++++ 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/app/interactors/admin_actions/add_collaborator.rb b/app/interactors/admin_actions/add_collaborator.rb index ac1a044f0..3e54fea88 100644 --- a/app/interactors/admin_actions/add_collaborator.rb +++ b/app/interactors/admin_actions/add_collaborator.rb @@ -67,7 +67,7 @@ def progressed_form_answers_will_be_orphaned? end def progressed_form_answers? - collaborator.form_answers.any? && collaborator.form_answers.any?(&:any_progress?) + collaborator.form_answers.touched.any? end def transfer_form_answers? diff --git a/app/models/form_answer.rb b/app/models/form_answer.rb index 7698a9b20..fd5e556cd 100644 --- a/app/models/form_answer.rb +++ b/app/models/form_answer.rb @@ -190,6 +190,12 @@ def secondary scope :vocf_free, -> { where(award_type: %w[mobility development]) } scope :provided_estimates, -> { where("document #>> '{product_estimated_figures}' = 'yes'") } + scope :touched, -> { + joins("LEFT JOIN eligibilities ON eligibilities.form_answer_id = form_answers.id") + .where("eligibilities.id IS NOT NULL AND eligibilities.type = form_answers.award_type AND eligibilities.answers::jsonb <> '{}'::jsonb") + .or(where("(document::jsonb - 'organization_name' - 'company_name') <> '{}'::jsonb")) + } + # callbacks before_save :set_award_year, unless: :award_year before_save :set_urn @@ -344,12 +350,6 @@ def fill_progress_in_percents ((fill_progress || 0) * 100).floor.to_s + "%" end - def any_progress? - return unless fill_progress - - document.except(comp_attr).any? || eligibility&.answers&.any? - end - def performance_years case award_type when "innovation" diff --git a/spec/models/form_answer_spec.rb b/spec/models/form_answer_spec.rb index 455d3eb7e..14bba7076 100644 --- a/spec/models/form_answer_spec.rb +++ b/spec/models/form_answer_spec.rb @@ -9,6 +9,27 @@ target = FormAnswer.submitted.where("feedback_hard_copy_generated" => true).to_sql expect(target).to eq FormAnswer.hard_copy_generated("feedback").to_sql end + + describe ".touched" do + let!(:eligibility_with_answers) { create(:eligibility, answers: { anything: :something }, type: :trade) } + let!(:form_answer_with_touched_document) { create(:form_answer, document: { anything: :something }) } + + before do + create(:form_answer, document: {}) + create(:form_answer, document: { organization_name: :anything }) + create(:form_answer, document: { company_name: :anything }) + create(:eligibility, answers: {}, type: :trade) + end + + it "returns only touched form_answers" do + expect(FormAnswer.touched).to match_array( + [ + eligibility_with_answers.form_answer, + form_answer_with_touched_document, + ], + ) + end + end end describe "#unsuccessful?" do From c5d16435334d46f4cd094f19ddeba24b63458d2a Mon Sep 17 00:00:00 2001 From: phil-l-brockwell Date: Wed, 11 Dec 2024 09:44:13 +0000 Subject: [PATCH 5/5] Add warning copy to User transfer form --- app/views/admin/users/collaborators/_transfer_form.html.slim | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/views/admin/users/collaborators/_transfer_form.html.slim b/app/views/admin/users/collaborators/_transfer_form.html.slim index 76fafddf8..fb3042544 100644 --- a/app/views/admin/users/collaborators/_transfer_form.html.slim +++ b/app/views/admin/users/collaborators/_transfer_form.html.slim @@ -22,8 +22,11 @@ .question-group h3= f.label :transfer_form_answers, { label: "Transfer existing Application/s", for: "transfer_form_answers_#{collaborator.id}" } .row - .col-sm-2 + .col-sm-1 = f.check_box :transfer_form_answers, value: true, id: "transfer_form_answers_#{collaborator.id}" + .col-sm-11 + .alert.alert-warning + p Ticking this box will transfer all applications to the new account. All collaborators on the new account will have access to these applications and the associated information. br = f.submit "Transfer", class: "btn btn-secondary"