Skip to content

fix(devices): address release-PR review on device sync#3387

Merged
tofikwest merged 1 commit into
mainfrom
fix/device-sync-review-follow-ups
Jul 10, 2026
Merged

fix(devices): address release-PR review on device sync#3387
tofikwest merged 1 commit into
mainfrom
fix/device-sync-review-follow-ups

Conversation

@tofikwest

@tofikwest tofikwest commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Follow-ups for the three cubic findings on the production deploy PR (#3382) — verified all three as valid:

  1. N+1 queries in available-providers — the endpoint made one query per provider plus a second for each unconnected one. Now a single batched query fetches every provider's connections (newest first) and status is resolved in memory. Semantics preserved and spec-tested, including the stale-error-under-newer-disconnect case and a new active-beats-newer-broken-row case.

  2. Uncontrolled select hid the saved selection — the device-sync select renders directly on the page (the popover-dismissal reason for the uncontrolled people-sync selects doesn't apply), so it's now controlled: the open dropdown marks the saved provider as selected for keyboard/screen-reader users. The value is guarded to items that actually exist — a saved-but-no-longer-listed provider falls back to no selection instead of breaking the select. Test asserts aria-selected on the open dropdown.

  3. Test fixture didn't omit connectionStatus — the older-API test inherited 'active' from the base fixture and passed by coincidence; it now explicitly sets undefined.

Tests: api 5/5, app devices 90/90, typecheck clean.


Summary by cubic

Batch the device sync provider connection lookup to remove N+1 queries and make the device sync select controlled so the saved provider stays selected. Also fixes a test fixture to correctly omit connectionStatus for older API cases.

  • Bug Fixes
    • API: Replace per-provider lookups with one findMany (newest-first) and resolve status in memory; prefer an active row over newer non-active ones, and don’t resurface stale error under a newer disconnect.
    • App: Make DeviceSyncProviderSelector controlled and guard value to listed items so the open dropdown marks the saved provider (aria-selected) and doesn’t break if the saved provider is missing.
    • Tests: Older-API fixture now omits connectionStatus (uses undefined) to reflect real responses.

Written for commit 094a918. Summary will update on new commits.

Review in cubic

- available-providers: one batched connections query for all providers
  (newest-first, resolved in memory) instead of up to two point lookups
  per provider
- device-sync select is controlled (value guarded to listed items) so
  the open dropdown marks the saved selection for keyboard and
  screen-reader users
- older-API test fixture now genuinely omits connectionStatus instead
  of inheriting 'active' from the base fixture
@vercel

vercel Bot commented Jul 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
app Ready Ready Preview, Comment Jul 10, 2026 4:02am
comp-framework-editor Ready Ready Preview, Comment Jul 10, 2026 4:02am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
portal Skipped Skipped Jul 10, 2026 4:02am

Request Review

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 4 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@tofikwest tofikwest merged commit f0c9f2c into main Jul 10, 2026
11 checks passed
@tofikwest tofikwest deleted the fix/device-sync-review-follow-ups branch July 10, 2026 04:03
claudfuen pushed a commit that referenced this pull request Jul 10, 2026
# [3.100.0](v3.99.3...v3.100.0) (2026-07-10)

### Bug Fixes

* **devices:** address cubic review on device-sync connection status ([4fa7598](4fa7598))
* **devices:** address release-PR review on device sync ([#3387](#3387)) ([f0c9f2c](f0c9f2c))
* **devices:** show reconnect hint when device-sync connection is errored ([d41bb5c](d41bb5c))
* **devices:** surface broken saved sync source on the closed trigger ([d59f0a0](d59f0a0))

### Features

* **devices:** count imported devices in the compliance chart ([#3384](#3384)) ([f94f02a](f94f02a))
* **devices:** universal source-reported compliance + last-seen for device sync ([#3383](#3383)) ([c45fa5f](c45fa5f))
@claudfuen

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.100.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants