chore(add-money): PIX-BR onramp recovery toggle + maintenance-flag cleanup#2269
Draft
Hugo0 wants to merge 2 commits into
Draft
chore(add-money): PIX-BR onramp recovery toggle + maintenance-flag cleanup#2269Hugo0 wants to merge 2 commits into
Hugo0 wants to merge 2 commits into
Conversation
…eanup Draft follow-up to #2268, to merge once the BRL-via-PIX onramp (Manteca Brazil) is stable again: flips pixBrazilOnrampMaintenance off so the "Maintenance" tag + in-flow banner disappear. The warn-only machinery stays in place, ready to flip back on if the onramp degrades again. Also fixes two smells surfaced in the #2268 review: - both surfaces (the list tag and the in-flow banner) now read the maintenance state through one isPixBrazilOnrampUnderMaintenance() selector instead of two separate flag reads that could drift apart if the rule ever gains complexity (e.g. a time window or extra regions). - the maintenance test snapshots and restores the shipped flag instead of hardcoding the restore value to `true`, so it no longer corrupts shared module state now that the committed default is `false`.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughConfiguration constants and flag naming are refactored in ChangesPIX Onramp Maintenance Configuration and Consumer Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
Contributor
Code-analysis diffPainscore total: 5762.17 → 5756.21 (-5.96) 🆕 New findings (11)
✅ Resolved (11)
|
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
…tenance surfaces Self-review follow-up on the #2268 surfaces — the maintenance config had drifted into mixed naming styles and read patterns. Make them consistent: - enablePixOnrampMaintenanceWarning replaces the noun-shaped pixBrazilOnrampMaintenance so it reads like its true siblings (enableFullMaintenance, enableMaintenanceBanner): verb-first, true = the maintenance behavior is ON. The config header now documents the convention (enable<Behavior> vs disable<Feature>) so new flags can't reintroduce the ambiguity. - drop the lone isPixBrazilOnrampUnderMaintenance() getter and read the flag inline like every other flag in the file. The shared copy constant already de-dupes the only thing worth sharing; each surface's context check (pix-add method / country BR) is legitimately local. - PIX_ONRAMP_MAINTENANCE_COPY replaces PIX_BRAZIL_ONRAMP_MAINTENANCE — drops the redundant "Brazil" (PIX is Brazil-only) and marks it as copy. - read the config under one name everywhere (underMaintenanceConfig); three files had aliased the default import to maintenanceConfig. The enable*/disable* polarity of the existing flags is deliberately left untouched: flipping booleans across a prod kill-switch file is an outage risk and out of scope here. The new header convention documents that polarity rather than changing it.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(mobile-ui)/qr-pay/__tests__/qr-pay-states.test.tsx:
- Around line 869-870: Convert the dynamic require() statement for the
underMaintenance.config to an ES6 import statement at the top of the
qr-pay-states.test.tsx file to comply with the
`@typescript-eslint/no-require-imports` ESLint rule. Add the import statement at
the file level, then replace the require() call on line 869 with direct usage of
the imported underMaintenanceConfig object, keeping the mutation of the
disabledPaymentProviders property and its cleanup within the test function scope
as it is currently structured.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e38c99bf-c945-4a3d-8eac-a9ba41b2d32d
📒 Files selected for processing (8)
src/app/(mobile-ui)/qr-pay/__tests__/qr-pay-states.test.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/components/AddMoney/components/MantecaAddMoney.tsxsrc/components/AddWithdraw/AddWithdrawCountriesList.tsxsrc/components/AddWithdraw/__tests__/AddWithdrawCountriesList.test.tsxsrc/components/Global/Banner/index.tsxsrc/config/underMaintenance.config.tssrc/proxy.ts
✅ Files skipped from review due to trivial changes (2)
- src/components/Global/Banner/index.tsx
- src/proxy.ts
Comment on lines
+869
to
+870
| const underMaintenanceConfig = require('@/config/underMaintenance.config').default | ||
| underMaintenanceConfig.disabledPaymentProviders = ['MANTECA'] |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify test structure around the config mutation lines
sed -n '860,890p' src/app/\(mobile-ui\)/qr-pay/__tests__/qr-pay-states.test.tsx | cat -nRepository: peanutprotocol/peanut-ui
Length of output: 1497
🏁 Script executed:
# Find the describe block containing this test to see if other tests need the same config
sed -n '850,920p' 'src/app/(mobile-ui)/qr-pay/__tests__/qr-pay-states.test.tsx' | cat -nRepository: peanutprotocol/peanut-ui
Length of output: 3140
Convert require() to ES6 import to comply with ESLint rules.
The dynamic require() call on line 869 violates the project's @typescript-eslint/no-require-imports rule. Since the setup and cleanup are already properly scoped within the test function, add an ES6 import at the top of the file and keep the mutation and cleanup within the test:
🔧 Suggested refactor
At the top of the test file, add:
import underMaintenanceConfig from '`@/config/underMaintenance.config`'Then replace line 869 with direct usage:
- const underMaintenanceConfig = require('`@/config/underMaintenance.config`').default
underMaintenanceConfig.disabledPaymentProviders = ['MANTECA']Keep the cleanup at the end of the test as-is (line 881).
🧰 Tools
🪛 ESLint
[error] 869-869: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`(mobile-ui)/qr-pay/__tests__/qr-pay-states.test.tsx around lines 869
- 870, Convert the dynamic require() statement for the underMaintenance.config
to an ES6 import statement at the top of the qr-pay-states.test.tsx file to
comply with the `@typescript-eslint/no-require-imports` ESLint rule. Add the
import statement at the file level, then replace the require() call on line 869
with direct usage of the imported underMaintenanceConfig object, keeping the
mutation of the disabledPaymentProviders property and its cleanup within the
test function scope as it is currently structured.
Source: Linters/SAST tools
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.
Follow-up to #2268.
Why
#2268 shipped the warn-only "PIX under maintenance" tag + in-flow banner with the flag on. This is the matching recovery PR — ready in advance so flipping PIX back to normal is a one-merge action, not a scramble. It also cleans up two smells from the #2268 review.
What
pixBrazilOnrampMaintenance: true → falseinunderMaintenance.config.ts. The tag + banner disappear; the warn-only machinery stays in place, ready to flip back on (set the flag totrue) if the onramp degrades again.isPixBrazilOnrampUnderMaintenance()selector instead of two separateunderMaintenanceConfig.pixBrazilOnrampMaintenancereads. The maintenance determination lives in one place, so if the rule ever gains complexity (a time window, more regions) the two surfaces can't diverge. Each caller still keeps its own context check (thepix-addmethod / countryBR).afterEachrestore totrue. Previously, once the committed default becamefalse(i.e. this PR), the oldafterEachwould have left the shared config singleton stuck attruefor anything running after it.Base
Cut onto
main, notdev: thepixBrazilOnrampMaintenanceflag + UI only exist onmainso far (the #2268 main→dev back-merge is still pending), somainis the only base where this is a clean, minimal diff. When the back-merge lands, this change rides along with it.Scope / notes
falsebeyond the maintenance UI being hidden — i.e. the pre-feat(add-money): flag BRL-via-PIX onramp as under maintenance (warn-only) #2268 deposit experience.Tests (local gate, run in worktree)
prettier✓ (all 4 files unchanged after--write)tsc✓ — 0 errors in changed filesjest✓ —AddWithdrawCountriesList(7) +add-money-states(41) pass; maintenance tests pass with the flag both on and offnext buildleft to CI (authoritative — submodules + env)