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

feat(core): add middleware for getting the wishlist sets upon shared wishlist id changes #847

Conversation

danpadrinom
Copy link
Contributor

@danpadrinom danpadrinom commented Aug 22, 2023

Description

This adds a middleware for getting the wishlist sets when the sharedWishlistId has changed. It ensures we are always accessing the updated data from the wishlist sets after creating or deleting a shared wishlist.

Also, the action UPDATE_WISHLIST_SET_SUCCESS was added to the updateSharedWishlistUponItemsChanges middleware and the shared wishlist reducer (result) was adjusted.

Dependencies

None

Checklist

  • The commit message follows our guidelines
  • Tests for the respective changes have been added
  • The code is commented, particularly in hard-to-understand areas
  • The labels and/or milestones were added

Disclaimer

By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement

@github-actions github-actions bot added 📦 core Relative to the `@farfetch/blackout-core` package type: feature New feature labels Aug 22, 2023
@danpadrinom danpadrinom changed the title feat(core): add shared wishlist middleware for getting the wishlist sets upon shared wishlist id changes feat(core): add middleware for getting the wishlist sets upon shared wishlist id changes Aug 22, 2023
@danpadrinom danpadrinom requested a review from boliveira August 23, 2023 08:12
@boliveira
Copy link

Also there is a problem I have just noticed in the implementation, at least in main and I believe v1 also has this problem. In packages/redux/src/sharedWishlists/reducer.ts when the REMOVE_SHARED_WISHLIST_SUCCESS is dispatched, the result slice is not updated which means the getSharedWishlistId selector will keep returning the previous shared wishlist id. Can you take the opportunity to take a look at that case as well, please? If you prefer, you can have it in another PR but that should be tackled. Thank you!

@danpadrinom danpadrinom force-pushed the feat_add-shared-wishlist-middleware-for-getting-wishlist-set branch from 1cd3837 to ea14909 Compare August 24, 2023 14:12
@github-actions github-actions bot added type: feature New feature and removed type: feature New feature labels Aug 24, 2023
@danpadrinom
Copy link
Contributor Author

The scope of this PR has been changed as it now adds an action to the updateSharedWishlistUponItemsChanges middleware.

@danpadrinom danpadrinom force-pushed the feat_add-shared-wishlist-middleware-for-getting-wishlist-set branch from ea14909 to 29c6c00 Compare August 24, 2023 15:20
@github-actions github-actions bot added type: feature New feature and removed type: feature New feature labels Aug 24, 2023
@danpadrinom danpadrinom force-pushed the feat_add-shared-wishlist-middleware-for-getting-wishlist-set branch from 29c6c00 to 7b0a421 Compare August 25, 2023 13:13
@github-actions github-actions bot added type: feature New feature and removed type: feature New feature labels Aug 25, 2023
@danpadrinom danpadrinom added this to the dev-v1-22-08-23 milestone Aug 28, 2023
@danpadrinom danpadrinom merged commit f3978da into dev-v1-16-08-23 Aug 29, 2023
@danpadrinom danpadrinom deleted the feat_add-shared-wishlist-middleware-for-getting-wishlist-set branch August 29, 2023 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 core Relative to the `@farfetch/blackout-core` package type: feature New feature
Development

Successfully merging this pull request may close these issues.

2 participants