Skip to content

@W-18771949 Get rule based bonus products from API#2641

Closed
sf-madhuri-uppu wants to merge 11 commits intofeature/rule-based-bonus-productsfrom
t/cc-shark/W-18771949
Closed

@W-18771949 Get rule based bonus products from API#2641
sf-madhuri-uppu wants to merge 11 commits intofeature/rule-based-bonus-productsfrom
t/cc-shark/W-18771949

Conversation

@sf-madhuri-uppu
Copy link
Contributor

@sf-madhuri-uppu sf-madhuri-uppu commented Jun 25, 2025

Description

Fetch rule based bonus products by calling product search API. Handle scenarios where a product can qualify for multiple rule based and list based promotions (sometimes both too). Formatted the response of rule based promotions to look like list based response so that modal's logic need not differentiate between the 2 kinds.

Screen.Recording.2025-06-27.at.12.57.55.AM.mov

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Breaking change (could cause existing functionality to not work as expected)
  • Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

  • Removing a public function or component or prop
  • Adding a required argument to a function
  • Changing the data type of a function parameter or return value
  • Adding a new peer dependency to package.json

Changes

  • (change1)

How to Test-Drive This PR

  • (step1)

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

  • There are no changes to UI

or...

Localization

  • Changes include a UI text update in the Retail React App (which requires translation)

@sf-madhuri-uppu sf-madhuri-uppu requested a review from a team as a code owner June 25, 2025 07:31
@cc-prodsec
Copy link
Collaborator

cc-prodsec commented Jun 25, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

@sf-madhuri-uppu sf-madhuri-uppu changed the base branch from feature/bonus-products to feature/rule-based-bonus-products June 26, 2025 19:07
@sf-madhuri-uppu sf-madhuri-uppu requested a review from a team June 26, 2025 19:49
@sf-madhuri-uppu sf-madhuri-uppu added skip changelog Skip the "Changelog Check" GitHub Actions step even if the Changelog.md files are not updated ready for review PR is ready to be reviewed labels Jun 26, 2025
Copy link
Contributor

@sf-vrushal-kulkarni sf-vrushal-kulkarni left a comment

Choose a reason for hiding this comment

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

Is this also handling the the modal on the cart page? Can see only the list based bonus products on cart page

Copy link
Contributor

@sf-vrushal-kulkarni sf-vrushal-kulkarni left a comment

Choose a reason for hiding this comment

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

I am not able to see any bonus products either for rule based or list based. I was able to see the rule based bonus products after adding the product 2 - 3 times to cart
Screenshot 2025-06-26 at 7 28 03 PM

@sf-vrushal-kulkarni sf-vrushal-kulkarni self-requested a review June 27, 2025 02:38
@sf-madhuri-uppu
Copy link
Contributor Author

@sf-vrushal-kulkarni I made changes to product view component so we will not able to see rule based promotions on cart page. I can see list based promotions on the bonus modal on PDP. Let me look into why we are seeing rule based promotion after adding the item to cart for the second time.

Screen.Recording.2025-06-26.at.10.11.57.PM.mov

@sf-madhuri-uppu
Copy link
Contributor Author

https://github.com/user-attachments/assets/8cea3bda-c01c-4ece-9023-0d2d2dd63ae9
@sf-vrushal-kulkarni Fixed the rule based promotion render issue. They will now be seen in the modal after the first add to cart itself

const [promotionIdToSearch, setPromotionIdToSearch] = useState(null)
const {data: bonusProductSearchResult} = useBonusProductSearch(promotionIdToSearch)

// State to track all promotion IDs and their results
Copy link
Contributor

Choose a reason for hiding this comment

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

The business logic here seems to be isolated to bonus products only. Is it possibel to extract it out into a hook to reduce code in this component? E.g useBonusProduct()

maxBonusItems:
bonusItemsForModal?.promotionIdToMaxBonusItemsMap?.[promotionIdToSearch]
}
ruleBasedPromotionsRef.current.push(currentPromotionResult)
Copy link
Contributor

@alexvuong alexvuong Jun 30, 2025

Choose a reason for hiding this comment

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

For React ref, we are re-assigning current

ruleBasedPromotionsRef.current = currentPromotionResult

productId: bonusProduct.productId,
productName: bonusProduct.productName,
c_productUrl: bonusProduct.c_productUrl
}
Copy link
Contributor

@alexvuong alexvuong Jun 30, 2025

Choose a reason for hiding this comment

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

If this is the data structure we are dealing for bonus product. You can avoid doing this here by taking advantage of react-query select data transformation during useBonusProductSearch hook. then the data returns will always have this shape.

https://tanstack.com/query/v4/docs/framework/react/reference/useQuery
See useProductViewModal for an example

P/s: You can definitely create promotion id map from here as well if that is more convenient and reduce the amount of data transformation logic in the handleAddToCart

if (pendingPromotionIds.length > 0) {
const nextPromotionId = pendingPromotionIds[0]
setPendingPromotionIds((prev) => prev.slice(1))
setPromotionIdToSearch(nextPromotionId)
Copy link
Contributor

@alexvuong alexvuong Jun 30, 2025

Choose a reason for hiding this comment

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

Can we filter out any pending promotion from useBonusProducts hook select transformation step so all promotions returned are always valid promotion.
//pseu-do code

const {data: bonusProd} = useBonusProductSearch(args, {
   enabdle: !! promotionId,
   select: (res) => {
       const validPromo = res.hits.map(promo => !promo.isPending)
       return res.hits.map (promo => ({
           id: ...,
           name: ...
        })
   
   }
 } )

let listBasedBonusProducts = newBonusItems.filter(
(item) => item.bonusProducts
)
// Collect all promotion IDs for rule-based promotions
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's clean up comment here unless it is necessary. Too much comments accumulated over time can affect bundle size if every line of code has one comment explaining what it does

isOpen: false,
data: {},
bonusProducts: basket?.bonusDiscountLineItems || []
existingBonusProducts: basket?.bonusDiscountLineItems || []
Copy link
Contributor

Choose a reason for hiding this comment

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

why changing it to existingBonusProducts? What made it different from bonusProd. from this hook POV, it only cares about basket.bonusDiscountLineItems as bonusProd. What is the non-existingBonusProd if we take this name?
So any un-added/un-set items from outside that is not added to cart is irrelavent

.filter(Boolean)

//create maps of promotionId to id and maxBonusItems for rule based promotions
const {promotionIdToIdMap, promotionIdToMaxBonusItemsMap} =
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we reduce this logic here if we take advantage of useProductSearch hook select for data transformation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review PR is ready to be reviewed skip changelog Skip the "Changelog Check" GitHub Actions step even if the Changelog.md files are not updated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants