CORE2016: add support for addon rates. #613
Conversation
| @@ -16,7 +16,7 @@ import { formatDistance, fromUnixTime } from "date-fns"; | |||
| function formatDate(longDate, dateFormat = dateConstants.LONG_DATE_FORMAT) { | |||
| const longDateInt = parseInt(longDate, 10); | |||
| return longDateInt | |||
| ? lightFormat(toDate(new Date(longDateInt)), dateFormat) | |||
| ? format(toDate(new Date(longDateInt)), dateFormat) | |||
There was a problem hiding this comment.
The switch from lightFormat to format caused me a little bit of concern. As far as I can tell format is fully compatible with lightFormat but not vice versa. If anyone knows of a case (that applies to us) where this is not true, please let me know and I'll create a separate function for formatting ISO8601 dates.
|
There's currently a field marked as being required in Terrain that shouldn't be in the case of an update. I'll fix that before merging this PR. |
| "addons": "Add-ons", | ||
| "addonRates": "Add-on Rates", |
There was a problem hiding this comment.
Was addons intentionally changed to addonRates?
It looks like the addons label is still in use, since in the screenshot of these updates, the tab in the /admin/subscriptions page now reads addons instead of Add-ons (and if you view the browser console in storybook or dev mode, the i18n lib should report a warning that the addons messages key was not found).
So should addonRates just be a new key/message?
| onClick={() => { | ||
| onDelete(key); | ||
| }} |
There was a problem hiding this comment.
I don't think the key prop is accessible here, since it's just for React:
Note that your components won’t receive key as a prop. It’s only used as a hint by React itself. If your component needs an ID, you have to pass it as a separate prop:
<Profile key={id} userId={id} />.
The onDelete prop can be updated below to use the index directly, then this can become:
| onClick={() => { | |
| onDelete(key); | |
| }} | |
| onClick={onDelete} |
| baseId={buildID(baseId, index)} | ||
| fieldName={`${fieldName}.${index}`} | ||
| key={index} | ||
| onDelete={onDelete} |
There was a problem hiding this comment.
| onDelete={onDelete} | |
| onDelete={() => onDelete(index)} |
| <TableContainer> | ||
| <Table> | ||
| <Typography component="caption"> | ||
| {t("subscriptions:addonRates")} |
There was a problem hiding this comment.
It's not required to namespace these subscriptions messages, since the i18n lib will look for these keys in the first namespace passed to useTranslation by default.
| {t("subscriptions:addonRates")} | |
| {t("addonRates")} |
| return { | ||
| uuid, | ||
| effectiveDate: addonRate.effective_date, | ||
| key: `addonRates.${index}`, |
There was a problem hiding this comment.
I don't think the key field is used, so it can be removed here.
| @@ -30,6 +38,14 @@ function mapPropsToValues(addon) { | |||
| : default_amount, | |||
| defaultPaid: default_paid, | |||
| resourceType: resource_type.unit, | |||
| addonRates: addon_rates.map((addonRate, index) => { | |||
| return { | |||
| uuid, | |||
There was a problem hiding this comment.
This uuid was destructured from the parent addon, so the addon's uuid is getting reused here.
This should probably be:
| uuid, | |
| uuid: addonRate.uuid, |
| @@ -60,6 +77,13 @@ function formatAddonSubmission(values, resourceTypes, update = false) { | |||
| resource_type: { | |||
| uuid: id, | |||
| }, | |||
| addon_rates: addonRates.map((addonRate) => { | |||
| return { | |||
| uuid, | |||
There was a problem hiding this comment.
This also looks like the addon's uuid (destructured from values above).
| uuid, | |
| uuid: addonRate.uuid, |
| addonRate={addonRate} | ||
| baseId={buildID(baseId, index)} | ||
| fieldName={`${fieldName}.${index}`} | ||
| key={index} |
There was a problem hiding this comment.
It's possible using the index as a key may cause issues when addon rates get deleted in the form (though it should be tested since it might only cause issues when rates are deleted and inserted, or if they need to be rearranged in the future).
It might be better to use addonRate.uuid as the key, but then a new uuid would have to be created in the onAdd function with crypto.randomUUID().
This PR adds support for addon rates to the subscription admin page in the DE. The screen will now look like this:
While this change was being made. The admin page was also updated to add support for the
consumableflag in the resource type object. This addon update form hadn't been working since theconsumableflag was added.