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

[PACKS] #10 Add custom request units to Request export #4405

Open
3 tasks
cielf opened this issue May 30, 2024 · 6 comments · May be fixed by #4608
Open
3 tasks

[PACKS] #10 Add custom request units to Request export #4405

cielf opened this issue May 30, 2024 · 6 comments · May be fixed by #4608

Comments

@cielf
Copy link
Collaborator

cielf commented May 30, 2024

Summary

Request export for banks includes custom request units, if applicable.

Why

This is step #9 of adding the ability to specify "packs" versus "individual" for requests

Details

If any items for the organization have customized units

Instead of having one column per item, have one column per item/unit combination. (eg. Kids (Newborn) and Kids (Newborn) - packs)

This is based on the request units available on the item, rather than whether the unit has ever been requested.

Visual Aid

Image

N.B.

All of the changes for PACKS must be implemented behind a flipper flag "enable_packs"
1/ Flipper works by enabling or disabling a tag (for the PACKS issues, that is enable_packs)
2/ Here is a code snippet illustrating how to use it in your code, with enable_packs as the example tag:
if Flipper.enabled?(:enable_packs)
// do the thing we are guarding with the tag
end
3/ How to check out if it works manually (with the example tag: enable_packs ):
You have to enable the flipper tag on your localhost (note - the tag is stored in your db, so if you reset your db you have to do it again)

localhost:3000/flipper

userid: admin
password: password
Sign In

Click: Add feature
enable_packs
Click: Add feature

Click: Fully enable

To set it back (to check that your nifty changes haven’t broken anything when the flag is off)
Sign in as above:
click on “enable_packs”
click “Delete”
type enable_packs in the “Are you sure” dialog and click ok

Criteria for completion

  • functionality as described above
  • tests that support the functionality as described above

Implication for UI where user selects unit:

  • Adding a unit to an item adds a column to the request export, which may require tweaks to pre-existing spreadsheets of exports

Background

The following sections have been identified as required for the PACKS implementation. These should be implemented in numerical order.

Image

@cielf cielf added the Ruby for Good 2024 DC Issues for RFG 2024 DC label May 30, 2024
@awwaiid awwaiid added this to the Packs milestone May 31, 2024
@cielf cielf added Help Wanted Groomed + open to all! Difficulty—Intermediate and removed Ruby for Good 2024 DC Issues for RFG 2024 DC labels Jul 7, 2024
@awwaiid awwaiid self-assigned this Aug 4, 2024
@github-actions github-actions bot removed the Help Wanted Groomed + open to all! label Aug 4, 2024
@awwaiid
Copy link
Collaborator

awwaiid commented Aug 4, 2024

@cielf I'm looking at this and in some of my test data was a bit surprised. For some reason I thought that this was supposed to (even before this change) have a column for every item from the org. But instead it has a column for every item for the exported requests. Is that what you expected too?

In which case if someone is exporting requests which have ONLY used "Adult Briefs (Small/Medium) (Packs)" then that is the only item column that would get exported, right? It wouldn't export the POSSIBLE units, only the ones that are used in any of the exported requests. That's what the code is leading to based on current behavior, but thought I'd check.

@awwaiid
Copy link
Collaborator

awwaiid commented Aug 4, 2024

Or now that I'm reading the description again, it would be that if the item was used on ANY of the exported requests, go ahead and include a column for ALL of the units. In which case, I should include the implicit "Unit" unit too, right?

@cielf
Copy link
Collaborator Author

cielf commented Aug 5, 2024

@cielf I'm looking at this and in some of my test data was a bit surprised. For some reason I thought that this was supposed to (even before this change) have a column for every item from the org. But instead it has a column for every item for the exported requests. Is that what you expected too?

In which case if someone is exporting requests which have ONLY used "Adult Briefs (Small/Medium) (Packs)" then that is the only item column that would get exported, right? It wouldn't export the POSSIBLE units, only the ones that are used in any of the exported requests. That's what the code is leading to based on current behavior, but thought I'd check.

The question of whether should all the items on the org be included? The easy answer would be yes, but/and a certain number of the items are not visible to partners, so I'm not so sure that it would be useful across the board(I know of one bank that only offers their assembled kits to the partners for direct request).

We could, perhaps, make it all items that are visible to partners -- that would, at least, be more stable under filtering -- but I'd wonder about whether we have banks that take things out of visibility to the partners when they no longer have a reliable supply of them. It's out of scope for this change in any case.

But I want to muse about this. Maybe ask the stakeholder circle for input on Wednesday.

@cielf
Copy link
Collaborator Author

cielf commented Aug 5, 2024

Or now that I'm reading the description again, it would be that if the item was used on ANY of the exported requests, go ahead and include a column for ALL of the units. In which case, I should include the implicit "Unit" unit too, right?

Short answer -- I think yes (though my ultimate reason for thinking so is stability, informed by the discussion above) But also -- what if they delete a unit (which they can, right?) . There could be item requests using that unit -- so we would need to make sure those are included.

Edge cases, man.

@awwaiid awwaiid linked a pull request Aug 24, 2024 that will close this issue
@awwaiid awwaiid linked a pull request Aug 24, 2024 that will close this issue
Copy link
Contributor

github-actions bot commented Sep 5, 2024

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

@github-actions github-actions bot added the stale label Sep 5, 2024
Copy link
Contributor

Automatically unassigned after 7 days of inactivity.

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

Successfully merging a pull request may close this issue.

2 participants