-
Notifications
You must be signed in to change notification settings - Fork 367
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
feat: [M3-7096] - Show Region label
in addition to id
on Invoices
#9663
feat: [M3-7096] - Show Region label
in addition to id
on Invoices
#9663
Conversation
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.
Thanks for the updates with some good clean up and tests! 🧹 Left a couple tiny suggestions.
Also: can we update the MSW invoice items endpoint (rest.get('*invoices/555/items',
) so we're seeing an accurate representation of what an overage line item in the global transfer pool?
It looks like that includes the substring "Transfer Overage" and we've currently got "Outbound Transfer".
packages/manager/src/features/Billing/PdfGenerator/utils.test.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/Billing/PdfGenerator/utils.test.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/Billing/InvoiceDetail/InvoiceTable.tsx
Outdated
Show resolved
Hide resolved
<TableCell colSpan={columns}> | ||
<TableCell | ||
colSpan={columns} | ||
sx={{ paddingLeft: '0px !important' }} |
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.
What was the crazy styling issue here?
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.
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.
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.
LGTM, thanks! 🚢
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.
Would be nice to have tests for the PDF generation too. Could be a follow up but it wouldn't be too labor intensive using pdf-parse - will make a ticket
Description 📝
label
andid
in the DC specific pricing invoices. This ticket is a follow up to address some feedback from feat: [M3-6980] - Add DC Specific Pricing Invoice Support #9597.Preview 📷
Important
The CSV does not contain the region
label
because I followed the current pattern of showing "pure" API data. (Notice how the dates are in the API's format)How to test 🧪
label
s andid
s on the Invoices with theDC Specific Pricing
feature flag on