From 77a7a8873b1d469016135307e49fa8f7eda60b40 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Tue, 28 Jul 2026 11:00:41 -0500 Subject: [PATCH] fix(locations): send venue/room ids when resyncing a room to the materializer The Resync Room button sent venue and room NAMES. The materializer's per-room route stopped accepting names in dropbox-materializer PR #24 (merged 2026-06-15), which replaced sync/materialize//// with sync/materialize//// and shipped no name-based fallback. That PR's own commit message called for a coordinated change here; it was never made. Since the materializer deployed, every click has 404'd and shown a "Not Found" dialog instead of resyncing. LocationForm already receives the room id and was discarding it to look the room's name back up, so no new data is needed. It now passes this.props.entity.id (the persisted venue, so an unsaved rename in the form cannot become the target) and the room id straight through, guarding against a venue that has never been saved (id 0). The existing action tests only asserted that postRequest was called, never the URL, which is why they stayed green through the regression. Added a test pinning the built URL, and a LocationForm test that opens the Rooms panel, clicks the real resync action and asserts the ids reach onRoomResync. Both fail if the old behavior is restored. Full suite: 154 suites, 1410 tests, green. Co-Authored-By: Claude --- .../__tests__/dropbox-sync-actions.test.js | 20 +- src/actions/dropbox-sync-actions.js | 60 ++-- .../forms/__tests__/location-form.test.js | 261 ++++++++++-------- src/components/forms/location-form.js | 20 +- src/pages/locations/edit-location-page.js | 4 +- 5 files changed, 210 insertions(+), 155 deletions(-) diff --git a/src/actions/__tests__/dropbox-sync-actions.test.js b/src/actions/__tests__/dropbox-sync-actions.test.js index a18c8e93c..ccabe90e2 100644 --- a/src/actions/__tests__/dropbox-sync-actions.test.js +++ b/src/actions/__tests__/dropbox-sync-actions.test.js @@ -381,7 +381,7 @@ describe("dropbox sync actions", () => { }); test("resyncRoom dispatches START_LOADING, RESYNC_ROOM_DISPATCHED, STOP_LOADING", async () => { - store.dispatch(DropboxSyncActions.resyncRoom("Main Venue", "Room A")); + store.dispatch(DropboxSyncActions.resyncRoom(12, 345)); await flushPromises(); const actions = store.getActions(); @@ -394,6 +394,22 @@ describe("dropbox sync actions", () => { expect(postRequest).toHaveBeenCalled(); }); + test("resyncRoom targets the materializer's integer venue/room route", async () => { + // The materializer route is ////. + // It previously took names, and sending names does not resolve at all — + // the request 404s and the resync silently never happens. The older + // assertions here only checked that postRequest was called, so they stayed + // green through that regression; pin the URL itself. + store.dispatch(DropboxSyncActions.resyncRoom(12, 345)); + await flushPromises(); + + expect(postRequest).toHaveBeenCalledTimes(1); + const url = postRequest.mock.calls[0][2]; + expect(url).toBe( + "https://test-api.example.com/api/v1/sync/materialize/1/12/345/" + ); + }); + test("getSyncConfig dispatches RECEIVE_SYNC_CONFIG with empty payload on failure", async () => { getRequest.mockImplementation(mockRequestImplReject); @@ -442,7 +458,7 @@ describe("dropbox sync actions", () => { test("resyncRoom dispatches STOP_LOADING on failure", async () => { postRequest.mockImplementation(() => mockRequestImplReject()); - store.dispatch(DropboxSyncActions.resyncRoom("Main Venue", "Room A")); + store.dispatch(DropboxSyncActions.resyncRoom(12, 345)); await flushPromises(); const actions = store.getActions(); diff --git a/src/actions/dropbox-sync-actions.js b/src/actions/dropbox-sync-actions.js index b0791857a..5dd1f7f88 100644 --- a/src/actions/dropbox-sync-actions.js +++ b/src/actions/dropbox-sync-actions.js @@ -352,38 +352,38 @@ export const rebuildSync = () => async (dispatch, getState) => { }); }; -export const resyncRoom = - (venueName, roomName) => async (dispatch, getState) => { - const { currentSummitState } = getState(); - const baseUrl = getBaseUrl(); - const summitId = currentSummitState?.currentSummit?.id; - - if (!baseUrl || !summitId) return; +// The materializer's per-room route takes integer venue/room ids +// (/api/v1/sync/materialize/{summitId}/{venueId}/{roomId}/). It previously +// took names; sending names now does not resolve at all. +export const resyncRoom = (venueId, roomId) => async (dispatch, getState) => { + const { currentSummitState } = getState(); + const baseUrl = getBaseUrl(); + const summitId = currentSummitState?.currentSummit?.id; - const accessToken = await getAccessTokenSafely(); + if (!baseUrl || !summitId) return; - dispatch(startLoading()); + const accessToken = await getAccessTokenSafely(); - const params = { - access_token: accessToken - }; + dispatch(startLoading()); - return postRequest( - null, - createAction(RESYNC_ROOM_DISPATCHED), - `${baseUrl}/api/v1/sync/materialize/${summitId}/${encodeURIComponent( - venueName - )}/${encodeURIComponent(roomName)}/`, - null, - authErrorHandler - )(params)(dispatch) - .then(() => { - dispatch(stopLoading()); - dispatch( - showSuccessMessage(T.translate("dropbox_sync.resync_dispatched")) - ); - }) - .catch(() => { - dispatch(stopLoading()); - }); + const params = { + access_token: accessToken }; + + return postRequest( + null, + createAction(RESYNC_ROOM_DISPATCHED), + `${baseUrl}/api/v1/sync/materialize/${summitId}/${venueId}/${roomId}/`, + null, + authErrorHandler + )(params)(dispatch) + .then(() => { + dispatch(stopLoading()); + dispatch( + showSuccessMessage(T.translate("dropbox_sync.resync_dispatched")) + ); + }) + .catch(() => { + dispatch(stopLoading()); + }); +}; diff --git a/src/components/forms/__tests__/location-form.test.js b/src/components/forms/__tests__/location-form.test.js index 502bfec7e..f934fe4f6 100644 --- a/src/components/forms/__tests__/location-form.test.js +++ b/src/components/forms/__tests__/location-form.test.js @@ -1,124 +1,129 @@ import React from "react"; import { render } from "@testing-library/react"; import { screen } from "@testing-library/dom"; +import userEvent from "@testing-library/user-event"; import LocationForm from "../location-form"; import currentSummitMock from "../../../__mocks__/currentSummitMock"; -describe("LocationForm", () => { - beforeEach(() => { - const props = { - history: { - length: 8, - action: "POP", - location: { - pathname: "/app/summits/69/locations/781", - search: "", - hash: "", - key: "wh0sst" - } - }, - currentSummit: currentSummitMock, - allClasses: [ - { - name: "string", - short_name: "string", - description: "string", - type: ["External", "Internal"], - banners: "array", - order: "integer", - opening_hour: "integer", - closing_hour: "integer", - address_1: "string", - address_2: "string", - zip_code: "string", - city: "string", - state: "string", - country: "string", - website_url: "string", - lng: "string", - lat: "string", - display_on_site: "boolean", - details_page: "boolean", - location_message: "string", - images: "array", - class_name: "SummitVenue", - is_main: "boolean", - floors: "array", - rooms: "array" - }, - { - name: "string", - short_name: "string", - description: "string", - type: ["External", "Internal"], - banners: "array", - order: "integer", - opening_hour: "integer", - closing_hour: "integer", - address_1: "string", - address_2: "string", - zip_code: "string", - city: "string", - state: "string", - country: "string", - website_url: "string", - lng: "string", - lat: "string", - display_on_site: "boolean", - details_page: "boolean", - location_message: "string", - images: "array", - class_name: "SummitAirport", - capacity: "integer", - airport_type: ["International", "Domestic"] - } - ], - entity: { - id: 781, - name: "International Barcelona Convention Center", - short_name: "CCIB", +const buildProps = (overrides = {}) => { + const props = { + history: { + length: 8, + action: "POP", + location: { + pathname: "/app/summits/69/locations/781", + search: "", + hash: "", + key: "wh0sst" + } + }, + currentSummit: currentSummitMock, + allClasses: [ + { + name: "string", + short_name: "string", + description: "string", + type: ["External", "Internal"], + banners: "array", + order: "integer", + opening_hour: "integer", + closing_hour: "integer", + address_1: "string", + address_2: "string", + zip_code: "string", + city: "string", + state: "string", + country: "string", + website_url: "string", + lng: "string", + lat: "string", + display_on_site: "boolean", + details_page: "boolean", + location_message: "string", + images: "array", class_name: "SummitVenue", - description: "", - location_type: "Internal", - address_1: "Plaça de Willy Brandt, 11-14", - address_2: "", - zip_code: "08019", - city: "Sant Marti", - state: "Barcelona", - country: "ES", - website_url: "", - lng: "2.2193", - lat: "41.4088", - display_on_site: false, - details_page: false, - is_main: false, - location_message: "", - maps: [], - images: [], - rooms: [], - floors: [], - capacity: 0, - booking_link: "", - sold_out: false, - airport_type: "", - hotel_type: "", - created: 1762190581, - last_edited: 1762190581, - order: 48, - opening_hour: "", - closing_hour: "" + is_main: "boolean", + floors: "array", + rooms: "array" }, - errors: {}, - onSubmit: jest.fn(), - onMapUpdate: jest.fn(), - onMarkerDragged: jest.fn(), - onFloorDelete: jest.fn(), - onRoomDelete: jest.fn(), - onImageDelete: jest.fn(), - onMapDelete: jest.fn() - }; + { + name: "string", + short_name: "string", + description: "string", + type: ["External", "Internal"], + banners: "array", + order: "integer", + opening_hour: "integer", + closing_hour: "integer", + address_1: "string", + address_2: "string", + zip_code: "string", + city: "string", + state: "string", + country: "string", + website_url: "string", + lng: "string", + lat: "string", + display_on_site: "boolean", + details_page: "boolean", + location_message: "string", + images: "array", + class_name: "SummitAirport", + capacity: "integer", + airport_type: ["International", "Domestic"] + } + ], + entity: { + id: 781, + name: "International Barcelona Convention Center", + short_name: "CCIB", + class_name: "SummitVenue", + description: "", + location_type: "Internal", + address_1: "Plaça de Willy Brandt, 11-14", + address_2: "", + zip_code: "08019", + city: "Sant Marti", + state: "Barcelona", + country: "ES", + website_url: "", + lng: "2.2193", + lat: "41.4088", + display_on_site: false, + details_page: false, + is_main: false, + location_message: "", + maps: [], + images: [], + rooms: [], + floors: [], + capacity: 0, + booking_link: "", + sold_out: false, + airport_type: "", + hotel_type: "", + created: 1762190581, + last_edited: 1762190581, + order: 48, + opening_hour: "", + closing_hour: "" + }, + errors: {}, + onSubmit: jest.fn(), + onMapUpdate: jest.fn(), + onMarkerDragged: jest.fn(), + onFloorDelete: jest.fn(), + onRoomDelete: jest.fn(), + onImageDelete: jest.fn(), + onMapDelete: jest.fn() + }; - render(); + return { ...props, ...overrides }; +}; + +describe("LocationForm", () => { + beforeEach(() => { + render(); }); describe("IsMain? checkbox", () => { @@ -128,3 +133,35 @@ describe("LocationForm", () => { }); }); }); + +describe("LocationForm Resync Room action", () => { + // The materializer's per-room resync route takes integer venue/room ids. + // This form is handed the room id already; it previously looked the room's + // NAME back up and sent that, which cannot resolve against the route. + const VENUE_ID = 781; + const ROOM_ID = 5002; + + test("passes the venue id and room id, not their names", async () => { + const onRoomResync = jest.fn(); + const props = buildProps({ syncEnabled: true, onRoomResync }); + props.entity = { + ...props.entity, + id: VENUE_ID, + rooms: [{ id: ROOM_ID, name: "Room A", capacity: 10, floor_name: "" }] + }; + + const { container } = render(); + + // The Rooms panel is collapsed by default; open it the way a user would. + await userEvent.click(screen.getByText("edit_location.rooms")); + + // uicore's Table renders custom row actions as . + const resyncButton = container.querySelector( + "[data-tip='dropbox_sync.resync_tooltip']" + ); + expect(resyncButton).toBeInTheDocument(); + await userEvent.click(resyncButton); + + expect(onRoomResync).toHaveBeenCalledWith(VENUE_ID, ROOM_ID); + }); +}); diff --git a/src/components/forms/location-form.js b/src/components/forms/location-form.js index ff6e309b8..591968896 100644 --- a/src/components/forms/location-form.js +++ b/src/components/forms/location-form.js @@ -14,10 +14,10 @@ import React from "react"; import T from "i18n-react/dist/i18n-react"; import "awesome-bootstrap-checkbox/awesome-bootstrap-checkbox.css"; -import Dropdown from "openstack-uicore-foundation/lib/components/inputs/dropdown" -import CountryDropdown from "openstack-uicore-foundation/lib/components/inputs/country-dropdown" -import Input from "openstack-uicore-foundation/lib/components/inputs/text-input" -import Table from "openstack-uicore-foundation/lib/components/table" +import Dropdown from "openstack-uicore-foundation/lib/components/inputs/dropdown"; +import CountryDropdown from "openstack-uicore-foundation/lib/components/inputs/country-dropdown"; +import Input from "openstack-uicore-foundation/lib/components/inputs/text-input"; +import Table from "openstack-uicore-foundation/lib/components/table"; import Panel from "openstack-uicore-foundation/lib/components/sections/panel"; import TextEditorV3 from "openstack-uicore-foundation/lib/components/inputs/editor-input-v3"; import { GMap } from "openstack-uicore-foundation/lib/components/google-map"; @@ -212,11 +212,13 @@ class LocationForm extends React.Component { } handleRoomResync(roomId) { - const { entity } = this.state; - const persistedVenueName = this.props.entity.name; - const room = entity.rooms.find((r) => r.id === roomId); - if (room && this.props.onRoomResync) { - this.props.onRoomResync(persistedVenueName, room.name); + // IDs, not names: the materializer's resync route takes integer + // venue/room ids. Read the venue id off props (the persisted entity), not + // state, so an unsaved rename in the form cannot be sent as the target. A + // venue that has never been saved has id 0 and no rooms to resync. + const venueId = this.props.entity.id; + if (venueId && roomId && this.props.onRoomResync) { + this.props.onRoomResync(venueId, roomId); } } diff --git a/src/pages/locations/edit-location-page.js b/src/pages/locations/edit-location-page.js index cac98a18f..07671b88d 100644 --- a/src/pages/locations/edit-location-page.js +++ b/src/pages/locations/edit-location-page.js @@ -130,8 +130,8 @@ class EditLocationPage extends React.Component { }); } - handleRoomResync(venueName, roomName) { - this.props.resyncRoom(venueName, roomName); + handleRoomResync(venueId, roomId) { + this.props.resyncRoom(venueId, roomId); } render() {