Revert "Search for Zendesk user with identities and add retry"

weni-ai · Dec 16, 2024 · 1277caf · 1277caf
1 parent 60fd599
commit 1277caf
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 96 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
-	}
-}