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

Fixed: On updating the facility, the settings related to the facility are also being updated (#500) #502

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

R-Sourabh
Copy link
Contributor

Related Issues

#500

Short Description and Why It's Useful

  • Added the call to update the reroute fulfillment configuration when updating the facility and also handled the case when it has no configuration or when the API fails.
  • Added the action call for updating the partial order rejection configuration on facility change.

Screenshots of Visual Changes before/after (If There Are Any)

Contribution and Currently Important Rules Acceptance

@dt2patel
Copy link
Contributor

Re-route configurations are not dependent on facility, why are we updating them on facility update?

@R-Sourabh
Copy link
Contributor Author

@dt2patel, when updating the facility, we fetch the related store. Therefore, we need to retrieve the re-route configuration for the updated store.

@dt2patel
Copy link
Contributor

Why not fetch re-route config on store update? If new facility is part of same product store, then why fetch new data even when store didn't change?

@R-Sourabh
Copy link
Contributor Author

@dt2patel Yes, we will handle this situation by adding a check to prevent re-fetching the re-route config for the same store. However, if the settings change for that store during a facility change, we will not get the updated settings.

…hipment methods when the store does not change during a facility update(hotwax#500)
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.

2 participants