From 1277cafdf59bf2a3f6f8404102d29c69e2d3884f Mon Sep 17 00:00:00 2001 From: Sandro Meireles <49874732+Sandro-Meireles@users.noreply.github.com> Date: Mon, 16 Dec 2024 10:36:21 -0300 Subject: [PATCH] Revert "Search for Zendesk user with identities and add retry" --- core/handlers/ticket_opened_test.go | 2 - services/tickets/zendesk/client.go | 35 ++++------- services/tickets/zendesk/client_test.go | 2 - services/tickets/zendesk/service.go | 84 +++++-------------------- 4 files changed, 27 insertions(+), 96 deletions(-) diff --git a/core/handlers/ticket_opened_test.go b/core/handlers/ticket_opened_test.go index 56f42b419..5fee45c30 100644 --- a/core/handlers/ticket_opened_test.go +++ b/core/handlers/ticket_opened_test.go @@ -33,8 +33,6 @@ func TestTicketOpened(t *testing.T) { }`), }, "https://nyaruka.zendesk.com/api/v2/users/search.json?query=b699a406-7e44-49be-9f01-1a82893e8a10": { - httpx.NewMockResponse(200, nil, `{"users": []}`), //now with retry the search repeats in these cases - httpx.NewMockResponse(200, nil, `{"users": []}`), httpx.NewMockResponse(200, nil, `{"users": []}`), }, "https://nyaruka.zendesk.com/api/v2/users/search.json?query=type:user details:\"b699a406-7e44-49be-9f01-1a82893e8a10\"": { diff --git a/services/tickets/zendesk/client.go b/services/tickets/zendesk/client.go index 0c01dac67..cb9b86afd 100644 --- a/services/tickets/zendesk/client.go +++ b/services/tickets/zendesk/client.go @@ -350,31 +350,18 @@ type SearchUserResponse struct { Users []User `json:"users"` } -// SearchUser returns the user or null if it does not exist, with retry logic for consistency delays. -func (c *RESTClient) SearchUser(query string) (*User, *httpx.Trace, error) { - endpoint := fmt.Sprintf("users/search.json?query=%s", query) - maxRetries := 3 - delay := 2 * time.Second - var ( - response SearchUserResponse - trace *httpx.Trace - err error - ) - - for i := 0; i < maxRetries; i++ { - trace, err = c.get(endpoint, nil, &response) - if err != nil { - return nil, trace, err - } - - if len(response.Users) > 0 { - return &response.Users[0], trace, nil - } - - time.Sleep(delay) +// SearchUser returns the user with the given external ID or nil if it doesn't exist +func (c *RESTClient) SearchUser(externalID string) (*User, *httpx.Trace, error) { + endpoint := fmt.Sprintf("users/search.json?query=%s", externalID) + var response SearchUserResponse + trace, err := c.get(endpoint, nil, &response) + if err != nil { + return nil, trace, err } - - return nil, trace, nil + if len(response.Users) == 0 { + return nil, trace, nil + } + return &response.Users[0], trace, nil } // MergeUser merge two users diff --git a/services/tickets/zendesk/client_test.go b/services/tickets/zendesk/client_test.go index 332728201..af51f0d86 100644 --- a/services/tickets/zendesk/client_test.go +++ b/services/tickets/zendesk/client_test.go @@ -316,8 +316,6 @@ func TestSearchUser(t *testing.T) { fmt.Sprintf("https://mocked_subdomain.zendesk.com/api/v2/users/search.json?query=%s", userUUID): { httpx.MockConnectionError, httpx.NewMockResponse(400, nil, `{"description": "Something went wrong", "error": "Unknown"}`), - httpx.NewMockResponse(200, nil, `{"users": []}`), //now with retry the search repeats in these cases - httpx.NewMockResponse(200, nil, `{"users": []}`), httpx.NewMockResponse(200, nil, `{"users": []}`), httpx.NewMockResponse(200, nil, `{"users": [{"id": 35436,"name": "Dummy User"}]}`), }, diff --git a/services/tickets/zendesk/service.go b/services/tickets/zendesk/service.go index de9a0e7c9..20f4c5e25 100644 --- a/services/tickets/zendesk/service.go +++ b/services/tickets/zendesk/service.go @@ -80,27 +80,23 @@ func NewService(rtCfg *runtime.Config, httpClient *http.Client, httpRetries *htt // Open opens a ticket which for mailgun means just sending an initial email func (s *service) Open(session flows.Session, topic *flows.Topic, body string, assignee *flows.User, logHTTP flows.HTTPLogCallback) (*flows.Ticket, error) { ticket := flows.OpenTicket(s.ticketer, topic, body, assignee) - - contact := session.Contact() - if contact == nil { - return nil, fmt.Errorf("session contact is nil") - } - contactDisplay := session.Contact().Format(session.Environment()) contactUUID := string(session.Contact().UUID()) - var trace *httpx.Trace var phoneNumber string - preferredURN := contact.PreferredURN() - if preferredURN != nil && preferredURN.URN().Scheme() == "whatsapp" { - phoneNumber = string(preferredURN.URN().Path()) + urn := session.Contact().PreferredURN().URN() + if urn.Scheme() == "whatsapp" { + phoneNumber = string(session.Contact().PreferredURN().URN().Path()) } - user, err := s.searchUserWithIdentities(contactUUID, phoneNumber, logHTTP) - if err != nil && user == nil { + user, trace, err := s.restClient.SearchUser(contactUUID) + if trace != nil { + logHTTP(flows.NewHTTPLog(trace, flows.HTTPStatusFromCode, s.redactor)) + } + if err != nil && trace.Response.StatusCode != http.StatusNotFound { return nil, err } - if user == nil { + if trace.Response.StatusCode == http.StatusNotFound || user == nil { newUser := &User{ Name: contactDisplay, ExternalID: contactUUID, @@ -201,18 +197,16 @@ func (s *service) Open(session flows.Session, topic *flows.Topic, body string, a if trace != nil { logHTTP(flows.NewHTTPLog(trace, flows.HTTPStatusFromCode, s.redactor)) } - if err != nil { + if err != nil && trace.Response.StatusCode != http.StatusNotFound { return nil, err } - if unmergedUser != nil { - _, trace, err = s.restClient.MergeUser(user.ID, unmergedUser.ID) - if trace != nil { - logHTTP(flows.NewHTTPLog(trace, flows.HTTPStatusFromCode, s.redactor)) - } - if err != nil { - return nil, err - } + _, trace, err = s.restClient.MergeUser(user.ID, unmergedUser.ID) + if trace != nil { + logHTTP(flows.NewHTTPLog(trace, flows.HTTPStatusFromCode, s.redactor)) + } + if err != nil && trace.Response.StatusCode != http.StatusNotFound { + return nil, err } return ticket, nil @@ -408,49 +402,3 @@ func (s *service) convertAttachments(attachments []utils.Attachment) ([]string, } return fileURLs, nil } - -func (s *service) searchUserWithIdentities(contactUUID string, phoneNumber string, logHTTP flows.HTTPLogCallback) (*User, error) { - fetchUser := func(identifier string) (*User, error) { - if identifier == "" { - return nil, nil - } - user, trace, err := s.restClient.SearchUser(identifier) - if trace != nil { - logHTTP(flows.NewHTTPLog(trace, flows.HTTPStatusFromCode, s.redactor)) - } - if err != nil && trace.Response.StatusCode != http.StatusNotFound { - return nil, err - } - return user, nil - } - - userContactUUID, err := fetchUser(contactUUID) - if err != nil { - return nil, err - } - - userPhoneNumber, err := fetchUser(phoneNumber) - if err != nil { - return nil, err - } - - switch { - case userContactUUID == nil && userPhoneNumber == nil: - return nil, nil - case userContactUUID != nil && userPhoneNumber == nil: - return userContactUUID, nil - case userPhoneNumber != nil && userContactUUID == nil: - return userPhoneNumber, nil - case userContactUUID.ID == userPhoneNumber.ID: - return userContactUUID, nil - default: - mergedUser, trace, err := s.restClient.MergeUser(userContactUUID.ID, userPhoneNumber.ID) - if trace != nil { - logHTTP(flows.NewHTTPLog(trace, flows.HTTPStatusFromCode, s.redactor)) - } - if err != nil && trace.Response.StatusCode != http.StatusNotFound { - return nil, err - } - return mergedUser, nil - } -}