Skip to content

[TEST] Invoice overhaul#115241

Open
itsdangold wants to merge 6 commits intomasterfrom
design/invoice-overhaul
Open

[TEST] Invoice overhaul#115241
itsdangold wants to merge 6 commits intomasterfrom
design/invoice-overhaul

Conversation

@itsdangold
Copy link
Copy Markdown
Contributor

This is an experimental PR improving the invoice detail UI

@itsdangold itsdangold requested a review from a team as a code owner May 8, 2026 22:31
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 8, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

📊 Type Coverage Diff

✅ No new type safety issues introduced. Coverage: 93.46%

Comment on lines +288 to +290
return 'subscription';
}
if (type.startsWith('ondemand_') || type === 'ondemand') {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The bucketFor function incorrectly classifies reserved_seer_budget as a 'subscription' item due to its broad prefix check, causing display issues on invoices.
Severity: MEDIUM

Suggested Fix

Update the bucketFor function to explicitly exclude reserved_seer_budget from the 'subscription' bucket logic. It should be handled as a special case, potentially falling into the 'adjustment' bucket or another more appropriate category. For example: if ((type === 'subscription' || type.startsWith('reserved_')) && type !== 'reserved_seer_budget').

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/gsApp/views/invoiceDetails/index.tsx#L288-L290

Potential issue: The `bucketFor` function incorrectly classifies `reserved_seer_budget`
items into the 'subscription' bucket because its logic checks if the type starts with
`reserved_`. However, `reserved_seer_budget` is defined as a "Special case" and is
treated as a distinct add-on product elsewhere in the codebase, not a standard reserved
volume item. This misclassification causes it to be displayed in the "Subscription"
section of an invoice with quantity and rate columns. Since it's a shared budget without
per-item quantity data, these columns will display '—', potentially causing confusion
for customers viewing their invoice.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f652fa5. Configure here.

)}
{!!taxNumber && (
<Text size="sm" variant="secondary">
{taxNumberName}: {taxNumber}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Double colon rendered in tax number display

Medium Severity

taxNumberName is defined as `${getTaxFieldInfo(countryCode).label}:` which already includes a trailing colon (e.g. "ABN:"). The template on line 259 then renders {taxNumberName}: {taxNumber}, producing a double colon like "ABN:: 12345". The old code used taxNumberName in a <dt> element where the trailing colon was appropriate, but the new inline layout adds a second colon.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f652fa5. Configure here.

<Tag variant="warning" icon={<IconTimer />}>
{t('Waiting for payment')}
</Tag>
)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Closed invoice status incorrectly shows waiting for payment

Medium Severity

The old code distinguished three invoice states: paid, awaiting payment, and closed. The new status badge only checks invoice.isPaid, so a closed-but-unpaid invoice now displays "Waiting for payment" instead of a closed/finalized state. This is misleading since isClosed invoices are no longer accepting payment — and the code already uses isClosed to hide the "Pay Now" button on line 91.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f652fa5. Configure here.

</Heading>
<Text size="md" variant="secondary">
{t('Charged')} <DateTime date={invoice.dateCreated} dateOnly year />.
</Text>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Misleading "Charged" text shown for unpaid invoices

Medium Severity

The text "Charged" is displayed unconditionally next to dateCreated, even for invoices that are unpaid or closed. This directly contradicts the "Waiting for payment" badge shown above it. The old code used a neutral "Date:" label. The actions footer correctly uses "Generated" instead, highlighting this as an oversight.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f652fa5. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant