feat: Improving item search order by functionality#1391
Draft
bmyoungquist wants to merge 5 commits into
Draft
Conversation
… picking the direction of the order
Contributor
WalkthroughAdds client-controlled sorting direction to items listing: API docs and handler annotations updated, backend ItemQuery and repository ordering logic extended with Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (Client)
participant Frontend as Frontend App
participant API as API Server
participant Repo as Repository/DB
Browser->>Frontend: user changes orderBy/orderByDirection (route query)
Frontend->>Frontend: update route-backed state, run search()
Frontend->>API: GET /v1/items?orderBy=...&orderByDirection=...
API->>Repo: build query with OrderBy + OrderByDirection
Repo->>Repo: map orderBy -> column, apply ASC/DESC
Repo-->>API: return paginated ItemSummary results
API-->>Frontend: return repo.PaginationResult[ItemSummary]
Frontend-->>Browser: render sorted items
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Security Recommendations
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
coderabbitai
Bot
requested review from
katosdev,
tankerkiller125 and
tonyaellie
coderabbitai
Bot
added
📖 documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (7)
backend/internal/data/repo/repo_items.go (2)
501-504: Consider DB indexes for newly sortable fields.If groups can be large, sorting by
purchasePriceandquantitymay become expensive without supporting indexes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/data/repo/repo_items.go` around lines 501 - 504, The new sortable cases using item.FieldPurchasePrice and item.FieldQuantity can cause slow ORDER BY on large groups—add explicit DB indexes for the underlying columns (purchase_price, quantity) via a migration and/or model schema change so queries that use the sorting branch in repo_items.go run efficiently; update the DB migration to create B-tree indexes (or composite indexes if sorting is commonly combined with other filtering columns), ensure any ORM model tags/annotations reflect the indexed columns, and add tests or explainers confirming the query planner uses the new indexes.
509-514: Add a deterministic tie-breaker to avoid page drift.Sorting only by non-unique fields can reorder equal rows between requests and cause duplicates/misses across paginated pages.
♻️ Proposed change
switch q.OrderByDirection { case "desc": - qb = qb.Order(ent.Desc(orderByField)) + qb = qb.Order(ent.Desc(orderByField), ent.Desc(item.FieldID)) default: // "asc" - qb = qb.Order(ent.Asc(orderByField)) + qb = qb.Order(ent.Asc(orderByField), ent.Asc(item.FieldID)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/data/repo/repo_items.go` around lines 509 - 514, The current ordering uses only q.OrderByDirection on orderByField which can cause page drift for non-unique fields; update the ordering logic in repo_items.go (the switch that calls qb.Order(ent.Desc(orderByField)) / ent.Asc(orderByField)) to append a deterministic tie-breaker sort on the unique primary key (e.g., the entity ID field) so equal values are consistently ordered—e.g., after sorting by orderByField, add qb.Order(ent.Asc("id")) (or ent.Desc("id") if you must keep reverse stable order) to guarantee stable pagination and avoid duplicates/misses across pages.docs/public/api/swagger-2.0.yaml (1)
2466-2473: Add enums fororderByandorderByDirectionin Swagger 2.0.This keeps the contract strict and helps clients send only supported values.
♻️ Suggested schema tightening
- description: Field to order the results by in: query name: orderBy + enum: + - name + - createdAt + - updatedAt + - assetId + - purchasePrice + - quantity type: string - description: Direction to order the results by in: query name: orderByDirection + enum: + - asc + - desc type: string🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/public/api/swagger-2.0.yaml` around lines 2466 - 2473, The query params orderBy and orderByDirection currently lack enums in the Swagger 2.0 spec; update their parameter schemas to include explicit enum lists so clients can only send supported values. Locate the parameter definitions for name: orderBy and name: orderByDirection and add an enum array for each (e.g., orderBy -> allowed sortable fields used by your API; orderByDirection -> ["asc","desc"] or your accepted direction tokens), keeping their type as string and preserving description/in:query metadata. Ensure the enums match the actual server-side accepted values so the API contract stays strict.docs/public/api/openapi-3.0.yaml (1)
481-490: ConstrainorderByandorderByDirectionwith enums.Documenting explicit allowed values improves client validation and narrows malformed-input surface.
♻️ Suggested schema tightening
- description: Field to order the results by name: orderBy in: query schema: type: string + enum: [name, createdAt, updatedAt, assetId, purchasePrice, quantity] - description: Direction to order the results by name: orderByDirection in: query schema: type: string + enum: [asc, desc]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/public/api/openapi-3.0.yaml` around lines 481 - 490, The two query parameters orderBy and orderByDirection should be constrained with enums: update the OpenAPI schema for the parameters named orderBy and orderByDirection to include an enum of allowed values (for orderBy list the valid sortable fields your API supports, e.g., "createdAt","updatedAt","name","id" or the canonical field names used in your backend) and for orderByDirection set enum: ["asc","desc"] (optionally add a default like "desc"); ensure the parameter entries still have type: string and include any descriptions/examples to reflect the allowed values.docs/public/api/openapi-3.0.json (2)
790-804: Clarify conflicting photo-filter semantics.Line 790 and Line 798 can both be
true, but the spec does not define the resulting behavior. Please document precedence or mark them as mutually exclusive to avoid client ambiguity.Suggested docs tweak
{ - "description": "Only return items that have a photo", + "description": "Only return items that have a photo. Mutually exclusive with onlyWithoutPhoto.", "name": "onlyWithPhoto", "in": "query", "schema": { "type": "boolean" } }, { - "description": "Only return items that don't have a photo", + "description": "Only return items that don't have a photo. Mutually exclusive with onlyWithPhoto.", "name": "onlyWithoutPhoto", "in": "query", "schema": { "type": "boolean" } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/public/api/openapi-3.0.json` around lines 790 - 804, The two query params onlyWithPhoto and onlyWithoutPhoto have ambiguous combined semantics; update the OpenAPI entry for these parameters so behavior is explicit — either (A) document precedence (e.g., onlyWithoutPhoto overrides onlyWithPhoto or vice versa) by editing the "description" fields for onlyWithPhoto and onlyWithoutPhoto to state the exact outcome when both are true, or (B) mark them as mutually exclusive in the spec and server validation (e.g., add a clear note and an API-level validation rule/error response) so clients cannot set both; reference the parameter names onlyWithPhoto and onlyWithoutPhoto when making the change.
814-827: DocumentorderByDirectionas enum constraint andorderByallowlist in OpenAPI schema (API contract tightening).Lines 814-827 currently accept free-form strings in the OpenAPI specification. While the backend correctly validates these fields using a switch statement with an explicit allowlist (lines 494–514 in
repo_items.go), the API contract should document and enforce this constraint to improve client-side validation and reduce invalid requests.Specifically:
orderByDirection: Should be constrained to["asc", "desc"]orderBy: Should enumerate the supported fields:createdAt,updatedAt,assetId,purchasePrice,quantity,nameSuggested OpenAPI constraint
{ "description": "Field to order the results by", "name": "orderBy", "in": "query", "schema": { - "type": "string" + "type": "string", + "enum": ["createdAt", "updatedAt", "assetId", "purchasePrice", "quantity", "name"] } }, { "description": "Direction to order the results by", "name": "orderByDirection", "in": "query", "schema": { - "type": "string" + "type": "string", + "enum": ["asc", "desc"] } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/public/api/openapi-3.0.json` around lines 814 - 827, Update the OpenAPI schema for the query parameters named "orderBy" and "orderByDirection" in docs/public/api/openapi-3.0.json to match backend validation: change "orderByDirection" to an enum with values ["asc","desc"] and change "orderBy" to an enum listing ["createdAt","updatedAt","assetId","purchasePrice","quantity","name"]; this mirrors the allowlist enforced in the repo_items.go switch (the validation logic around lines 494–514) so clients get the same constraints as the server.docs/public/api/swagger-2.0.json (1)
782-793: Addenumconstraints fororderByandorderByDirectionparameters for API clarity.The
orderByandorderByDirectionparameters lack enum constraints in the API spec, making it unclear to API consumers which values are valid. While the backend properly validates these parameters with a whitelist (falling back to safe defaults), the Swagger documentation should explicitly list valid values for better developer experience.📚 Suggested documentation improvement
{ "type": "string", - "description": "Field to order the results by", + "description": "Field to order the results by", + "enum": ["name", "createdAt", "updatedAt", "assetId", "purchasePrice", "quantity"], "name": "orderBy", "in": "query" }, { "type": "string", - "description": "Direction to order the results by", + "description": "Direction to order the results by", + "enum": ["asc", "desc"], "name": "orderByDirection", "in": "query" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/public/api/swagger-2.0.json` around lines 782 - 793, The Swagger parameter objects for orderBy and orderByDirection are missing enum constraints; update the parameter with "name": "orderBy" (the orderBy parameter) to include an "enum" array populated with the exact whitelist of sortable fields used by the backend (e.g., createdAt, updatedAt, name, id — match your backend whitelist), and update the parameter with "name": "orderByDirection" to include an "enum" with the allowed directions (e.g., "asc", "desc"); ensure both enum values exactly mirror the backend validation/whitelist so the API spec and implementation stay in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/api/handlers/v1/v1_ctrl_items.go`:
- Around line 73-75: Validate and normalize the incoming sorting params at the
handler boundary: check params.Get("orderBy") against a predefined allowlist of
permitted column names (e.g., the set of sortable fields used by the repository)
and only accept params.Get("orderByDirection") if it normalizes to "asc" or
"desc" (case-insensitive); if either is invalid, return an HTTP 400 error from
the handler instead of passing raw values into the query struct (the fields
OrderBy and OrderByDirection) that is sent to the repository. Ensure you
normalize the accepted direction to a consistent lowercase value before
constructing the query and reference the same allowlist used by the repository
to avoid mismatches.
- Line 33: The Swagger `@Param` for negateTags is wrong — the code parses query
param "negateTags" as a boolean but the comment documents it as []string; update
the handler's Swagger annotation for negateTags in v1_ctrl_items.go to declare
it as type=bool (instead of []string) and mark it as query, include a short
description like "Negate tag matching" and whether it's required, so the API
contract matches the runtime parsing used in the corresponding handler that
reads the negateTags query parameter.
In `@docs/public/api/openapi-3.0.yaml`:
- Around line 442-449: The OpenAPI parameter "negateTags" is incorrectly defined
as an array; change its schema to type: boolean and remove array-specific fields
(explode and items) so clients send a boolean flag; update or keep the
description to reflect it is a boolean that toggles exclusion of tags (reference
symbol: negateTags in the parameter definition).
In `@docs/public/api/swagger-2.0.yaml`:
- Around line 2434-2439: The Swagger param "negateTags" is documented as an
array but should be a boolean; update the "negateTags" parameter definition to
use type: boolean (remove the items: block and type: array), keep in: query and
the description, and ensure any examples/defaults reflect a boolean value so the
generated clients use the correct wire format for the negateTags parameter.
In `@frontend/locales/cs-CZ.json`:
- Around line 544-546: Revert the direct edit to the non-English locale: remove
the added "order": {"order_by": "Seřadit podle"} entry from
frontend/locales/cs-CZ.json so that only source strings are changed in this PR;
instead make your string change in frontend/locales/en.json (update the
corresponding "order.order_by" key there) and leave all other locale files
(including cs-CZ.json) untouched for Weblate to manage.
In `@frontend/locales/id-ID.json`:
- Around line 281-283: Add the missing translation keys for the items.order
group in the id-ID locale: include "items.order.direction",
"items.order.ascending", and "items.order.descending" with appropriate
Indonesian strings (e.g., direction label and ascending/descending words),
ensuring they are placed under the existing items.order object (alongside
"order_by") and that the JSON stays valid (commas, quotes). Use the exact key
names items.order.direction, items.order.ascending, items.order.descending so
the UI lookup matches.
In `@frontend/locales/it.json`:
- Around line 521-523: The locale is missing three translation keys used by the
updated items filters UI: add "items.order.direction", "items.order.ascending",
and "items.order.descending" to the Italian locale alongside the existing
"items.order.order_by" entry; provide appropriate Italian strings (e.g.,
"Direzione", "Crescente", "Decrescente") so the UI's sort direction control can
render localized labels for the settings in the items filters.
In `@frontend/locales/pl.json`:
- Around line 544-546: The Polish locale is missing three new i18n keys under
the existing "order" object: add "direction", "ascending", and "descending"
alongside the existing "order_by" key so the sort-direction selector is
localized; update the "order" entry (the object that contains "order_by") to
include "direction", "ascending", and "descending" with appropriate Polish
translations.
In `@frontend/locales/ro-RO.json`:
- Around line 224-226: The locale is missing the items.order.direction,
items.order.ascending, and items.order.descending keys; update the ro-RO JSON to
include these translations alongside the existing items.order.order_by so the
direction dropdown shows localized labels (add entries for
"items.order.direction", "items.order.ascending", and "items.order.descending"
with appropriate Romanian strings).
In `@frontend/locales/ru.json`:
- Around line 543-545: The RU locale is missing translations for the new
sort-direction dropdown keys; add entries for "items.order.direction",
"items.order.ascending", and "items.order.descending" into
frontend/locales/ru.json under the same "order" object (or the appropriate
"items" object) so the UI keys resolve; provide correct Russian strings for each
key (e.g., a short label for direction and localized words for
ascending/descending) and ensure JSON syntax and nesting match other locale
files.
In `@frontend/locales/sk-SK.json`:
- Around line 536-538: Add the missing Slovak translation keys under the
existing "order" object: include "direction", "ascending", and "descending"
entries (alongside the existing "order_by") so the UI's items sort controls have
localized strings; update the items.order object to contain keys "direction",
"ascending", and "descending" with appropriate Slovak values matching the
project's translation style.
In `@frontend/locales/sl.json`:
- Around line 292-294: The Slovenian locale only defines "order.order_by" and is
missing required keys for the updated filters UI; add the missing translations
for "items.order.direction", "items.order.ascending", and
"items.order.descending" (and ensure they live under the same "items.order"
object alongside "order_by") so the UI can read all three keys; update the
translations in the same file to include these keys with appropriate Slovenian
strings.
In `@frontend/locales/zh-CN.json`:
- Around line 544-546: The localization for the ordering UI is incomplete: under
the "order" object (currently containing "order_by") add the missing keys
"direction", "ascending", and "descending" with appropriate Simplified Chinese
translations (e.g., "direction": "方向", "ascending": "升序", "descending": "降序") so
the new direction selector shows proper labels for users; update the "order"
object in frontend/locales/zh-CN.json alongside the existing "order_by" entry.
In `@frontend/locales/zh-TW.json`:
- Around line 544-546: The items.order translation is incomplete: add the
missing keys under "items.order"—add "direction", "ascending", and "descending"
alongside the existing "order_by" so the new sort-direction UI has Traditional
Chinese labels; update the zh-TW JSON entry for items.order to include these
three keys (e.g., items.order.direction, items.order.ascending,
items.order.descending) with appropriate Traditional Chinese translations.
---
Nitpick comments:
In `@backend/internal/data/repo/repo_items.go`:
- Around line 501-504: The new sortable cases using item.FieldPurchasePrice and
item.FieldQuantity can cause slow ORDER BY on large groups—add explicit DB
indexes for the underlying columns (purchase_price, quantity) via a migration
and/or model schema change so queries that use the sorting branch in
repo_items.go run efficiently; update the DB migration to create B-tree indexes
(or composite indexes if sorting is commonly combined with other filtering
columns), ensure any ORM model tags/annotations reflect the indexed columns, and
add tests or explainers confirming the query planner uses the new indexes.
- Around line 509-514: The current ordering uses only q.OrderByDirection on
orderByField which can cause page drift for non-unique fields; update the
ordering logic in repo_items.go (the switch that calls
qb.Order(ent.Desc(orderByField)) / ent.Asc(orderByField)) to append a
deterministic tie-breaker sort on the unique primary key (e.g., the entity ID
field) so equal values are consistently ordered—e.g., after sorting by
orderByField, add qb.Order(ent.Asc("id")) (or ent.Desc("id") if you must keep
reverse stable order) to guarantee stable pagination and avoid duplicates/misses
across pages.
In `@docs/public/api/openapi-3.0.json`:
- Around line 790-804: The two query params onlyWithPhoto and onlyWithoutPhoto
have ambiguous combined semantics; update the OpenAPI entry for these parameters
so behavior is explicit — either (A) document precedence (e.g., onlyWithoutPhoto
overrides onlyWithPhoto or vice versa) by editing the "description" fields for
onlyWithPhoto and onlyWithoutPhoto to state the exact outcome when both are
true, or (B) mark them as mutually exclusive in the spec and server validation
(e.g., add a clear note and an API-level validation rule/error response) so
clients cannot set both; reference the parameter names onlyWithPhoto and
onlyWithoutPhoto when making the change.
- Around line 814-827: Update the OpenAPI schema for the query parameters named
"orderBy" and "orderByDirection" in docs/public/api/openapi-3.0.json to match
backend validation: change "orderByDirection" to an enum with values
["asc","desc"] and change "orderBy" to an enum listing
["createdAt","updatedAt","assetId","purchasePrice","quantity","name"]; this
mirrors the allowlist enforced in the repo_items.go switch (the validation logic
around lines 494–514) so clients get the same constraints as the server.
In `@docs/public/api/openapi-3.0.yaml`:
- Around line 481-490: The two query parameters orderBy and orderByDirection
should be constrained with enums: update the OpenAPI schema for the parameters
named orderBy and orderByDirection to include an enum of allowed values (for
orderBy list the valid sortable fields your API supports, e.g.,
"createdAt","updatedAt","name","id" or the canonical field names used in your
backend) and for orderByDirection set enum: ["asc","desc"] (optionally add a
default like "desc"); ensure the parameter entries still have type: string and
include any descriptions/examples to reflect the allowed values.
In `@docs/public/api/swagger-2.0.json`:
- Around line 782-793: The Swagger parameter objects for orderBy and
orderByDirection are missing enum constraints; update the parameter with "name":
"orderBy" (the orderBy parameter) to include an "enum" array populated with the
exact whitelist of sortable fields used by the backend (e.g., createdAt,
updatedAt, name, id — match your backend whitelist), and update the parameter
with "name": "orderByDirection" to include an "enum" with the allowed directions
(e.g., "asc", "desc"); ensure both enum values exactly mirror the backend
validation/whitelist so the API spec and implementation stay in sync.
In `@docs/public/api/swagger-2.0.yaml`:
- Around line 2466-2473: The query params orderBy and orderByDirection currently
lack enums in the Swagger 2.0 spec; update their parameter schemas to include
explicit enum lists so clients can only send supported values. Locate the
parameter definitions for name: orderBy and name: orderByDirection and add an
enum array for each (e.g., orderBy -> allowed sortable fields used by your API;
orderByDirection -> ["asc","desc"] or your accepted direction tokens), keeping
their type as string and preserving description/in:query metadata. Ensure the
enums match the actual server-side accepted values so the API contract stays
strict.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 415fe138-bb29-4d3e-93b6-e018d7911cd1
⛔ Files ignored due to path filters (6)
backend/app/api/static/docs/docs.gois excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.yamlis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.yamlis excluded by!backend/app/api/static/docs/**backend/go.sumis excluded by!**/*.sum
📒 Files selected for processing (36)
backend/app/api/handlers/v1/v1_ctrl_items.gobackend/internal/data/repo/repo_items.godocs/public/api/openapi-3.0.jsondocs/public/api/openapi-3.0.yamldocs/public/api/swagger-2.0.jsondocs/public/api/swagger-2.0.yamlfrontend/lib/api/classes/items.tsfrontend/locales/ca.jsonfrontend/locales/cs-CZ.jsonfrontend/locales/da-DK.jsonfrontend/locales/de.jsonfrontend/locales/en.jsonfrontend/locales/es.jsonfrontend/locales/fi-FI.jsonfrontend/locales/fr.jsonfrontend/locales/hu.jsonfrontend/locales/id-ID.jsonfrontend/locales/it.jsonfrontend/locales/ja-JP.jsonfrontend/locales/nb-NO.jsonfrontend/locales/nl.jsonfrontend/locales/pl.jsonfrontend/locales/pt-BR.jsonfrontend/locales/pt-PT.jsonfrontend/locales/ro-RO.jsonfrontend/locales/ru.jsonfrontend/locales/sk-SK.jsonfrontend/locales/sl.jsonfrontend/locales/sv.jsonfrontend/locales/tr.jsonfrontend/locales/uk-UA.jsonfrontend/locales/zh-CN.jsonfrontend/locales/zh-HK.jsonfrontend/locales/zh-MO.jsonfrontend/locales/zh-TW.jsonfrontend/pages/items.vue
Comment on lines
73
to
75
| OrderBy: params.Get("orderBy"), | ||
| OrderByDirection: params.Get("orderByDirection"), | ||
| } |
Contributor
There was a problem hiding this comment.
Harden sorting query params with an allowlist before passing to repository.
orderByDirection is accepted as raw input and forwarded directly. Please normalize/validate orderBy and orderByDirection (asc/desc) at the handler boundary and reject invalid values with 400 instead of silently coercing them. This is a security hardening and behavior-consistency improvement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/api/handlers/v1/v1_ctrl_items.go` around lines 73 - 75, Validate
and normalize the incoming sorting params at the handler boundary: check
params.Get("orderBy") against a predefined allowlist of permitted column names
(e.g., the set of sortable fields used by the repository) and only accept
params.Get("orderByDirection") if it normalizes to "asc" or "desc"
(case-insensitive); if either is invalid, return an HTTP 400 error from the
handler instead of passing raw values into the query struct (the fields OrderBy
and OrderByDirection) that is sent to the repository. Ensure you normalize the
accepted direction to a consistent lowercase value before constructing the query
and reference the same allowlist used by the repository to avoid mismatches.
Comment on lines
+544
to
+546
| "order": { | ||
| "order_by": "Seřadit podle" | ||
| }, |
Contributor
There was a problem hiding this comment.
Please keep non-English locale edits out of this feature PR.
This direct edit should be reverted and handled by Weblate; this PR should only carry source-string changes in frontend/locales/en.json.
Based on learnings: "Submit PRs by only adding or updating translation strings in frontend/locales/en.json. Do not modify other locale files (e.g., ar-AA.json, bs-BA.json, ca.json, cs-CZ.json, de.json, es.json, fr.json, etc.) since those translations are maintained via Weblate."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/locales/cs-CZ.json` around lines 544 - 546, Revert the direct edit
to the non-English locale: remove the added "order": {"order_by": "Seřadit
podle"} entry from frontend/locales/cs-CZ.json so that only source strings are
changed in this PR; instead make your string change in frontend/locales/en.json
(update the corresponding "order.order_by" key there) and leave all other locale
files (including cs-CZ.json) untouched for Weblate to manage.
Comment on lines
+543
to
+545
| "order": { | ||
| "order_by": "Сортировка по" | ||
| }, |
Contributor
There was a problem hiding this comment.
Missing Russian strings for sort direction labels.
Please add items.order.direction, items.order.ascending, and items.order.descending to match the new sort-direction dropdown.
As per coding guidelines, "frontend/locales/**/*: i18n translations must be maintained in frontend/locales/ to support multiple languages in the UI".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/locales/ru.json` around lines 543 - 545, The RU locale is missing
translations for the new sort-direction dropdown keys; add entries for
"items.order.direction", "items.order.ascending", and "items.order.descending"
into frontend/locales/ru.json under the same "order" object (or the appropriate
"items" object) so the UI keys resolve; provide correct Russian strings for each
key (e.g., a short label for direction and localized words for
ascending/descending) and ensure JSON syntax and nesting match other locale
files.
Comment on lines
+536
to
+538
| "order": { | ||
| "order_by": "Radiť podľa" | ||
| }, |
Contributor
There was a problem hiding this comment.
items.order needs the direction/ascending/descending keys too.
Line 536 introduces order_by only. The updated items sort UI also requires items.order.direction, items.order.ascending, and items.order.descending for full Slovak localization.
As per coding guidelines, frontend/locales/**/*: i18n translations must be maintained in frontend/locales/ to support multiple languages in the UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/locales/sk-SK.json` around lines 536 - 538, Add the missing Slovak
translation keys under the existing "order" object: include "direction",
"ascending", and "descending" entries (alongside the existing "order_by") so the
UI's items sort controls have localized strings; update the items.order object
to contain keys "direction", "ascending", and "descending" with appropriate
Slovak values matching the project's translation style.
Comment on lines
+292
to
+294
| "order": { | ||
| "order_by": "Razvrsti po" | ||
| }, |
Contributor
There was a problem hiding this comment.
Slovenian locale needs the full items.order key set.
order_by was added, but items.order.direction, items.order.ascending, and items.order.descending are also required by the updated filters UI.
Based on learnings, "Applies to frontend/locales/**/* : i18n translations must be maintained in frontend/locales/ to support multiple languages in the UI".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/locales/sl.json` around lines 292 - 294, The Slovenian locale only
defines "order.order_by" and is missing required keys for the updated filters
UI; add the missing translations for "items.order.direction",
"items.order.ascending", and "items.order.descending" (and ensure they live
under the same "items.order" object alongside "order_by") so the UI can read all
three keys; update the translations in the same file to include these keys with
appropriate Slovenian strings.
Comment on lines
+544
to
+546
| "order": { | ||
| "order_by": "排序方式" | ||
| }, |
Contributor
There was a problem hiding this comment.
Please complete items.order for the new direction selector.
Line 544 defines order_by but not direction, ascending, and descending. Add those keys so Simplified Chinese users see proper labels instead of i18n key paths.
As per coding guidelines, frontend/locales/**/*: i18n translations must be maintained in frontend/locales/ to support multiple languages in the UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/locales/zh-CN.json` around lines 544 - 546, The localization for the
ordering UI is incomplete: under the "order" object (currently containing
"order_by") add the missing keys "direction", "ascending", and "descending" with
appropriate Simplified Chinese translations (e.g., "direction": "方向",
"ascending": "升序", "descending": "降序") so the new direction selector shows
proper labels for users; update the "order" object in
frontend/locales/zh-CN.json alongside the existing "order_by" entry.
Comment on lines
+544
to
+546
| "order": { | ||
| "order_by": "排序" | ||
| }, |
Contributor
There was a problem hiding this comment.
items.order is incomplete for the new sort-direction UI.
Line 544 defines order_by only. Please also add direction, ascending, and descending under items.order so Traditional Chinese users get translated labels for the direction select.
As per coding guidelines, frontend/locales/**/*: i18n translations must be maintained in frontend/locales/ to support multiple languages in the UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/locales/zh-TW.json` around lines 544 - 546, The items.order
translation is incomplete: add the missing keys under "items.order"—add
"direction", "ascending", and "descending" alongside the existing "order_by" so
the new sort-direction UI has Traditional Chinese labels; update the zh-TW JSON
entry for items.order to include these three keys (e.g., items.order.direction,
items.order.ascending, items.order.descending) with appropriate Traditional
Chinese translations.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/app/api/handlers/v1/v1_ctrl_items.go (1)
73-75:⚠️ Potential issue | 🟠 MajorValidate and normalize
orderBy/orderByDirectionat the handler boundary.Line 73 and Line 74 still pass raw query values downstream. Please enforce an allowlist for
orderByand restrictorderByDirectiontoasc|desc(normalized), returning400on invalid input. This is an important security hardening and API-behavior consistency fix.🔐 Suggested hardening diff
func (ctrl *V1Controller) HandleItemsGetAll() errchain.HandlerFunc { - extractQuery := func(r *http.Request) repo.ItemQuery { + extractQuery := func(r *http.Request) (repo.ItemQuery, error) { params := r.URL.Query() @@ + orderBy := strings.TrimSpace(params.Get("orderBy")) + if orderBy != "" { + allowedOrderBy := map[string]struct{}{ + "name": {}, "createdAt": {}, "updatedAt": {}, + "purchasePrice": {}, "quantity": {}, "assetId": {}, + } + if _, ok := allowedOrderBy[orderBy]; !ok { + return repo.ItemQuery{}, validate.NewRequestError( + errors.New("invalid orderBy"), + http.StatusBadRequest, + ) + } + } + + orderByDirection := strings.ToLower(strings.TrimSpace(params.Get("orderByDirection"))) + if orderByDirection == "" { + orderByDirection = "asc" + } else if orderByDirection != "asc" && orderByDirection != "desc" { + return repo.ItemQuery{}, validate.NewRequestError( + errors.New("invalid orderByDirection"), + http.StatusBadRequest, + ) + } + v := repo.ItemQuery{ @@ - OrderBy: params.Get("orderBy"), - OrderByDirection: params.Get("orderByDirection"), + OrderBy: orderBy, + OrderByDirection: orderByDirection, } @@ - return v + return v, nil } return func(w http.ResponseWriter, r *http.Request) error { ctx := services.NewContext(r.Context()) + q, err := extractQuery(r) + if err != nil { + return err + } - items, err := ctrl.repo.Items.QueryByGroup(ctx, ctx.GID, extractQuery(r)) + items, err := ctrl.repo.Items.QueryByGroup(ctx, ctx.GID, q)As per coding guidelines, "Return errors directly from adapter functions or use
validate.NewRequestError()for HTTP error responses".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/handlers/v1/v1_ctrl_items.go` around lines 73 - 75, Validate and normalize the raw params.Get("orderBy") and params.Get("orderByDirection") values in the handler before building the downstream request: enforce an allowlist of permitted orderBy fields (check membership of orderBy against that slice) and normalize orderByDirection to lowercase allowing only "asc" or "desc"; if either check fails return a 400 using validate.NewRequestError(...) rather than passing the raw values through to OrderBy / OrderByDirection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/public/api/openapi-3.0.yaml`:
- Around line 463-472: The two query parameters onlyWithPhoto and
onlyWithoutPhoto are ambiguous—document their mutual exclusivity and how the
server validates them: update the parameter descriptions for onlyWithPhoto and
onlyWithoutPhoto to state they are mutually exclusive and that if both are
present the server will return a 400 Bad Request (or describe the precedence
rule if you prefer), and add/mention the corresponding validation behavior
(error response code and message) so clients know how to construct requests
reliably.
- Around line 478-487: Update the OpenAPI parameter definitions for the query
params named orderBy and orderByDirection to constrain allowed values and
declare defaults: for orderBy (string) add enum
["createdAt","updatedAt","assetId","purchasePrice","quantity","name"] with
default "name", and for orderByDirection (string) add enum ["asc","desc"] with
default "asc"; ensure these enums and defaults replace the current free-form
string schemas for the parameters named orderBy and orderByDirection so the spec
accurately documents allowed sorting fields and direction.
---
Duplicate comments:
In `@backend/app/api/handlers/v1/v1_ctrl_items.go`:
- Around line 73-75: Validate and normalize the raw params.Get("orderBy") and
params.Get("orderByDirection") values in the handler before building the
downstream request: enforce an allowlist of permitted orderBy fields (check
membership of orderBy against that slice) and normalize orderByDirection to
lowercase allowing only "asc" or "desc"; if either check fails return a 400
using validate.NewRequestError(...) rather than passing the raw values through
to OrderBy / OrderByDirection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a3bc8507-4dcd-482e-ae22-a6567c47a8ce
⛔ Files ignored due to path filters (5)
backend/app/api/static/docs/docs.gois excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.yamlis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.yamlis excluded by!backend/app/api/static/docs/**
📒 Files selected for processing (5)
backend/app/api/handlers/v1/v1_ctrl_items.godocs/public/api/openapi-3.0.jsondocs/public/api/openapi-3.0.yamldocs/public/api/swagger-2.0.jsondocs/public/api/swagger-2.0.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/public/api/swagger-2.0.yaml
- docs/public/api/openapi-3.0.json
Comment on lines
+478
to
+487
| - description: Field to order the results by | ||
| name: orderBy | ||
| in: query | ||
| schema: | ||
| type: string | ||
| - description: Direction to order the results by | ||
| name: orderByDirection | ||
| in: query | ||
| schema: | ||
| type: string |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate relevant backend files:"
fd -t f 'v1_ctrl_items.go|repo_items.go'
echo
echo "Inspect request parsing for order params:"
rg -n -C4 'orderByDirection|orderBy|OrderBy|OrderByDirection' backend/app/api/handlers/v1/v1_ctrl_items.go
echo
echo "Inspect repository whitelist/mapping for ORDER BY fields and direction:"
rg -n -C6 'OrderByDirection|orderBy|switch|case|asc|desc|Order\(' backend/internal/data/repo/repo_items.goRepository: sysadminsmedia/homebox
Length of output: 8809
Constrain sorting params with explicit enums and document defaults in the OpenAPI spec.
The orderBy and orderByDirection parameters are currently defined as free-form strings without enum constraints. Add enums to document the accepted values and specify defaults to enforce the contract at the API level:
orderBy: accepted values are"createdAt","updatedAt","assetId","purchasePrice","quantity","name"(default:"name")orderByDirection: accepted values are"asc","desc"(default:"asc")
While backend validation via switch statements mitigates injection risk, the spec should precisely reflect supported values to prevent invalid client requests and reduce the surface for misuse.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/public/api/openapi-3.0.yaml` around lines 478 - 487, Update the OpenAPI
parameter definitions for the query params named orderBy and orderByDirection to
constrain allowed values and declare defaults: for orderBy (string) add enum
["createdAt","updatedAt","assetId","purchasePrice","quantity","name"] with
default "name", and for orderByDirection (string) add enum ["asc","desc"] with
default "asc"; ensure these enums and defaults replace the current free-form
string schemas for the parameters named orderBy and orderByDirection so the spec
accurately documents allowed sorting fields and direction.
Author
|
Coderabbit is telling me to revert changes in non-english locale files. I would understand if it is a new term, but not updating the translations in this PR will break functionality. Should I acquiesce to the AI's request and then update them all by hand via Weblate, or can I leave these changes in the PR? |
Collaborator
|
I know that messing with the translations can cause issues, though not sure in this case, @tankerkiller125 |
Author
|
Converting to draft for now. I was reading #1083 and I think I might take a crack at it later this week. |
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
2 participants
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.
What type of PR is this?
What this PR does / why we need it:
This PR aims to improve the order by functionality on the search page by doing the following:
Which issue(s) this PR fixes:
None - I can create one if needed
Testing
Manually tested by interacting with the components on the search page
Summary by CodeRabbit
New Features
Localization