-
Notifications
You must be signed in to change notification settings - Fork 7
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
filter subscriptions for disabled plans #470
Conversation
WalkthroughThe Changes
Sequence DiagramsequenceDiagram
participant Hook as useUserSubscriptions
participant API as Service API
participant Filter as Subscription Filter
alt ServiceId Provided
Hook->>API: getServiceOffering(serviceId)
API-->>Hook: Return Service Offering
Hook->>Filter: Filter Subscriptions by ProductTierId
else No ServiceId
Hook->>API: getServiceOfferingIds()
API-->>Hook: Return Available Services
Hook->>Filter: Filter Subscriptions by ServiceId and ProductTierId
end
Filter-->>Hook: Return Filtered Subscriptions
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/hooks/query/useUserSubscriptions.js (3)
15-43
: Add error handling for API callsThe asynchronous functions within
queryFn
are making API calls without error handling. While React Query handles errors by moving the query into an error state, adding try-catch blocks around your API calls can provide more granular control and allow for custom error messages or fallback logic.Example refactor:
queryFn: async () => { try { const response = await listSubscriptions({ serviceId, environmentType }); const subscriptions = response.data.subscriptions || []; if (serviceId) { const serviceOfferingRes = await getServiceOffering(serviceId, environmentType); const existingProductTierIds = serviceOfferingRes?.data?.offerings?.map( (offering) => offering?.productTierID ); return subscriptions?.filter((subscription) => existingProductTierIds?.includes(subscription?.productTierId) ); } else { const servicesRes = await getServiceOfferingIds(environmentType); const services = servicesRes?.data?.services; return subscriptions?.filter((subscription) => services?.find( (service) => service?.serviceId === subscription?.serviceId && service?.offerings?.find( (offering) => offering?.productTierID === subscription.productTierId ) ) ); } } catch (error) { // Handle error appropriately, e.g., log or transform error throw error; } },
31-42
: Optimize filtering logic for better performanceThe current implementation uses nested
.filter()
and.find()
methods, which can lead to performance issues with larger datasets due to higher time complexity. Consider optimizing this logic by converting the services and their offerings into a lookup map or set for faster access times.Example refactor:
// Create a Map for quick lookup of serviceId to a Set of productTierIDs const serviceOfferingsMap = new Map(); services?.forEach((service) => { const offeringsSet = new Set(service?.offerings?.map((offering) => offering?.productTierID)); serviceOfferingsMap.set(service?.serviceId, offeringsSet); }); return subscriptions?.filter((subscription) => { const offeringsSet = serviceOfferingsMap.get(subscription?.serviceId); return offeringsSet?.has(subscription?.productTierId); });
46-46
: Redundant 'select' function can be omittedThe
select
option inuseQuery
is used to transform or derive data from the query result. Sinceselect: (response) => response
doesn't modify the response, it can be omitted to simplify the code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/hooks/query/useUserSubscriptions.js
(1 hunks)
🔇 Additional comments (2)
src/hooks/query/useUserSubscriptions.js (2)
38-39
: [Duplicate] Inconsistent property naming: 'productTierID' vs 'productTierId'
This inconsistency also appears here between offering?.productTierID
and subscription.productTierId
. Ensure that property names are used consistently throughout the codebase.
31-32
: Possible missing 'environmentType' parameter in 'getServiceOfferingIds'
The function getServiceOffering
is called with environmentType
(lines 19-22), but getServiceOfferingIds
is called without it. If environmentType
is necessary for fetching service offerings, consider passing it to getServiceOfferingIds
as well to ensure consistent behavior across different environments.
Summary by CodeRabbit
New Features
Bug Fixes