Bug #1220 - Adding alpha ordering to /items/fields endpoint#1394
Bug #1220 - Adding alpha ordering to /items/fields endpoint#1394bmyoungquist wants to merge 1 commit intosysadminsmedia:mainfrom
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Security Note: No security concerns identified. This change only adds deterministic ordering to field name retrieval with no impact on authorization, data exposure, or input validation. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/internal/data/repo/repo_items.go (1)
1190-1198:⚠️ Potential issue | 🟠 MajorField names are sorted, but field values are still unsorted
Line 1196 fixes deterministic ordering for names, but Issue
#1220also asks for the Values dropdown to be alphanumeric.GetAllCustomFieldValuesstill usesUnique(true)+Select(...)withoutOrder(...), so value order remains nondeterministic.Suggested follow-up patch
func (e *ItemsRepository) GetAllCustomFieldValues(ctx context.Context, gid uuid.UUID, name string) ([]string, error) { type st struct { Value string `json:"text_value"` } var values []st err := e.db.Item.Query(). Where( item.HasGroupWith(group.ID(gid)), ). QueryFields(). Where( itemfield.Name(name), ). Unique(true). + Order(ent.Asc(itemfield.FieldTextValue)). Select(itemfield.FieldTextValue). Scan(ctx, &values)Security recommendation: keep the
group.ID(gid)tenant predicate in both names and values queries (as currently implemented) to avoid cross-tenant metadata leakage.🤖 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 1190 - 1198, GetAllCustomFieldValues currently returns deduplicated values without deterministic ordering; update the query that builds values to include an Order clause (e.g., Order(ent.Asc(itemfield.FieldValue))) in the chain that uses Unique(true) + Select(...) so the Values dropdown is alphanumeric and still scoped by the tenant predicate (item.HasGroupWith(group.ID(gid))). Locate the query in GetAllCustomFieldValues that calls e.db.Item.Query()...QueryFields()...Select(itemfield.FieldValue) and append the appropriate Order(...) using itemfield.FieldValue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/internal/data/repo/repo_items.go`:
- Around line 1190-1198: GetAllCustomFieldValues currently returns deduplicated
values without deterministic ordering; update the query that builds values to
include an Order clause (e.g., Order(ent.Asc(itemfield.FieldValue))) in the
chain that uses Unique(true) + Select(...) so the Values dropdown is
alphanumeric and still scoped by the tenant predicate
(item.HasGroupWith(group.ID(gid))). Locate the query in GetAllCustomFieldValues
that calls e.db.Item.Query()...QueryFields()...Select(itemfield.FieldValue) and
append the appropriate Order(...) using itemfield.FieldValue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b7eb8a7a-4426-4678-bc71-e613634c8bc0
📒 Files selected for processing (1)
backend/internal/data/repo/repo_items.go
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #1220
Testing
Manually tested by interacting with the custom field filter on the search page
Summary by CodeRabbit