Skip to content

[MON-1122] Changes the rounding mode to .down instead of .plain#5821

Merged
polpielladev merged 11 commits into
mainfrom
mon-1122-revert-variable-rounding-logic-to-not-round-up
Nov 24, 2025
Merged

[MON-1122] Changes the rounding mode to .down instead of .plain#5821
polpielladev merged 11 commits into
mainfrom
mon-1122-revert-variable-rounding-logic-to-not-round-up

Conversation

@polpielladev

Copy link
Copy Markdown
Collaborator

When shipping Paywalls v2, there was a regression with the way that we show prices. We changed from not rounding or rounding down to actually rounding up.

Where this is specially important is when a dev picks a price that is explicitly designed to get just under a dollar threshold, like $83.99, which is either 6.99 or 7.00 — and 6.99 is the actual price they're aiming to put in front of customers.

Checklist

  • If applicable, unit tests
  • If applicable, create follow-up issues for purchases-android and hybrids

@polpielladev polpielladev requested a review from a team as a code owner November 18, 2025 11:43
@polpielladev polpielladev added the pr:fix A bug fix label Nov 18, 2025
@emerge-tools

emerge-tools Bot commented Nov 18, 2025

Copy link
Copy Markdown

📸 Snapshot Test

12 modified, 468 unchanged

Name Added Removed Modified Renamed Unchanged Errored Approval
RevenueCat
com.revenuecat.PaywallsTester
0 0 7 0 233 0 ✅ Approved
RevenueCat
com.revenuecat.PaywallsTester.mac-catalyst-optimized-for-mac
0 0 5 0 235 0 ✅ Approved

🛸 Powered by Emerge Tools

@ajpallares ajpallares left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for fixing this!

I was wondering if this is indeed the behavior we want, but I've seen the internal discussion where this is agreed 👍

@ajpallares

Copy link
Copy Markdown
Member

By the way @polpielladev, there are some snapshots that have been modified now due to this change, which is expected. Yo can totally go ahead, take a look at them and approve if they look ok.

Also, could you please run a @RCGitBot please test to run the full test suite? Thanks!

@polpielladev

Copy link
Copy Markdown
Collaborator Author

@ajpallares Awesome, I do now have access to Emerge, so will go about fixing things shortly. Am I right in thinking that this change will the propagate to the hybrids without having to change the logic there? I saw a customer report from the capacitor SDK but I can also see that the logic there is pulled from the Purchases Hybrid Common repo, which should use the native SDK right?

@ajpallares

Copy link
Copy Markdown
Member

Am I right in thinking that this change will the propagate to the hybrids without having to change the logic there? I saw a customer report from the capacitor SDK but I can also see that the logic there is pulled from the Purchases Hybrid Common repo, which should use the native SDK right?

That's right! 👍

@polpielladev polpielladev force-pushed the mon-1122-revert-variable-rounding-logic-to-not-round-up branch from 60bc625 to 0fa4d09 Compare November 19, 2025 08:47
@polpielladev

Copy link
Copy Markdown
Collaborator Author

@ajpallares It looks like I have some snapshot failures for the paywall tests, how do I re-record the snapshots? Is it documented somewhere?

@tonidero

tonidero commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

@ajpallares It looks like I have some snapshot failures for the paywall tests, how do I re-record the snapshots? Is it documented somewhere?

@polpielladev You can just enter into the emerge link for the failing tests and approve the changes. Once merged, that will become the new "golden"

Ignore me, didn't take a closer look 😅

@ajpallares

Copy link
Copy Markdown
Member

@tonidero I think @polpielladev meant this failure (https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/31809/workflows/65f93dd3-2212-489d-b87b-dfb30ee78b38/jobs/412055) from non-Emerge snapshots tests.

He already run the workflow to re-generate them with the changes of this PR (https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/31819) and the automatically-generated PRs in purchases-ios-snapshots have already been merged
RevenueCat/purchases-ios-snapshots#211
RevenueCat/purchases-ios-snapshots#209
RevenueCat/purchases-ios-snapshots#210
RevenueCat/purchases-ios-snapshots#208
RevenueCat/purchases-ios-snapshots#207

@ajpallares

Copy link
Copy Markdown
Member

Now @polpielladev, in your branch, you can just run

fastlane ios update_snapshots_repo

to update the commit reference of purchases-ios-snapshots (or you can just replace it in https://github.com/RevenueCat/purchases-ios/blob/main/Tests/purchases-ios-snapshots-commit with a7110f83ee04ee306015d8fd4b7368568473b172).

After that, you can push the changes and the CI will build again, hopefully now with successful snapshot test run

@polpielladev polpielladev requested a review from a team as a code owner November 19, 2025 14:55
@polpielladev

Copy link
Copy Markdown
Collaborator Author

@RCGitBot please test

@polpielladev

Copy link
Copy Markdown
Collaborator Author

@RCGitBot please test

@polpielladev polpielladev merged commit 5108941 into main Nov 24, 2025
47 checks passed
@polpielladev polpielladev deleted the mon-1122-revert-variable-rounding-logic-to-not-round-up branch November 24, 2025 14:54
dpannasch added a commit to RevenueCat/purchases-js that referenced this pull request Mar 19, 2026
## Summary
- Per-period price calculations (`pricePerWeek`, `pricePerMonth`,
`pricePerYear`) were using `Math.round` / `Intl.NumberFormat` halfExpand
rounding, causing prices like $83.99/yr to display as $7.00/mo instead
of $6.99/mo
- Added shared `floorMicrosToCurrencyUnit` helper with cached
`Intl.NumberFormat` lookup and IEEE 754 epsilon correction
- Fixed both code paths: `offerings.ts` (`toPricingPhase` entity
builder) and `paywall-price-helpers.ts` (paywall variable formatting)
- Matches iOS SDK behavior from
[purchases-ios#5821](RevenueCat/purchases-ios#5821)

## Test plan
- [x] Updated existing per-period price tests across 3 test files (538
total tests pass)
- [x] Added unit tests for `floorMicrosToCurrencyUnit` covering USD,
EUR, JPY, zero values, and FP epsilon
- [x] TypeScript typecheck and ESLint pass
- [ ] Manual verification with paywall displaying annual/6-month
packages

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants