Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bizarre issue with individual requests #4069

Closed
3 tasks
cielf opened this issue Jan 31, 2024 · 18 comments
Closed
3 tasks

Fix bizarre issue with individual requests #4069

cielf opened this issue Jan 31, 2024 · 18 comments

Comments

@cielf
Copy link
Collaborator

cielf commented Jan 31, 2024

Summary

There is a sequence that causes the amount on the individual requests to be large and strange. See details.

Why fix?

Too many support requests from partners. Partner confusion and frustration for both the partner and bank.

Recreation of the problem

Sign in as a partner. Click on "Create request" under "# of individuals.
Fill in a number, without selecting an item.
Click submit request.
This gives you an error, AND the item selection is a text field rather than a selection field. (That's a problem, to be sure.)
Then click on dashboard, and Click on "Create request" under "# of individuals.
Select an item and provide a number. Click "submit request".
The request created will (I should say may, but I tried it a couple of times) have a value other than the one entered.

Criteria for completion

  • In the screen that is shown after you have an error on a request, the item is a selection from the available items.
  • The sequence described above produces a request with the value entered by the user
  • tests to support the above.

Image

@elasticspoon
Copy link
Collaborator

elasticspoon commented Feb 5, 2024

So I figured out the issue with the text field, but I am having some issues reproducing the request number issues.

I follow the steps: fail the first time, return and make a request for Briefs for 1 Person. My request is created for 50 briefs.
I do the same steps except I don't fail the first time: My request is created for 50 briefs.

@cielf I can't reproduce a discrepancy in those values between failing first and not failing. Could you make a video maybe?

Or by chance do the partners have confusion as to why a request for 1 individual results in an order of 50 items?

@elasticspoon
Copy link
Collaborator

That said there is also a discrepancy in the select box behavior. On the default page it shows these options partner.requestable_items.active.visible:
image

while on the error page it shows these organization.items.active.visible.sort :
image

I feel like the same values should be used in both situations, however, I am not sure which to go with.

@cielf
Copy link
Collaborator Author

cielf commented Feb 9, 2024

It should be partner. That could be a separate issue, though it might be related...

@cielf
Copy link
Collaborator Author

cielf commented Feb 9, 2024

Were you trying to recreate the bizarre numbers before or after you addresssed the text field -- could be related? Don't have time to make a video in the moment.

@elasticspoon
Copy link
Collaborator

I haven't made any changes yet. So before.

The code multiplies the number of users by a default amount per item. So if you enter 10 people and select diapers, you'll get 500 diapers cause the default is 50 per person. Just want to make sure that isn't what is causing confusion.

@cielf
Copy link
Collaborator Author

cielf commented Feb 9, 2024

Oh, no that's not the confusion. It's more like you enter 10 people and get 20470 or some other strange number. I'm not even sure it's a multiple of the number of people always.

@elasticspoon
Copy link
Collaborator

Gotcha. I could not reproduce that, so when you get a chance if you could upload a video. In the meantime I'll make a draft PR for just the input field fixes (maybe that will fix it?) and take a video of my own steps (maybe I'm doing something wrong?)

Also could the partner type matter? I was using the verified partner account.

@cielf
Copy link
Collaborator Author

cielf commented Feb 9, 2024

Yeah... I just tried it with verified and it worked. I was probably reproducing it with a copy of the production date, so I'm wondering if it might be due to something like an unset quantity on the item. (Edit -- the Adult Briefs L/XL does not have a quantity set in the seed, so that's not it...)

@cielf
Copy link
Collaborator Author

cielf commented Feb 10, 2024

I swear this happened! (I even demonstrated it to someone else). But I haven't been able to recreate it for that last half hour.

elasticspoon added a commit to elasticspoon/human-essentials that referenced this issue Feb 12, 2024
Addresses part of issue rubyforgood#4069:

After a request bad request is submitted the select
input disappears and turns into a text input.

This was caused by formatted_requestable_items not being set.

Refactors setting @formatted_requestable_items to a private method call.
Unifies all the behavior of setting requestable items.

Adds additional tests to ensure select fields have
correct values.

feat: adds TODOs
@elasticspoon
Copy link
Collaborator

I swear this happened! (I even demonstrated it to someone else). But I haven't been able to recreate it for that last half hour.

I got the rest of it fixed. So up to you as to the next steps, my PR is reviewable, I am just leaving it as a draft for if we do manage to reproduce the bug.

@cielf
Copy link
Collaborator Author

cielf commented Feb 12, 2024

Well -- if it blocks the bug from happening, that's pretty good. I will try a few more times, but we might have to concede that it's hiding well. Also -- fixing the select issue will significantly reduce the support load from the partners, I believe. So, I would say let's go ahead with reviewing your PR as a partial fix

elasticspoon added a commit to elasticspoon/human-essentials that referenced this issue Feb 12, 2024
Addresses part of issue rubyforgood#4069:

After a request bad request is submitted the select
input disappears and turns into a text input.

This was caused by formatted_requestable_items not being set.

Refactors setting @formatted_requestable_items to a private method call.
Unifies all the behavior of setting requestable items.

Adds additional tests to ensure select fields have
correct values.

feat: adds TODOs
@cielf
Copy link
Collaborator Author

cielf commented Feb 18, 2024

@elasticspoon I just noticed that if you do have an error on the individual requests page, you lose all the input. That's not happening on the quantity request page, so it may be related to the bizarre issue.

awwaiid added a commit that referenced this issue Feb 25, 2024
…ssue

fix: select fields on request pages (#4069 partial)
@cancelei
Copy link
Contributor

cancelei commented Mar 29, 2024

@cielf I was trying to reproduce the second part of the issue to complete it fully. But couldn't find it, if you remember how to do it, I would be happy to handle part 2 of this issue. To close it within a short period.

In the process, I found other potential issues related to this feature, but they are more minor and most likely not the priority. Here is a short list to collaborate with PM:

Potential Issues:

  • Exported Requests are unordered, data are not desc or asc and status is pending to fulfill. with mixed dates.

  • When generating the PDF report of a donation, the shipping cost if exists does not appear on it.

  • When inserting an invalid date, with 5 or more digits at the year position, there is no error warning and it saves the wrong date.

@cielf
Copy link
Collaborator Author

cielf commented Mar 29, 2024

I'm closing this issue. I think the fixes elasticspoon putt in effectively block the problem from happening, so it is far less of an issue.

@cielf cielf closed this as completed Mar 29, 2024
@cielf
Copy link
Collaborator Author

cielf commented Mar 29, 2024

And I'll add the items you mentioned to the input queue.

@cielf
Copy link
Collaborator Author

cielf commented Mar 29, 2024

Though the donation shouldn't have a shipping cost. We've only implemented that for distributions.

@cielf
Copy link
Collaborator Author

cielf commented Mar 29, 2024

Technically a 5 digit year is valid, just obviously wrong in context.

@cancelei
Copy link
Contributor

@cielf happy to have helped just with the research I did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants