Repository navigation
Add stale-room deactivation specs for media preservation - #463
Merged
Merged
Conversation
Cover the success path that hides rooms via visible=false while keeping contacts, characteristics, and Active Storage panoramas/images, plus the already-hidden fail-closed invariant in BuildingsApi and ClassroomApi. Co-authored-by: rsmoke <rsmoke@umich.edu>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused test-only changes correctly cover the documented deactivation and preservation scenarios.
0 open findings
What changed in this PR
Adds regression coverage ensuring stale-room deactivation preserves room records, associations, and media.
Changes:
- Tests successful soft deactivation and media preservation.
- Tests fail-closed behavior for already-hidden rooms.
| File | Description |
|---|---|
spec/lib/buildings_api_spec.rb |
Covers building room deactivation behavior. |
spec/lib/classroom_api_spec.rb |
Covers classroom deactivation behavior. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
rsmoke
added a commit
that referenced
this pull request
Oct 14, 2026
* Update tailwindcss dependencies, modify Procfile for CSS watch command, and enhance support info modal with improved accessibility and styling. * Add DepartmentApi and SentryMetrics regression specs (#462) Cover department lookup success/failure shaping used by BuildingsApi room updates, and ensure Sentry metric failures are swallowed so jobs/rake keep running. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * Add stale-room deactivation regression specs for media preservation. (#463) Cover the success path that hides rooms via visible=false while keeping contacts, characteristics, and Active Storage panoramas/images, plus the already-hidden fail-closed invariant in BuildingsApi and ClassroomApi. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * Add ApiUpdateDatabase PhaseResult/RunResult regression specs (#464) Cover structured phase/run reporting and runner early-exit when auth token acquisition fails, so nightly classroom sync failures stay visible in ApiUpdateLog without continuing later phases. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * Bump rubyzip from 3.2.2 to 3.4.0 in the bundler group across 1 directory (#465) Bumps the bundler group with 1 update in the / directory: [rubyzip](https://github.com/rubyzip/rubyzip). Updates `rubyzip` from 3.2.2 to 3.4.0 - [Release notes](https://github.com/rubyzip/rubyzip/releases) - [Changelog](https://github.com/rubyzip/rubyzip/blob/main/Changelog.md) - [Commits](rubyzip/rubyzip@v3.2.2...v3.4.0) --- updated-dependencies: - dependency-name: rubyzip dependency-version: 3.4.0 dependency-type: indirect dependency-group: bundler ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: rsmokeUM <rsmoke@umich.edu> * Add TaskResultLog, TokenStore, and Okta display_name regression specs (#467) * Add TaskResultLog, TokenStore, and Okta display_name regression specs Cover nightly API update logging status/payload persistence, token cache expiry and failure paths, and Okta SAML display_name mapping on omniauth callback so auth/name regressions are caught early. Co-authored-by: rsmoke <rsmoke@umich.edu> * Improve error logging in ApiUpdateLog by including detailed error messages when save fails. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * Add ApplicationHelper api_status/api_log_text regression specs (#470) * Add ApplicationHelper api_status and api_log_text regression specs. Cover the admin header banner signals for missing nightly runs, error vs success recency, and blank log fallback so false-positive or missed API failure alerts are less likely to regress. Co-authored-by: rsmoke <rsmoke@umich.edu> * Potential fix for pull request finding 'Test does not verify log recency filtering' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Add FloorPolicy authorization regression specs (#471) * Add FloorPolicy authorization regression specs. Lock viewer show vs admin mutation permissions and deny show when membership groups are empty outside production. Co-authored-by: rsmoke <rsmoke@umich.edu> * Improve error logging in TaskResultLog specs by including specific error messages for invalid records --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * Add AnalyticsDashboard and ApiUpdateLog policy regression specs. (#476) Lock admin-only index/show/refresh gates so non-admin viewers cannot access API update logs or refresh the analytics dashboard. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * restore allowed_clock_drift in saml attributes (#478) Co-authored-by: rsmokeUM <rsmoke@umich.edu> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: cursor[bot] <206951365+cursor[bot]@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Margarita Barvinok <56089936+britaumich@users.noreply.github.com>
rsmoke
added a commit
that referenced
this pull request
Oct 21, 2026
…480) * Update tailwindcss dependencies, modify Procfile for CSS watch command, and enhance support info modal with improved accessibility and styling. * Add DepartmentApi and SentryMetrics regression specs (#462) Cover department lookup success/failure shaping used by BuildingsApi room updates, and ensure Sentry metric failures are swallowed so jobs/rake keep running. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * Add stale-room deactivation regression specs for media preservation. (#463) Cover the success path that hides rooms via visible=false while keeping contacts, characteristics, and Active Storage panoramas/images, plus the already-hidden fail-closed invariant in BuildingsApi and ClassroomApi. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * Add ApiUpdateDatabase PhaseResult/RunResult regression specs (#464) Cover structured phase/run reporting and runner early-exit when auth token acquisition fails, so nightly classroom sync failures stay visible in ApiUpdateLog without continuing later phases. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * Bump rubyzip from 3.2.2 to 3.4.0 in the bundler group across 1 directory (#465) Bumps the bundler group with 1 update in the / directory: [rubyzip](https://github.com/rubyzip/rubyzip). Updates `rubyzip` from 3.2.2 to 3.4.0 - [Release notes](https://github.com/rubyzip/rubyzip/releases) - [Changelog](https://github.com/rubyzip/rubyzip/blob/main/Changelog.md) - [Commits](rubyzip/rubyzip@v3.2.2...v3.4.0) --- updated-dependencies: - dependency-name: rubyzip dependency-version: 3.4.0 dependency-type: indirect dependency-group: bundler ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: rsmokeUM <rsmoke@umich.edu> * Add TaskResultLog, TokenStore, and Okta display_name regression specs (#467) * Add TaskResultLog, TokenStore, and Okta display_name regression specs Cover nightly API update logging status/payload persistence, token cache expiry and failure paths, and Okta SAML display_name mapping on omniauth callback so auth/name regressions are caught early. Co-authored-by: rsmoke <rsmoke@umich.edu> * Improve error logging in ApiUpdateLog by including detailed error messages when save fails. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * Add ApplicationHelper api_status/api_log_text regression specs (#470) * Add ApplicationHelper api_status and api_log_text regression specs. Cover the admin header banner signals for missing nightly runs, error vs success recency, and blank log fallback so false-positive or missed API failure alerts are less likely to regress. Co-authored-by: rsmoke <rsmoke@umich.edu> * Potential fix for pull request finding 'Test does not verify log recency filtering' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Add FloorPolicy authorization regression specs (#471) * Add FloorPolicy authorization regression specs. Lock viewer show vs admin mutation permissions and deny show when membership groups are empty outside production. Co-authored-by: rsmoke <rsmoke@umich.edu> * Improve error logging in TaskResultLog specs by including specific error messages for invalid records --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * Add AnalyticsDashboard and ApiUpdateLog policy regression specs. (#476) Lock admin-only index/show/refresh gates so non-admin viewers cannot access API update logs or refresh the analytics dashboard. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: rsmoke <rsmoke@umich.edu> * restore allowed_clock_drift in saml attributes (#478) Co-authored-by: rsmokeUM <rsmoke@umich.edu> * Count already-hidden rooms as deactivated so the API sync can finish. A stale classroom that was already hidden aborted the rooms phase and skipped the rest of the update. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: cursor[bot] <206951365+cursor[bot]@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Margarita Barvinok <56089936+britaumich@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds regression coverage for the success path of
deactivate_stale_roomsin BuildingsApi and ClassroomApi — previously only dry-run and rollback-on-mismatch were tested.Risky behavior now covered
visible: false) instead of destroyedroom_image/room_panoramaattachments are retainedTest files added/updated
spec/lib/buildings_api_spec.rbspec/lib/classroom_api_spec.rbWhy this reduces regression risk
A regression back to
destroy/deleteon stale rooms would permanently lose panoramas and gallery images. The prior suite only asserted rollback failure cases and dry-run warnings, not the production success path that preserves media.Validation
bundle exec rspec spec/lib/buildings_api_spec.rb spec/lib/classroom_api_spec.rb— 14 examples, 0 failures (SimpleCov minimum fails on subset runs as expected).Note
Low Risk
Test-only changes; no application or sync behavior is modified.
Overview
Adds regression specs for the success path of
deactivate_stale_roomson BuildingsApi and ClassroomApi—coverage that was missing next to existing rollback and dry-run examples.Each API now has examples that stale rooms are hidden (
visible: false) while Room rows, contacts, characteristics, and Active Storageroom_image/room_panoramastay intact, and that deactivation returns false when every listed room is already hidden (count mismatch / fail-closed).Reviewed by Cursor Bugbot for commit 10ac91e. Bugbot is set up for automated code reviews on this repo. Configure here.