Fixed problem when entering one million items for donation

[GiovannyCordeiro]

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]

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

[dorner]

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

[cielf]

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.

[cielf]

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

[GiovannyCordeiro]

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]

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]

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]

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]

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]

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]

@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]

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]

@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.

[GiovannyCordeiro]

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]

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

[dorner]

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]

@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]

@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]

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

[github-actions[bot]]

@GiovannyCordeiro: Your PR Fixed problem when entering one million items for donation is part of today's Human Essentials production release: 2025.06.29. Thank you very much for your contribution!