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

#4403: Show units in new/edit distribution #4586

Merged
merged 9 commits into from
Aug 25, 2024
Merged

Conversation

dorner
Copy link
Collaborator

@dorner dorner commented Aug 9, 2024

This makes a number of changes related to showing requested items and packs on the distribution pages.

image

  • Editing distributions will silently remove any items that have a quantity of 0. Before this threw an error. All other itemizables and new distributions should remain the same.
  • Distribution New / Edit pages now show all items requested (regardless of whether there is a current line item in the distribution with that requested item). All requested items are disabled on the distribution page and cannot be changed. (Quantity can be changed to 0 though.)
  • Request units are now shown on the distribution page if relevant.

See the original issue (#4403) for more details.

@dorner dorner requested a review from cielf August 9, 2024 20:23
Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks pretty good to me. I'm not 100% convinced that the larger font on the requested column is the way to go. It feels a little strange but I can see it drawing attention to the packs aspect when there.

For further discussion -- should we have the barcodes in place in this situation -- do they serve any purpose?

@cielf cielf requested a review from awwaiid August 11, 2024 17:01
@dorner
Copy link
Collaborator Author

dorner commented Aug 16, 2024

Good question. I think it makes sense to remove them.

@dorner
Copy link
Collaborator Author

dorner commented Aug 16, 2024

Fixed!
image

@cielf
Copy link
Collaborator

cielf commented Aug 16, 2024

I like it -- it's going to be much more compact. I do want to show it to the stakeholder circle pre-release in case there is a reason we need to have them ( though, if we do, we're going to have to do some fancy error handling on the barcodes)

@awwaiid Over to you for technical review, please!

Copy link
Collaborator

@awwaiid awwaiid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you save the distribution without selecting the storage location you get an error, as expected. Not as expected is that this then flips all of the requested lines to be the barcode style and loses the units.

Before save:
image

After save:
image

@dorner
Copy link
Collaborator Author

dorner commented Aug 23, 2024

Should be fixed now!

@dorner dorner requested a review from awwaiid August 23, 2024 19:56
Copy link
Collaborator

@awwaiid awwaiid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Holding off on merge until after the release tomorrow.

@awwaiid awwaiid added this to the Request Units (Packs) milestone Aug 25, 2024
@awwaiid awwaiid merged commit f47a634 into main Aug 25, 2024
38 checks passed
@awwaiid awwaiid deleted the 4403-packs-distribution branch August 25, 2024 20:12
Copy link
Contributor

github-actions bot commented Sep 1, 2024

@dorner: Your PR #4403: Show units in new/edit distribution is part of today's Human Essentials production release: 2024.09.01.
Thank you very much for your contribution!

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

Successfully merging this pull request may close these issues.

[PACKS] #8 New / Edit Distribution for the banks shows custom request units, if applicable
3 participants