From b43741e1c2bccbf706898cf021a80a6e4d74886a Mon Sep 17 00:00:00 2001 From: mk-5 Date: Tue, 19 Dec 2023 21:30:53 +0100 Subject: [PATCH] fix(#95): fetch all available users for assign user view --- internal/issues/search_issues.go | 18 ++---- internal/issues/search_issues_test.go | 9 ++- internal/jira/jira.go | 1 + internal/jira/jira_user.go | 12 +++- internal/jira/jira_user_test.go | 49 ++++++++++++++- internal/users/fetch.go | 28 +++++++++ internal/users/fuzzy.go | 26 ++++++++ internal/users/fuzzy_test.go | 89 +++++++++++++++++++++++++++ internal/users/user_assign.go | 15 +---- internal/users/user_assign_test.go | 1 + 10 files changed, 214 insertions(+), 34 deletions(-) create mode 100644 internal/users/fetch.go create mode 100644 internal/users/fuzzy.go create mode 100644 internal/users/fuzzy_test.go diff --git a/internal/issues/search_issues.go b/internal/issues/search_issues.go index 5b06336..e05c934 100644 --- a/internal/issues/search_issues.go +++ b/internal/issues/search_issues.go @@ -229,15 +229,13 @@ func (view *searchIssuesView) runSelectStatus() { func (view *searchIssuesView) runSelectUser() { app.GetApp().ClearNow() app.GetApp().Loading(true) - us := view.fetchUsers(view.project.Key) - us = append(us, jira.User{DisplayName: ui.MessageAll}) - usersStrings := users.FormatJiraUsers(us) - view.fuzzyFind = app.NewFuzzyFind(ui.MessageSelectUser, usersStrings) + var us *[]jira.User + view.fuzzyFind, us = users.NewFuzzyFind(view.project.Key, view.api) app.GetApp().Loading(false) if user := <-view.fuzzyFind.Complete; true { app.GetApp().ClearNow() - if user.Index >= 0 && len(us) > 0 { - searchForUser = &us[user.Index] + if user.Index >= 0 && len(*us) > 0 { + searchForUser = &(*us)[user.Index] view.dirty = true } go view.runIssuesFuzzyFind() @@ -311,14 +309,6 @@ func (view *searchIssuesView) fetchStatuses(projectId string) []jira.IssueStatus return ss } -func (view *searchIssuesView) fetchUsers(projectKey string) []jira.User { - us, err := view.api.FindUsers(projectKey) - if err != nil { - app.Error(err.Error()) - } - return us -} - func (view *searchIssuesView) findLabels(query string) []string { app.GetApp().LoadingWithText(true, ui.MessageSearchLabelsLoading) labels, err := view.api.FindLabels(nil, query) diff --git a/internal/issues/search_issues_test.go b/internal/issues/search_issues_test.go index e44ed9b..f2730a6 100644 --- a/internal/issues/search_issues_test.go +++ b/internal/issues/search_issues_test.go @@ -295,12 +295,11 @@ func Test_fjiraSearchIssuesView_runSelectUser(t *testing.T) { view.runSelectUser() done <- struct{}{} }() - for { - if view.fuzzyFind != nil { - break - } - <-time.NewTimer(10 * time.Millisecond).C + for view.fuzzyFind == nil { + <-time.After(10 * time.Millisecond) } + view.fuzzyFind.SetDebounceDisabled(true) + view.fuzzyFind.Update() query := "John" for _, key := range query { view.fuzzyFind.HandleKeyEvent(tcell.NewEventKey(-1, key, tcell.ModNone)) diff --git a/internal/jira/jira.go b/internal/jira/jira.go index 5b8b613..869d467 100644 --- a/internal/jira/jira.go +++ b/internal/jira/jira.go @@ -12,6 +12,7 @@ type Api interface { SearchJql(query string) ([]Issue, error) SearchJqlPageable(query string, page int32, pageSize int32) ([]Issue, int32, int32, error) FindUsers(project string) ([]User, error) + FindUsersWithQuery(project string, query string) ([]User, error) FindProjects() ([]Project, error) FindLabels(issue *Issue, query string) ([]string, error) AddLabel(issueId string, label string) error diff --git a/internal/jira/jira_user.go b/internal/jira/jira_user.go index 69ecd52..99b9a44 100644 --- a/internal/jira/jira_user.go +++ b/internal/jira/jira_user.go @@ -26,15 +26,23 @@ const ( var UserSearchDeserializeErr = errors.New("Cannot deserialize jira user search response.") type findUserQueryParams struct { - Project string `url:"project"` - MaxResults int `url:"maxResults"` + Project string `url:"project"` + MaxResults int `url:"maxResults"` + Query *string `url:"query"` } func (api *httpApi) FindUsers(project string) ([]User, error) { + return api.FindUsersWithQuery(project, "") +} + +func (api *httpApi) FindUsersWithQuery(project string, query string) ([]User, error) { queryParams := &findUserQueryParams{ Project: project, MaxResults: 10000, } + if query != "" { + queryParams.Query = &query + } response, err := api.jiraRequest("GET", FindUser, queryParams, nil) if err != nil { return nil, err diff --git a/internal/jira/jira_user_test.go b/internal/jira/jira_user_test.go index e32d357..5fbd48a 100644 --- a/internal/jira/jira_user_test.go +++ b/internal/jira/jira_user_test.go @@ -1,6 +1,7 @@ package jira import ( + "github.com/stretchr/testify/assert" "net/http" "reflect" "testing" @@ -9,6 +10,7 @@ import ( func Test_httpJiraApi_FindUsers(t *testing.T) { type args struct { project string + query string } tests := []struct { name string @@ -24,6 +26,14 @@ func Test_httpJiraApi_FindUsers(t *testing.T) { }, false, }, + {"should find users with query without error", + args{project: "FJIR", query: "test"}, + []User{ + {AccountId: "456", EmailAddress: "test@test.pl", DisplayName: "Mateusz Kulawik", Active: true, TimeZone: "Europe/Warsaw", Locale: "en_GB", AvatarUrls: nil}, + {AccountId: "123", EmailAddress: "", DisplayName: "mateusz.test", Active: true, TimeZone: "Europe/Warsaw", Locale: "en_US", AvatarUrls: nil}, + }, + false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -53,7 +63,13 @@ func Test_httpJiraApi_FindUsers(t *testing.T) { ` w.Write([]byte(body)) //nolint:errcheck }) - got, err := api.FindUsers(tt.args.project) + var got []User + var err error + if tt.args.query == "" { + got, err = api.FindUsers(tt.args.project) + } else { + got, err = api.FindUsersWithQuery(tt.args.project, tt.args.query) + } if (err != nil) != tt.wantErr { t.Errorf("FindUsers() error = %v, wantErr %v", err, tt.wantErr) return @@ -64,3 +80,34 @@ func Test_httpJiraApi_FindUsers(t *testing.T) { }) } } + +func Test_httpJiraApi_FindUsers_returnError(t *testing.T) { + type args struct { + project string + query string + } + tests := []struct { + name string + args args + wantErr bool + }{ + {"should return error when search failed", + args{project: "FJIR"}, + true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // given + api := NewJiraApiMock(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(500) + }) + + // when + _, err := api.FindUsersWithQuery(tt.args.project, tt.args.query) + + // then + assert.Error(t, err) + }) + } +} diff --git a/internal/users/fetch.go b/internal/users/fetch.go new file mode 100644 index 0000000..2b7d64f --- /dev/null +++ b/internal/users/fetch.go @@ -0,0 +1,28 @@ +package users + +import ( + "github.com/mk-5/fjira/internal/app" + "github.com/mk-5/fjira/internal/jira" +) + +type RecordsProvider interface { + FetchUsers(projectKey string, query string) []jira.User +} + +type apiRecordsProvider struct { + api jira.Api +} + +func NewApiRecordsProvider(api jira.Api) RecordsProvider { + return &apiRecordsProvider{ + api: api, + } +} + +func (r *apiRecordsProvider) FetchUsers(projectKey string, query string) []jira.User { + us, err := r.api.FindUsersWithQuery(projectKey, query) + if err != nil { + app.Error(err.Error()) + } + return us +} diff --git a/internal/users/fuzzy.go b/internal/users/fuzzy.go new file mode 100644 index 0000000..3ee9653 --- /dev/null +++ b/internal/users/fuzzy.go @@ -0,0 +1,26 @@ +package users + +import ( + "github.com/mk-5/fjira/internal/app" + "github.com/mk-5/fjira/internal/jira" + "github.com/mk-5/fjira/internal/ui" +) + +const ( + typeaheadSearchThreshold = 100 +) + +func NewFuzzyFind(projectKey string, api jira.Api) (*app.FuzzyFind, *[]jira.User) { + var us []jira.User + provider := NewApiRecordsProvider(api) + return app.NewFuzzyFindWithProvider(ui.MessageSelectUser, func(query string) []string { + // it searches up to {typeaheadThreshold} records using typeahead - then it do regular fuzzy-find + if len(us) > 0 && len(us) < typeaheadSearchThreshold { + return FormatJiraUsers(us) + } + us = provider.FetchUsers(projectKey, query) + us = append(us, jira.User{DisplayName: ui.MessageAll}) + usersStrings := FormatJiraUsers(us) + return usersStrings + }), &us +} diff --git a/internal/users/fuzzy_test.go b/internal/users/fuzzy_test.go new file mode 100644 index 0000000..28f5d3d --- /dev/null +++ b/internal/users/fuzzy_test.go @@ -0,0 +1,89 @@ +package users + +import ( + "github.com/gdamore/tcell/v2" + "github.com/mk-5/fjira/internal/app" + "github.com/mk-5/fjira/internal/jira" + "github.com/stretchr/testify/assert" + "net/http" + "strings" + "testing" +) + +func TestNewFuzzyFind(t *testing.T) { + screen := tcell.NewSimulationScreen("utf-8") + _ = screen.Init() //nolint:errcheck + defer screen.Fini() + app.InitTestApp(screen) + + tests := []struct { + name string + }{ + {"should use api find up to typeaheadThreshold, then fuzzy-find"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // given + apiCall := false + sb2User := strings.Builder{} + sb2User.WriteString(`[{"id": "U1", "displayName": "Bob"}, {"id": "U2", "displayName": "John"}]`) + sb1000Users := strings.Builder{} + sb1000Users.WriteString("[") + for i := 0; i < 1000; i++ { + sb1000Users.WriteString(`{"id": "U1", "displayName": "Bob"}`) + if i != 999 { + sb1000Users.WriteString(",") + } + } + sb1000Users.WriteString("]") + var apiResult *strings.Builder + api := jira.NewJiraApiMock(func(w http.ResponseWriter, r *http.Request) { + apiCall = true + w.WriteHeader(200) + _, _ = w.Write([]byte(apiResult.String())) + }) + fuzzyFind, us := NewFuzzyFind("ABC", api) + fuzzyFind.SetDebounceDisabled(true) + + // when + apiCall = false + apiResult = &sb1000Users + fuzzyFind.SetQuery("") + fuzzyFind.Update() + + // then + assert.True(t, apiCall) + assert.Equal(t, 1001, len(*us)) + + // when + apiCall = false + apiResult = &sb1000Users + fuzzyFind.SetQuery("b") + fuzzyFind.Update() + + // then + assert.True(t, apiCall) + assert.Equal(t, 1001, len(*us)) + + // when + apiCall = false + apiResult = &sb2User + fuzzyFind.SetQuery("bo") + fuzzyFind.Update() + + // then + assert.True(t, apiCall) + assert.Equal(t, 3, len(*us)) + + // when + apiCall = false + apiResult = &sb2User + fuzzyFind.SetQuery("bo") + fuzzyFind.Update() + + // then + assert.False(t, apiCall, "api shouldn't be called because previous call returned 2 records") + assert.Equal(t, 3, len(*us)) + }) + } +} diff --git a/internal/users/user_assign.go b/internal/users/user_assign.go index ffe10f7..1e99bea 100644 --- a/internal/users/user_assign.go +++ b/internal/users/user_assign.go @@ -68,9 +68,8 @@ func (view *userAssignChangeView) HandleKeyEvent(ev *tcell.EventKey) { func (view *userAssignChangeView) startUsersSearching() { app.GetApp().ClearNow() app.GetApp().Loading(true) - users := view.findUser(view.issue.Fields.Project.Key) - usersStrings := FormatJiraUsers(users) - view.fuzzyFind = app.NewFuzzyFind(ui.MessageUsersFuzzyFind, usersStrings) + var us *[]jira.User + view.fuzzyFind, us = NewFuzzyFind(view.issue.Fields.Project.Key, view.api) view.fuzzyFind.MarginBottom = 0 app.GetApp().Loading(false) if user := <-view.fuzzyFind.Complete; true { @@ -82,18 +81,10 @@ func (view *userAssignChangeView) startUsersSearching() { return } view.fuzzyFind = nil - view.assignUserToTicket(view.issue, &users[user.Index]) + view.assignUserToTicket(view.issue, &(*us)[user.Index]) } } -func (view *userAssignChangeView) findUser(project string) []jira.User { - users, err := view.api.FindUsers(project) - if err != nil { - app.Error(err.Error()) - } - return users -} - func (view *userAssignChangeView) assignUserToTicket(issue *jira.Issue, user *jira.User) { if user == nil { view.goBackFn() diff --git a/internal/users/user_assign_test.go b/internal/users/user_assign_test.go index 3917d33..eafbfd9 100644 --- a/internal/users/user_assign_test.go +++ b/internal/users/user_assign_test.go @@ -118,6 +118,7 @@ func Test_fjiraAssignChangeView_assignUserToTicket(t *testing.T) { for view.fuzzyFind == nil { <-time.After(10 * time.Millisecond) } + view.fuzzyFind.SetDebounceDisabled(true) view.fuzzyFind.HandleKeyEvent(tcell.NewEventKey(-1, 'B', tcell.ModNone)) view.fuzzyFind.HandleKeyEvent(tcell.NewEventKey(tcell.KeyEnter, -1, tcell.ModNone)) // wait for confirmation