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

4294 Disable deleting purchases and donations with inactive items #4302

Merged

Conversation

pshong79
Copy link
Collaborator

Resolves #4294

Description

If a donation or a purchase contains an inactive item, the system is likely to return an error.
Currently, when trying to delete a purchase with inactive items in Production, it is returning an inventory error.

To mitigate this, we should just not allow for deleting a purchase or donation if there are inactive items and display a message instructing the user to make the inactive item active.

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Tested manually
Updated donations_request_spec.rb and purchases_request_spec.rb to verify that the Delete button is disabled for the organization admin user.

Screenshots

Purchases
Before:
purchases_before

After:
purchases_after

Donations
Before:
donations_before

After:
donations_after

Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

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

Minor nitpick, otherwise looks good!

<%= delete_button_to donation_path(@donation), {
size: "md",
enabled: !@donation.has_inactive_item?,
confirm: "Are you sure you want to permanently remove this donation?" } if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a pretty hefty statement now - can we break the if into the block format instead of it being at the end of it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dorner Thanks for the suggestion. The change has been made.

cielf
cielf previously requested changes Apr 27, 2024
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.

Hey @pshong79 -- Please also change the text in the message to include deletion, as described in the issue.

@pshong79
Copy link
Collaborator Author

@cielf 🤦 Sorry! I missed that! It should be corrected now.

@cielf
Copy link
Collaborator

cielf commented Apr 29, 2024

Looks good to me -- @dorner -- if you're happy with the correction to the nitpick, we can merge.

Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

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

LGTM!

@dorner dorner merged commit e68403a into rubyforgood:main May 2, 2024
19 checks passed
Copy link
Contributor

github-actions bot commented May 5, 2024

@pshong79: Your PR 4294 Disable deleting purchases and donations with inactive items is part of today's Human Essentials production release: 2024.05.05.
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.

Disable *deletion* of donations and purchases if they include an inactive item
3 participants