Add a confirmation page to partner requests.

[cielf]

Summary

Add a confirmation step to partner requests

Justification

Non-profits are often staffed with folks with limited technical skills and sometimes make mistakes with their requests. This puts and extra burden on the essentials banks as they then need to get in contact with their partners and see if they really did intend to order 5000 rather than their usual 500 diapers. We need to add in a confirmation page summarizing the request and ask for confirmation from the partner before this request is placed to the diaper bank.

Details

There are three different ways to make a request (partners/requests/new, partners/family_requests/new, partners/individuals_request/new). The confirmation page information is the same for each of them

It will show a list of the items requested and number requested for each, along with the text "Please confirm that the above list is what you meant to request" and buttons "No, I need to change something." and "Yes, it's right"

Additionally, if a quota is in place, warn partners from accidentally making a request that is over the quota unintentionally. "Oops! You are ordering 9999999 of something, are you sure?"

Clicking "No..." will take them back to the requests page they came from so that they can adjust their ask. Clicking "Yes..." will take them to the current

Clicking "Yes" will submit the request and show the submitted page for that request, as it does now.

Things we should know

History: This was raised by Sean a long time ago as https://github.com/rubyforgood/partner/issues/354. There was even a pull request for it. It was closed but remained on the project initiatives board. CL took the backlogs for partner-facing items to the July 2022 partnerbase working group, where this was identified as one of the higher priority asks, as it does burden the essentials banks.

There has been some discussion in the partnerbase working group about whether the quota is a weekly or monthly quota. For the purposes of this issue, treat it as a per request quota of total items, as most partners only do one request for month.

Criteria for Completion

-[] Add in a confirmation page when partners issue a request, as described above -[] Add in relevant tests -[] Tests pass

[edwinthinks]

@cielf I'd love to take this one :)

[cielf]

Sounds good to me

Sent from my iPhone

On Jul 31, 2022, at 7:15 AM, Edwin Mak @.***> wrote:

 @cielf I'd love to take this one :)

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[cielf]

FYI the same functionality, but for distributions is in the backlog as well.

[github-actions[bot]]

This issue has been inactive for 251 hours (10.46 days) and will be automatically unassigned after 109 more hours (4.54 days).

[edwinthinks]

I'll need some help thinking of the UX for this

[github-actions[bot]]

This issue has been inactive for 250 hours (10.42 days) and will be automatically unassigned after 110 more hours (4.58 days).

[github-actions[bot]]

This issue has been inactive for 370 hours (15.42 days) and is past the limit of 360 hours (15.00 days) so is being unassigned.

[dorner]

This is also blocked on #3214 .

[cielf]

I'm pretty sure this is no longer blocked on #3214. Not sure what the issue was that was blocking it before -- @edwinthinks - can you clarify -- is it blocked for you, because of need of help with thinking of the UX, or blocked for anybody?

[cielf]

I don't think this is still blocked.

[lokisk1155]

I can take this one also

[dorner]

@lokisk1155 this and the other one are both pretty hefty pieces of work. I'd suggest picking one.

[lokisk1155]

@dorner cielf said that this one and #3090 are very similar so it makes sense to take both. But I can start with just one.

[cielf]

Well - it's also possible that you'll run out of time on one before finishing the other -- it's the same pattern, but @dorner is right that they are good chunks of work. They are also old enough that the odds of someone else taking the other one in the meantime is relatively low.

[lokisk1155]

@cielf @dorner Np I will start with this one because you guys are already in the thread

[github-actions[bot]]

This issue has been inactive for 248 hours (10.33 days) and will be automatically unassigned after 112 more hours (4.67 days).

[github-actions[bot]]

This issue has been inactive for 368 hours (15.33 days) and is past the limit of 360 hours (15.00 days) so is being unassigned.

[danielabar]

Is this still in a help wanted status? I was looking for a next issue to pick up, but then noticed there's an open PR from about a year ago: https://github.com/rubyforgood/human-essentials/pull/3638

[dorner]

@danielabar this is a pretty meaty change - it might make sense to grab that PR and use it as a basis for a new PR. It's pretty out of date and has a bunch of conflicts at this point.

[cielf]

(Note -- @danielabar is doing #3090 first, but I would consider this reserved for her for the next couple of weeks anyway.)

[danielabar]

I can pick up this one if no one else has started on it yet. Hopefully will be able to re-use some of #3090 .

[cielf]

You have it!

[danielabar]

Just want to make sure I'm on the right track:

Here is a screenshot for Partner Request -> Quantity:

For Partner Request -> Individual, looks like it converts "Number of Individuals" to a Quantity, although I'm not entirely sure how that's done. So the modal should display the quantity not individuals?

For the above screenshots, it's using the same partner request template to generate the modal body content.

[cielf]

That is a fine, fine question. Our founding bank has not disabled individual requests - so I'll reach out to them and see if they have an opinion. We may, at least, need to tweak the headings to make it clear.

I was leaning toward showing the number of individuals, but we are already showing them the quantity in their request view, so that's probably the right way to go. Similarly for children.

[cielf]

I wasn't able to get a definite call from the business. Let's go with showing the quantity (which is what is show in their request view, and to the banks) rather than the number of individuals.

[danielabar]

Got the confirm modal working for family request as well:

Let me know what, if any changes might be required in the wording among the 3 request types. We could potentially vary the modal heading, currently Request Confirmation.

Another possibility: I removed the sentence that was in Distribution Confirmation You are about to create a distribution for... because it wasn't in the description for this. But I could bring that back, with a different text for each request type?

Maybe something like:

1. You are about to create a quantity request
2. You are about to create a family request
3. You are about to create an individual request

[danielabar]

A separate question re: "if a quota is in place, warn partners from accidentally making a request that is over the quota unintentionally. "Oops! You are ordering 9999999 of something, are you sure?"

How do quotas work currently? I see there's a quota column on partners table, although none populated in seed data. And don't see any system tests that use this feature?

I'm unsure if this quota warning should show up somehow instead of the modal? i.e. could this be considered a separate enhancement from the confirmation modal?

[cielf]

Quotas are advisory only -- for most banks they end up corresponding to the number of items on a request, because most banks run a monthly cycle and use a monthly quota (I don't know that there are any exceptions, but I can well imagine someone using it as an annual thing instead.)

[cielf]

Let's add the additional text iff there is a non-zero quota on the partner and the total number of items in the request is over that. Maybe use the same colouration that we do when someone has had an intervening audit when editing a donation?

[cielf]

As for wording tweaks -- How about we change "Quantity" to "Total Items". I think that will drive home a bit more that, for individual and child, this is not what you entered, but the result of what you entered? I don't think we need it to be different for each request type.

[danielabar]

re: "use the same colouration that we do when someone has had an intervening audit when editing a donation"

I'm not familiar with audits, what are the steps to have the above displayed?

Also, re: "for most banks they end up corresponding to the number of items on a request, because most banks run a monthly cycle and use a monthly quota"

I'm a little unclear on this, I searched for quota in db/schema.rb and it only turns up for the partners table, but none of the partners in the seed data have this populated. I'm wondering if to see any quota related code in action requires updating a partner in the database to have a quota? Also, there doesn't seem to be a place to specify whether its a monthly or annual quota.

# db/schema.rb
create_table "partners", id: :serial, force: :cascade do |t|
  ...
  t.integer "quota"
  ...
end

[cielf]

I know.... it isn't specified whether it's monthly or annual (and so, it can only ever be info only) -- A legacy issue, I'm afraid. And yes, it's on a partner basis -- and yes, I believe there isn't any in the seed data. (Note to self, put in an issue to fix that). There is very little quota-based code. Basically just editing it and showing it in the partner view.

[cielf]

Note that we aren't going to show the quota to the partners -- just give them a nudge that their numbers seem a bit high if they happen to go over it.

Why? It is felt that if the partners know their quota, they might order to the quota.

[danielabar]

Does the partner quota apply to a per line item quantity or total request quantity?

Eg: Suppose a partner has a quota of 100. Then they place an order for 5 different items, 100 of each. So the total request would be for 500 items which is greater than the quota of 100. But each line item is within the quota of 100.

In this case, should the nudge message be displayed in the confirmation modal?

[cielf]

The comparison should be to the total items in the request, not the individual line items. (So in the case you mentioned, we would show the message)

[danielabar]

For the partner quota "oops" message, I modified the wording slightly to indicate total items rather than "something". For example, I updated a partner to have a quota of 10, then submitted a request of two items, quantity 5, and 6 so it just goes over the quota. Then they see the following, but they can still submit the request:

[cielf]

That seems right.

Now that I see i in place, I want to modify the wording slightly -- Remove the "Oops!", please. Although I think it might be more effective if it were nearer the buttons, we'll probably be ok if we use the colours in the warning message we use when someone is editing a distrubution that was scheduled in the past.

[danielabar]

With the above changes, it will look like this:

Let me know if that's about right or needs further changes?

[cielf]

Looks good to me so far!

[danielabar]

Just posting a status here (still wip), I had the family request confirmation working in an earlier commit at comment here, but after rebasing, it no longer works. It thinks the request is invalid (completely empty request) even though the request is actually valid. Will investigate further...

[cielf]

Hey @danielabar -- Just thought you should be aware of the [PACKS] line of issues if you aren't already. I'm pretty sure this will be going in well before we're ready to enable them, but we will be needing to update this as well (after the fact) to bring it in line before we do. [So many moving parts!] That probably doesn't have to be you.

[danielabar]

I'm not familiar with the PACKS feature, can you point me to the ticket(s).

For the partner confirmation, it's working and tested, just left with some refactoring to do.

[cielf]

s 4396 to 4308 plus 4501 (an overall qa pass)

[danielabar]

Update: Addressed PR feedback although the most recent commit has a test failure when the events flag is on: https://github.com/rubyforgood/human-essentials/actions/runs/10237022770/job/28319804139

Will try to enable that flag locally and see if/how that distribution test fails.

[danielabar]

Update: All the tests are passing now, although I didn't change anything from yesterday's comment when there was a failure. Wondering if someone re-ran the tests on CI and that's a known intermittent?