feat: [M3-6980] - Add DC Specific Pricing Invoice Support #9597
Conversation
There was a problem hiding this comment.
Looks great! Just a couple things unrelated to the feature itself:
a min-width on the date fields would help readability on mobile
you may not know, but worth asking the question, is the billing name.address still relevant?
lastly, i noticed that none of the code additions user alphabetical ordering - is that intentional?
bnussman-akamai
added
Add'tl Approval Needed
packages/manager/src/features/Billing/InvoiceDetail/InvoiceTable.tsx
Outdated
Show resolved
Hide resolved
@cpathipa |
|
@bnussman thx! nevermind, i read wrong, i was seeing ordered arguments as unalphabetised but that is irrelevant. |
| export const getInvoiceRegion = (invoiceItem: InvoiceItem) => { | ||
| // If the invoice item is not regarding transfer, just return the region. | ||
| if (!invoiceItem.label.toLowerCase().includes('transfer')) { | ||
| return invoiceItem.region; |
There was a problem hiding this comment.
In tables, I know we generally sub out the region id, which we get from invoiceItem.region, with the human readable label (e.g. Jakarta, ID). Do we want to do that here as well?
| @@ -279,6 +294,21 @@ const truncateLabel = (label: string) => { | |||
| return label.length > 20 ? `${label.substr(0, 20)}...` : label; | |||
| }; | |||
|
|
|||
| export const getInvoiceRegion = (invoiceItem: InvoiceItem) => { | |||
| // If the invoice item is not regarding transfer, just return the region. | |||
| if (!invoiceItem.label.toLowerCase().includes('transfer')) { | |||
There was a problem hiding this comment.
We can expect region to return a null image, but I think falling back to an empty string in that case is fine. API can't think of another situation in which a region would be null for a service's invoice item -- and we're still checking on whether it will be null for transfer overages in the global/general pool.
bnussman-akamai
added
Approved
|
Somehow I accidentally submitted my review before I meant to. I meant to add:
|
|
@bnussman-akamai These most recent failures are caused by the changes in #9524, so merging in the latest changes from |
* add basic region columns to pdf, csv, and table * handle region transfer * Added changeset: DC Specific Pricing Invoice Support * fix error state * add min width * improve invoice label condition check * fix loading state * make comment more clear --------- Co-authored-by: Banks Nussman <banks@nussman.us>
Description 📝
Major Changes 🔄
regioncolumn to Invoice Details, PDF Invoice, and CSV invoicePreview 📷
Invoice Detail 🧾
CSV 🧾
PDF 🧾
How to test 🧪
http://localhost:3000/account/billing/invoices/555