Added lastUpdatedAt, Updated Condition in DSR, written tests, changed… - #806
Added lastUpdatedAt, Updated Condition in DSR, written tests, changed…#806Rishav-Bagri wants to merge 1 commit into
Conversation
… the variable to precedence
📝 WalkthroughWalkthroughThe search rule models now support precedence, nested query/time/filter conditions, filter value maps, and ChangesDynamic search rules
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integration/search_rules_test.go`:
- Line 158: The search-rules integration assertions incorrectly use assert.Len
on pointer-based Conditions objects. In the create-path test near line 158,
assert rule.Conditions, rule.Conditions.Query, and rule.Conditions.Time
directly; in the update-path test near line 201, assert updatedRule.Conditions
and updatedRule.Conditions.Query directly, preserving the expected presence
checks.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e9da6aea-c7e1-4c30-8dab-5996397a9bc1
📒 Files selected for processing (2)
integration/search_rules_test.gomodels_search.go
| assert.Equal(t, 10, rule.Priority) | ||
| assert.Equal(t, 10, rule.Precedence) | ||
| assert.True(t, rule.Active) | ||
| assert.Len(t, rule.Conditions, 2) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does github.com/stretchr/testify/assert Len support a pointer to a Go struct in version v1.11.1?
💡 Result:
No, the assert.Len function in version v1.11.1 of the github.com/stretchr/testify/assert package does not natively support pointers to Go structs [1][2]. The implementation of assert.Len relies on a helper function named getLen, which uses the reflect package to determine the length of the provided object [1]: // getLen tries to get the length of an object. // It returns (0, false) if impossible. func getLen(x interface{}) (length int, ok bool) { v:= reflect.ValueOf(x) defer func { ok = recover == nil } return v.Len, true } In Go, the reflect.Value.Len method supports types such as Array, Chan, Map, Slice, and String. It does not support pointers to structs. If you pass a pointer to a struct to assert.Len, the function will determine that it is impossible to get the length and return a failure message indicating that the object could not be applied to the builtin len function [1].
Citations:
- 1: https://github.com/stretchr/testify/blob/v1.11.1/assert/assertions.go
- 2: https://github.com/stretchr/testify/blob/v1.11.1/assert/assertion_forward.go
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)search_rules_test\.go$|go\.mod$|assertions\.go$' || true
echo "== search_rules_test relevant lines =="
if [ -f integration/search_rules_test.go ]; then
nl -ba integration/search_rules_test.go | sed -n '130,215p'
fi
echo "== module testify version =="
if [ -f go.mod ]; then
rg 'github.com/stretchr/testify' go.mod go.sum | sed -n '1,20p'
fi
echo "== Condition declarations =="
rg -n "type Condition|Conditions" -S . --glob '!vendor/**' --glob '!**/testdata/**' | sed -n '1,120p'Repository: meilisearch/meilisearch-go
Length of output: 295
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository root =="
pwd
echo "== relevant files =="
git ls-files | grep -E '(^|/)search_rules_test\.go$|^go\.mod$|^go\.sum$' || true
echo "== search_rules_test.go relevant lines =="
sed -n '130,215p' integration/search_rules_test.go
echo "== module testify version =="
grep -n 'github.com/stretchr/testify' go.mod go.sum 2>/dev/null || true
echo "== Condition declarations/usages =="
grep -RInE 'type Condition|Conditions' . --exclude-dir=vendor --exclude-dir=testdata --exclude-dir=.git | head -120 || trueRepository: meilisearch/meilisearch-go
Length of output: 4276
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== models_search.go outline/contents =="
sed -n '1,90p' models_search.go
echo "== SearchRulesRequest/create/update API handling relevant symbols =="
grep -RInE 'SearchRulesRequest|update search rule|search rule|SearchRule|search_rules\|conditions' --exclude-dir=vendor --exclude-dir=testdata --exclude-dir=.git . | head -200Repository: meilisearch/meilisearch-go
Length of output: 29642
Replace collection-length checks with nested-condition assertions.
Rules.Conditions is *meilisearch.Condition, so assert.Len checks object length rather than condition count. Assert rule.Conditions, .Query, and .Time for the create path, and updatedRule.Conditions and .Query for the update path instead.
📍 Affects 1 file
integration/search_rules_test.go#L158-L158(this comment)integration/search_rules_test.go#L201-L201
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integration/search_rules_test.go` at line 158, The search-rules integration
assertions incorrectly use assert.Len on pointer-based Conditions objects. In
the create-path test near line 158, assert rule.Conditions,
rule.Conditions.Query, and rule.Conditions.Time directly; in the update-path
test near line 201, assert updatedRule.Conditions and
updatedRule.Conditions.Query directly, preserving the expected presence checks.
1 participant
… the variable to precedence
Pull Request
Related issue
Fixes #805
What does this PR do?
a. Condition is now a pointer rather than a slice
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
LastUpdatedAtinformation to search rule responses.