Skip to content

fix(frontend): only send active log sort directions#1822

Merged
riderx merged 2 commits into
mainfrom
codex/fix-log-order-validation
Mar 17, 2026
Merged

fix(frontend): only send active log sort directions#1822
riderx merged 2 commits into
mainfrom
codex/fix-log-order-validation

Conversation

@riderx
Copy link
Copy Markdown
Member

@riderx riderx commented Mar 17, 2026

Summary (AI generated)

  • Fix the logs table request payload so only active sort directions are sent to /private/stats
  • Apply the same serialization fix to the deployment table, which used the same invalid order shape
  • Prevent frontend requests from sending sortable: true, which the backend rejects

Motivation (AI generated)

The device logs page was failing with invalid_body because the frontend serialized all sortable columns into the order array, including unsorted columns represented as true. The backend validates order[*].sortable as only asc or desc, so the request was rejected before any data could load.

Business Impact (AI generated)

This restores the logs experience for users investigating device behavior and update failures. It removes a frontend/backend contract mismatch in an operational debugging flow, reducing support friction and preserving trust in the console for troubleshooting production issues.

Test Plan (AI generated)

  • Run bun eslint src/components/tables/LogTable.vue src/components/tables/DeploymentTable.vue
  • Open a device logs page and confirm logs load without the invalid_body console error
  • Open the deployment table and confirm sorting/filtering requests still succeed

Generated with AI

Summary by CodeRabbit

  • Bug Fixes
    • Improved table sorting reliability in Deployment and Log tables by refining how sort directions are handled, ensuring more consistent and predictable ordering of results.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

Both DeploymentTable.vue and LogTable.vue were updated to restrict the order parameter construction. Previously, any truthy sortable value was included in API requests. Now, only columns with sortable as a string are included, with explicit 'asc' or 'desc' type casting.

Changes

Cohort / File(s) Summary
Table Order Parameter Refinement
src/components/tables/DeploymentTable.vue, src/components/tables/LogTable.vue
Updated order array filtering to require sortable as a string type and cast to explicit 'asc' | 'desc' directions instead of accepting any truthy value. Improves type safety in API payload construction.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Whisker-twitched precision found,
Where sorting strings stood proud and sound,
No more truthy shadows stray,
Asc and desc light up the way!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description includes a comprehensive summary, motivation, and business impact. However, the required test plan sections are incomplete: manual testing steps for logs and deployment table are marked incomplete. Complete the pending test plan items by confirming logs load without errors and deployment table sorting requests still succeed, then update the PR description.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses the main change: filtering sort directions to only send explicit values, fixing the invalid payload issue described in the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-log-order-validation
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/components/tables/LogTable.vue (1)

290-292: Consider extracting order serialization into a shared helper.

This logic is duplicated here, in exportCsv(), and in DeploymentTable.vue. A shared helper would reduce drift risk if sorting rules change again.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/tables/LogTable.vue` around lines 290 - 292, Extract the logic
that builds the serialized sort order (currently implemented as
columns.value.filter(elem => typeof elem.sortable === 'string').map(elem => ({
key: elem.key as string, sortable: elem.sortable as 'asc' | 'desc' })) ) into a
single shared helper (e.g., serializeColumnOrder(columns):
Array<{key:string,sortable:'asc'|'desc'}>), export it from a common utilities
module, and replace the in-place code in LogTable.vue, exportCsv(), and
DeploymentTable.vue to call that helper so all three use the same implementation
and avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/tables/LogTable.vue`:
- Around line 290-292: Extract the logic that builds the serialized sort order
(currently implemented as columns.value.filter(elem => typeof elem.sortable ===
'string').map(elem => ({ key: elem.key as string, sortable: elem.sortable as
'asc' | 'desc' })) ) into a single shared helper (e.g.,
serializeColumnOrder(columns): Array<{key:string,sortable:'asc'|'desc'}>),
export it from a common utilities module, and replace the in-place code in
LogTable.vue, exportCsv(), and DeploymentTable.vue to call that helper so all
three use the same implementation and avoid duplication.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2213628c-9403-4de9-94dc-d66526aeb6df

📥 Commits

Reviewing files that changed from the base of the PR and between a521f9f and ef74073.

📒 Files selected for processing (2)
  • src/components/tables/DeploymentTable.vue
  • src/components/tables/LogTable.vue

@riderx riderx merged commit acc2e87 into main Mar 17, 2026
13 checks passed
@riderx riderx deleted the codex/fix-log-order-validation branch March 17, 2026 15:42
@sonarqubecloud
Copy link
Copy Markdown

riderx added a commit that referenced this pull request Mar 19, 2026
* fix(api): harden plan usage org rpc access

* Merge branch 'main' into fix/ghsa-wh77-plan-rpc-access

* fix(tests): correct plan usage rpc test typings

* Merge branch 'main' into fix/ghsa-wh77-plan-rpc-access

* test(db): authenticate hardened plan rpc assertions

* test(db): keep plan rpc auth context through negative case

* test: cover plan usage rpc via authenticated client

* test: parallelize plan usage rpc checks

* fix: ensure FORCE_JAVASCRIPT_ACTIONS_TO_NODE24 is set in all workflow files

* chore(release): 12.116.63

* fix(webhooks): stop following webhook redirects (#1803)

* chore(release): 12.116.64

* fix(devices): restore pagination after page 1 (#1820)

* fix(devices): normalize analytics pagination cursor timestamps

* test(devices): run cloudflare datetime checks concurrently

* chore(release): 12.116.65

* fix(sso): prevent orphan org on first login and use correct IdP URLs

- Add DB migration to skip auto-org creation in generate_org_on_user_create
  when the new user's email domain has an active SSO provider. Previously,
  auth.ts lazily creating the public.users row would trigger an unwanted
  personal org for brand-new SSO users.
- Replace client-side SP metadata URL computation in SsoConfiguration.vue
  (which used VITE_SUPABASE_URL / custom domain) with a fetch to the
  existing /private/sso/sp-metadata endpoint, which derives ACS URL and
  Entity ID from SUPABASE_URL — the same value Supabase uses internally
  for SAML processing.
- Add sp_metadata_url to sp-metadata.ts response to match the SpMetadata
  interface consumed by the frontend.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore: update bun lockfile after install

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sso): harden trigger and sp-metadata after security audit

- generate_org_on_user_create: add auth.users provider check to avoid
  suppressing personal org for email/password signups on SSO domains
  (Finding 2 — High). Email/password users on a corporate domain still
  get a personal org; provision-user.ts rejects them anyway.
- generate_org_on_user_create: add sso_enabled org flag to EXISTS query,
  matching check_domain_sso and all other SSO lookups (Finding 1 — Critical).
- generate_org_on_user_create: apply btrim to domain component to match
  the lower(btrim(domain)) normalization contract from migration
  20260312183000 (Finding 6 — Medium).
- sp-metadata.ts: add return before quickError to prevent silent
  fall-through if quickError signature ever changes (Finding 3 — High).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sso): surface toast on SP metadata fetch failures

Both error paths in fetchSpMetadata (non-ok response and network error)
now show a toast.error alongside the existing console.error, consistent
with all other error handling in SsoConfiguration.vue.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sso): add structured cloudlog to sp-metadata unauthorized branch

Add Context<MiddlewareKeyVariables> type annotation, cloudlog import,
and requestId to the app.get handler. The !auth branch now emits a
cloudlog entry with requestId, message, and auth value before returning
quickError, consistent with the logging pattern in provision-user.ts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sso): restrict org-skip to SAML providers only in trigger

The previous logic checked has_sso AFTER the provider IF, and skipped
org creation for any non-email/phone provider when has_sso was true —
incorrectly affecting OAuth users (google, github, etc.) whose email
domain happened to match an active SSO provider.

Restructure: compute has_sso first, then use a single condition
NOT (user_provider ~ '^sso:' AND has_sso) to gate the INSERT.
Supabase sets app_metadata.provider to 'sso:<uuid>' for SAML sessions,
so OAuth providers never match '^sso:' and always get a personal org.
This also eliminates the duplicate INSERT path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sso): merge existing users on SSO login

* fix(sso): stop editing merged migration

* fix(sso): harden SSO login and merge flow

* fix(sso): align SAML guards and Cloudflare routes

* fix(sso): sync auth flags with enforcement

* fix(sso): close shared pg pool on workerd

* fix(frontend): only send active log sort directions (#1822)

* chore(release): 12.116.66

* chore(release): 12.116.67

* chore(release): 12.116.68

* fix(db): avoid cache writes for plan usage read-only calls

* fix(cron): use writable pg client

* fix(api): keep plan usage RPC read-only

* fix(api): restore plan usage read-only guard

* fix(api): drop unused plan rpc variable

* Merge origin/main

* fix: resolve package manifest conflict

* docs(sql): add docstrings for plan usage RPCs
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.

1 participant