Skip to content

add filter to room report - #24

Merged
smarcet merged 1 commit into
masterfrom
fixes-on-reports
Oct 27, 2021
Merged

add filter to room report#24
smarcet merged 1 commit into
masterfrom
fixes-on-reports

Conversation

@smarcet smarcet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

looks good to me

@smarcet
smarcet merged commit 501a200 into master Oct 27, 2021
smarcet pushed a commit that referenced this pull request Oct 27, 2021
@santipalenque
santipalenque deleted the fixes-on-reports branch November 24, 2021 20:43
caseylocker added a commit that referenced this pull request Jul 30, 2026
…rializer

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/<int:summit_id>/<str:venue>/<str:room>/

with

  sync/materialize/<int:summit_id>/<int:venue_id>/<int:room_id>/

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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants