Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds cross-team search functionality by introducing a new "SearchPostsAllTeams" action across multiple controllers and updating corresponding mappings.
- Updated frequency and added a new search action in loadtest/control/simulcontroller/controller.go
- Implemented searchPostsAllTeams in both simulcontroller and action definitions and updated simplecontroller mapping
- Minor formatting cleanups in loadtest/control/actions.go
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| loadtest/control/simulcontroller/controller.go | Added a new "SearchPostsAllTeams" action with updated frequency settings |
| loadtest/control/simulcontroller/actions.go | Added the searchPostsAllTeams implementation; changed loop syntax for word generation |
| loadtest/control/simplecontroller/controller.go | Updated action mapping to include "SearchPostsAllTeams" |
| loadtest/control/actions.go | Added a new SearchPostsAllTeams function and removed extraneous whitespace in error checks |
Files not reviewed (1)
- config/simplecontroller.sample.json: Language not supported
|
What's the status of this PR, @JulienTant? Do you want me to take a look already? |
Go ahead! :) |
agarciamontoro
left a comment
There was a problem hiding this comment.
Thank you for working on this, @JulienTant, and sorry for the delay in the review! I left some comments below with a couple of things to discuss.
| func searchPostsAllTeams(u user.User) control.UserActionResponse { | ||
| var words []string | ||
| var opts control.PostsSearchOpts | ||
| // This is an arbitrary limit on the number of words to search for. | ||
| // TODO: possibly use user analytics data to improve this. | ||
| count := 1 + rand.Intn(4) | ||
|
|
||
| if rand.Float64() < 0.2 { | ||
| // We limit the search to 7 days. | ||
| t := time.Now().Add(-time.Duration(rand.Intn(7)) * time.Hour * 24) | ||
| switch rand.Intn(3) { | ||
| case 0: | ||
| opts.On = t | ||
| case 1: | ||
| opts.Before = t | ||
| case 2: | ||
| opts.After = t | ||
| } | ||
| } | ||
|
|
||
| if rand.Float64() < 0.2 { | ||
| opts.Excluded = []string{control.PickRandomWord()} | ||
| } | ||
|
|
||
| if rand.Float64() < 0.2 { | ||
| opts.IsPhrase = true | ||
| } | ||
|
|
||
| for range count { | ||
| words = append(words, control.PickRandomWord()) | ||
| } | ||
|
|
||
| term := control.GeneratePostsSearchTerm(words, opts) | ||
|
|
||
| list, err := u.SearchPosts("", term, false) | ||
| if err != nil { | ||
| return control.UserActionResponse{Err: control.NewUserError(err)} | ||
| } | ||
|
|
||
| return control.UserActionResponse{Info: fmt.Sprintf("found %d posts in all teams", len(list.Posts))} | ||
| } | ||
|
|
There was a problem hiding this comment.
Could we refactor the code in this function and in searchPosts out to an util function? Then searchPosts can call it with the ID of the current team and searchPostsAllTeams with an empty string.
There was a problem hiding this comment.
Yeah it's reasonable! on it! :)
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis pull request adds a new cross-team post search action called Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.3)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
agarciamontoro
left a comment
There was a problem hiding this comment.
This makes sense to me, @JulienTant, thank you!
Summary
todo
Ticket Link
Jira https://mattermost.atlassian.net/browse/MM-63512
Summary by CodeRabbit
New Features
Chores