-
-
Notifications
You must be signed in to change notification settings - Fork 665
feat: add service expiry badge, service tag & category to addons #541
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
base: main
Are you sure you want to change the base?
Conversation
- Implemented ServiceExpiryBadge component for displaying service expiry status with customizable colors and text. - Developed ServiceExpiryDatePicker component for selecting expiry dates, including navigation between months and displaying the current date. - Introduced useServiceExpiry hook to manage service expiry logic, including fetching expiry data from various providers and caching results. - Added utility functions for handling service expiry preferences, caching, and calculating days remaining until expiry.
…r additional services
WalkthroughAdds a frontend service-expiry system: new badge and date-picker UI, a hook that fetches and caches expiry from multiple providers with per-service preference overrides and localStorage persistence, integration into services/addons UI and template options, plus two new option types in core schema. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Services UI
participant Hook as useServiceExpiry
participant Cache as Client Cache
participant Storage as localStorage
participant Provider as Expiry Provider API
User->>UI: Open services menu / view service
UI->>Hook: init(useServiceExpiry(serviceId))
Hook->>Storage: readOverrides(serviceId)
Storage-->>Hook: preference
alt preference = hidden
Hook-->>UI: state=disabled
else preference = manual
Hook-->>UI: state=manual, expiresAt
else preference = auto
Hook->>Cache: getCachedExpiry(serviceId)
alt cache hit (fresh)
Cache-->>Hook: cached expiresAt
Hook-->>UI: badge (daysRemaining, source=cache)
else cache miss/stale
Hook->>Provider: fetchExpiryFromProvider(serviceId)
Provider-->>Hook: expiresAt / error
Hook->>Cache: setCachedExpiry(serviceId, expiresAt)
Hook-->>UI: badge (daysRemaining, source=api)
end
end
User->>UI: change preference or select date
UI->>Storage: setExpiryPreference(serviceId, preference)
Storage-->>UI: saved
UI->>Hook: dispatch EXPIRY_OVERRIDE_EVENT
Hook->>Cache: clear/update as needed
Hook-->>UI: updated badge/state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 2
🧹 Nitpick comments (4)
packages/frontend/src/components/menu/service-expiry-date-picker.tsx (1)
104-136: Consider memoisingtoday()to avoid repeated Date instantiations.
today()is called on every iteration when rendering calendar cells (line 114). This creates a newDateobject for each cell. Extract it before the map or memoise it.+ const todayDate = useMemo(() => today(), []); + return ( <Popover ... <div className="grid grid-cols-7 gap-1 text-sm"> {weeks.map((week, weekIndex) => week.map((date, dayIndex) => { if (!date) { return <span key={`${weekIndex}-${dayIndex}`} />; } const isSelected = selectedDate ? isSameDay(date, selectedDate) : false; - const isToday = isSameDay(date, today()); + const isToday = isSameDay(date, todayDate);packages/frontend/src/hooks/useServiceExpiry.ts (1)
224-242: Consider adding request timeouts to prevent indefinite hangs.The fetch calls lack timeout handling. If a provider API is slow or unresponsive, the hook will remain in
loadingstate indefinitely. Consider usingAbortControllerwith a timeout.async function fetchRealDebridExpiry( credentials: Credentials ): Promise<string | null> { const apiKey = credentials?.apiKey?.trim(); if (!apiKey) { throw new Error('Enter your Real-Debrid API key to view expiry.'); } + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); const response = await fetch('https://api.real-debrid.com/rest/1.0/user', { headers: { Authorization: `Bearer ${apiKey}`, }, cache: 'no-store', + signal: controller.signal, }); + clearTimeout(timeoutId); if (!response.ok) { throw new Error(`Real-Debrid API error (${response.status})`); }Apply the same pattern to other provider fetch functions.
packages/frontend/src/utils/service-expiry.ts (1)
189-196: RedundantwriteOverridescall when no override exists.The else branch writes overrides even when
overrides[serviceId]doesn't exist, resulting in an unnecessary localStorage write.if (!preference || preference.mode === 'auto') { if (overrides[serviceId]) { delete overrides[serviceId]; writeOverrides(overrides); - } else { - writeOverrides(overrides); } } else {packages/frontend/src/components/menu/services.tsx (1)
12-15: Remove commented-out import.The
SERVICE_DETAILSimport is commented out. If it's no longer needed, remove it entirely to keep the code clean.import { - // SERVICE_DETAILS, ServiceId, } from '../../../../core/src/utils/constants';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/frontend/src/components/menu/service-expiry-badge.tsx(1 hunks)packages/frontend/src/components/menu/service-expiry-date-picker.tsx(1 hunks)packages/frontend/src/components/menu/services.tsx(1 hunks)packages/frontend/src/hooks/useServiceExpiry.ts(1 hunks)packages/frontend/src/utils/service-expiry.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/frontend/src/components/menu/service-expiry-date-picker.tsx (1)
packages/frontend/src/components/ui/button/button.tsx (1)
Button(126-222)
packages/frontend/src/utils/service-expiry.ts (1)
packages/core/src/utils/constants.ts (1)
ServiceId(243-243)
packages/frontend/src/components/menu/services.tsx (5)
packages/core/src/utils/constants.ts (1)
ServiceId(243-243)packages/frontend/src/hooks/useServiceExpiry.ts (1)
useServiceExpiry(49-202)packages/frontend/src/components/menu/service-expiry-badge.tsx (1)
ServiceExpiryBadge(12-29)packages/frontend/src/utils/service-expiry.ts (5)
ExpiryMode(10-10)getExpiryPreference(175-179)canServiceAutoFetch(64-66)isTrackedService(52-54)setExpiryPreference(181-217)packages/frontend/src/components/menu/service-expiry-date-picker.tsx (1)
ServiceExpiryDatePicker(17-158)
🪛 Biome (2.1.2)
packages/frontend/src/components/menu/services.tsx
[error] 69-69: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 70-70: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 71-71: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 72-72: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 73-73: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 74-74: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 135-135: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 170-170: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 171-171: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 169-169: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 179-179: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (6)
packages/frontend/src/components/menu/service-expiry-badge.tsx (1)
1-29: LGTM!Clean, focused presentational component with appropriate use of
memofor performance. The props interface is well-defined and the styling approach using inline styles for dynamic colours alongside utility classes is sensible.packages/frontend/src/components/menu/service-expiry-date-picker.tsx (1)
160-226: LGTM!The utility functions are well-implemented. The
parseDatevalidation correctly catches invalid calendar dates (e.g., Feb 30th), andbuildCalendarproperly handles week padding for calendar grid alignment.packages/frontend/src/hooks/useServiceExpiry.ts (1)
49-202: LGTM!The hook is well-structured with proper lifecycle management, cancellation handling, and comprehensive state transitions. The credential signature approach for dependency tracking is appropriate.
packages/frontend/src/utils/service-expiry.ts (1)
1-75: LGTM!The module is well-organised with clear separation between cache and override management. The SSR guards, JSON parsing safety, and legacy preference migration are thoughtfully implemented.
packages/frontend/src/components/menu/services.tsx (2)
477-558: LGTM!The
SortableServiceItemcomponent is well-implemented with proper integration of the expiry hook, drag-and-drop functionality, and conditional badge rendering based on expiry status.
559-727: LGTM!The
ServiceModalcomponent cleanly integrates expiry controls with credential editing. The mode resolution logic correctly falls back to manual when auto-fetch isn't supported.
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
♻️ Duplicate comments (1)
packages/frontend/src/components/menu/services.tsx (1)
66-75: Critical: Hooks still called conditionally after early return (duplicate issue).This is the same critical Rules of Hooks violation flagged in the previous review. All hooks (
useUserData,useMenu,useState,useEffecton line 136,useSensorson lines 170-178, anduseEffecton line 180) are called after the early return on line 75, causing them to execute conditionally. This will cause runtime errors and undefined behaviour.The fix remains the same: move all hook calls before the early return, or extract the conditional logic into a child component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/frontend/src/components/menu/services.tsx(1 hunks)packages/frontend/src/hooks/useServiceExpiry.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/frontend/src/hooks/useServiceExpiry.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/frontend/src/components/menu/services.tsx (9)
packages/frontend/src/context/status.tsx (1)
useStatus(23-23)packages/frontend/src/context/userData.tsx (1)
useUserData(334-340)packages/frontend/src/context/menu.tsx (1)
useMenu(122-122)packages/core/src/utils/constants.ts (1)
ServiceId(243-243)packages/core/src/main.ts (1)
setUserData(115-117)packages/core/src/db/schemas.ts (2)
StatusResponse(1050-1050)UserData(503-503)packages/frontend/src/components/menu/service-expiry-badge.tsx (1)
ServiceExpiryBadge(12-29)packages/core/src/db/db.ts (1)
open(101-127)packages/frontend/src/components/menu/service-expiry-date-picker.tsx (1)
ServiceExpiryDatePicker(17-158)
🪛 Biome (2.1.2)
packages/frontend/src/components/menu/services.tsx
[error] 136-136: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 171-171: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 172-172: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 170-170: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 180-180: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (1)
packages/frontend/src/components/menu/services.tsx (1)
625-638: The referenced code does not exist in the current file.The review comment cites lines 625-638 in
packages/frontend/src/components/menu/services.tsx, but the file only contains 602 lines. Additionally, none of the referenced code elements exist in the repository:
- No
setExpiryPreferencefunction- No
useServiceExpiryhook- No
expiryMode,manualExpiryInput, ormanualExpiryUpdatedAtstate- No
ServiceExpiryBadgecomponent- The
ServiceModalcomponent in the actual file (lines 526-602) handles only credential configuration, not expiry preferencesThe review appears to be based on a different version of the codebase or an entirely different file. The actual implementation does not include any expiry preference functionality.
Likely an incorrect or invalid review comment.
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/core/src/db/schemas.ts(1 hunks)packages/frontend/src/components/menu/addons.tsx(7 hunks)packages/frontend/src/components/menu/user-addon-metadata.ts(1 hunks)packages/frontend/src/components/shared/template-option.tsx(2 hunks)packages/frontend/src/components/ui/badge/badge.tsx(1 hunks)packages/frontend/src/components/ui/badge/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/frontend/src/components/shared/template-option.tsx (3)
packages/frontend/src/components/menu/service-expiry-date-picker.tsx (1)
ServiceExpiryDatePicker(17-158)packages/frontend/src/components/menu/user-addon-metadata.ts (3)
DEFAULT_USER_TAG(111-111)UserTagType(85-85)USER_TAG_OPTIONS(78-83)packages/frontend/src/components/ui/badge/badge.tsx (1)
ServiceExpiryBadge(108-176)
packages/frontend/src/components/ui/badge/badge.tsx (1)
packages/frontend/src/components/ui/badge/index.tsx (4)
BadgeProps(2-2)Badge(1-1)ServiceExpiryBadgeProps(2-2)ServiceExpiryBadge(1-1)
packages/frontend/src/components/menu/addons.tsx (2)
packages/frontend/src/components/menu/user-addon-metadata.ts (6)
UserAddonCategory(41-41)getUserCategoryMetadata(66-74)UserTagType(85-85)getUserCategoryOptions(54-64)getUserTagOptions(95-104)DEFAULT_USER_TAG(111-111)packages/frontend/src/components/ui/badge/badge.tsx (1)
ServiceExpiryBadge(108-176)
🔇 Additional comments (16)
packages/frontend/src/components/ui/badge/index.tsx (1)
1-2: LGTM!Clean barrel file following standard re-export patterns for components and their associated types.
packages/frontend/src/components/shared/template-option.tsx (3)
17-23: LGTM!Imports are well-organised and correctly sourced from the new badge and metadata modules.
414-433: LGTM!The
datecase follows the established patterns from other option types, with correct handling of forced values, defaults, and theemptyIsUndefinedbehaviour.
434-520: LGTM!The
service-tagcase is well-implemented with:
- Proper fallback to
DEFAULT_USER_TAG- Dynamic option injection for custom tag values
- Conditional date picker rendering for the 'expires' type
- Real-time preview badge for user feedback
Minor observation: the cast
as UserTagTypeon line 442 is redundant sinceUserTagTypeis typed asstring, but it doesn't cause any issues and adds semantic clarity.packages/frontend/src/components/menu/user-addon-metadata.ts (3)
3-9: LGTM!The
formatLabelhelper is well-implemented with proper handling of edge cases including multiple consecutive separators and empty results.
13-74: LGTM!Category utilities are well-structured:
- Clean derivation of
USER_CATEGORY_METADATAfrom optionsgetUserCategoryOptionscorrectly ensures custom values are always selectablegetUserCategoryMetadataprovides sensible fallbacks for unknown categories
78-111: LGTM!Tag utilities follow a consistent pattern with the category code:
- Sensible default options covering common subscription states
getUserTagLabelprovides proper fallback chainDEFAULT_USER_TAGis a safe default valuepackages/frontend/src/components/ui/badge/badge.tsx (3)
11-40: LGTM!Well-structured CVA variants with:
- Semantic intent options covering common UI states
- Flexible size variants
- Smart 'custom' intent allowing full style customisation
46-96: LGTM!The Badge component is well-implemented with:
- Proper
forwardRefusage for ref forwarding- Clean custom styling pattern via
backgroundColor/textColorprops- Automatic border addition for custom backgrounds improving contrast
displayNameset for React DevTools
108-176: LGTM!Excellent implementation of
ServiceExpiryBadge:
- Correct local date parsing to avoid timezone issues
- Clear expiry comparison logic normalised to midnight
- User-friendly label formatting with appropriate tense ("Expires" vs "Expired")
- Semantic intent mapping provides intuitive visual feedback
packages/frontend/src/components/menu/addons.tsx (6)
87-95: LGTM!Imports are correctly added for the new badge component and user metadata utilities.
873-934: LGTM!Well-implemented visual enhancements:
- Category-coloured left border provides clear visual grouping
ServiceExpiryBadgedisplays subscription status at a glance- Responsive hiding of the badge on mobile is a sensible space optimisation
- Safe optional chaining for accessing nested options
1262-1276: Verify the useEffect dependency array to avoid potential state synchronisation issues.The effect sets
userCategorytocurrentCategoryValuewhen not present, butcurrentCategoryValueis derived fromvalues.options?.userCategory. This creates a dependency cycle where:
currentCategoryValueis derived fromvalues- Effect runs when
currentCategoryValuechanges- Effect modifies
valuesThe condition
!('userCategory' in nextOptions)should prevent an infinite loop, but consider whether this effect is necessary given thatcurrentCategoryValuealready falls back toinitialValues.options?.userCategoryon line 1249. If the intent is only to initialiseuserTagwith the default, thecurrentCategoryValuehandling in this effect might be redundant.
1416-1430: LGTM!The submit handler correctly merges defaults for any undefined option values, with appropriate special handling for
userTag's object default.
1348-1360: LGTM!Clean sorting implementation that ensures
userCategoryanduserTagoptions consistently appear at the end of the form, withuserCategorybeforeuserTag.
432-500: Remove review comment — referenced code does not exist in the file.The grouping logic with
lastCategorytracking,getUserCategoryMetadata, and category headers is not present in lines 432-500 or anywhere else in the file. The actual implementation is a simple direct mapping of presets toSortableAddonItemcomponents without category grouping features. The code snippet in your review comment does not match the current codebase.Likely an incorrect or invalid review comment.
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/utils/config.ts(1 hunks)
🔇 Additional comments (1)
packages/core/src/utils/config.ts (1)
943-968: The review comment references code that does not exist in the codebase.The service-tag validation code at lines 943-968 is not present in
packages/core/src/utils/config.ts. The actual code at those lines contains proxy configuration validation, not service-tag option validation. Additionally, no "service-tag" option type is defined or used anywhere in the repository. This review comment cannot be applied as written.Likely an incorrect or invalid review comment.
…ngs and use Zod for ISO format
|
Demo: https://streamable.com/69nnyn It expires in 2 days. |
|
Someone on discord suggested adding an option to select, select all, select none, select based on category, select based on tags. I will skip this request for now to avoid over complication but these are the options that we can think of after having this basic version. |
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.