diff --git a/Gemfile b/Gemfile index 52f4200cf0..7a48096758 100644 --- a/Gemfile +++ b/Gemfile @@ -14,7 +14,7 @@ gem "pg", "~> 1.6.3" # Web server. gem "puma" # Rails web framework. -gem "rails", "~> 8.0.2" +gem "rails", "~> 8.1.0" ###### MODELS / DATABASE ####### diff --git a/Gemfile.lock b/Gemfile.lock index efaaca3764..9f574469f4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -2,29 +2,31 @@ GEM remote: https://rubygems.org/ specs: Ascii85 (2.0.1) - actioncable (8.0.2.1) - actionpack (= 8.0.2.1) - activesupport (= 8.0.2.1) + action_text-trix (2.1.19) + railties + actioncable (8.1.3.1) + actionpack (= 8.1.3.1) + activesupport (= 8.1.3.1) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (8.0.2.1) - actionpack (= 8.0.2.1) - activejob (= 8.0.2.1) - activerecord (= 8.0.2.1) - activestorage (= 8.0.2.1) - activesupport (= 8.0.2.1) + actionmailbox (8.1.3.1) + actionpack (= 8.1.3.1) + activejob (= 8.1.3.1) + activerecord (= 8.1.3.1) + activestorage (= 8.1.3.1) + activesupport (= 8.1.3.1) mail (>= 2.8.0) - actionmailer (8.0.2.1) - actionpack (= 8.0.2.1) - actionview (= 8.0.2.1) - activejob (= 8.0.2.1) - activesupport (= 8.0.2.1) + actionmailer (8.1.3.1) + actionpack (= 8.1.3.1) + actionview (= 8.1.3.1) + activejob (= 8.1.3.1) + activesupport (= 8.1.3.1) mail (>= 2.8.0) rails-dom-testing (~> 2.2) - actionpack (8.0.2.1) - actionview (= 8.0.2.1) - activesupport (= 8.0.2.1) + actionpack (8.1.3.1) + actionview (= 8.1.3.1) + activesupport (= 8.1.3.1) nokogiri (>= 1.8.5) rack (>= 2.2.4) rack-session (>= 1.0.1) @@ -32,42 +34,43 @@ GEM rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) - actiontext (8.0.2.1) - actionpack (= 8.0.2.1) - activerecord (= 8.0.2.1) - activestorage (= 8.0.2.1) - activesupport (= 8.0.2.1) + actiontext (8.1.3.1) + action_text-trix (~> 2.1.15) + actionpack (= 8.1.3.1) + activerecord (= 8.1.3.1) + activestorage (= 8.1.3.1) + activesupport (= 8.1.3.1) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (8.0.2.1) - activesupport (= 8.0.2.1) + actionview (8.1.3.1) + activesupport (= 8.1.3.1) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - activejob (8.0.2.1) - activesupport (= 8.0.2.1) + activejob (8.1.3.1) + activesupport (= 8.1.3.1) globalid (>= 0.3.6) - activemodel (8.0.2.1) - activesupport (= 8.0.2.1) - activerecord (8.0.2.1) - activemodel (= 8.0.2.1) - activesupport (= 8.0.2.1) + activemodel (8.1.3.1) + activesupport (= 8.1.3.1) + activerecord (8.1.3.1) + activemodel (= 8.1.3.1) + activesupport (= 8.1.3.1) timeout (>= 0.4.0) - activestorage (8.0.2.1) - actionpack (= 8.0.2.1) - activejob (= 8.0.2.1) - activerecord (= 8.0.2.1) - activesupport (= 8.0.2.1) + activestorage (8.1.3.1) + actionpack (= 8.1.3.1) + activejob (= 8.1.3.1) + activerecord (= 8.1.3.1) + activesupport (= 8.1.3.1) marcel (~> 1.0) - activesupport (8.0.2.1) + activesupport (8.1.3.1) base64 - benchmark (>= 0.3) bigdecimal concurrent-ruby (~> 1.0, >= 1.3.1) connection_pool (>= 2.2.5) drb i18n (>= 1.6, < 2) + json logger (>= 1.4.2) minitest (>= 5.1) securerandom (>= 0.3) @@ -112,7 +115,6 @@ GEM nokogiri (~> 1, >= 1.10.8) base64 (0.3.0) bcrypt (3.1.22) - benchmark (0.5.0) better_errors (2.10.1) erubi (>= 1.0.0) rack (>= 0.9.0) @@ -547,20 +549,20 @@ GEM rack (>= 1.3) rackup (2.3.1) rack (>= 3) - rails (8.0.2.1) - actioncable (= 8.0.2.1) - actionmailbox (= 8.0.2.1) - actionmailer (= 8.0.2.1) - actionpack (= 8.0.2.1) - actiontext (= 8.0.2.1) - actionview (= 8.0.2.1) - activejob (= 8.0.2.1) - activemodel (= 8.0.2.1) - activerecord (= 8.0.2.1) - activestorage (= 8.0.2.1) - activesupport (= 8.0.2.1) + rails (8.1.3.1) + actioncable (= 8.1.3.1) + actionmailbox (= 8.1.3.1) + actionmailer (= 8.1.3.1) + actionpack (= 8.1.3.1) + actiontext (= 8.1.3.1) + actionview (= 8.1.3.1) + activejob (= 8.1.3.1) + activemodel (= 8.1.3.1) + activerecord (= 8.1.3.1) + activestorage (= 8.1.3.1) + activesupport (= 8.1.3.1) bundler (>= 1.15.0) - railties (= 8.0.2.1) + railties (= 8.1.3.1) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) actionview (>= 5.0.1.rc1) @@ -577,13 +579,14 @@ GEM rails-html-sanitizer (1.7.0) loofah (~> 2.25) nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) - railties (8.0.2.1) - actionpack (= 8.0.2.1) - activesupport (= 8.0.2.1) + railties (8.1.3.1) + actionpack (= 8.1.3.1) + activesupport (= 8.1.3.1) irb (~> 1.13) rackup (>= 1.0.0) rake (>= 12.2) thor (~> 1.0, >= 1.2.2) + tsort (>= 0.2) zeitwerk (~> 2.6) rainbow (3.1.1) rake (13.4.2) @@ -838,7 +841,7 @@ DEPENDENCIES pry-remote puma rack-mini-profiler - rails (~> 8.0.2) + rails (~> 8.1.0) rails-controller-testing rails-erd recaptcha diff --git a/app/controllers/kits_controller.rb b/app/controllers/kits_controller.rb index ec23122ba5..3718320ce5 100644 --- a/app/controllers/kits_controller.rb +++ b/app/controllers/kits_controller.rb @@ -19,6 +19,20 @@ def new @kit.line_items.build end + def validate + @kit = current_organization.kits.new(kit_params) + @kit.line_items.combine! + @kit.valid? + @kit.errors.add(:base, "At least one item is required") if @kit.line_items.empty? + + if @kit.errors.none? + body = render_to_string(template: "kits/validate", formats: [:html], layout: false) + render json: {valid: true, body: body} + else + render json: {valid: false} + end + end + def create kit_creation = KitCreateService.new(organization_id: current_organization.id, kit_params: kit_params) kit_creation.call diff --git a/app/javascript/controllers/confirmation_controller.js b/app/javascript/controllers/confirmation_controller.js index bb04894b7f..ebb4056d65 100644 --- a/app/javascript/controllers/confirmation_controller.js +++ b/app/javascript/controllers/confirmation_controller.js @@ -15,6 +15,10 @@ import { Controller } from "@hotwired/stimulus" * If the user clicks the "Yes..." button from the modal, it submits the form. * If the user clicks the "No..." button from the modal, it closes and user remains on the same url. + * + * The button that opened the modal is remembered and passed back into requestSubmit, so that any other + * Stimulus controller composed onto the same form (e.g. duplicate-items) still sees it as `event.submitter` + * when the form is eventually (re)submitted. */ export default class extends Controller { static targets = [ @@ -29,6 +33,8 @@ export default class extends Controller { openModal(event) { event.preventDefault(); + this.submitter = event.currentTarget; + const formData = new FormData(this.formTarget); const formObject = this.buildNestedObject(formData); @@ -49,7 +55,7 @@ export default class extends Controller { this.modalTarget.innerHTML = data.body; $(this.modalTarget).modal("show"); } else { - this.formTarget.requestSubmit(); + this.formTarget.requestSubmit(this.submitter); } }) .catch((error) => { @@ -57,7 +63,7 @@ export default class extends Controller { // In this case, just submit the form as if the user had clicked Save. // NICE TO HAVE: Send to bugsnag but need to install/configure https://www.npmjs.com/package/@bugsnag/js console.log(`=== ConfirmationController ERROR ${error}`); - this.formTarget.requestSubmit(); + this.formTarget.requestSubmit(this.submitter); }); } @@ -106,6 +112,6 @@ export default class extends Controller { $(this.modalTarget).find('#modalYes').prop('disabled', true); $(this.modalTarget).find('#modalNo').prop('disabled', true); $(this.modalTarget).modal("hide"); - this.formTarget.requestSubmit(); + this.formTarget.requestSubmit(this.submitter); } } diff --git a/app/javascript/controllers/duplicate_items_controller.js b/app/javascript/controllers/duplicate_items_controller.js index f341a20c61..e2b2e7632b 100644 --- a/app/javascript/controllers/duplicate_items_controller.js +++ b/app/javascript/controllers/duplicate_items_controller.js @@ -6,10 +6,10 @@ export default class extends Controller { connect() { this.boundHandleSubmit = this.handleSubmit.bind(this) this.element.addEventListener("submit", this.boundHandleSubmit) - + // Disable Rails UJS for this form to prevent "Saving" state this.element.removeAttribute('data-remote') - + // Remove data-disable-with from all submit buttons const buttons = this.element.querySelectorAll('input[type="submit"], button[type="submit"]') buttons.forEach(button => { @@ -23,9 +23,9 @@ export default class extends Controller { if (!this.itemSubmitButtonTargets.includes(submitter)) return event.preventDefault() - + const duplicates = this.findDuplicates() - + if (duplicates.length > 0) { this.showModal(duplicates, submitter.name) } else { @@ -100,10 +100,10 @@ export default class extends Controller { document.getElementById('duplicateItemsModal')?.remove() document.body.insertAdjacentHTML('beforeend', modalHtml) - + const modal = new bootstrap.Modal(document.getElementById('duplicateItemsModal')) modal.show() - + document.getElementById('confirmMerge').addEventListener('click', () => { this.mergeAndSubmit(duplicates, buttonName) }) @@ -115,29 +115,29 @@ export default class extends Controller { // Separate the first entry from remaining entries const [firstEntry, ...remainingEntries] = item.entries - + // Update the first entry with the merged total firstEntry.section.querySelector('input[name*="[quantity]"]').value = total - + // Remove all duplicate entries from the form submission remainingEntries.forEach(entry => entry.section.remove()) }) const modal = new bootstrap.Modal(document.getElementById('duplicateItemsModal')) modal.hide() - + this.submitForm(buttonName) } submitForm(buttonName) { this.element.removeEventListener('submit', this.boundHandleSubmit) - + const input = document.createElement('input') input.type = 'hidden' input.name = buttonName input.value = '1' this.element.appendChild(input) - + this.element.submit() } } diff --git a/app/views/kits/_form.html.erb b/app/views/kits/_form.html.erb index f8cd43b4f1..e50b12a1ae 100644 --- a/app/views/kits/_form.html.erb +++ b/app/views/kits/_form.html.erb @@ -1,4 +1,4 @@ -<%= simple_form_for @kit, remote: request.xhr?, data: { controller: "form-input duplicate-items" }, html: { class: 'form-horizontal' } do |f| %> +<%= simple_form_for @kit, remote: request.xhr?, data: { controller: "form-input duplicate-items", confirmation_target: "form" }, html: { class: 'form-horizontal' } do |f| %>
@@ -34,7 +34,10 @@

NB: You will not be able to change the composition of the kit once saved. Partner visibility and name can be changed via the kit's item.

- <%= submit_button({}, {"duplicate-items-target": "itemSubmitButton"}) %> + <%= submit_button({}, { + "duplicate-items-target": "itemSubmitButton", + action: "click->confirmation#openModal" + }) %>
@@ -42,3 +45,14 @@
<% end %> + +<%# Confirmation modal: See confirmation_controller.js for how this gets displayed %> +<%# and app/controllers/kits_controller.rb#validate for how it gets populated. %> + diff --git a/app/views/kits/new.html.erb b/app/views/kits/new.html.erb index d13a03006d..63f6102d05 100644 --- a/app/views/kits/new.html.erb +++ b/app/views/kits/new.html.erb @@ -20,5 +20,7 @@ -<%= render 'form' %> +
+ <%= render 'form' %> +
<%= render partial: "barcode_items/barcode_modal" %> diff --git a/app/views/kits/validate.html.erb b/app/views/kits/validate.html.erb new file mode 100644 index 0000000000..ea03e1c437 --- /dev/null +++ b/app/views/kits/validate.html.erb @@ -0,0 +1,37 @@ +
+
+
+
Kit Creation Confirmation
+ +
+
+

You are about to create a kit named + <%= @kit.name %> + with value + $<%= format("%.2f", @kit.value_in_dollars) %> +

+ + + + + + + + + <% @kit.line_items.each do |line_item| %> + + + + + <% end %> + +
Item NameQuantity
<%= line_item.name %><%= line_item.quantity %>
+

Please confirm that this is the correct composition of the kit. Note: You will + not be able to edit the items contained in the kit.

+
+
+ + +
+
+
diff --git a/config/routes.rb b/config/routes.rb index 74bf981a79..44b200303c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -187,6 +187,9 @@ def set_up_flipper end resources :kits do + collection do + post :validate + end member do get :allocations post :allocate diff --git a/spec/requests/distributions_requests_spec.rb b/spec/requests/distributions_requests_spec.rb index 39c58c4b1f..c429032754 100644 --- a/spec/requests/distributions_requests_spec.rb +++ b/spec/requests/distributions_requests_spec.rb @@ -665,7 +665,7 @@ storage_location_id: location.id, 'issued_at(1i)' => issued_at.to_date.year, 'issued_at(2i)' => issued_at.to_date.month, - 'issued_at(3i)' => nil # day part of date missing + 'issued_at(3i)' => '' # day part of date missing }} end diff --git a/spec/requests/kit_requests_spec.rb b/spec/requests/kit_requests_spec.rb index 0f346a487d..dc6f704674 100644 --- a/spec/requests/kit_requests_spec.rb +++ b/spec/requests/kit_requests_spec.rb @@ -19,6 +19,64 @@ end end + describe "POST #validate" do + let(:item) { create(:item, organization: organization) } + + it "returns valid and the confirmation modal body for a valid kit" do + post validate_kits_url(format: :json), params: { + kit: { + name: "A new kit", + value_in_dollars: "10.10", + line_items_attributes: {"0": {item_id: item.id, quantity: 5}} + } + } + + expect(response).to be_successful + json = JSON.parse(response.body) + expect(json["valid"]).to eq(true) + expect(json["body"]).to include("A new kit") + expect(json["body"]).to include("$10.10") + expect(json["body"]).to include(item.name) + end + + it "returns invalid when the name is blank" do + post validate_kits_url(format: :json), params: { + kit: { + name: "", + value_in_dollars: "10.10", + line_items_attributes: {"0": {item_id: item.id, quantity: 5}} + } + } + + expect(response).to be_successful + json = JSON.parse(response.body) + expect(json["valid"]).to eq(false) + expect(json["body"]).to be_nil + end + + it "returns invalid when there are no line items" do + post validate_kits_url(format: :json), params: { + kit: {name: "A new kit", value_in_dollars: "10.10"} + } + + expect(response).to be_successful + json = JSON.parse(response.body) + expect(json["valid"]).to eq(false) + end + + it "does not persist the kit" do + expect { + post validate_kits_url(format: :json), params: { + kit: { + name: "A new kit", + value_in_dollars: "10.10", + line_items_attributes: {"0": {item_id: item.id, quantity: 5}} + } + } + }.not_to change(Kit, :count) + end + end + describe "GET #index" do before do # this shouldn't be shown diff --git a/spec/requests/requests_requests_spec.rb b/spec/requests/requests_requests_spec.rb index af085e1e91..471b61f04a 100644 --- a/spec/requests/requests_requests_spec.rb +++ b/spec/requests/requests_requests_spec.rb @@ -196,7 +196,7 @@ get requests_path(fulfilled_request) page = Nokogiri::HTML(response.body) - cancel_button = page.at_css('button') { |el| el.text.strip == 'Cancel' } + cancel_button = page.at_css('button[data-disable-with="Please wait..."], input[value="Cancel"]') expect(cancel_button).not_to be_present end diff --git a/spec/services/calendar_service_spec.rb b/spec/services/calendar_service_spec.rb index 6dd7f598b7..7dbbb11958 100644 --- a/spec/services/calendar_service_spec.rb +++ b/spec/services/calendar_service_spec.rb @@ -92,7 +92,7 @@ ["Newfoundland -03:30", "America/St_Johns"]] result = described_class.time_zones - expect(result.size).to eq(151) + expect(result.size).to eq(152) expect(result[0..11]).to eq(expected) end end diff --git a/spec/system/account_request_system_spec.rb b/spec/system/account_request_system_spec.rb index d34dd86673..4350df53bf 100644 --- a/spec/system/account_request_system_spec.rb +++ b/spec/system/account_request_system_spec.rb @@ -35,12 +35,14 @@ expect(AccountRequest.count).to eq(0) - expect { click_button 'Submit' }.to change(AccountRequest, :count).by(1) + click_button 'Submit' - created_account_request = AccountRequest.last - - # Request Received + # Wait for the redirect before asserting on the count, otherwise the + # async form submission may not have been processed yet. expect(page).to have_content('Request Received!') + expect(AccountRequest.count).to eq(1) + + created_account_request = AccountRequest.last expect(page).to have_content("We've sent you a email with instructions on next steps at #{created_account_request.email}!") # Access link within email they would have received diff --git a/spec/system/kit_system_spec.rb b/spec/system/kit_system_spec.rb index 224daf0a53..18f8f6190f 100644 --- a/spec/system/kit_system_spec.rb +++ b/spec/system/kit_system_spec.rb @@ -47,11 +47,48 @@ click_button "Save" + expect(page).to have_css("#kitConfirmationModal", visible: true) + within "#kitConfirmationModal" do + expect(page).to have_content("Kit Creation Confirmation") + expect(find(:element, "data-testid": "kit-confirmation-name")).to have_text(kit_traits[:name]) + expect(find(:element, "data-testid": "kit-confirmation-value")).to have_text("$10.10") + expect(page).to have_content(item.name) + expect(page).to have_content(quantity_per_kit.to_s) + expect(page).to have_css(".text-danger", text: "not") + click_button "Yes, it's correct" + end + expect(page.find(".alert")).to have_content "Kit created successfully" expect(page).to have_content(kit_traits[:name]) expect(page).to have_content("#{quantity_per_kit} #{item.name}") end + it "returns to the form with values intact when declining the confirmation" do + visit new_kit_path + kit_traits = attributes_for(:kit) + + fill_in "Name", with: kit_traits[:name] + find(:css, '#kit_value_in_dollars').set('10.10') + + item = Item.last + quantity_per_kit = 5 + select item.name, from: "kit_line_items_attributes_0_item_id" + find(:css, '#kit_line_items_attributes_0_quantity').set(quantity_per_kit) + + click_button "Save" + + expect(page).to have_css("#kitConfirmationModal", visible: true) + within "#kitConfirmationModal" do + click_button "No, I need to make changes" + end + + expect(page).to have_no_css("#kitConfirmationModal", visible: true) + expect(page).to have_current_path(new_kit_path) + expect(page).to have_no_content("Kit created successfully") + expect(page).to have_field("Name", with: kit_traits[:name]) + expect(page).to have_field("kit_line_items_attributes_0_quantity", with: quantity_per_kit.to_s) + end + it "can add items correctly" do visit new_kit_path new_barcode = "1234567890" @@ -210,7 +247,6 @@ describe "when missing required fields" do it "displays error indicating missing field and preserves filled out fields" do visit new_kit_path - kit_traits = attributes_for(:kit) find(:css, '#kit_value_in_dollars').set('10.10') @@ -221,8 +257,9 @@ click_button "Save" + expect(page).to have_no_css("#kitConfirmationModal", visible: true) expect(page.find(".alert")).to have_content "Name can't be blank" - expect(page).to have_content(kit_traits[:quantity]) + expect(page).to have_field("kit_line_items_attributes_0_quantity", with: quantity_per_kit.to_s) expect(page).to have_content(item.name) end end @@ -253,10 +290,15 @@ fill_in quantity_input[:id], with: "15" end - # Try to save - should trigger duplicate detection modal + # Try to save - the confirmation preview appears first click_button "Save" - # JavaScript modal should appear + expect(page).to have_css("#kitConfirmationModal", visible: true) + within "#kitConfirmationModal" do + click_button "Yes, it's correct" + end + + # Confirming then triggers duplicate detection expect(page).to have_css("#duplicateItemsModal", visible: true) expect(page).to have_content("Multiple Item Entries Detected") expect(page).to have_content("Merge Items") diff --git a/spec/system/partner_system_spec.rb b/spec/system/partner_system_spec.rb index 2e525e4136..d4c9d1d9fe 100644 --- a/spec/system/partner_system_spec.rb +++ b/spec/system/partner_system_spec.rb @@ -766,9 +766,10 @@ def post_refresh visit partners_path click_on 'Groups' - assert page.has_content? existing_partner_group.name, wait: page_content_wait - - click_on 'Edit' + within '#nav-partner-groups', wait: page_content_wait do + assert page.has_content? existing_partner_group.name + click_on 'Edit' + end post_refresh end