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

Allow user stock locations to be deleted #4389

Merged

Conversation

rmparr
Copy link
Contributor

@rmparr rmparr commented May 30, 2022

Description
Currently, if using Spree::UserStockLocation associations, once a user has an associated stock location, it is impossible to remove all associated stock locations. This is because the template is missing a hidden field for user[:stock_location_ids].

This PR remedies the problem by mirroring the conventions used for the user's roles. It also adds coverage around the expected behavior in the controller, which was missing.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

rmparr added 4 commits May 30, 2022 15:57
This is related to roles, not stock locations.
This provides spec parity for other HABTM relationships.
This will allow for removing all user-stock location relations.
Without this, once a user has an associated stock location, they
must ALWAYS have at least one.
This mirrors the user roles specs, and tests the fixed behavior
around user stock locations.
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks!

@waiting-for-dev waiting-for-dev merged commit fc88402 into solidusio:master Jun 1, 2022
@waiting-for-dev
Copy link
Contributor

Thanks, @rmparr! Would you like to create backport PRs for supported Solidus versions? (v2.11, v3.0 & v3.1).

@rmparr
Copy link
Contributor Author

rmparr commented Jun 1, 2022

@waiting-for-dev Absolutely!
#4398
#4399
#4400

@rmparr rmparr deleted the fix-user-restricted-stock-management branch June 1, 2022 16:12
@waiting-for-dev
Copy link
Contributor

Thanks, @rmparr! ❤️

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.

4 participants