-
Notifications
You must be signed in to change notification settings - Fork 7
fix(reset): keep unknown credit counts distinct from zero in the confirmation #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -142,7 +142,7 @@ function resetAccountOptions(payload: OpenDialogPayload): ResetDialogOption[] { | |||||||||||
| account.usedPercent === undefined | ||||||||||||
| ? 'quota unavailable' | ||||||||||||
| : `${account.usedPercent}%` | ||||||||||||
| const counts = `${account.applicableAvailableCount ?? 0}/${account.availableCount ?? 0}` | ||||||||||||
| const counts = `${account.applicableAvailableCount === undefined ? '?' : account.applicableAvailableCount}/${account.availableCount ?? '?'}` | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: When availableCount is undefined, the main account list renders 'credits unavailable', but the dialog account option renders '?/?'. These two surfaces describe the same unknown state differently, contradicting the PR's stated consistency goal. Use the same wording in resetAccountOptions (e.g. fall back to 'credits unavailable' when availableCount is undefined). Prompt for AI agents
Suggested change
|
||||||||||||
| const status = account.eligible | ||||||||||||
| ? 'eligible' | ||||||||||||
| : (account.reason ?? 'unavailable') | ||||||||||||
|
|
@@ -278,7 +278,7 @@ function openResetDialog( | |||||||||||
| const preview = state.knobs.preview as ResetPreviewKnob | undefined | ||||||||||||
| const accountKey = preview?.accountKey | ||||||||||||
| const chatgptAccountId = preview?.chatgptAccountId | ||||||||||||
| const availableCount = preview?.availableCount ?? 0 | ||||||||||||
| const availableCount = preview?.availableCount ?? 'unknown' | ||||||||||||
| const DialogConfirm = api.ui.DialogConfirm | ||||||||||||
| api.ui.dialog.replace(() => ( | ||||||||||||
| <DialogConfirm | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: The
not.toContain('does not currently count')assertion in this test is inert:''args render the account-list view (renderResetAccountList), and that disclosure string exists only in renderResetConfirm, so the assertion passes regardless of the implementation. Drop it or move it to a select/confirm-view test to actually guard the disclosure behavior.Prompt for AI agents