Fixed problem when entering one million items for donation #5155

GiovannyCordeiro · 2025-04-14T21:34:46Z

Resolves #5068

Description

Changing the logic to ensure proper functioning.

Type of change

Change event to validate before the request is sent
Implementing conditionals to set the appropriate value according to the user's selection

How Has This Been Tested?

I don't know if it's possible to run tests

cielf · 2025-04-16T13:56:31Z

LGTM functionally -- @dorner -- can you give @GiovannyCordeiro some advice on how to build tests for this?

dorner · 2025-04-16T16:28:02Z

It would have to be a system test. That should handle whatever interactions are necessary.

cielf · 2025-04-16T17:24:43Z

We don't currently have any tests around the donation confirmation process (which is why we only saw this problem during manual testing). I expect that looking at spec/system/distribution_spec.rb lines 29-56 would be instructive for how to check that a modal comes up, and to click the button.

…ories (rubyforgood#5145) * Fix miscategorized reporting category for menstrual liners * Refactor item reporting category scopes * Refactor period supply report service * Refactor diaper report service * Refactor children served report service * Fix export partners csv service spec * Fix mistake from find and replace * Fix remaining specs * Remove unrelated db schema change * Fix migration * Cleanup mistake * Refactor adult incontinence report to use reporting_category

cielf · 2025-04-20T01:55:06Z

spec/system/donation_system_spec.rb

@@ -389,6 +389,20 @@
          end.to change { Donation.count }.by(1)
        end

+        it "Remaining on the page when the user cancels a large donation", js: true do
+          select Donation::SOURCES[:misc], from: "donation_source"


I see a very minor thing with the test -- best practice is that the "it" clause reads like a sentence -- so this would be "remains on the page when the user cancels a large donation", rather than "Remaining...."

OOHH, I had no idea, that's interesting, I'll correct it Cielf!

cielf · 2025-04-20T01:57:09Z

Back to @dorner for technical comments -- I had a small quibble with the test clause.

GiovannyCordeiro · 2025-04-22T20:25:56Z

Guys, I was making the test for this issue, which basically aimed to guarantee the logic of when the JS confirmation message was rejected by the user.

I used dismiss_confirm for this test, however, even though I had the behavior of the application in the negative case in the application, the test still didn't work. I thought it was because of the implementation that was already in JS that used .each and so the capybara could lose the .preventDefault() reference, but when I did an implementation without using .each the test continued to fail even with the correct logic in the application.

I decided, after a lot of head banging, to go to another test that used accept_confirm and change it to dismiss_confirm, just to confirm that it would fail if I made this change, and what happened was that the test simply passed. Which means that in practice there is no difference between these two methods, they go to the positive case in a confirmation window sent by JS.

I don't think it's because we're using the local_cuprite driver instead of selenium, maybe that's because something in the library, which from what I've seen hasn't received a commit in a year, or if the problem is between my computer and my chair.

Could someone help me about this question please? I would really like to do an effective test for this issue!

dorner · 2025-04-22T23:13:57Z

If it's passing both ways, then there might be something in the logic that's ignoring the result of the confirm dialog.

Either answer_confirm is not fully equal to false (might it be another falsey value like ""?) or preventDefault is not returning true. Maybe add some debug logs to see what's going on.

dorner · 2025-04-22T23:16:15Z

I can't find any specification that says when preventDefault will return true. There is a separate defaultPrevented attribute on the event that you can use instead. https://dom.spec.whatwg.org/#ref-for-dom-event-preventdefault%E2%91%A2

GiovannyCordeiro · 2025-04-24T22:58:17Z

I checked the return values of .preventDefault() with isDefaultPrevented() and it does indeed return true. What happens is that the test simulation still creates a new donation by pressing cancel, even though my interaction when I click on the page doesn't create it.

Probably dismiss_confirm is ignoring the response but I don't know exactly why, I searched these days and found nothing about it. Could you give me another reference so I can look into it further, please?

dorner · 2025-04-25T19:25:17Z

I haven't found anything specific. You might need to put in some debug logs or an actual debugger session to see what Capybara is doing.

GiovannyCordeiro · 2025-05-02T21:43:21Z

Just to update the progress of the issue, I still haven't managed to define exactly why the tests don't exactly replicate the user's behavior, although when a test user actually does it works correctly.

I've even tested it in a personal project and it worked correctly, but I couldn't replicate it in the project.

cielf · 2025-05-17T17:04:35Z

@dorner : it sounds like @GiovannyCordeiro is stuck on this. Can we get a judgement call on whether it's valid to proceed without the test they are having problems with or need to send it back to the drawing board?

GiovannyCordeiro · 2025-05-20T22:24:56Z

I genuinely searched extensively for a solution, but sadly, I wasn’t able to find anything that could apply my test effectively. @cielf @dorner

dorner · 2025-06-06T19:47:54Z

@GiovannyCordeiro can you merge main into your branch? I know this is taking a while but it's been busy on my end. Once the conflicts are solved I can hopefully take a look next week.

* First pass at adding the profile fields to the seed * Added the unused fields application_data and distributor_type to ignored_columns and added comments recording the relationship betwween Partners::Profile fields and partials * First pass at expanding the fields exported to the csv * Removed contact_person as it was only used in the exprt_partners_csv_service and it makes sense to have all the logic for the csv in one place, removed accidental duplicate contact columns * Removed agency_info as it was only used in the exprt_partners_csv_service and it makes sense to have all the logic for the csv in one place * Cleaned and DRYed up definition of population and poverty served percentages * First pass at modifying the specs to check for all of the new fields in the export * Seperated expected headers and values by associated partial, added spec to verify that only columns for enabled partials are exported * Effects of adding application_data and distributor_type to unused fields * Changes made by linter * Added test to specify how a partner with an empty profile should be exported * Added partials_to_show function to the Organization model, reworked the Partner model to refer to Organization model for the partials_to_show and ALL_PARTIALS * Added organization argument to ExportPartnersCSVService so which partials to show can be determined even when no partials are present * Added tests to verify the export service works when there are no partners * Expanded partners request tests to account for new exported fields * Changes made by linter * Updated user documentation about exporting partners * Added comment explaining addition of partner_form_fields * Updated the order of columns in the partners export to match the order of fields in the partner profile form * Updated the order to columns to more closely match the order of fields in the partner profile form, renamed column headers to match fields in the form * Updated partner profile view to have same field names as edit form and export * Updated user documentation about exporting partners to reflect new column order * Removed duplicate hash key * Hardcoded expected values for tests on partner export * Split up massive base_table function * Fixed creating adding non priod supply item

…ood#5114)

… improvements (rubyforgood#5161) * change submit button class to use 'btn-success' for better visibility * comment out default button class in SimpleForm configuration * add product_drive model translation to en.yml * fix: correct button text for creating product drives * fix: remove default button class configuration in SimpleForm setup

…od#5080) * Add migration to set default distribution quantity * Set default distribution quantity in after_initialize if new record * Display default value in the form * Add tests * Remove changes to form view * Remove unecessary valid? call * Update Schema version to match main * WIP * Add Gemfile.lock to gitignore * Update values for recent addition to children_served_report_service_spec * Update values for recent addition to children_served_report_service_spec * Update values for recent addition to children_served_report_service_spec * Attempt to remove Gemfile and .gitignore from PR. * Attempt to remove Gemfile and .gitignore from PR. * Add distribution_quantity of 1 to items in test setup * Add distribution_quantity of one to kit items * Update total_people_served_with_loose_supplies_per_month to exclude kits * Merge main * Fix merge main mistake. * Add new line to fix linting error --------- Co-authored-by: CL Fisher <[email protected]>

* Partner dashboard: add 'comment and sender' to Requests in Progress * Comment is truncated to 20 chars; rest displayed via tooltip * Partner requests: add 'comment and sender' to Request History * Add specs * Refactor script for tooltip

Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.18.7 to 1.18.8. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md) - [Commits](sparklemotion/nokogiri@v1.18.7...v1.18.8) --- updated-dependencies: - dependency-name: nokogiri dependency-version: 1.18.8 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…byforgood#5192) * [rubyforgood#4922] handle invalid date ranges in date range input * Fixed issue where tabbing into date range field and typing invalid data bypassed Litepicker validation * Updated DateRangeHelper to catch parsing errors and show flash notice instead of raising 500 * Added custom client-side check in Stimulus controller to catch bad input on blur * Introduced global isLitePickerActive flag in application.js so that custom date range controller can avoid alerting user if Litepicker is open * temporarily remove invalid date range system tests to investigate CI failures * Revert "temporarily remove invalid date range system tests to investigate CI failures" This reverts commit f8ccea2. * temp changes to investigate js timing failure for system tests on ci * remove attempt to focus tests for ci failure investigation * temp ci failure investigation - run only distribution system tests which include invalid date range testing * temp investigation for ci failure - try to get to failure quickly * investigate ci failure - attempt to submit form without js having a chance to simulate invalid data getting to server * temp ci investigation - focus only on server side validation test * more ci failure investigation - how far does it get before js alert popups up * experiment to temp disable custom js validation in test to simulate server side validation * now that server side validation test passes on ci, try also with client side validation test * try with all tests * cleanup CI test failure investigation code * remove unused data test id from filter button * document disable validation option for client side JS * consolidate all js execution in test * undo changes to rspec system workflow file

* Add specs for adjusment csv export * Add service for csv exporting adjustments * Change header to comply with existing documentation * Lint the specs * Move adjustment export format tests into requests spec * Adapt adjustments specs to test csv export text and use module method * Hardcode adjustments_requests spec strings --------- Co-authored-by: Daniel Orner <[email protected]>

…rubyforgood#5206)

rubyforgood#5202) * bug-5152 Adds test coverage to transfer export, which already includes inactive items * bug-5152 cleans up rspec file to use let

…ood#5208) Co-authored-by: Amal PB <[email protected]>

Bumps [webmock](https://github.com/bblimke/webmock) from 3.24.0 to 3.25.1. - [Changelog](https://github.com/bblimke/webmock/blob/master/CHANGELOG.md) - [Commits](bblimke/webmock@v3.24.0...v3.25.1) --- updated-dependencies: - dependency-name: webmock dependency-version: 3.25.1 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [pry-doc](https://github.com/pry/pry-doc) from 1.5.0 to 1.6.0. - [Changelog](https://github.com/pry/pry-doc/blob/master/CHANGELOG.md) - [Commits](pry/pry-doc@v1.5.0...v1.6.0) --- updated-dependencies: - dependency-name: pry-doc dependency-version: 1.6.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [bootsnap](https://github.com/Shopify/bootsnap) from 1.18.4 to 1.18.6. - [Changelog](https://github.com/Shopify/bootsnap/blob/main/CHANGELOG.md) - [Commits](Shopify/bootsnap@v1.18.4...v1.18.6) --- updated-dependencies: - dependency-name: bootsnap dependency-version: 1.18.6 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [shoulda-matchers](https://github.com/thoughtbot/shoulda-matchers) from 6.4.0 to 6.5.0. - [Release notes](https://github.com/thoughtbot/shoulda-matchers/releases) - [Changelog](https://github.com/thoughtbot/shoulda-matchers/blob/main/CHANGELOG.md) - [Commits](thoughtbot/shoulda-matchers@v6.4.0...v6.5.0) --- updated-dependencies: - dependency-name: shoulda-matchers dependency-version: 6.5.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [rspec-rails](https://github.com/rspec/rspec-rails) from 7.1.0 to 8.0.0. - [Changelog](https://github.com/rspec/rspec-rails/blob/main/Changelog.md) - [Commits](rspec/rspec-rails@v7.1.0...v8.0.0) --- updated-dependencies: - dependency-name: rspec-rails dependency-version: 8.0.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [dry-struct](https://github.com/dry-rb/dry-struct) from 1.6.0 to 1.8.0. - [Release notes](https://github.com/dry-rb/dry-struct/releases) - [Changelog](https://github.com/dry-rb/dry-struct/blob/main/CHANGELOG.md) - [Commits](dry-rb/dry-struct@v1.6.0...v1.8.0) --- updated-dependencies: - dependency-name: dry-struct dependency-version: 1.8.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [dotenv-rails](https://github.com/bkeepers/dotenv) from 3.1.7 to 3.1.8. - [Release notes](https://github.com/bkeepers/dotenv/releases) - [Changelog](https://github.com/bkeepers/dotenv/blob/main/Changelog.md) - [Commits](bkeepers/dotenv@v3.1.7...v3.1.8) --- updated-dependencies: - dependency-name: dotenv-rails dependency-version: 3.1.8 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [omniauth-google-oauth2](https://github.com/zquestz/omniauth-google-oauth2) from 1.2.0 to 1.2.1. - [Release notes](https://github.com/zquestz/omniauth-google-oauth2/releases) - [Changelog](https://github.com/zquestz/omniauth-google-oauth2/blob/master/CHANGELOG.md) - [Commits](zquestz/omniauth-google-oauth2@v1.2.0...v1.2.1) --- updated-dependencies: - dependency-name: omniauth-google-oauth2 dependency-version: 1.2.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [knapsack_pro](https://github.com/KnapsackPro/knapsack_pro-ruby) from 8.1.2 to 8.3.0. - [Changelog](https://github.com/KnapsackPro/knapsack_pro-ruby/blob/main/CHANGELOG.md) - [Commits](KnapsackPro/knapsack_pro-ruby@v8.1.2...v8.3.0) --- updated-dependencies: - dependency-name: knapsack_pro dependency-version: 8.3.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>

Bumps [rails](https://github.com/rails/rails) from 8.0.1 to 8.0.2. - [Release notes](https://github.com/rails/rails/releases) - [Commits](rails/rails@v8.0.1...v8.0.2) --- updated-dependencies: - dependency-name: rails dependency-version: 8.0.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>

GiovannyCordeiro · 2025-06-06T20:55:42Z

I believe I've solved it, the conflict was quite simple, but I'm finding this PR messy with several updates, I can open another PR with the change and upload it again if you think it's necessary.

dorner · 2025-06-08T14:54:10Z

@GiovannyCordeiro as long as the actual changes are small, the commit history should be squashable. I can look at this hopefully Friday.

dorner · 2025-06-20T21:09:32Z

Long story short, I've traced this all the way to here: https://github.com/rubycdp/cuprite/blob/main/lib/capybara/cuprite/page.rb#L172

This sends a message via Chrome's DevTools Protocol which looks like this:

method: "Page.handleJavaScriptDialog", params: { accept: false}

Unfortunately, the parameter seems to be gone when it hits Chrome.

Not sure what's going on, but it's a definite low-level bug.

This doesn't seem to change if I use accept_confirm - it just uses the default for this method, which is to accept, but still doesn't pass any parameters.

dorner · 2025-06-20T21:15:04Z

@GiovannyCordeiro unfortunately I don't have time to file an issue on the Cuprite repo. They'll probably want a more easily reproducible test case than "clone Human Essentials and run this test" :) If you can come up with one, we can probably file an issue to get some help.

awwaiid · 2025-06-22T14:49:30Z

@GiovannyCordeiro ok -- the remaining failure isn't really related to what you implemented; I think everything you have is great and correct. But we'd like @dorner to take over this PR instead of merging it until we get the dialog dismissal testing sorted out. So ... you can consider your work done here! And we'll still notify you when it is actually merged. Eventually.

dorner · 2025-06-27T20:12:26Z

So the test suddenly started passing locally. I'm wondering if a recent Chome update fixed it. 😮

fix: canceling after saving 1M items

daf3730

GiovannyCordeiro force-pushed the 5068-fixing-million-donations-save branch from e871b61 to daf3730 Compare April 14, 2025 21:35

cielf requested a review from dorner April 16, 2025 14:01

coalest and others added 2 commits April 19, 2025 15:30

test: guaranteeing the annulment of large donations

14ba2c9

cielf reviewed Apr 20, 2025

View reviewed changes

fix: improving the readability of the test

385673d

fix: changing click event of entire form to button only

fc0b631

Naraveni and others added 9 commits June 6, 2025 20:47

5002 - Filter by Category Added to Donations And Purchases Page

7301599

5084 Display Drive Count on Summary page for Selected Dates (rubyforg…

a29ff70

…ood#5114)

rubyforgood#5100: Fix unknown format error on expired session

61ba5cc

Add tests

49af734

danielabar and others added 20 commits June 6, 2025 20:47

5193: Fix eventable filter to be across all time

806f58e

fixes

47e1166

Adding link to User Guide for org_admin and org_users

5582388

updating user guide to match changes

d10bbca

Fix: Includes audited and discarded storage locations in audits index (…

dd1eab9

…rubyforgood#5206)

bug-5152 Adds test coverage to transfer export, which already include… (

7d64ec7

rubyforgood#5202) * bug-5152 Adds test coverage to transfer export, which already includes inactive items * bug-5152 cleans up rspec file to use let

give new response when existing user is assigned a new role (rubyforg…

82f3aa7

…ood#5208) Co-authored-by: Amal PB <[email protected]>

Merge branch 'main' into 5068-fixing-million-donations-save

c13fa30

awwaiid assigned dorner Jun 22, 2025

Merge branch 'main' into 5068-fixing-million-donations-save

148e762

dorner approved these changes Jun 27, 2025

View reviewed changes

dorner merged commit e25b608 into rubyforgood:main Jun 27, 2025
16 of 17 checks passed

Uh oh!

Fixed problem when entering one million items for donation #5155

Fixed problem when entering one million items for donation #5155

Uh oh!

Conversation

GiovannyCordeiro commented Apr 14, 2025

Description

Type of change

How Has This Been Tested?

Uh oh!

cielf commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dorner commented Apr 16, 2025

Uh oh!

cielf commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cielf Apr 20, 2025

Choose a reason for hiding this comment

Uh oh!

GiovannyCordeiro Apr 20, 2025

Choose a reason for hiding this comment

Uh oh!

cielf commented Apr 20, 2025

Uh oh!

GiovannyCordeiro commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dorner commented Apr 22, 2025

Uh oh!

dorner commented Apr 22, 2025

Uh oh!

GiovannyCordeiro commented Apr 24, 2025

Uh oh!

dorner commented Apr 25, 2025

Uh oh!

GiovannyCordeiro commented May 2, 2025

Uh oh!

cielf commented May 17, 2025

Uh oh!

GiovannyCordeiro commented May 20, 2025

Uh oh!

dorner commented Jun 6, 2025

Uh oh!

GiovannyCordeiro commented Jun 6, 2025

Uh oh!

dorner commented Jun 8, 2025

Uh oh!

dorner commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dorner commented Jun 20, 2025

Uh oh!

awwaiid commented Jun 22, 2025

Uh oh!

dorner commented Jun 27, 2025

Uh oh!

Uh oh!

Uh oh!

cielf commented Apr 16, 2025 •

edited

Loading

cielf commented Apr 16, 2025 •

edited

Loading

GiovannyCordeiro commented Apr 22, 2025 •

edited

Loading

dorner commented Jun 20, 2025 •

edited

Loading