Skip to content

Update dependencies, enhance accessibility, and add regression specs - #481

Merged
rsmoke merged 15 commits into
mainfrom
staging
Oct 21, 2026
Merged

rsmoke merged 15 commits into
mainfrom
staging

Conversation

@rsmoke

@rsmoke rsmoke commented Oct 21, 2026

Copy link
Copy Markdown
Member

This pull request updates the logic for deactivating (hiding) rooms in both the BuildingsApi and ClassroomApi, ensuring that rooms already hidden are correctly counted as "deactivated" and improving the safety and clarity of the deactivation process. The test suites for both APIs are expanded to cover these new behaviors, including concurrency safety.

Key changes:

Core logic improvements

  • The deactivate_stale_rooms method in both lib/buildings_api.rb and lib/classroom_api.rb now counts rooms that are already hidden as successfully deactivated, and only updates rooms that are visible or have unset visibility. The method also acquires a lock on the relevant room rows before performing updates to ensure data consistency. [1] [2]

Test coverage enhancements

  • The specs for both APIs (spec/lib/buildings_api_spec.rb and spec/lib/classroom_api_spec.rb) have been expanded:
    • Tests now verify that already hidden rooms are counted as deactivated, and that the method returns the correct count. [1] [2] [3]
    • New tests ensure that when a mix of hidden and visible rooms are processed, only the visible ones are updated and the total is correct. [1] [2]
    • Tests confirm that a database lock is acquired before updating or counting rooms, ensuring proper concurrency handling. [1] [2]
    • Additional tests cover the case where a room's visibility is unset (nil). [1] [2]

rsmoke and others added 14 commits October 14, 2026 11:35
…d, and enhance support info modal with improved accessibility and styling.
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>
…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>
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>
…ory (#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>
…#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 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.

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>
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>
Co-authored-by: rsmokeUM <rsmoke@umich.edu>
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>
… ClassroomApi

Introduce a locking mechanism to ensure that rooms are locked before counting hidden rooms during the deactivation process. This change is accompanied by new tests to verify that the locking occurs correctly before any updates are made to room visibility.
Copilot AI balanced review requested due to automatic review settings October 21, 2026 20:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Static review found no blocking issues in the focused logic changes or regression specs; tests were not executed.

No open issues 🎉

What changed in this PR

Improves stale-room deactivation in both APIs so already-hidden rooms no longer cause failure.

Changes:

  • Locks room rows before counting and updating.
  • Accepts already-hidden rooms and hides rooms with true or unset visibility.
  • Adds regression specs for mixed visibility, rollback, and lock ordering.
File Description
spec/​lib/​classroom_api_spec.rb Adds classroom deactivation regression coverage.
spec/​lib/​buildings_api_spec.rb Adds building-room deactivation regression coverage.
lib/​classroom_api.rb Updates classroom locking and visibility handling.
lib/​buildings_api.rb Updates building-room locking and visibility handling.

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 21, 2026 20:56
@rsmoke
rsmoke merged commit 722e49f into main Oct 21, 2026
4 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Both regression specs must verify locking precedes counting, not just updating.

2 open issues

Review effort: Balanced

Comment on lines +127 to +129
lock_index = queries.index { |sql| sql.include?("FOR UPDATE") }
update_index = queries.index { |sql| sql.match?(/UPDATE "rooms"/) }
expect(lock_index).to be < update_index
Comment on lines +135 to +137
lock_index = queries.index { |sql| sql.include?("FOR UPDATE") }
update_index = queries.index { |sql| sql.match?(/UPDATE "rooms"/) }
expect(lock_index).to be < update_index

This branch was successfully deployed

1 active deployment
staging — 722e49fa Deployed Oct 21, 2026 by hatchbox-io[bot]
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.

3 participants