Add discount timer badge to header #2583
Conversation
WalkthroughThe pull request introduces a new feature for displaying a discount timer badge across various components of the application. This feature allows users to see the remaining time for a promotional discount on yearly plans. The changes include adding a new component for the discount timer badge, updating header and navigation controls to conditionally render this badge, creating utility functions for time formatting, and implementing comprehensive acceptance tests to validate the new functionality. Additionally, entries were added to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 (
|
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. Additional details and impacted files📢 Thoughts on this report? Let us know! |
Bundle ReportChanges will increase total bundle size by 13.81kB (0.04%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
ryan-gang
force-pushed
the
discount-timer-badge
branch
from
abe2deb to
254128c
Compare
ryan-gang
force-pushed
the
discount-timer-badge
branch
from
3e5b271 to
a05920b
Compare
ryan-gang
force-pushed
the
discount-timer-badge
branch
from
a05920b to
0644912
Compare
ryan-gang
changed the title
ryan-gang
force-pushed
the
discount-timer-badge
branch
from
50cdc40 to
989cbaa
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
app/components/billing-status-badge/discount-timer-badge.hbs (1)
9-17: Consider internationalizing the "left" text.
The hardcoded "left" text might need localization if the application supports multiple languages.tests/pages/components/header.js (1)
6-10: Add test coverage for hover and click interactions.
ThediscountTimerBadgepage object is properly scoped. Ensure acceptance or integration tests cover the overlay/tooltip display on hover and redirection after a click.app/components/billing-status-badge/discount-timer-badge.ts (1)
30-32: Handle expired or near-expiry edge cases.
If the discount expiration is close or already passed, consider providing a fallback message fortimeLeftandtooltipTextinstead of showing potentially misleading values.Also applies to: 34-36
app/utils/time-formatting.ts (1)
59-65: Consider unified handling of zero vs. singular/plural durations.Currently,
formatTimeDurationSingularOrPluraltreats all non-1 values as plural, which can yield "0 seconds," while the badge’s negative duration block returns "0 second." For consistency, harmonize how the code handles zero durations across these functions.app/components/course-page/header/navigation-controls.hbs (1)
30-30: Extract common badge logic for maintainability.Line 30 highlights a plan to refactor recurring discount badge code. Merging the discount-related conditions (lines 35–38) into a shared component now could reduce duplication and simplify future updates.
Also applies to: 35-38
tests/acceptance/view-discount-countdown-test.js (2)
18-28: Correct the test description to match the actual behavior.The test name says it redirects to "/tracks" page, but the assertion checks for "/catalog." Update the test description or the assertion for clarity.
38-42: Remove or update outdated TODO comment about assert.dom usage.Despite the comment,
assert.dom()is successfully used just below. If there's no technical limitation, consider deleting the leftover TODO.app/components/header.hbs (1)
61-61: Consider extracting the badge logic into a shared component.The TODO comment mentions creating a dedicated
BillingStatusBadgeComponent. Doing so would make the code more reusable and reduce duplication across the header and course page header.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.gitignore(1 hunks)app/components/billing-status-badge/discount-timer-badge.hbs(1 hunks)app/components/billing-status-badge/discount-timer-badge.ts(1 hunks)app/components/course-page/header/navigation-controls.hbs(1 hunks)app/components/course-page/header/navigation-controls.ts(2 hunks)app/components/header.hbs(1 hunks)app/components/header.ts(2 hunks)app/utils/time-formatting.ts(1 hunks)tests/acceptance/view-discount-countdown-test.js(1 hunks)tests/pages/components/header.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🔇 Additional comments (8)
app/components/billing-status-badge/discount-timer-badge.hbs (2)
1-8: Verify target route and styling classes.
Ensure that the "pay" route definitely exists and confirm the Tailwind utility classes remain consistent across different theme variations or breakpoints. Otherwise, all good.
19-20: Tooltip text might need a fallback.
If there's a scenario wheretooltipTextis empty or null, ensure the tooltip handles this gracefully.app/components/billing-status-badge/discount-timer-badge.ts (1)
5-5:⚠️ Potential issueTypo in import statement.
Check whetherformatTimeDurationForCoundownis a valid function name or if it should beformatTimeDurationForCountdown.- import { formatTimeDurationForBadge, formatTimeDurationForCoundown } from 'codecrafters-frontend/utils/time-formatting'; + import { formatTimeDurationForBadge, formatTimeDurationForCountdown } from 'codecrafters-frontend/utils/time-formatting';app/components/course-page/header/navigation-controls.ts (2)
11-11: Import is correct.
No issues spotted; thePromotionalDiscountModelimport aligns with usage.
32-38: Ensure robust feature-flag checks.
canSeeDiscountCountdownusage is valid. Make sure dependent template logic cleanly handles false or null discount states.app/utils/time-formatting.ts (1)
35-57: Revisit handling of durations >24 hours and negative values.This function correctly splits the duration into hours, minutes, and seconds, but there's a TODO for durations over 24 hours. Additionally, unlike the older function which logs negative durations via Sentry, here it simply returns "0 second." If that's intentional, ensure it's consistently handled across the codebase.
Would you like help implementing a full "X days left" approach or preserving the Sentry logging for negative durations?
app/components/header.ts (1)
31-42: Ensure robust handling for user and discount data.These new getters (
activeDiscountForYearlyPlanandcanSeeDiscountCountdown) rely on a valid user session and feature flag. Confirm that upstream logic properly manages scenarios where the user is null or when the discount is missing. Add test coverage if needed.app/components/header.hbs (1)
66-69: Ensure test coverage for both discount visibility conditions.It might be beneficial to add test cases to confirm that the badge is rendered only if
activeDiscountForYearlyPlanis truthy andcanSeeDiscountCountdownis true, and that it remains hidden otherwise.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/billing-status-badge/discount-timer-badge.ts (1)
17-37: Consider adding error boundaries and analytics tracking.As this component is part of a critical business feature (discounts), consider:
- Implementing error boundaries to gracefully handle any rendering failures
- Adding analytics tracking to monitor user interactions with the discount timer
This will help track the effectiveness of the discount feature and ensure robust error handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/components/billing-status-badge/discount-timer-badge.ts(1 hunks)app/components/pay-page/experimental-referral-discount-notice.ts(2 hunks)app/components/pay-page/experimental-signup-discount-notice.ts(2 hunks)app/utils/time-formatting.ts(2 hunks)tests/unit/utils/time-formatting-test.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- app/components/pay-page/experimental-signup-discount-notice.ts
- app/components/pay-page/experimental-referral-discount-notice.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/utils/time-formatting.ts
🔇 Additional comments (9)
tests/unit/utils/time-formatting-test.ts (6)
1-1: Great catch correcting the function import name!Updating the import statement from the old misspelling helps maintain consistency and clarity across the codebase.
10-10: The function call reference looks correct.Validating hours, minutes, and seconds is essential for a countdown. Well done including it in the tests.
17-17: Proper function reference for minutes/seconds check.This test helps confirm the correct formatting when hours are zero. Good coverage.
24-24: Good handling of seconds-only scenario.Ensuring countdown displays just seconds when hours and minutes are zero is crucial for usability.
30-30: Verified negative time difference scenario.Returning '00s' here prevents confusion around expired countdowns. This test is a nice edge-case check.
33-33: Correctly formats zero seconds edge case.Ensuring the countdown doesn't display a negative or invalid time is important. Nice coverage.
app/components/billing-status-badge/discount-timer-badge.ts (3)
1-15: LGTM! Well-structured imports and type definitions.The imports are properly organized, and the TypeScript interface provides clear typing for the component's arguments with good documentation for the size prop.
34-36: Avoid hardcoding discount percentage.The tooltip text has a hardcoded "40% off" value. This should be derived from the discount model to ensure accuracy and maintainability.
- return `Upgrade in ${formatTimeDurationForCountdown(this.expiresAt, this.time.currentTime)} to get 40% off the annual plan. Click to view details.`; + return `Upgrade in ${formatTimeDurationForCountdown(this.expiresAt, this.time.currentTime)} to get ${this.args.discount.percentageOff}% off the annual plan. Click to view details.`;
39-43: LGTM! Proper Glint type registry declaration.The component is correctly registered in the Glint environment, following TypeScript best practices.
| get timeLeft() { | ||
| return formatTimeDurationForBadge(this.expiresAt, this.time.currentTime); | ||
| } |
There was a problem hiding this comment.
Add guard for expired discounts in timeLeft getter.
The timeLeft getter should handle cases where the current time is after the expiration time to prevent showing negative durations.
get timeLeft() {
+ if (this.isExpired) {
+ return 'Expired';
+ }
return formatTimeDurationForBadge(this.expiresAt, this.time.currentTime);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| get timeLeft() { | |
| return formatTimeDurationForBadge(this.expiresAt, this.time.currentTime); | |
| } | |
| get timeLeft() { | |
| if (this.isExpired) { | |
| return 'Expired'; | |
| } | |
| return formatTimeDurationForBadge(this.expiresAt, this.time.currentTime); | |
| } |
|
A single unrelated code-mirror test is failing. |
Implement the DiscountTimerBadge component to display a countdown for promotional discounts. Integrate the component into the header and navigation controls, replacing existing VIP and member badge logic. Add acceptance tests to verify the discount countdown functionality. This enhances user experience by providing timely information on discounts available.
…rove time display - Refactored the DiscountTimerBadge component to use the promotional discount model for expiration date and status. - Updated the time display to dynamically show the remaining time left until the discount expires. - Removed hardcoded time strings for a more flexible and accurate countdown representation.
- Added logic to conditionally render the DiscountTimerBadge based on the active discount for the yearly plan. - Introduced a computed property to retrieve the active discount from the current user, enhancing the user interface with real-time discount information.
- Introduced `formatTimeDurationForBadge` to calculate and format time left for display on badges. - Added `formatTimeDurationSingularOrPlural` to handle singular and plural forms of time units. - Enhances the user interface by providing clear and accurate time representations for badge notifications.
…ly plan - Updated the navigation controls to conditionally render the DiscountTimerBadge with the active discount information for the yearly plan. - Introduced a computed property to retrieve the active discount from the current user, improving the user interface with real-time discount visibility.
- Updated header and navigation controls to conditionally render the DiscountTimerBadge based on the active discount for the yearly plan. - Enhanced user interface by providing real-time visibility of discounts, improving user experience. - Removed commented-out code for clarity and maintainability.
…ontrols - Introduced a computed property `canSeeDiscountCountdown` in both HeaderComponent and NavigationControlsComponent to determine visibility of the discount countdown based on feature flags. - This enhancement supports the conditional rendering of discount-related UI elements, improving user experience by providing real-time discount visibility.
…elated components
ryan-gang
force-pushed
the
discount-timer-badge
branch
from
4e1de70 to
d4720d6
Compare
|
I rebased the PR on top of |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
app/utils/time-formatting.ts (1)
41-43: Implement days formatting as noted in TODO.The comment indicates missing functionality for displaying durations longer than 24 hours.
Would you like me to help implement the days formatting functionality? Here's what we need to consider:
- Converting hours to days
- Rounding up to nearest day
- Handling plural/singular forms using the existing utility
app/components/header.ts (1)
39-41: Consider caching feature flag value.The
canSeeDiscountCountdowngetter might be called frequently during renders.+ @tracked private _canSeeDiscountCountdown: boolean | null = null; + get canSeeDiscountCountdown() { - return this.featureFlags.canSeeDiscountCountdown; + if (this._canSeeDiscountCountdown === null) { + this._canSeeDiscountCountdown = this.featureFlags.canSeeDiscountCountdown; + } + + return this._canSeeDiscountCountdown; }app/utils/code-mirror-gutter-rs.ts (3)
300-300: Avoid assignment in a complex expression for clarity.The lint warns about using the assignment
(markers = [])within a larger expression. While functionally correct, it decreases readability. If it’s feasible without diverging too much from upstream, consider extracting the assignment for clarity:- if (marker) (markers || (markers = [])).push(marker) + if (!markers && marker) { + markers = [] + } + if (marker) { + markers.push(marker) + }🧰 Tools
🪛 Biome (1.9.4)
[error] 300-300: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
381-381: Rewrite assignment to improve clarity.Similar lint feedback applies here:
(this.above = above)is used within a ternary operator. Moving the assignment outside the expression improves readability and aligns with typical TypeScript best practices, if permissible:- this.dom.style.marginTop = (this.above = above) ? above + "px" : "" + this.above = above + this.dom.style.marginTop = above ? above + "px" : ""🧰 Tools
🪛 Biome (1.9.4)
[error] 381-381: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
1-531: Add or improve test coverage for newly introduced lines.Multiple newly added lines (e.g., lines 95, 169, 219-222, 299, etc.) are flagged as uncovered. Consider adding tests that exercise these configurations, event handlers, and marker logic to improve confidence and maintainability.
Would you like help drafting unit or integration tests to ensure these features are adequately covered?
🧰 Tools
🪛 Biome (1.9.4)
[error] 300-300: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 381-381: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
🪛 GitHub Check: codecov/patch
[warning] 95-95: app/utils/code-mirror-gutter-rs.ts#L95
Added line #L95 was not covered by tests
[warning] 169-169: app/utils/code-mirror-gutter-rs.ts#L169
Added line #L169 was not covered by tests
[warning] 204-204: app/utils/code-mirror-gutter-rs.ts#L204
Added line #L204 was not covered by tests
[warning] 206-206: app/utils/code-mirror-gutter-rs.ts#L206
Added line #L206 was not covered by tests
[warning] 219-222: app/utils/code-mirror-gutter-rs.ts#L219-L222
Added lines #L219 - L222 were not covered by tests
[warning] 224-224: app/utils/code-mirror-gutter-rs.ts#L224
Added line #L224 was not covered by tests
[warning] 226-227: app/utils/code-mirror-gutter-rs.ts#L226-L227
Added lines #L226 - L227 were not covered by tests
[warning] 230-231: app/utils/code-mirror-gutter-rs.ts#L230-L231
Added lines #L230 - L231 were not covered by tests
[warning] 234-235: app/utils/code-mirror-gutter-rs.ts#L234-L235
Added lines #L234 - L235 were not covered by tests
[warning] 246-246: app/utils/code-mirror-gutter-rs.ts#L246
Added line #L246 was not covered by tests
[warning] 259-259: app/utils/code-mirror-gutter-rs.ts#L259
Added line #L259 was not covered by tests
[warning] 293-293: app/utils/code-mirror-gutter-rs.ts#L293
Added line #L293 was not covered by tests
[warning] 299-299: app/utils/code-mirror-gutter-rs.ts#L299
Added line #L299 was not covered by tests
[warning] 325-326: app/utils/code-mirror-gutter-rs.ts#L325-L326
Added lines #L325 - L326 were not covered by tests
[warning] 328-330: app/utils/code-mirror-gutter-rs.ts#L328-L330
Added lines #L328 - L330 were not covered by tests
[warning] 332-332: app/utils/code-mirror-gutter-rs.ts#L332
Added line #L332 was not covered by tests
[warning] 334-334: app/utils/code-mirror-gutter-rs.ts#L334
Added line #L334 was not covered by tests
[warning] 340-342: app/utils/code-mirror-gutter-rs.ts#L340-L342
Added lines #L340 - L342 were not covered by tests
[warning] 350-350: app/utils/code-mirror-gutter-rs.ts#L350
Added line #L350 was not covered by tests
[warning] 425-425: app/utils/code-mirror-gutter-rs.ts#L425
Added line #L425 was not covered by tests
[warning] 446-448: app/utils/code-mirror-gutter-rs.ts#L446-L448
Added lines #L446 - L448 were not covered by tests
[warning] 451-451: app/utils/code-mirror-gutter-rs.ts#L451
Added line #L451 was not covered by tests
[warning] 458-458: app/utils/code-mirror-gutter-rs.ts#L458
Added line #L458 was not covered by tests
[warning] 460-460: app/utils/code-mirror-gutter-rs.ts#L460
Added line #L460 was not covered by tests
[warning] 462-462: app/utils/code-mirror-gutter-rs.ts#L462
Added line #L462 was not covered by tests
[warning] 466-466: app/utils/code-mirror-gutter-rs.ts#L466
Added line #L466 was not covered by tests
[warning] 472-472: app/utils/code-mirror-gutter-rs.ts#L472
Added line #L472 was not covered by tests
[warning] 475-475: app/utils/code-mirror-gutter-rs.ts#L475
Added line #L475 was not covered by tests
[warning] 478-479: app/utils/code-mirror-gutter-rs.ts#L478-L479
Added lines #L478 - L479 were not covered by testsapp/utils/code-mirror-collapse-unchanged-gutter.ts (2)
47-62: Consider consolidating marker classes.
CollapseUnchangedGutterMarkerRSlargely mirrorsCollapseUnchangedGutterMarkerin structure. If practical, centralizing shared logic could simplify maintenance. For example, both classes could derive from a common base, or the existing marker could accept a parameter to toggle the “RS” variant.
74-80: Unified gutter configurations.The
gutterRSblock duplicates logic in the adjacentgutterconfiguration. If you foresee future changes affecting both, consider an abstraction to reduce repetition while still calling the respective marker constructor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.gitignore(1 hunks).prettierignore(1 hunks)app/components/billing-status-badge/discount-timer-badge.hbs(1 hunks)app/components/billing-status-badge/discount-timer-badge.ts(1 hunks)app/components/code-mirror.ts(2 hunks)app/components/course-page/header/navigation-controls.hbs(1 hunks)app/components/course-page/header/navigation-controls.ts(2 hunks)app/components/header.hbs(1 hunks)app/components/header.ts(2 hunks)app/components/pay-page/experimental-referral-discount-notice.ts(2 hunks)app/components/pay-page/experimental-signup-discount-notice.ts(2 hunks)app/utils/code-mirror-collapse-unchanged-gutter.ts(3 hunks)app/utils/code-mirror-gutter-rs.ts(1 hunks)app/utils/time-formatting.ts(2 hunks)tests/acceptance/view-discount-countdown-test.js(1 hunks)tests/pages/components/header.js(1 hunks)tests/unit/utils/time-formatting-test.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .prettierignore
🚧 Files skipped from review as they are similar to previous changes (10)
- tests/pages/components/header.js
- app/components/pay-page/experimental-signup-discount-notice.ts
- app/components/header.hbs
- app/components/pay-page/experimental-referral-discount-notice.ts
- app/components/billing-status-badge/discount-timer-badge.hbs
- tests/unit/utils/time-formatting-test.ts
- app/components/course-page/header/navigation-controls.hbs
- app/components/course-page/header/navigation-controls.ts
- tests/acceptance/view-discount-countdown-test.js
- .gitignore
🧰 Additional context used
📓 Learnings (1)
app/utils/code-mirror-gutter-rs.ts (2)
Learnt from: VasylMarchuk
PR: codecrafters-io/frontend#2561
File: app/utils/code-mirror-gutter-rs.ts:381-381
Timestamp: 2025-01-21T18:50:13.901Z
Learning: The file `app/utils/code-mirror-gutter-rs.ts` is cloned from `codemirror/[email protected]/src/gutter.ts` with minimal modifications. Style suggestions should be avoided to maintain easier updatability with the upstream source.
Learnt from: VasylMarchuk
PR: codecrafters-io/frontend#2561
File: app/utils/code-mirror-gutter-rs.ts:301-301
Timestamp: 2025-01-21T18:49:53.963Z
Learning: The file `app/utils/code-mirror-gutter-rs.ts` is cloned from `codemirror/[email protected]/src/gutter.ts` and should maintain minimal modifications to make future updates easier by reapplying only the necessary changes.
🪛 GitHub Check: codecov/patch
app/components/billing-status-badge/discount-timer-badge.ts
[warning] 28-28: app/components/billing-status-badge/discount-timer-badge.ts#L28
Added line #L28 was not covered by tests
app/utils/code-mirror-gutter-rs.ts
[warning] 95-95: app/utils/code-mirror-gutter-rs.ts#L95
Added line #L95 was not covered by tests
[warning] 169-169: app/utils/code-mirror-gutter-rs.ts#L169
Added line #L169 was not covered by tests
[warning] 204-204: app/utils/code-mirror-gutter-rs.ts#L204
Added line #L204 was not covered by tests
[warning] 206-206: app/utils/code-mirror-gutter-rs.ts#L206
Added line #L206 was not covered by tests
[warning] 219-222: app/utils/code-mirror-gutter-rs.ts#L219-L222
Added lines #L219 - L222 were not covered by tests
[warning] 224-224: app/utils/code-mirror-gutter-rs.ts#L224
Added line #L224 was not covered by tests
[warning] 226-227: app/utils/code-mirror-gutter-rs.ts#L226-L227
Added lines #L226 - L227 were not covered by tests
[warning] 230-231: app/utils/code-mirror-gutter-rs.ts#L230-L231
Added lines #L230 - L231 were not covered by tests
[warning] 234-235: app/utils/code-mirror-gutter-rs.ts#L234-L235
Added lines #L234 - L235 were not covered by tests
[warning] 246-246: app/utils/code-mirror-gutter-rs.ts#L246
Added line #L246 was not covered by tests
[warning] 259-259: app/utils/code-mirror-gutter-rs.ts#L259
Added line #L259 was not covered by tests
[warning] 293-293: app/utils/code-mirror-gutter-rs.ts#L293
Added line #L293 was not covered by tests
[warning] 299-299: app/utils/code-mirror-gutter-rs.ts#L299
Added line #L299 was not covered by tests
[warning] 325-326: app/utils/code-mirror-gutter-rs.ts#L325-L326
Added lines #L325 - L326 were not covered by tests
[warning] 328-330: app/utils/code-mirror-gutter-rs.ts#L328-L330
Added lines #L328 - L330 were not covered by tests
[warning] 332-332: app/utils/code-mirror-gutter-rs.ts#L332
Added line #L332 was not covered by tests
[warning] 334-334: app/utils/code-mirror-gutter-rs.ts#L334
Added line #L334 was not covered by tests
[warning] 340-342: app/utils/code-mirror-gutter-rs.ts#L340-L342
Added lines #L340 - L342 were not covered by tests
[warning] 350-350: app/utils/code-mirror-gutter-rs.ts#L350
Added line #L350 was not covered by tests
[warning] 425-425: app/utils/code-mirror-gutter-rs.ts#L425
Added line #L425 was not covered by tests
[warning] 446-448: app/utils/code-mirror-gutter-rs.ts#L446-L448
Added lines #L446 - L448 were not covered by tests
[warning] 451-451: app/utils/code-mirror-gutter-rs.ts#L451
Added line #L451 was not covered by tests
[warning] 458-458: app/utils/code-mirror-gutter-rs.ts#L458
Added line #L458 was not covered by tests
[warning] 460-460: app/utils/code-mirror-gutter-rs.ts#L460
Added line #L460 was not covered by tests
[warning] 462-462: app/utils/code-mirror-gutter-rs.ts#L462
Added line #L462 was not covered by tests
[warning] 466-466: app/utils/code-mirror-gutter-rs.ts#L466
Added line #L466 was not covered by tests
[warning] 472-472: app/utils/code-mirror-gutter-rs.ts#L472
Added line #L472 was not covered by tests
[warning] 475-475: app/utils/code-mirror-gutter-rs.ts#L475
Added line #L475 was not covered by tests
[warning] 478-479: app/utils/code-mirror-gutter-rs.ts#L478-L479
Added lines #L478 - L479 were not covered by tests
🪛 Biome (1.9.4)
app/utils/code-mirror-gutter-rs.ts
[error] 300-300: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 381-381: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (8)
app/components/billing-status-badge/discount-timer-badge.ts (3)
26-28: Add test coverage for isExpired getter.The static analysis indicates that line 28 is not covered by tests.
Please add test cases to cover the
isExpiredgetter, including both expired and non-expired scenarios.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 28-28: app/components/billing-status-badge/discount-timer-badge.ts#L28
Added line #L28 was not covered by tests
30-32:⚠️ Potential issueAdd guard clause for expired discounts.
The
timeLeftgetter should handle expired discounts to prevent showing negative durations.get timeLeft() { + if (this.isExpired) { + return 'Expired'; + } return formatTimeDurationForBadge(this.expiresAt, this.time.currentTime); }Likely invalid or redundant comment.
34-36: 🛠️ Refactor suggestionAvoid hardcoding discount percentage and improve accessibility.
The tooltip text has hardcoded values that should be derived from the model for maintainability.
get tooltipText() { - return `Upgrade in ${formatTimeDurationForCountdown(this.expiresAt, this.time.currentTime)} to get 40% off the annual plan. Click to view details.`; + return `Upgrade in ${formatTimeDurationForCountdown(this.expiresAt, this.time.currentTime)} to get ${this.args.discount.percentageOff}% off the annual plan. Click to view details.`; }Likely invalid or redundant comment.
app/utils/time-formatting.ts (1)
44-46: 🛠️ Refactor suggestionStandardize error handling approach.
There's an inconsistency in error handling between the formatting functions:
formatTimeDurationForCountdownlogs to SentryformatTimeDurationForBadgesilently returns '0 second'if (distanceInSeconds < 0) { + Sentry.captureMessage(`Received negative time duration for discount badge`, { + extra: { + expiryDate: laterDate, + currentDate: earlierDate, + }, + }); + return '0 second'; }Likely invalid or redundant comment.
app/components/header.ts (1)
31-33:⚠️ Potential issueAdd null check for currentUser access.
The getter might throw if
currentUseris null.get activeDiscountForYearlyPlan(): PromotionalDiscountModel | null { - return this.currentUser?.activeDiscountForYearlyPlan || null; + if (!this.currentUser) { + return null; + } + + return this.currentUser.activeDiscountForYearlyPlan || null; }Likely invalid or redundant comment.
app/utils/code-mirror-gutter-rs.ts (1)
1-4: Preserve minimal modifications from upstream.According to the retrieved learnings, this file is cloned from an upstream CodeMirror source. Making style or structural changes could complicate future merges from the parent repository. Keep differences minimal unless strictly necessary.
app/components/code-mirror.ts (2)
41-41: Check for potential duplication in highlight imports.You import
highlightActiveLineGutterRS, but you also importhighlightActiveLineGutterfrom the base library. If they both enable similar line highlighting, confirm that both are needed, or consider unifying them if they serve the same purpose.
70-71: Evaluate the combined highlight approach.The
highlightActiveLinearray now includes two separate active-line-gutter extensions. Confirm through testing that running both doesn’t produce duplication or visual conflicts in the gutter.
Prototype for discount timer badge
This is part 2 of 2 in a stack made with GitButler:
Summary by CodeRabbit
New Features
Bug Fixes
Chores
.gitignoreto exclude IDE and development tool-specific directories..prettierignoreto exclude specific TypeScript files from formatting.