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. |
| 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().
There was a problem hiding this comment.
Using the UUID as the key makes more sense to me than using the index. I'll update it. Thanks for the suggestion!
|
Also, I don't see any Chromatic builds associated with this PR, so you probably still need to set up a Chromatic token for your fork. |
|
Ah, thanks for catching that! I'll set one up.
Sarah
On January 14, 2025 at 5:37:29 PM, Paul Sarando ***@***.***) wrote:
Also, I don't see any Chromatic builds associated with this PR, so you
probably still need to set up a Chromatic token for your fork
<https://github.com/cyverse-de/sonora/blob/main/guidelines.md#chromatic-setup>
.
—
Reply to this email directly, view it on GitHub
<#613 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADFLJZIOQL2ZDHC73H7AG32KWUUTAVCNFSM6AAAAABUYWYK76VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOJRGM4TCMJWGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
| import ids from "../../ids"; | ||
|
|
||
| function AddonRateEditorRow(props) { | ||
| const { baseId, fieldName, key, onDelete } = props; |
There was a problem hiding this comment.
Note that the GitHub Action is now reporting a warning here (which should also be reported if you run npm run check locally):
'key' is assigned a value but never used
There was a problem hiding this comment.
Sounds good, thanks. I think I have that fixed locally, but I haven't pushed that change yet because I ran into some errors during testing after making some recent changes.
Co-authored-by: Paul Sarando <[email protected]>
Co-authored-by: Paul Sarando <[email protected]>
Co-authored-by: Paul Sarando <[email protected]>
Co-authored-by: Paul Sarando <[email protected]>
Co-authored-by: Paul Sarando <[email protected]>
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.