From e11f7c72afc6a2fc89dbae8c689b7be3aae48c5d Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Fri, 31 Jul 2026 17:41:44 -0300 Subject: [PATCH 1/6] chore: call delete speaker pic when pic removed --- src/actions/speaker-actions.js | 40 ++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/src/actions/speaker-actions.js b/src/actions/speaker-actions.js index 414216865..302dc4a80 100644 --- a/src/actions/speaker-actions.js +++ b/src/actions/speaker-actions.js @@ -61,6 +61,7 @@ export const UPDATE_SPEAKER = "UPDATE_SPEAKER"; export const SPEAKER_UPDATED = "SPEAKER_UPDATED"; export const SPEAKER_ADDED = "SPEAKER_ADDED"; export const PIC_ATTACHED = "PIC_ATTACHED"; +export const PIC_DELETED = "PIC_DELETED"; export const BIG_PIC_ATTACHED = "BIG_PIC_ATTACHED"; export const MERGE_SPEAKERS = "MERGE_SPEAKERS"; export const SPEAKER_MERGED = "SPEAKER_MERGED"; @@ -141,6 +142,18 @@ const uploadProfilePic = (entity, file) => async (dispatch) => { }); }; +const deleteProfilePic = (speakerId) => async (dispatch) => { + const accessToken = await getAccessTokenSafely(); + + return deleteRequest( + null, + createAction(PIC_DELETED), + `${window.API_BASE_URL}/api/v1/speakers/${speakerId}/photo?access_token=${accessToken}`, + null, + authErrorHandler + )({})(dispatch); +}; + const uploadBigPic = (entity, file) => async (dispatch) => { const accessToken = await getAccessTokenSafely(); @@ -157,6 +170,18 @@ const uploadBigPic = (entity, file) => async (dispatch) => { }); }; +const deleteBigPic = (speakerId) => async (dispatch) => { + const accessToken = await getAccessTokenSafely(); + + return deleteRequest( + null, + createAction(PIC_DELETED), + `${window.API_BASE_URL}/api/v1/speakers/${speakerId}/big-photo?access_token=${accessToken}`, + null, + authErrorHandler + )({})(dispatch); +}; + export const initSpeakersList = () => async (dispatch) => { dispatch(createAction(INIT_SPEAKERS_LIST_PARAMS)()); }; @@ -343,7 +368,12 @@ export const resetSpeakerForm = () => (dispatch) => { dispatch(createAction(RESET_SPEAKER_FORM)({})); }; -export const saveSpeaker = (entity) => async (dispatch) => { +export const saveSpeaker = (entity) => async (dispatch, getState) => { + const { currentSpeakerState } = getState(); + const { entity: prevSpeaker } = currentSpeakerState; + const removeProfilePic = prevSpeaker?.pic && !entity.pic; + const removeBigPic = prevSpeaker?.big_pic && !entity.big_pic; + const accessToken = await getAccessTokenSafely(); dispatch(startLoading()); @@ -358,7 +388,13 @@ export const saveSpeaker = (entity) => async (dispatch) => { normalizedEntity, authErrorHandler, entity - )({})(dispatch).then(() => { + )({})(dispatch).then(async () => { + if (removeProfilePic) { + await dispatch(deleteProfilePic(entity.id)); + } + if (removeBigPic) { + await dispatch(deleteBigPic(entity.id)); + } dispatch(showSuccessMessage(T.translate("edit_speaker.speaker_saved"))); }); } else { From 70b594db4e00ac3d728b7b22af425330f8e8e3f5 Mon Sep 17 00:00:00 2001 From: smarcet Date: Fri, 14 Aug 2026 00:46:11 -0300 Subject: [PATCH 2/6] chore: fix pre-existing lint errors in speaker reducer and edit page These 33 ESLint errors predate this branch and block the pre-commit hook as soon as either file is staged. No behaviour change: - speaker-reducer.js: drop unreachable `break` after `return` and the redundant blocks around each case, remove five never-read locals, `var` to `const` in the for-in loops, template literals for the cache-busting pic suffix, property shorthand, and reorder imports so uicore comes first - edit-summit-speaker-page.js: drop the unused componentDidUpdate params Both files go from 31 and 2 errors to 0. --- .../speakers/edit-summit-speaker-page.js | 2 +- src/reducers/speakers/speaker-reducer.js | 210 ++++++++---------- 2 files changed, 94 insertions(+), 118 deletions(-) diff --git a/src/pages/speakers/edit-summit-speaker-page.js b/src/pages/speakers/edit-summit-speaker-page.js index f8e91e15b..225f6bbe2 100644 --- a/src/pages/speakers/edit-summit-speaker-page.js +++ b/src/pages/speakers/edit-summit-speaker-page.js @@ -43,7 +43,7 @@ class EditSummitSpeakerPage extends React.Component { } } - componentDidUpdate(prevProps, prevState, snapshot) { + componentDidUpdate(prevProps) { const oldId = prevProps.match.params.speaker_id; const newId = this.props.match.params.speaker_id; diff --git a/src/reducers/speakers/speaker-reducer.js b/src/reducers/speakers/speaker-reducer.js index cb00ee95d..ee06a8941 100644 --- a/src/reducers/speakers/speaker-reducer.js +++ b/src/reducers/speakers/speaker-reducer.js @@ -9,8 +9,10 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. - **/ + * */ +import { VALIDATE } from "openstack-uicore-foundation/lib/utils/actions"; +import { LOGOUT_USER } from "openstack-uicore-foundation/lib/security/actions"; import { RECEIVE_SPEAKER, RESET_SPEAKER_FORM, @@ -20,15 +22,11 @@ import { PIC_ATTACHED, BIG_PIC_ATTACHED } from "../../actions/speaker-actions"; - import { AFFILIATION_ADDED, AFFILIATION_DELETED } from "../../actions/member-actions"; -import { VALIDATE } from "openstack-uicore-foundation/lib/utils/actions"; -import { LOGOUT_USER } from "openstack-uicore-foundation/lib/security/actions"; - export const DEFAULT_ENTITY = { id: 0, title: "", @@ -57,135 +55,113 @@ const DEFAULT_STATE = { const speakerReducer = (state = DEFAULT_STATE, action) => { const { type, payload } = action; switch (type) { - case LOGOUT_USER: - { - // we need this in case the token expired while editing the form - if (payload.hasOwnProperty("persistStore")) { - return state; - } else { - return { ...state, entity: { ...DEFAULT_ENTITY }, errors: {} }; - } - } - break; - case RESET_SPEAKER_FORM: - { - return DEFAULT_STATE; - } - break; - case UPDATE_SPEAKER: - { - return { ...state, entity: { ...payload }, errors: {} }; + case LOGOUT_USER: { + // we need this in case the token expired while editing the form + if (payload.hasOwnProperty("persistStore")) { + return state; } - break; + return { ...state, entity: { ...DEFAULT_ENTITY }, errors: {} }; + } + case RESET_SPEAKER_FORM: { + return DEFAULT_STATE; + } + case UPDATE_SPEAKER: { + return { ...state, entity: { ...payload }, errors: {} }; + } case SPEAKER_ADDED: - case RECEIVE_SPEAKER: - { - let entity = { ...payload.response }; - let registration_code = "", - on_site_phone = "", - registered = false, - checked_in = false, - confirmed = false; + case RECEIVE_SPEAKER: { + const entity = { ...payload.response }; - for (var key in entity) { - if (entity.hasOwnProperty(key)) { - entity[key] = entity[key] == null ? "" : entity[key]; - } + for (const key in entity) { + if (entity.hasOwnProperty(key)) { + entity[key] = entity[key] == null ? "" : entity[key]; } + } - if (entity.hasOwnProperty("presentations")) { - entity.all_presentations = [ - ...entity.all_presentations, - ...entity.presentations - ]; - } + if (entity.hasOwnProperty("presentations")) { + entity.all_presentations = [ + ...entity.all_presentations, + ...entity.presentations + ]; + } - if (entity.hasOwnProperty("moderated_presentations")) { - entity.all_presentations = [ - ...entity.all_presentations, - ...entity.moderated_presentations - ]; - } + if (entity.hasOwnProperty("moderated_presentations")) { + entity.all_presentations = [ + ...entity.all_presentations, + ...entity.moderated_presentations + ]; + } - entity.all_presentations = entity.all_presentations.filter( - (v, i, a) => a.findIndex((t) => t.id === v.id) === i - ); + entity.all_presentations = entity.all_presentations.filter( + (v, i, a) => a.findIndex((t) => t.id === v.id) === i + ); - if (entity.hasOwnProperty("affiliations")) { - entity.affiliations = entity.affiliations.map((a) => { - let affiliationTmp = {}; - for (var key in a) { - affiliationTmp[key] = a[key] == null ? "" : a[key]; - } - return affiliationTmp; - }); - } + if (entity.hasOwnProperty("affiliations")) { + entity.affiliations = entity.affiliations.map((a) => { + const affiliationTmp = {}; + for (const key in a) { + affiliationTmp[key] = a[key] == null ? "" : a[key]; + } + return affiliationTmp; + }); + } - if (entity.hasOwnProperty("registration_code")) { - entity.registration_code = entity.registration_code.code; - entity.code_redeemed = entity.registration_code.redeemed; - } + if (entity.hasOwnProperty("registration_code")) { + entity.registration_code = entity.registration_code.code; + entity.code_redeemed = entity.registration_code.redeemed; + } - if (entity.hasOwnProperty("summit_assistance")) { - entity.on_site_phone = entity.summit_assistance.on_site_phone; - entity.registered = entity.summit_assistance.registered; - entity.checked_in = entity.summit_assistance.checked_in; - entity.confirmed = entity.summit_assistance.confirmed; - } + if (entity.hasOwnProperty("summit_assistance")) { + entity.on_site_phone = entity.summit_assistance.on_site_phone; + entity.registered = entity.summit_assistance.registered; + entity.checked_in = entity.summit_assistance.checked_in; + entity.confirmed = entity.summit_assistance.confirmed; + } - delete entity.languages; - delete entity.areas_of_expertise; - delete entity.organizational_roles; + delete entity.languages; + delete entity.areas_of_expertise; + delete entity.organizational_roles; - return { - ...state, - entity: { ...DEFAULT_ENTITY, ...entity }, - errors: {} - }; - } - break; + return { + ...state, + entity: { ...DEFAULT_ENTITY, ...entity }, + errors: {} + }; + } case PIC_ATTACHED: { - let pic = state.entity.pic + "?" + new Date().getTime(); - return { ...state, entity: { ...state.entity, pic: pic } }; + const pic = `${state.entity.pic}?${new Date().getTime()}`; + return { ...state, entity: { ...state.entity, pic } }; } case BIG_PIC_ATTACHED: { - let pic = state.entity.big_pic + "?" + new Date().getTime(); + const pic = `${state.entity.big_pic}?${new Date().getTime()}`; return { ...state, entity: { ...state.entity, big_pic: pic } }; } - case SPEAKER_UPDATED: - { - return state; - } - break; - case VALIDATE: - { - return { ...state, errors: payload.errors }; - } - break; - case AFFILIATION_ADDED: - { - let affiliation = { ...payload.response }; - return { - ...state, - entity: { - ...state.entity, - affiliations: [...state.entity.affiliations, affiliation] - } - }; - } - break; - case AFFILIATION_DELETED: - { - let { affiliationId } = payload; - let affiliations = state.entity.affiliations.filter( - (a) => a.id !== affiliationId - ); - return { - ...state, - entity: { ...state.entity, affiliations: affiliations } - }; - } - break; + case SPEAKER_UPDATED: { + return state; + } + case VALIDATE: { + return { ...state, errors: payload.errors }; + } + case AFFILIATION_ADDED: { + const affiliation = { ...payload.response }; + return { + ...state, + entity: { + ...state.entity, + affiliations: [...state.entity.affiliations, affiliation] + } + }; + } + case AFFILIATION_DELETED: { + const { affiliationId } = payload; + const affiliations = state.entity.affiliations.filter( + (a) => a.id !== affiliationId + ); + return { + ...state, + entity: { ...state.entity, affiliations } + }; + } default: return state; } From c31ca58dfe3a612a3b671a8b46979af1a6468cb7 Mon Sep 17 00:00:00 2001 From: smarcet Date: Fri, 14 Aug 2026 00:47:57 -0300 Subject: [PATCH 3/6] fix: delete speaker photo from the remove button with a confirm Removing a speaker photo deferred the DELETE to the next Save and inferred it by diffing the submitted entity against the one in the store. Because putRequest dispatches UPDATE_SPEAKER before the request, the pic was already cleared in the store by the time the DELETE ran, so a failed delete left the form claiming the photo was gone while it still existed server side. Move the deletion to the remove button, matching the upload path which already posts immediately, and guard it with showConfirmDialog since it is now irreversible: - add removeAttachedPicture(speakerId, picAttr), symmetric with attachPicture - split BIG_PIC_DELETED out of PIC_DELETED and handle both in speakerReducer so each action clears only its own field - drop the getState() diffing and the delete branches from saveSpeaker - an unsaved speaker (id 0) clears the field locally, nothing to delete A failed delete now keeps the photo visible instead of hiding it optimistically. Covers the flow with the first tests this domain has: endpoint selection per picAttr, the reducer clearing only the matching field, no delete on save, and the confirm gate (deletes on confirm, does nothing on cancel). --- src/actions/__tests__/speaker-actions.test.js | 113 ++++++++++++++++++ src/actions/speaker-actions.js | 34 +++--- .../forms/__tests__/speaker-form.test.js | 112 +++++++++++++++++ src/components/forms/speaker-form.js | 33 +++-- src/i18n/en.json | 1 + .../speakers/edit-summit-speaker-page.js | 8 +- .../__tests__/speaker-reducer.test.js | 38 ++++++ src/reducers/speakers/speaker-reducer.js | 10 +- 8 files changed, 324 insertions(+), 25 deletions(-) create mode 100644 src/actions/__tests__/speaker-actions.test.js create mode 100644 src/components/forms/__tests__/speaker-form.test.js create mode 100644 src/reducers/speakers/__tests__/speaker-reducer.test.js diff --git a/src/actions/__tests__/speaker-actions.test.js b/src/actions/__tests__/speaker-actions.test.js new file mode 100644 index 000000000..7e5b22360 --- /dev/null +++ b/src/actions/__tests__/speaker-actions.test.js @@ -0,0 +1,113 @@ +/** + * @jest-environment jsdom + */ +import configureStore from "redux-mock-store"; +import thunk from "redux-thunk"; +import { + deleteRequest, + putRequest +} from "openstack-uicore-foundation/lib/utils/actions"; +import { removeAttachedPicture, saveSpeaker } from "../speaker-actions"; +import * as methods from "../../utils/methods"; + +jest.mock("openstack-uicore-foundation/lib/utils/actions", () => ({ + __esModule: true, + ...jest.requireActual("openstack-uicore-foundation/lib/utils/actions"), + deleteRequest: jest.fn(), + putRequest: jest.fn() +})); + +const SPEAKER_ID = 42; + +describe("removeAttachedPicture", () => { + const mockStore = configureStore([thunk]); + + beforeEach(() => { + jest.clearAllMocks(); + window.API_BASE_URL = "https://api.test"; + jest.spyOn(methods, "getAccessTokenSafely").mockResolvedValue("TOKEN"); + deleteRequest.mockImplementation( + (_requestAction, receiveAction) => () => (dispatch) => { + dispatch(receiveAction({ response: {} })); + return Promise.resolve({ response: {} }); + } + ); + }); + + afterEach(() => { + jest.restoreAllMocks(); + delete window.API_BASE_URL; + }); + + it("deletes the profile photo through the photo endpoint", async () => { + const store = mockStore({}); + + await store.dispatch(removeAttachedPicture(SPEAKER_ID, "profile")); + + expect(deleteRequest).toHaveBeenCalledTimes(1); + expect(deleteRequest.mock.calls[0][2]).toBe( + `https://api.test/api/v1/speakers/${SPEAKER_ID}/photo?access_token=TOKEN` + ); + expect(store.getActions().map((a) => a.type)).toContain("PIC_DELETED"); + }); + + it("deletes the big photo through the big-photo endpoint", async () => { + const store = mockStore({}); + + await store.dispatch(removeAttachedPicture(SPEAKER_ID, "big")); + + expect(deleteRequest.mock.calls[0][2]).toBe( + `https://api.test/api/v1/speakers/${SPEAKER_ID}/big-photo?access_token=TOKEN` + ); + expect(store.getActions().map((a) => a.type)).toContain("BIG_PIC_DELETED"); + }); + + it("does not emit a delete action when the request fails", async () => { + deleteRequest.mockImplementation( + () => () => () => Promise.reject(new Error("boom")) + ); + const store = mockStore({}); + + await expect( + store.dispatch(removeAttachedPicture(SPEAKER_ID, "profile")) + ).resolves.toBeUndefined(); + + const types = store.getActions().map((a) => a.type); + expect(types).not.toContain("PIC_DELETED"); + expect(types).not.toContain("STOP_LOADING"); + }); +}); + +describe("saveSpeaker", () => { + const mockStore = configureStore([thunk]); + + beforeEach(() => { + jest.clearAllMocks(); + window.API_BASE_URL = "https://api.test"; + jest.spyOn(methods, "getAccessTokenSafely").mockResolvedValue("TOKEN"); + putRequest.mockImplementation( + (requestAction, receiveAction, _url, _body, _err, payload) => + () => + (dispatch) => { + dispatch(requestAction(payload)); + dispatch(receiveAction({ response: {} })); + return Promise.resolve({ response: {} }); + } + ); + }); + + afterEach(() => { + jest.restoreAllMocks(); + delete window.API_BASE_URL; + }); + + it("never deletes photos as a side effect of saving", async () => { + const store = mockStore({}); + + await store.dispatch( + saveSpeaker({ id: SPEAKER_ID, title: "Dev", pic: "", big_pic: "" }) + ); + + expect(deleteRequest).not.toHaveBeenCalled(); + }); +}); diff --git a/src/actions/speaker-actions.js b/src/actions/speaker-actions.js index 302dc4a80..f80f7e974 100644 --- a/src/actions/speaker-actions.js +++ b/src/actions/speaker-actions.js @@ -63,6 +63,7 @@ export const SPEAKER_ADDED = "SPEAKER_ADDED"; export const PIC_ATTACHED = "PIC_ATTACHED"; export const PIC_DELETED = "PIC_DELETED"; export const BIG_PIC_ATTACHED = "BIG_PIC_ATTACHED"; +export const BIG_PIC_DELETED = "BIG_PIC_DELETED"; export const MERGE_SPEAKERS = "MERGE_SPEAKERS"; export const SPEAKER_MERGED = "SPEAKER_MERGED"; @@ -175,7 +176,7 @@ const deleteBigPic = (speakerId) => async (dispatch) => { return deleteRequest( null, - createAction(PIC_DELETED), + createAction(BIG_PIC_DELETED), `${window.API_BASE_URL}/api/v1/speakers/${speakerId}/big-photo?access_token=${accessToken}`, null, authErrorHandler @@ -368,12 +369,7 @@ export const resetSpeakerForm = () => (dispatch) => { dispatch(createAction(RESET_SPEAKER_FORM)({})); }; -export const saveSpeaker = (entity) => async (dispatch, getState) => { - const { currentSpeakerState } = getState(); - const { entity: prevSpeaker } = currentSpeakerState; - const removeProfilePic = prevSpeaker?.pic && !entity.pic; - const removeBigPic = prevSpeaker?.big_pic && !entity.big_pic; - +export const saveSpeaker = (entity) => async (dispatch) => { const accessToken = await getAccessTokenSafely(); dispatch(startLoading()); @@ -388,13 +384,7 @@ export const saveSpeaker = (entity) => async (dispatch, getState) => { normalizedEntity, authErrorHandler, entity - )({})(dispatch).then(async () => { - if (removeProfilePic) { - await dispatch(deleteProfilePic(entity.id)); - } - if (removeBigPic) { - await dispatch(deleteBigPic(entity.id)); - } + )({})(dispatch).then(() => { dispatch(showSuccessMessage(T.translate("edit_speaker.speaker_saved"))); }); } else { @@ -445,6 +435,22 @@ export const attachPicture = (entity, file, picAttr) => async (dispatch) => { }); }; +export const removeAttachedPicture = + (speakerId, picAttr) => async (dispatch) => { + const deleteFile = picAttr === "profile" ? deleteProfilePic : deleteBigPic; + + dispatch(startLoading()); + + return dispatch(deleteFile(speakerId)) + .then(() => { + dispatch(stopLoading()); + }) + .catch(() => { + // authErrorHandler already reported the failure and stopped loading; + // the reducer is left untouched so the picture stays visible. + }); + }; + /** ************************************************************************************************* */ /* SPEAKER ATTENDANCE */ /** ************************************************************************************************* */ diff --git a/src/components/forms/__tests__/speaker-form.test.js b/src/components/forms/__tests__/speaker-form.test.js new file mode 100644 index 000000000..3c90b489f --- /dev/null +++ b/src/components/forms/__tests__/speaker-form.test.js @@ -0,0 +1,112 @@ +/** + * @jest-environment jsdom + */ +import React from "react"; +import { render, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import "@testing-library/jest-dom"; +import SpeakerForm from "../speaker-form"; +import showConfirmDialog from "../../mui/showConfirmDialog"; + +jest.mock("i18n-react/dist/i18n-react", () => ({ + __esModule: true, + default: { translate: (key) => key } +})); + +jest.mock("../../mui/showConfirmDialog", () => ({ + __esModule: true, + default: jest.fn() +})); + +const buildEntity = (overrides = {}) => ({ + id: 42, + title: "Dev", + first_name: "Ada", + last_name: "Lovelace", + email: "ada@test.com", + member: null, + bio: "", + irc: "", + twitter: "", + company: "", + phone_number: "", + pic: "https://cdn.test/pic.png", + big_pic: "https://cdn.test/big.png", + all_presentations: [], + registration_codes: [], + summit_assistances: [], + affiliations: [], + ...overrides +}); + +const renderForm = (props = {}) => { + const onRemoveAttach = jest.fn(); + const { container } = render( + + ); + return { onRemoveAttach, container }; +}; + +// uicore's UploadInput only renders its `.remove` control while the preview is hovered +const previews = (container) => container.querySelectorAll(".file-box"); + +const clickRemove = async (container, index) => { + const preview = previews(container)[index]; + await userEvent.hover(preview); + await userEvent.click(preview.querySelector(".remove")); +}; + +describe("SpeakerForm photo removal", () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it("deletes the profile photo only after the user confirms", async () => { + showConfirmDialog.mockResolvedValue(true); + const { onRemoveAttach, container } = renderForm(); + + await clickRemove(container, 0); + + await waitFor(() => + expect(onRemoveAttach).toHaveBeenCalledWith(42, "profile") + ); + expect(showConfirmDialog).toHaveBeenCalledTimes(1); + }); + + it("deletes nothing when the user cancels the confirm", async () => { + showConfirmDialog.mockResolvedValue(false); + const { onRemoveAttach, container } = renderForm(); + + await clickRemove(container, 0); + + await waitFor(() => expect(showConfirmDialog).toHaveBeenCalledTimes(1)); + expect(onRemoveAttach).not.toHaveBeenCalled(); + }); + + it("targets the big photo endpoint from the big photo input", async () => { + showConfirmDialog.mockResolvedValue(true); + const { onRemoveAttach, container } = renderForm(); + + await clickRemove(container, 1); + + await waitFor(() => expect(onRemoveAttach).toHaveBeenCalledWith(42, "big")); + }); + + it("clears the field locally without confirming for an unsaved speaker", async () => { + const { onRemoveAttach, container } = renderForm({ entity: { id: 0 } }); + + await clickRemove(container, 0); + + await waitFor(() => expect(previews(container)).toHaveLength(1)); + expect(showConfirmDialog).not.toHaveBeenCalled(); + expect(onRemoveAttach).not.toHaveBeenCalled(); + }); +}); diff --git a/src/components/forms/speaker-form.js b/src/components/forms/speaker-form.js index 9ee1713bc..b3834a21e 100644 --- a/src/components/forms/speaker-form.js +++ b/src/components/forms/speaker-form.js @@ -14,9 +14,9 @@ import React from "react"; import T from "i18n-react/dist/i18n-react"; import "awesome-bootstrap-checkbox/awesome-bootstrap-checkbox.css"; -import MemberInput from "openstack-uicore-foundation/lib/components/inputs/member-input" -import UploadInput from "openstack-uicore-foundation/lib/components/inputs/upload-input" -import Input from "openstack-uicore-foundation/lib/components/inputs/text-input" +import MemberInput from "openstack-uicore-foundation/lib/components/inputs/member-input"; +import UploadInput from "openstack-uicore-foundation/lib/components/inputs/upload-input"; +import Input from "openstack-uicore-foundation/lib/components/inputs/text-input"; import Panel from "openstack-uicore-foundation/lib/components/sections/panel"; import TextEditorV3 from "openstack-uicore-foundation/lib/components/inputs/editor-input-v3"; import { AffiliationsTable } from "../tables/affiliationstable"; @@ -29,6 +29,7 @@ import { } from "../../utils/methods"; import { mustReplaceSpeakerFieldsWithMemberInfo } from "../../models/app-config"; import CopyClipboard from "../buttons/copy-clipboard"; +import showConfirmDialog from "../mui/showConfirmDialog"; class SpeakerForm extends React.Component { constructor(props) { @@ -124,11 +125,27 @@ class SpeakerForm extends React.Component { this.props.onAttach(this.state.entity, formData, "big"); } - handleRemoveFile(picAttr) { + async handleRemoveFile(picAttr) { const entity = { ...this.state.entity }; - entity[picAttr] = ""; - this.setState({ entity }); + // a speaker that was not persisted yet has nothing to delete server side + if (!entity.id) { + entity[picAttr === "profile" ? "pic" : "big_pic"] = ""; + this.setState({ entity }); + return; + } + + const confirmed = await showConfirmDialog({ + title: T.translate("general.are_you_sure"), + text: T.translate("edit_speaker.remove_pic_warning"), + iconType: "warning", + confirmButtonText: T.translate("general.yes_delete"), + confirmButtonColor: "error" + }); + + if (!confirmed) return; + + this.props.onRemoveAttach(entity.id, picAttr); } handleSubmit(publish, ev) { @@ -430,7 +447,7 @@ class SpeakerForm extends React.Component { this.handleRemoveFile("pic")} + handleRemove={() => this.handleRemoveFile("profile")} className="dropzone col-md-6" multiple={false} accept="image/*" @@ -448,7 +465,7 @@ class SpeakerForm extends React.Component { this.handleRemoveFile("big_pic")} + handleRemove={() => this.handleRemoveFile("big")} className="dropzone col-md-6" multiple={false} accept="image/*" diff --git a/src/i18n/en.json b/src/i18n/en.json index ae84c6051..005245cf0 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -768,6 +768,7 @@ "big_pic_size": "484 x 986px", "big_pic_format": "PNG or JPG", "affiliations": "Affiliations", + "remove_pic_warning": "This photo will be deleted right away and cannot be recovered.", "speaker_saved": "Speaker saved successfully.", "speaker_created": "Speaker created successfully.", "company": "Company", diff --git a/src/pages/speakers/edit-summit-speaker-page.js b/src/pages/speakers/edit-summit-speaker-page.js index 225f6bbe2..362ed16bb 100644 --- a/src/pages/speakers/edit-summit-speaker-page.js +++ b/src/pages/speakers/edit-summit-speaker-page.js @@ -20,7 +20,8 @@ import { getSpeaker, resetSpeakerForm, saveSpeaker, - attachPicture + attachPicture, + removeAttachedPicture } from "../../actions/speaker-actions"; import { loadSummits } from "../../actions/summit-actions"; import "../../styles/edit-summit-speaker-page.less"; @@ -64,6 +65,7 @@ class EditSummitSpeakerPage extends React.Component { history, saveSpeaker, attachPicture, + removeAttachedPicture, match } = this.props; const title = entity.id @@ -90,6 +92,7 @@ class EditSummitSpeakerPage extends React.Component { errors={errors} onSubmit={saveSpeaker} onAttach={attachPicture} + onRemoveAttach={removeAttachedPicture} /> ); @@ -106,5 +109,6 @@ export default connect(mapStateToProps, { getSpeaker, resetSpeakerForm, saveSpeaker, - attachPicture + attachPicture, + removeAttachedPicture })(EditSummitSpeakerPage); diff --git a/src/reducers/speakers/__tests__/speaker-reducer.test.js b/src/reducers/speakers/__tests__/speaker-reducer.test.js new file mode 100644 index 000000000..0861a2166 --- /dev/null +++ b/src/reducers/speakers/__tests__/speaker-reducer.test.js @@ -0,0 +1,38 @@ +/** + * @jest-environment jsdom + */ +import speakerReducer from "../speaker-reducer"; +import { BIG_PIC_DELETED, PIC_DELETED } from "../../../actions/speaker-actions"; + +const stateWithPics = { + entity: { + id: 42, + first_name: "Ada", + pic: "https://cdn.test/pic.png", + big_pic: "https://cdn.test/big.png" + }, + errors: {} +}; + +describe("speakerReducer photo deletion", () => { + it("clears pic and leaves big_pic untouched on PIC_DELETED", () => { + const next = speakerReducer(stateWithPics, { + type: PIC_DELETED, + payload: { response: {} } + }); + + expect(next.entity.pic).toBe(""); + expect(next.entity.big_pic).toBe("https://cdn.test/big.png"); + expect(next.entity.first_name).toBe("Ada"); + }); + + it("clears big_pic and leaves pic untouched on BIG_PIC_DELETED", () => { + const next = speakerReducer(stateWithPics, { + type: BIG_PIC_DELETED, + payload: { response: {} } + }); + + expect(next.entity.big_pic).toBe(""); + expect(next.entity.pic).toBe("https://cdn.test/pic.png"); + }); +}); diff --git a/src/reducers/speakers/speaker-reducer.js b/src/reducers/speakers/speaker-reducer.js index ee06a8941..768aafbef 100644 --- a/src/reducers/speakers/speaker-reducer.js +++ b/src/reducers/speakers/speaker-reducer.js @@ -20,7 +20,9 @@ import { SPEAKER_UPDATED, SPEAKER_ADDED, PIC_ATTACHED, - BIG_PIC_ATTACHED + PIC_DELETED, + BIG_PIC_ATTACHED, + BIG_PIC_DELETED } from "../../actions/speaker-actions"; import { AFFILIATION_ADDED, @@ -132,10 +134,16 @@ const speakerReducer = (state = DEFAULT_STATE, action) => { const pic = `${state.entity.pic}?${new Date().getTime()}`; return { ...state, entity: { ...state.entity, pic } }; } + case PIC_DELETED: { + return { ...state, entity: { ...state.entity, pic: "" } }; + } case BIG_PIC_ATTACHED: { const pic = `${state.entity.big_pic}?${new Date().getTime()}`; return { ...state, entity: { ...state.entity, big_pic: pic } }; } + case BIG_PIC_DELETED: { + return { ...state, entity: { ...state.entity, big_pic: "" } }; + } case SPEAKER_UPDATED: { return state; } From c84eb5e5624c56cab2fd515a3c93a588abfb7738 Mon Sep 17 00:00:00 2001 From: smarcet Date: Fri, 14 Aug 2026 12:03:26 -0300 Subject: [PATCH 4/6] fix: preserve unsaved speaker fields when a photo is removed componentDidUpdate replaced the whole local entity from props whenever props.entity changed, including the redux update triggered by removeAttachedPicture. That discarded any unsaved edits to other fields. handleRemoveFile now sets a pictureOnlyUpdate flag before dispatching the delete (Redux updates pic/big_pic synchronously during that call, before the returned promise resolves), and componentDidUpdate merges only pic/big_pic from props while the flag is set instead of overwriting the whole entity. --- .../forms/__tests__/speaker-form.test.js | 48 +++++++++++++++++-- src/components/forms/speaker-form.js | 17 ++++++- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/src/components/forms/__tests__/speaker-form.test.js b/src/components/forms/__tests__/speaker-form.test.js index 3c90b489f..77a75b278 100644 --- a/src/components/forms/__tests__/speaker-form.test.js +++ b/src/components/forms/__tests__/speaker-form.test.js @@ -40,10 +40,11 @@ const buildEntity = (overrides = {}) => ({ }); const renderForm = (props = {}) => { - const onRemoveAttach = jest.fn(); - const { container } = render( + const onRemoveAttach = props.onRemoveAttach || jest.fn().mockResolvedValue(); + const entity = buildEntity(props.entity); + const { container, rerender } = render( { onRemoveAttach={onRemoveAttach} /> ); - return { onRemoveAttach, container }; + const rerenderWithEntity = (nextEntity) => + rerender( + + ); + return { onRemoveAttach, container, entity, rerenderWithEntity }; }; // uicore's UploadInput only renders its `.remove` control while the preview is hovered @@ -109,4 +122,31 @@ describe("SpeakerForm photo removal", () => { expect(showConfirmDialog).not.toHaveBeenCalled(); expect(onRemoveAttach).not.toHaveBeenCalled(); }); + + it("preserves unsaved field edits when the removal updates props.entity", async () => { + showConfirmDialog.mockResolvedValue(true); + // real removeAttachedPicture resolves only after the PIC_DELETED-driven + // redux update has already flowed down as new props - a promise that + // never settles during this test reproduces that ordering + const onRemoveAttach = jest.fn(() => new Promise(() => {})); + const { container, entity, rerenderWithEntity } = renderForm({ + onRemoveAttach + }); + + const twitterInput = container.querySelector("#twitter"); + await userEvent.clear(twitterInput); + await userEvent.type(twitterInput, "unsaved-handle"); + expect(twitterInput.value).toBe("unsaved-handle"); + + await clickRemove(container, 0); + await waitFor(() => + expect(onRemoveAttach).toHaveBeenCalledWith(42, "profile") + ); + + // simulates the redux entity coming back down as props after PIC_DELETED, + // with only the picture field cleared and every other field unchanged + rerenderWithEntity({ ...entity, pic: "" }); + + expect(container.querySelector("#twitter").value).toBe("unsaved-handle"); + }); }); diff --git a/src/components/forms/speaker-form.js b/src/components/forms/speaker-form.js index b3834a21e..a5f59ee2c 100644 --- a/src/components/forms/speaker-form.js +++ b/src/components/forms/speaker-form.js @@ -57,7 +57,13 @@ class SpeakerForm extends React.Component { scrollToError(this.props.errors); if (!shallowEqual(prevProps.entity, this.props.entity)) { - state.entity = { ...this.props.entity }; + state.entity = this.pictureOnlyUpdate + ? { + ...this.state.entity, + pic: this.props.entity.pic, + big_pic: this.props.entity.big_pic + } + : { ...this.props.entity }; state.errors = {}; } @@ -145,7 +151,14 @@ class SpeakerForm extends React.Component { if (!confirmed) return; - this.props.onRemoveAttach(entity.id, picAttr); + // guards componentDidUpdate against clobbering unsaved edits when the + // redux entity update lands (Redux updates pic/big_pic synchronously + // during the dispatch below, before this promise ever resolves) + this.pictureOnlyUpdate = true; + + this.props.onRemoveAttach(entity.id, picAttr).finally(() => { + this.pictureOnlyUpdate = false; + }); } handleSubmit(publish, ev) { From c44c1f961b900f15cd4ac249e884894563180045 Mon Sep 17 00:00:00 2001 From: smarcet Date: Fri, 14 Aug 2026 12:17:31 -0300 Subject: [PATCH 5/6] fix: await saveSpeaker's request before resolving putRequest/postRequest were called but never returned, so dispatch(saveSpeaker(entity)) resolved before the update/create request actually completed. Return both promise chains so callers can rely on the dispatch settling only after the request finishes. --- src/actions/__tests__/speaker-actions.test.js | 33 ++++++++++++++ src/actions/speaker-actions.js | 44 +++++++++---------- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/src/actions/__tests__/speaker-actions.test.js b/src/actions/__tests__/speaker-actions.test.js index 7e5b22360..9d7c01e7f 100644 --- a/src/actions/__tests__/speaker-actions.test.js +++ b/src/actions/__tests__/speaker-actions.test.js @@ -110,4 +110,37 @@ describe("saveSpeaker", () => { expect(deleteRequest).not.toHaveBeenCalled(); }); + + it("resolves only after the update request settles", async () => { + let resolveRequest; + putRequest.mockImplementation( + (requestAction, receiveAction) => () => (dispatch) => { + dispatch(requestAction()); + return new Promise((resolve) => { + resolveRequest = () => { + dispatch(receiveAction({ response: {} })); + resolve({ response: {} }); + }; + }); + } + ); + const store = mockStore({}); + + let settled = false; + const dispatched = store + .dispatch( + saveSpeaker({ id: SPEAKER_ID, title: "Dev", pic: "", big_pic: "" }) + ) + .then(() => { + settled = true; + }); + + await Promise.resolve(); + await Promise.resolve(); + expect(settled).toBe(false); + + resolveRequest(); + await dispatched; + expect(settled).toBe(true); + }); }); diff --git a/src/actions/speaker-actions.js b/src/actions/speaker-actions.js index f80f7e974..a3cea557d 100644 --- a/src/actions/speaker-actions.js +++ b/src/actions/speaker-actions.js @@ -377,7 +377,7 @@ export const saveSpeaker = (entity) => async (dispatch) => { const normalizedEntity = normalizeEntity(entity); if (entity.id) { - putRequest( + return putRequest( createAction(UPDATE_SPEAKER), createAction(SPEAKER_UPDATED), `${window.API_BASE_URL}/api/v1/speakers/${entity.id}?access_token=${accessToken}`, @@ -387,28 +387,28 @@ export const saveSpeaker = (entity) => async (dispatch) => { )({})(dispatch).then(() => { dispatch(showSuccessMessage(T.translate("edit_speaker.speaker_saved"))); }); - } else { - const successMessage = { - title: T.translate("general.done"), - html: T.translate("edit_speaker.speaker_created"), - type: "success" - }; - - postRequest( - createAction(UPDATE_SPEAKER), - createAction(SPEAKER_ADDED), - `${window.API_BASE_URL}/api/v1/speakers?access_token=${accessToken}`, - normalizedEntity, - authErrorHandler, - entity - )({})(dispatch).then(() => { - dispatch( - showMessage(successMessage, () => { - history.push("/app/speakers"); - }) - ); - }); } + + const successMessage = { + title: T.translate("general.done"), + html: T.translate("edit_speaker.speaker_created"), + type: "success" + }; + + return postRequest( + createAction(UPDATE_SPEAKER), + createAction(SPEAKER_ADDED), + `${window.API_BASE_URL}/api/v1/speakers?access_token=${accessToken}`, + normalizedEntity, + authErrorHandler, + entity + )({})(dispatch).then(() => { + dispatch( + showMessage(successMessage, () => { + history.push("/app/speakers"); + }) + ); + }); }; export const attachPicture = (entity, file, picAttr) => async (dispatch) => { From f397042277cd4b65dddb3883932f2527608d44b3 Mon Sep 17 00:00:00 2001 From: smarcet Date: Fri, 14 Aug 2026 14:55:24 -0300 Subject: [PATCH 6/6] fix: track pending picture-only updates with a counter Overlapping profile-pic and big-pic deletions previously shared a single boolean guard (pictureOnlyUpdate). The first request to settle cleared it while the second was still pending, letting componentDidUpdate overwrite the whole entity and discard unsaved form edits. Use a pending-request counter instead so the guard stays active until all deletions settle. --- src/components/forms/speaker-form.js | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/components/forms/speaker-form.js b/src/components/forms/speaker-form.js index a5f59ee2c..8d8cdf20d 100644 --- a/src/components/forms/speaker-form.js +++ b/src/components/forms/speaker-form.js @@ -57,13 +57,14 @@ class SpeakerForm extends React.Component { scrollToError(this.props.errors); if (!shallowEqual(prevProps.entity, this.props.entity)) { - state.entity = this.pictureOnlyUpdate - ? { - ...this.state.entity, - pic: this.props.entity.pic, - big_pic: this.props.entity.big_pic - } - : { ...this.props.entity }; + state.entity = + this.pendingPictureOnlyUpdates > 0 + ? { + ...this.state.entity, + pic: this.props.entity.pic, + big_pic: this.props.entity.big_pic + } + : { ...this.props.entity }; state.errors = {}; } @@ -153,11 +154,13 @@ class SpeakerForm extends React.Component { // guards componentDidUpdate against clobbering unsaved edits when the // redux entity update lands (Redux updates pic/big_pic synchronously - // during the dispatch below, before this promise ever resolves) - this.pictureOnlyUpdate = true; + // during the dispatch below, before this promise ever resolves). + // Counted rather than boolean so overlapping profile/big-pic deletions + // each keep the guard active until they've all settled. + this.pendingPictureOnlyUpdates = (this.pendingPictureOnlyUpdates || 0) + 1; this.props.onRemoveAttach(entity.id, picAttr).finally(() => { - this.pictureOnlyUpdate = false; + this.pendingPictureOnlyUpdates -= 1; }); }