Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
122 changes: 25 additions & 97 deletions src/actions/sponsor-forms-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1231,37 +1231,6 @@ export const deleteSponsorFormItem =
});
};

const saveItemImages =
(formId, formItemId, images) => async (dispatch, getState) => {
const { currentSummitState } = getState();
const { currentSummit } = currentSummitState;
const accessToken = await getAccessTokenSafely();
const params = { access_token: accessToken };

const promises = images.map((file) => {
if (file.id) {
return putRequest(
null,
createAction(SPONSOR_FORM_ITEM_IMAGES_UPDATED),
`${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}/items/${formItemId}/images/${file.id}`,
file,
authErrorHandler,
file
)(params)(dispatch);
}
return postRequest(
null,
createAction(SPONSOR_FORM_ITEM_IMAGES_UPDATED),
`${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}/items/${formItemId}/images`,
file,
authErrorHandler,
file
)(params)(dispatch);
});

return Promise.all(promises);
};

export const saveSponsorFormItem =
(formId, entity) => async (dispatch, getState) => {
const { currentSummitState } = getState();
Expand All @@ -1276,85 +1245,44 @@ export const saveSponsorFormItem =

const normalizedEntity = normalizeItem(entity);

return postRequest(
null,
createAction(SPONSOR_FORM_ITEM_UPDATED),
`${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}/items`,
normalizedEntity,
snackbarErrorHandler
)(params)(dispatch)
.then(({ response }) => {
const promises = [Promise.resolve(0)];

if (normalizedEntity.images?.length > 0) {
const savingImages = saveItemImages(
formId,
response.id,
normalizedEntity.images
)(dispatch, getState);

promises.push(savingImages);
}

return Promise.all(promises).then(() => {
if (entity.id) {
return putRequest(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@tomrndom This branch was cut from 78ef677c (Aug 3) and does not contain #1002 (merged Aug 21), so this rewrite of saveSponsorFormItem sits on top of the pre-#1002 version of the file and reverts it. GitHub already reports the PR as CONFLICTING; master is 31 commits ahead.

#1002 deliberately stopped sending images in the item request body, because the nested-images path replaces the whole collection on update. origin/master:src/actions/sponsor-forms-actions.js:1424-1429:

// Images are never round-tripped inline: the item add/update endpoint's
// nested-images path only clones the file name (no S3 copy) and, on
// update, replaces the whole collection - wiping cloned-from-inventory
// images whose id it can't preserve. New uploads are persisted separately
// via saveNewItemImages once the item itself is saved.
delete normalizedEntity.images;

With this branch's normalizeItem, stored images are filtered out — file_path is write_only in purchases-api's ShowFormItemImageSerializer, so a fetched item only carries id + file_url — and the PUT sends images: []. ShowFormItemService.update then runs form_item.images.all().delete() (show_form_item_service.py:95-97). Concretely: upload an image, save, reopen the item, edit the name, save — every image on that item is gone.

What merging as-is would undo:

# master (post-#1002) this branch
1 delete normalizedEntity.images images.filter(img => img.file_path) → PUT sends images: [] → collection wiped
2 saveNewItemImages posts new uploads to /items/{id}/images removed
3 onImageDeleted on SponsorItemDialog, wired to MuiFormikUpload's onDelete absent
4 removeItemFile wired through handleRemoveItemImage in the list page absent
5 expand: "images" on the save request params absent
6 ~450 lines of saveSponsorFormItem / updateSponsorFormItem tests, including "omits persisted images from the update request body so they are never round-tripped" replaced by the pre-#1002 file

Could you rebase onto master and rebuild the change on top of #1002? The intent here — one shared dialog, a single save action, requireDefaultQuantity — still holds. On the new base it needs to keep delete normalizedEntity.images + saveNewItemImages, the expand: "images" param, onImageDeleted passed from sponsor-form-item-list-page/index.js, and master's existing tests, extended to cover the new POST/PUT branch.

null,
createAction(SPONSOR_FORM_ITEM_UPDATED),
`${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}/items/${entity.id}`,
normalizedEntity,
snackbarErrorHandler
)(params)(dispatch)
.then(() => {
dispatch(
snackbarSuccessHandler({
title: T.translate("general.success"),
html: T.translate("sponsor_form_item_list.edit_item.created")
html: T.translate("sponsor_form_item_list.edit_item.updated")
})
);
})
.catch((err) => {
throw err;
})
.finally(() => {
dispatch(stopLoading());
});
})
.finally(() => {
dispatch(stopLoading());
});
};

export const updateSponsorFormItem =
(formId, entity) => async (dispatch, getState) => {
const { currentSummitState } = getState();
const accessToken = await getAccessTokenSafely();
const { currentSummit } = currentSummitState;

dispatch(startLoading());

const params = {
access_token: accessToken
};

const normalizedEntity = normalizeItem(entity);
}

return putRequest(
return postRequest(
null,
createAction(SPONSOR_FORM_ITEM_UPDATED),
`${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}/items/${entity.id}`,
`${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}/items`,
normalizedEntity,
snackbarErrorHandler
)(params)(dispatch)
.then(() => {
const promises = [Promise.resolve(0)];

if (normalizedEntity.images?.length > 0) {
const savingImages = saveItemImages(
formId,
entity.id,
normalizedEntity.images
)(dispatch, getState);

promises.push(savingImages);
}

return Promise.all(promises).then(() => {
dispatch(
snackbarSuccessHandler({
title: T.translate("general.success"),
html: T.translate("sponsor_form_item_list.edit_item.updated")
})
);
});
})
.catch((err) => {
throw err;
dispatch(
snackbarSuccessHandler({
title: T.translate("general.success"),
html: T.translate("sponsor_form_item_list.edit_item.created")
})
);
})
.finally(() => {
dispatch(stopLoading());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
import React from "react";
import { render, screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import "@testing-library/jest-dom";
import SponsorItemDialog from "../sponsor-inventory-popup";

jest.mock("i18n-react/dist/i18n-react", () => ({
translate: jest.fn((key) => key)
}));

jest.mock("../../../../hooks/useScrollToError", () => jest.fn());

jest.mock("openstack-uicore-foundation/lib/components", () => ({
MuiFormikUpload: function MockMuiFormikUpload({ name }) {
return <div data-testid={`upload-${name}`} />;
}
}));

jest.mock(
"openstack-uicore-foundation/lib/components/mui/formik-inputs/additional-input-list",
() =>
function MockAdditionalInputList({ name }) {
return <div data-testid={`meta-fields-${name}`} />;
}
);

jest.mock(
"../../../../components/mui/formik-inputs/item-price-tiers",
() =>
function MockItemPriceTiers() {
return <div data-testid="price-tiers" />;
}
);

jest.mock(
"../../../../components/inputs/formik-text-editor",
() =>
function MockFormikTextEditor({ name }) {
return <textarea data-testid={`editor-${name}`} name={name} readOnly />;
}
);

const BASE_ENTITY = {
id: 0,
code: "",
name: "",
description: "",
early_bird_rate: "",
standard_rate: "",
onsite_rate: "",
quantity_limit_per_show: "",
quantity_limit_per_sponsor: "",
meta_fields: [],
images: []
};

const fillRequiredTextFields = async (user) => {
await user.type(document.querySelector("input[name=\"code\"]"), "CODE-1");
await user.type(document.querySelector("input[name=\"name\"]"), "Item 1");
};

const submit = async (user) => {
await user.click(
screen.getByRole("button", { name: "edit_inventory_item.save_changes" })
);
};

describe("SponsorItemDialog", () => {
let onSave;
let onClose;

beforeEach(() => {
jest.clearAllMocks();
onSave = jest.fn(() => Promise.resolve());
onClose = jest.fn();
});

it("titles itself by whether the entity has an id", () => {
const { rerender } = render(
<SponsorItemDialog
entity={BASE_ENTITY}
onSave={onSave}
onClose={onClose}
/>
);
expect(
screen.getByText("edit_inventory_item.new_item")
).toBeInTheDocument();

rerender(
<SponsorItemDialog
entity={{ ...BASE_ENTITY, id: 42 }}
onSave={onSave}
onClose={onClose}
/>
);
expect(
screen.getByText("edit_inventory_item.edit_item")
).toBeInTheDocument();
});

it("blocks save when code/name are empty", async () => {
const user = userEvent.setup();
render(
<SponsorItemDialog
entity={BASE_ENTITY}
onSave={onSave}
onClose={onClose}
/>
);

await submit(user);

expect(onSave).not.toHaveBeenCalled();
});

describe("default_quantity requirement", () => {
it("is optional by default: saves with no value and shows no required marker", async () => {
const user = userEvent.setup();
render(
<SponsorItemDialog
entity={BASE_ENTITY}
onSave={onSave}
onClose={onClose}
/>
);

expect(
screen.queryByText("edit_inventory_item.default_quantity *")
).not.toBeInTheDocument();

await fillRequiredTextFields(user);
await submit(user);

await waitFor(() => expect(onSave).toHaveBeenCalledTimes(1));
expect(onClose).toHaveBeenCalledTimes(1);
});

it("blocks save, shows the error and the required marker when required and empty", async () => {
const user = userEvent.setup();
render(
<SponsorItemDialog
entity={{ ...BASE_ENTITY, default_quantity: undefined }}
onSave={onSave}
onClose={onClose}
requireDefaultQuantity
/>
);

expect(
screen.getByText("edit_inventory_item.default_quantity *")
).toBeInTheDocument();

await fillRequiredTextFields(user);
await submit(user);

expect(onSave).not.toHaveBeenCalled();
expect(
await screen.findByText("validation.required")
).toBeInTheDocument();
});

it("allows save once a value is provided when required", async () => {
const user = userEvent.setup();
render(
<SponsorItemDialog
entity={{ ...BASE_ENTITY, default_quantity: "" }}
onSave={onSave}
onClose={onClose}
requireDefaultQuantity
/>
);

await fillRequiredTextFields(user);
await user.type(
document.querySelector("input[name=\"default_quantity\"]"),
"5"
);
await submit(user);

await waitFor(() => expect(onSave).toHaveBeenCalledTimes(1));
expect(onSave.mock.calls[0][0]).toEqual(
expect.objectContaining({ default_quantity: 5 })
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ const SponsorItemDialog = ({
onSave,
onMetaFieldTypeDeleted,
onMetaFieldTypeValueDeleted,
entity: initialEntity
entity: initialEntity,
requireDefaultQuantity = false
}) => {
const [isSaving, setIsSaving] = useState(false);

Expand All @@ -62,7 +63,11 @@ const SponsorItemDialog = ({
early_bird_rate: nullableDecimalValidation(),
standard_rate: nullableDecimalValidation(),
onsite_rate: nullableDecimalValidation(),
default_quantity: positiveNumberValidation(),
default_quantity: requireDefaultQuantity
? positiveNumberValidation().required(
T.translate("validation.required")
)
: positiveNumberValidation(),
quantity_limit_per_sponsor: positiveNumberValidation(),
quantity_limit_per_show: positiveNumberValidation(),
meta_fields: formMetafieldsValidation()
Expand Down Expand Up @@ -173,6 +178,7 @@ const SponsorItemDialog = ({
<Grid2 size={4}>
<InputLabel htmlFor="default_quantity">
{T.translate("edit_inventory_item.default_quantity")}
{requireDefaultQuantity && " *"}
</InputLabel>
<MuiFormikQuantityField
variant="outlined"
Expand Down Expand Up @@ -263,7 +269,8 @@ SponsorItemDialog.propTypes = {
onSave: PropTypes.func.isRequired,
onMetaFieldTypeDeleted: PropTypes.func,
onMetaFieldTypeValueDeleted: PropTypes.func,
entity: PropTypes.object
entity: PropTypes.object,
requireDefaultQuantity: PropTypes.bool
};

export default SponsorItemDialog;
Loading
Loading