Increased retention offer code entropy from 16-bit to 32-bit#26738
Increased retention offer code entropy from 16-bit to 32-bit#26738
Conversation
closes https://linear.app/ghost/issue/BER-3422/increase-retention-code-entropy - retention offer codes now use Uint32Array instead of Uint16Array, generating 8-character hex codes instead of 4-character ones, increasing the code space from ~65K to ~4.3 billion possible values - also extracted the offer name generation logic and added unit tests, to make sure we stay under the 40-char max. limit imposed by Stripe on coupon names --------- Co-authored-by: Kevin Ansfield <kevin@ghost.org>
WalkthroughThe changes implement validation constraints for retention offers by capping the maximum retention duration at 99 months and centralizing error messaging. A new Possibly related issues
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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.
🧹 Nitpick comments (1)
apps/admin-x-settings/src/components/settings/growth/offers/offer-helpers.ts (1)
62-64: Consider using a directexport constdeclaration.The constant is declared at the top without
export, then re-exported at the bottom. This works but is slightly unconventional. Consider declaring it asexport constdirectly on line 1 for cleaner code.♻️ Suggested refactor
-const MAX_RETENTION_OFFER_NAME_LENGTH = 40; +export const MAX_RETENTION_OFFER_NAME_LENGTH = 40;Then remove lines 62-64:
-export { - MAX_RETENTION_OFFER_NAME_LENGTH -};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/src/components/settings/growth/offers/offer-helpers.ts` around lines 62 - 64, The constant MAX_RETENTION_OFFER_NAME_LENGTH is declared without export and then re-exported at the bottom; change it to a direct exported declaration (export const MAX_RETENTION_OFFER_NAME_LENGTH = ...) where it’s originally defined and remove the separate export block (the export { MAX_RETENTION_OFFER_NAME_LENGTH };) to simplify and clarify the module’s API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/admin-x-settings/src/components/settings/growth/offers/offer-helpers.ts`:
- Around line 62-64: The constant MAX_RETENTION_OFFER_NAME_LENGTH is declared
without export and then re-exported at the bottom; change it to a direct
exported declaration (export const MAX_RETENTION_OFFER_NAME_LENGTH = ...) where
it’s originally defined and remove the separate export block (the export {
MAX_RETENTION_OFFER_NAME_LENGTH };) to simplify and clarify the module’s API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0caa826e-518a-4cc1-a8ef-99d232fc411d
📒 Files selected for processing (4)
apps/admin-x-settings/src/components/settings/growth/offers/edit-retention-offer-modal.tsxapps/admin-x-settings/src/components/settings/growth/offers/offer-helpers.tsapps/admin-x-settings/test/acceptance/membership/offers.test.tsapps/admin-x-settings/test/unit/components/settings/growth/offers/offer-helpers.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1b8083ccc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const isValidMonthDuration = (value: number): boolean => { | ||
| return Number.isInteger(value) && value > 0; | ||
| return Number.isInteger(value) && value > 0 && value <= MAX_RETENTION_OFFER_MONTHS; |
There was a problem hiding this comment.
Handle legacy retention offers above 99 months
The new value <= MAX_RETENTION_OFFER_MONTHS check rejects any existing retention offer whose duration_in_months is above 99, even if the admin only edits display text. Because onValidate runs before handleSave, those legacy offers can no longer be saved in their current form once opened in this modal, which is a backward-compatibility regression for stores that already created long-duration retention offers under the previous rules.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Acceptable, as feature is still in private beta and the limit is anyways much higher than the typical use case for a retention offer
🤖 Velo CI Failure AnalysisClassification: 🔴 HARD FAIL
|
closes https://linear.app/ghost/issue/BER-3422/increase-retention-code-entropy
Co-authored-by: Kevin Ansfield kevin@ghost.org