Skip to content

fix: Send the name of the default filter sort option - #41

Merged
joamag merged 1 commit into
masterfrom
bug/filter-sort-name
Oct 1, 2026
Merged

joamag merged 1 commit into
masterfrom
bug/filter-sort-name

Conversation

@joamag

@joamag joamag commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Closes #40, the server side being hivesolutions/colony-plugins#48, with the binaries in hivesolutions/uxf-bin#10.

Description

The filter adds a default sort option without a data-name (js/ui/filter.part.js), and the sort value of the query falls back to the HTML of the selected option when it has no data-name. When the browser translates the page (eg: Chrome page translation) the text of the option is wrapped in <font> elements, so the filter sends the translated markup as the sort value instead of default (seen in production in OMNI-LDJ-15, <font dir="auto" style="vertical-align: inherit;">...</font>:descending), which the server fails to handle.

Changes

Tests

  • test/ui/filter.js (mocha, on Node 20 as in CI), 6 new tests: the name of the default sort option, the query with the default sort option (before and after its contents are translated), the query with named sort options (including the toggling of their order) and the query with an option without a name wrapped in translation markup. The data query is replaced by a stub that records the queries, and the document and window of jsdom are exposed as globals (as in test/gateway/print.js), as the filter uses them. 3 of them fail against master (the translated one with the exact markup of the event), and the 47 tests pass.
  • Coverage of the changed statements: 7 of 7 (100%), including both sides of the fallback, measured with c8.
  • eslint passes (npm run lint).
  • The binaries are built with the join of uxf, which reproduces the current binaries (18b96ebe) byte for byte from master, so the changes of the new ones are only the ones of the filter (see chore: New binaries sending the name of the default filter sort option uxf-bin#10).

Merge

Merge after hivesolutions/uxf-bin#10. A squash (or rebase) merge of it changes 9be03c81, requiring the bin pointer of this branch to be re-pointed before merging.

Copilot AI balanced review requested due to automatic review settings September 30, 2026 19:59

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d17a7fc5-bbc8-4411-871a-55a802fa864b

📥 Commits

Reviewing files that changed from the base of the PR and between 81ecd60 and 43cfa2b.

📒 Files selected for processing (2)
  • bin
  • test/ui/filter.js

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The filter now uses a stable name for its default sort option and uses an option’s text content as the fallback sort value when no name is set. Tests cover the sort values passed to uxdataquery.

Changes

Filter sort values

Layer / File(s) Summary
Build sort query values
js/ui/filter.part.js, test/ui/filter.js
The default sort option now has data-name="default". For options without data-name, the filter uses text content instead of HTML. Tests check default, translated, named, direction-toggle, and fallback sort values.

Bin subproject reference

Layer / File(s) Summary
Update subproject reference
bin
The bin subproject reference changes from commit 18b96ebedfc07ef20fdffc5a36a2f2831a3881a4 to 9be03c8115c53cc8f82af7fd35edec2829bb0b1f.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 43cfa

The filter now sends a stable "default" sort value and uses an option's plain text, not its markup, when no name is set. This prevents translated markup from breaking list loading. The bundled binaries already include the fix. No merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 43cfa

The change stabilizes sorting without showing a new security boundary or privilege expansion. Verification remains incomplete for the published browser files and server-side handling.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The visible production effect is limited to sort values emitted by filters using this component and delivered to their existing data sources. The supplied evidence does not identify the downstream tenants, services, or data stores, so their maximum exposure remains unverified.

Trust Boundaries and Controls

  • observed — Sort keys continue to originate in client-side DOM attributes or text and can reach a remote query. Using text instead of HTML is not server-side validation or authorization. Those downstream controls were not supplied and cannot be inferred from this change.

Resilience and Maintainability Implications

  • inferred — The test recorder captures and restores the shared query method around each test. Restoration after ordinary test failure relies on Mocha cleanup semantics. The configured serial runner avoids overlapping replacements; abrupt process termination or an alternate overlapping harness is not covered, but no production security effect was established.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #40 requires data-name="default" on the default sort option and text fallback for unnamed options. js/ui/filter.part.js implements both changes. test/ui/filter.js verifies default sorting,…
Out of Scope Changes check ✅ Passed The source change and focused tests directly support issue #40. The bin subproject pointer change is consistent with delivering the related build output. The available diff does not demonstrate unre…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Title check ✅ Passed The title clearly and concisely describes the main fix: sending the default filter sort option name.
Description check ✅ Passed The description directly explains the browser-translation bug, the code changes, the tests, and the binary update.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-01T06:06:35.419193Z 43cfa2b Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

- The default sort option now has a name, sent whatever its contents
- Options without a name fall back to their text, never to their markup
- Pages translated by the browser no longer send markup as the sort value
- Tests cover the default, translated, named and unnamed sort options
@joamag

joamag commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@joamag
joamag merged commit 0d24b6e into master Oct 1, 2026
6 checks passed
@joamag
joamag deleted the bug/filter-sort-name branch October 1, 2026 06:05
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 43cfa2b29a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Send the name of the default filter sort option instead of its markup

2 participants