feat: default new users to INR instead of USD - #114
Merged
Conversation
Changes the default home currency for new accounts, the default currency on a newly created subscription in the mobile client, and the currency Entry placeholders. Existing accounts are untouched. The migration alters the users.preferred_currency column default only — it does not backfill. A default change should not silently overwrite a currency a user deliberately chose. Drops the redundant PreferredCurrency assignment in AuthService: the User entity initializer already carries the default, so registration had two places stating the same value. Two burn-rate integration tests created USD subscriptions and happened to match the old USD home currency, so no FX conversion ran. With the new default they would need a live USD->INR rate the test container cannot fetch. Switched them to INR so they keep testing burn-rate math.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Makes INR the default currency across the app instead of USD.
User.PreferredCurrency(entity default)USDINRusers.preferred_currency(DB column default)USDINRUSDINREntryplaceholders (subscription + settings pages)USDINRdocs/Design.mdDDL'USD''INR'Existing users are not touched
The migration (
DefaultPreferredCurrencyInr) alters the column default only — noUPDATE, no backfill. Anyone who already picked a currency keeps it. Changing a default should not overwrite a deliberate choice, so theDownmigration is a clean inverse for the same reason.Cleanup included
AuthService.RegisterAsyncsetPreferredCurrency = "USD"explicitly while theUserentity initializer already carried the same default — two places stating one value. Removed the assignment; the entity default is now the single source. Registration behavior is unchanged.No change needed in
FxRateRefreshBackgroundService: it derives its base/target currency pairs from the rows actually in use, so it picks up INR without a code change.Test changes
DashboardControllerTests.GetBurnRate_WithActiveMonthlySubscription_ReturnsCorrectTotalsandSubscriptionsControllerTests(viaValidRequest()) createdUSDsubscriptions. Those tests passed because the subscription currency happened to equal the oldUSDhome currency, so the burn-rate calculator never converted. With an INR home currency they would need a liveUSD -> INRrate that the Testcontainers-backed run has no way to fetch. Both now useINR, which keeps them testing burn-rate math rather than FX plumbing. Mixed-currency conversion stays covered byBurnRateCalculatorTests, which stubs rates.UsersControllerTestsnow assertsINRas the registration default — that assertion is the check on this change.Verification
All four test projects pass locally:
Notes for the reviewer
dotnet ef database update(thedb-migrateworkflow) — the default only applies to rows inserted after it runs.Entryon both mobile pages. A picker would prevent typos but is out of scope here.