Skip to content
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.

Sign up for GitHub

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

CORE2016: add support for addon rates. #613

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

CORE2016: add support for addon rates. #613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ceffc5e
CORE-2016: added initial support for addon rates
slr71 Dec 20, 2024
f8e4f16
CORE-2016: fixed a bug with initial value population in addon rates
slr71 Dec 20, 2024
63ca8f5
CORE-2016: added support for adding new addon rates
slr71 Dec 20, 2024
325d5de
CORE-2016: added support for removing rates from an addon
slr71 Dec 20, 2024
f8fdbe2
CORE-2016: fixed the problem with the consumable flag not being set i…
slr71 Jan 7, 2025
6c21780
CORE-2016: fixed the date format in addon update request bodies
slr71 Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion public/static/locales/en/subscriptions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"addonCreated": "Add-on created",
"addonDelete": "Add-on(s) deleted",
"addonName": "Add-on name",
"addons": "Add-ons",
"addonRates": "Add-on Rates",
Copy link
Member

Choose a reason for hiding this comment

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

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?

"addonUpdated": "Add-on updated",
"addSubscription": "Add subscription",
"addTooltip": "Add subscription for existing user.",
Expand All @@ -26,6 +26,7 @@
"editAddons": "Edit Add-ons",
"editQuotas": "Edit Quotas",
"editSubscription": "Edit Subscription",
"effectiveDate": "Effective Date",
"endDate": "End Date",
"false": "False",
"gib": "GiB",
Expand All @@ -43,6 +44,7 @@
"quota": "Quota",
"quotas": "Quotas",
"quotaUpdated": "Quota updated.",
"rate": "Rate",
"removeAddonAriaLabel": "Remove add-on from subscription.",
"removeAddonError": "Failed to remove add-on. Please try again.",
"resourceType": "Resource Type",
Expand Down
127 changes: 127 additions & 0 deletions src/components/subscriptions/addons/edit/AddonRatesEditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/**
* Form fields for adding, editing, and deleting app addon rates.
*
* @author sarahr
*/

import React from "react";
import { useTranslation } from "i18n";
import {
Button,
TableContainer,
Table,
TableHead,
TableRow,
TableCell,
TableBody,
Typography,
} from "@mui/material";
import {
FormNumberField,
FormTimestampField,
} from "components/forms/FormField";
import buildID from "components/utils/DebugIDUtil";
import { minValue, nonEmptyField } from "components/utils/validations";
import { Field } from "formik";
import { Add, Delete } from "@mui/icons-material";

import ids from "../../ids";

function AddonRateEditorRow(props) {
const { baseId, fieldName, key, onDelete } = props;
const { t: i18nUtil } = useTranslation("util");
const { t } = useTranslation(["common"]);

return (
<TableRow>
<TableCell>
<Field
component={FormNumberField}
id={buildID(baseId, ids.ADDONS_DLG.ADDON_RATE.RATE)}
name={`${fieldName}.rate`}
required
validate={(value) => minValue(value, i18nUtil)}
/>
</TableCell>
<TableCell>
<Field
component={FormTimestampField}
id={buildID(
baseId,
ids.ADDONS_DLG.ADDON_RATE.EFFECTIVE_DATE
)}
name={`${fieldName}.effectiveDate`}
required
validate={(value) => nonEmptyField(value, i18nUtil)}
/>
</TableCell>
<TableCell passing="none">
<Button
id={buildID(baseId, ids.DELETE_BUTTON)}
aria-label={t("common:delete")}
onClick={() => {
onDelete(key);
}}
Comment on lines +62 to +64
Copy link
Member

Choose a reason for hiding this comment

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

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:

Suggested change
onClick={() => {
onDelete(key);
}}
onClick={onDelete}

>
<Delete />
</Button>
</TableCell>
</TableRow>
);
}

function AddonRatesEditor(props) {
const { addonRates, baseId, fieldName, onAdd, onDelete } = props;

const { t } = useTranslation(["subscriptions", "common"]);

return (
<>
<TableContainer>
<Table>
<Typography component="caption">
{t("subscriptions:addonRates")}
Copy link
Member

Choose a reason for hiding this comment

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

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.

Suggested change
{t("subscriptions:addonRates")}
{t("addonRates")}

</Typography>
<TableHead>
<TableRow>
<TableCell>
<Typography>
{t("subscriptions:rate")}
</Typography>
</TableCell>
<TableCell>
<Typography>
{t("subscriptions:effectiveDate")}
</Typography>
</TableCell>
<TableCell>
<Button
id={buildID(baseId, ids.ADD_BUTTON)}
color="primary"
variant="outlined"
startIcon={<Add />}
onClick={onAdd}
>
{t("common:add")}
</Button>
</TableCell>
</TableRow>
</TableHead>
<TableBody>
{addonRates?.map((addonRate, index) => (
<AddonRateEditorRow
addonRate={addonRate}
baseId={buildID(baseId, index)}
fieldName={`${fieldName}.${index}`}
key={index}
Copy link
Member

Choose a reason for hiding this comment

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

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().

onDelete={onDelete}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onDelete={onDelete}
onDelete={() => onDelete(index)}

/>
))}
</TableBody>
</Table>
</TableContainer>
</>
);
}

export default AddonRatesEditor;
44 changes: 38 additions & 6 deletions src/components/subscriptions/addons/edit/EditAddon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import { useTranslation } from "i18n";
import buildID from "components/utils/DebugIDUtil";
import { Button, MenuItem } from "@mui/material";
import DEDialog from "components/utils/DEDialog";
import { FastField, Field, Form, Formik } from "formik";
import FormCheckbox from "components/forms/FormCheckbox";
import FormNumberField from "components/forms/FormNumberField";
import FormTextField from "components/forms/FormTextField";
import { FastField, Field, FieldArray, Form, Formik } from "formik";
import {
FormCheckbox,
FormNumberField,
FormTextField,
} from "components/forms/FormField";
import { nonEmptyField, nonZeroValue } from "components/utils/validations";
import ErrorTypographyWithDialog from "components/error/ErrorTypographyWithDialog";

Expand All @@ -24,6 +26,7 @@ import {
RESOURCE_TYPES_QUERY_KEY,
} from "serviceFacades/subscriptions";
import { announce } from "components/announcer/CyVerseAnnouncer";
import AddonRatesEditor from "./AddonRatesEditor";

function EditAddonDialog(props) {
const { addon, open, onClose, parentId } = props;
Expand Down Expand Up @@ -119,7 +122,7 @@ function EditAddonDialog(props) {
}}
enableReinitialize={true}
>
{({ handleSubmit, resetForm }) => {
{({ handleSubmit, resetForm, values }) => {
return (
<Form>
<DEDialog
Expand Down Expand Up @@ -170,6 +173,7 @@ function EditAddonDialog(props) {
/>
)}
<EditAddonForm
addon={values}
parentId={parentId}
resourceTypes={resourceTypes}
t={t}
Expand All @@ -183,7 +187,7 @@ function EditAddonDialog(props) {
}

function EditAddonForm(props) {
const { parentId, resourceTypes, t } = props;
const { addon, parentId, resourceTypes, t } = props;
const { t: i18nUtil } = useTranslation("util");
return (
<>
Expand Down Expand Up @@ -239,6 +243,34 @@ function EditAddonForm(props) {
label={t("defaultPaid")}
name="defaultPaid"
/>
<FieldArray
name="addonRates"
render={(arrayHelpers) => {
const onAdd = () => {
arrayHelpers.push({
rate: 0,
effectiveDate: Date.now().toString(),
});
};

const onDelete = (index) => {
arrayHelpers.remove(index);
};

return (
<AddonRatesEditor
addonRates={addon?.addonRates}
baseId={buildID(
parentId,
ids.ADDONS_DLG.ADDON_RATES
)}
fieldName="addonRates"
onAdd={onAdd}
onDelete={onDelete}
/>
);
}}
/>
</>
);
}
Expand Down
27 changes: 26 additions & 1 deletion src/components/subscriptions/addons/edit/formatters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import dateConstants from "components/utils/dateConstants";
import { formatDateObject } from "components/utils/DateFormatter";
import { bytesInGiB, bytesToGiB } from "../../utils";

function formatEffectiveDate(effectiveDate) {
return formatDateObject(new Date(effectiveDate), dateConstants.ISO_8601);
}

function mapPropsToValues(addon) {
let values = {
addonName: "",
description: "",
defaultAmount: 0,
defaultPaid: true,
resourceType: "",
addonRates: [],
};

if (addon) {
Expand All @@ -17,6 +24,7 @@ function mapPropsToValues(addon) {
default_amount,
default_paid,
resource_type,
addon_rates,
} = addon;

values = {
Expand All @@ -30,6 +38,14 @@ function mapPropsToValues(addon) {
: default_amount,
defaultPaid: default_paid,
resourceType: resource_type.unit,
addonRates: addon_rates.map((addonRate, index) => {
return {
uuid,
Copy link
Member

Choose a reason for hiding this comment

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

This uuid was destructured from the parent addon, so the addon's uuid is getting reused here.
This should probably be:

Suggested change
uuid,
uuid: addonRate.uuid,

effectiveDate: addonRate.effective_date,
key: `addonRates.${index}`,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the key field is used, so it can be removed here.

rate: addonRate.rate,
};
}),
};
}

Expand All @@ -44,13 +60,14 @@ function formatAddonSubmission(values, resourceTypes, update = false) {
defaultAmount,
defaultPaid,
resourceType,
addonRates,
} = values;

const resourceObj = resourceTypes.find(
(resource) => resourceType === resource.unit
);

const { id, unit, name } = resourceObj;
const { id, unit, name, consumable } = resourceObj;

const submission = {
name: addonName,
Expand All @@ -60,6 +77,13 @@ function formatAddonSubmission(values, resourceTypes, update = false) {
resource_type: {
uuid: id,
},
addon_rates: addonRates.map((addonRate) => {
return {
uuid,
Copy link
Member

Choose a reason for hiding this comment

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

This also looks like the addon's uuid (destructured from values above).

Suggested change
uuid,
uuid: addonRate.uuid,

effective_date: formatEffectiveDate(addonRate.effectiveDate),
rate: addonRate.rate,
};
}),
};

// Include the submission's UUID if an update is requested
Expand All @@ -70,6 +94,7 @@ function formatAddonSubmission(values, resourceTypes, update = false) {
...submission.resource_type,
unit,
name,
consumable,
};
}

Expand Down
5 changes: 5 additions & 0 deletions src/components/subscriptions/ids.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ export default {
NAME: "name",
RESOURCE_TYPE: "resourceType",
RESOURCE_UNIT: "resourceUnit",
ADDON_RATES: "addonRates",
ADDON_RATE: {
RATE: "rate",
EFFECTIVE_DATE: "effectiveDate",
},
},
ADDONS_MENU_ITEM: "addonsMenuItem",
CANCEL_BUTTON: "cancelButton",
Expand Down
4 changes: 2 additions & 2 deletions src/components/utils/DateFormatter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
@author sriram
*/

import lightFormat from "date-fns/lightFormat";
import format from "date-fns/format";
import toDate from "date-fns/toDate";
import dateConstants from "./dateConstants";
import { formatDistance, fromUnixTime } from "date-fns";
Expand All @@ -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)
Copy link
Member Author

@slr71 slr71 Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

: dateConstants.EMPTY_DATE;
}

Expand Down
1 change: 1 addition & 0 deletions src/components/utils/dateConstants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ export default {
DATE_FORMAT: "yyyy-MM-dd",
TIME_FORMAT: "HH:mm:ss",
LONG_DATE_FORMAT: "yyyy-MM-dd HH:mm:ss",
ISO_8601: "yyyy-MM-dd'T'HH:mm:ssXXX",
EMPTY_DATE: "-",
};
14 changes: 14 additions & 0 deletions stories/subscriptions/SubscriptionAddonsMocks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ export const availableAddons = {
name: "cpu.hours",
unit: "cpu hours",
},
addon_rates: [
{
uuid: "932ded25-fcad-4680-8e91-c44002b6e91b",
rate: 125.0,
effective_date: "2022-01-01T07:00:00Z",
},
],
},
{
uuid: "c21dd61f-aa41-40ad-8005-859679ceed9c",
Expand All @@ -23,6 +30,13 @@ export const availableAddons = {
name: "data.size",
unit: "bytes",
},
addon_rates: [
{
uuid: "932ded25-fcad-4680-8e91-c44002b6e91b",
rate: 125.0,
effective_date: "2022-01-01T07:00:00Z",
},
],
},
],
};
Expand Down
Loading