Merge pull request #27 from Ilhasoft/release-v5.5.24

Release v5.5.24

CHANGELOG.md

@@ -1,3 +1,8 @@
+v5.5.24
+----------
+ * Tickets fixes and improvements
+ * Update to goflow v0.87.0
+
 v5.5.17
 ----------
  * Send email when reopening mailgun ticket

hooks/ticket_opened.go

@@ -6,6 +6,7 @@ import (
 	"github.com/nyaruka/goflow/flows"
 	"github.com/nyaruka/goflow/flows/events"
 	"github.com/nyaruka/mailroom/models"
+	"github.com/nyaruka/mailroom/services/tickets"
 
 	"github.com/gomodule/redigo/redis"
 	"github.com/jmoiron/sqlx"
@@ -61,7 +62,7 @@ func handleTicketOpened(ctx context.Context, tx *sqlx.Tx, rp *redis.Pool, org *m
 		event.Ticket.Body,
 		map[string]interface{}{
 			"contact-uuid":    scene.Contact().UUID(),
-			"contact-display": scene.Contact().Format(org.Env()),
+			"contact-display": tickets.GetContactDisplay(org.Env(), scene.Contact()),
 		},
 	)
 

models/assets.go

@@ -283,6 +283,7 @@ func NewOrgAssets(ctx context.Context, db *sqlx.DB, orgID OrgID, prev *OrgAssets
 		}
 	} else {
 		o.ticketers = prev.ticketers
+		o.ticketersByID = prev.ticketersByID
 		o.ticketersByUUID = prev.ticketersByUUID
 	}
 

models/http_logs.go

@@ -147,5 +147,8 @@ func (h *HTTPLogger) Ticketer(t *Ticketer) flows.HTTPLogCallback {
 
 // Insert this logger's logs into the database
 func (h *HTTPLogger) Insert(ctx context.Context, tx Queryer) error {
-	return InsertHTTPLogs(ctx, tx, h.logs)
+	if len(h.logs) > 0 {
+		return InsertHTTPLogs(ctx, tx, h.logs)
+	}
+	return nil
 }

models/http_logs_test.go

@@ -1,32 +1,77 @@
-package models
+package models_test
 
 import (
+	"net/http"
 	"testing"
 	"time"
 
+	"github.com/nyaruka/goflow/flows"
+	"github.com/nyaruka/goflow/utils/httpx"
+	"github.com/nyaruka/mailroom/models"
 	"github.com/nyaruka/mailroom/testsuite"
+
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestHTTPLogs(t *testing.T) {
 	ctx := testsuite.CTX()
 	db := testsuite.DB()
 
 	// insert a log
-	log := NewClassifierCalledLog(Org1, WitID, "http://foo.bar", "GET /", "STATUS 200", false, time.Second, time.Now())
-	err := InsertHTTPLogs(ctx, db, []*HTTPLog{log})
+	log := models.NewClassifierCalledLog(models.Org1, models.WitID, "http://foo.bar", "GET /", "STATUS 200", false, time.Second, time.Now())
+	err := models.InsertHTTPLogs(ctx, db, []*models.HTTPLog{log})
 	assert.Nil(t, err)
 
 	testsuite.AssertQueryCount(t, db,
 		`SELECT count(*) from request_logs_httplog WHERE org_id = $1 AND classifier_id = $2 AND is_error = FALSE`,
-		[]interface{}{Org1, WitID}, 1)
+		[]interface{}{models.Org1, models.WitID}, 1)
 
 	// insert a log with nil response
-	log = NewClassifierCalledLog(Org1, WitID, "http://foo.bar", "GET /", "", true, time.Second, time.Now())
-	err = InsertHTTPLogs(ctx, db, []*HTTPLog{log})
+	log = models.NewClassifierCalledLog(models.Org1, models.WitID, "http://foo.bar", "GET /", "", true, time.Second, time.Now())
+	err = models.InsertHTTPLogs(ctx, db, []*models.HTTPLog{log})
 	assert.Nil(t, err)
 
 	testsuite.AssertQueryCount(t, db,
 		`SELECT count(*) from request_logs_httplog WHERE org_id = $1 AND classifier_id = $2 AND is_error = TRUE AND response IS NULL`,
-		[]interface{}{Org1, WitID}, 1)
+		[]interface{}{models.Org1, models.WitID}, 1)
+}
+
+func TestHTTPLogger(t *testing.T) {
+	ctx := testsuite.CTX()
+	db := testsuite.DB()
+
+	defer httpx.SetRequestor(httpx.DefaultRequestor)
+	httpx.SetRequestor(httpx.NewMockRequestor(map[string][]httpx.MockResponse{
+		"https://temba.io": {
+			httpx.NewMockResponse(200, nil, `hello`),
+			httpx.NewMockResponse(400, nil, `world`),
+		},
+	}))
+
+	mailgun, err := models.LookupTicketerByUUID(ctx, db, models.MailgunUUID)
+	require.NoError(t, err)
+
+	logger := &models.HTTPLogger{}
+	log := logger.Ticketer(mailgun)
+
+	// make and log a few HTTP requests
+	req1, err := http.NewRequest("GET", "https://temba.io", nil)
+	require.NoError(t, err)
+	trace1, err := httpx.DoTrace(http.DefaultClient, req1, nil, nil, -1)
+	require.NoError(t, err)
+	log(flows.NewHTTPLog(trace1, flows.HTTPStatusFromCode, nil))
+
+	req2, err := http.NewRequest("GET", "https://temba.io", nil)
+	require.NoError(t, err)
+	trace2, err := httpx.DoTrace(http.DefaultClient, req2, nil, nil, -1)
+	require.NoError(t, err)
+	log(flows.NewHTTPLog(trace2, flows.HTTPStatusFromCode, nil))
+
+	err = logger.Insert(ctx, db)
+	assert.NoError(t, err)
+
+	testsuite.AssertQueryCount(t, db,
+		`SELECT count(*) from request_logs_httplog WHERE org_id = $1 AND ticketer_id = $2`,
+		[]interface{}{models.Org1, models.MailgunID}, 2)
 }

models/tickets.go

@@ -80,6 +80,7 @@ func (t *Ticket) TicketerID() TicketerID  { return t.t.TicketerID }
 func (t *Ticket) ExternalID() null.String { return t.t.ExternalID }
 func (t *Ticket) Status() TicketStatus    { return t.t.Status }
 func (t *Ticket) Subject() string         { return t.t.Subject }
+func (t *Ticket) Body() string            { return t.t.Body }
 func (t *Ticket) Config(key string) string {
 	return t.t.Config.GetString(key, "")
 }
@@ -177,7 +178,7 @@ func loadTickets(ctx context.Context, db Queryer, query string, params ...interf
 	return tickets, nil
 }
 
-const selectTicketSQL = `
+const selectTicketByUUIDSQL = `
 SELECT
   id,
   uuid,
@@ -200,9 +201,40 @@ WHERE
 
 // LookupTicketByUUID looks up the ticket with the passed in UUID
 func LookupTicketByUUID(ctx context.Context, db Queryer, uuid flows.TicketUUID) (*Ticket, error) {
-	rows, err := db.QueryxContext(ctx, selectTicketSQL, string(uuid))
+	return lookupTicket(ctx, db, selectTicketByUUIDSQL, uuid)
+}
+
+const selectTicketByExternalIDSQL = `
+SELECT
+  id,
+  uuid,
+  org_id,
+  contact_id,
+  ticketer_id,
+  external_id,
+  status,
+  subject,
+  body,
+  config,
+  opened_on,
+  modified_on,
+  closed_on
+FROM
+  tickets_ticket
+WHERE
+  ticketer_id = $1 AND
+  external_id = $2
+`
+
+// LookupTicketByExternalID looks up the ticket with the passed in ticketer and external ID
+func LookupTicketByExternalID(ctx context.Context, db Queryer, ticketerID TicketerID, externalID string) (*Ticket, error) {
+	return lookupTicket(ctx, db, selectTicketByExternalIDSQL, ticketerID, externalID)
+}
+
+func lookupTicket(ctx context.Context, db Queryer, query string, params ...interface{}) (*Ticket, error) {
+	rows, err := db.QueryxContext(ctx, query, params...)
 	if err != nil && err != sql.ErrNoRows {
-		return nil, errors.Wrapf(err, "error querying for ticket for uuid: %s", string(uuid))
+		return nil, err
 	}
 	defer rows.Close()
 
@@ -213,7 +245,7 @@ func LookupTicketByUUID(ctx context.Context, db Queryer, uuid flows.TicketUUID)
 	ticket := &Ticket{}
 	err = rows.StructScan(&ticket.t)
 	if err != nil {
-		return nil, errors.Wrapf(err, "error reading ticket for uuid: %s", string(uuid))
+		return nil, err
 	}
 
 	return ticket, nil
@@ -414,6 +446,33 @@ func (t *Ticketer) AsService(ticketer *flows.Ticketer) (TicketService, error) {
 	return nil, errors.Errorf("unrecognized ticket service type '%s'", t.Type())
 }
 
+const updateTicketerConfigSQL = `
+UPDATE 
+	tickets_ticketer
+SET
+	config = $2
+WHERE 
+	id = $1
+`
+
+// UpdateConfig updates the configuration of this ticketer with the given values
+func (t *Ticketer) UpdateConfig(ctx context.Context, db *sqlx.DB, add map[string]string, remove map[string]bool) error {
+	for key, value := range add {
+		t.t.Config[key] = value
+	}
+	for key := range remove {
+		delete(t.t.Config, key)
+	}
+
+	// convert to null.Map to save
+	dbMap := make(map[string]interface{}, len(t.t.Config))
+	for key, value := range t.t.Config {
+		dbMap[key] = value
+	}
+
+	return Exec(ctx, "update ticketer config", db, updateTicketerConfigSQL, t.t.ID, null.NewMap(dbMap))
+}
+
 // TicketService extends the engine's ticket service and adds support for forwarding new incoming messages
 type TicketService interface {
 	flows.TicketService
@@ -470,7 +529,7 @@ func LookupTicketerByUUID(ctx context.Context, db Queryer, uuid assets.TicketerU
 	return ticketer, nil
 }
 
-const selectTicketersSQL = `
+const selectOrgTicketersSQL = `
 SELECT ROW_TO_JSON(r) FROM (SELECT
 	t.id as id,
 	t.uuid as uuid,
@@ -492,7 +551,7 @@ ORDER BY
 func loadTicketers(ctx context.Context, db sqlx.Queryer, orgID OrgID) ([]assets.Ticketer, error) {
 	start := time.Now()
 
-	rows, err := db.Queryx(selectTicketersSQL, orgID)
+	rows, err := db.Queryx(selectOrgTicketersSQL, orgID)
 	if err != nil && err != sql.ErrNoRows {
 		return nil, errors.Wrapf(err, "error querying ticketers for org: %d", orgID)
 	}

models/tickets_test.go

@@ -57,7 +57,7 @@ func TestTickets(t *testing.T) {
 		models.Org2,
 		models.AlexandriaID,
 		models.ZendeskID,
-		"EX7869",
+		"EX6677",
 		"Other Org Ticket",
 		"Where are my pants?",
 		nil,
@@ -79,9 +79,14 @@ func TestTickets(t *testing.T) {
 	testsuite.AssertQueryCount(t, db, `SELECT count(*) FROM tickets_ticket WHERE status = 'O' AND closed_on IS NULL`, nil, 3)
 
 	// can lookup a ticket by UUID
-	tk, err := models.LookupTicketByUUID(ctx, db, "2ef57efc-d85f-4291-b330-e4afe68af5fe")
+	tk1, err := models.LookupTicketByUUID(ctx, db, "2ef57efc-d85f-4291-b330-e4afe68af5fe")
+	assert.NoError(t, err)
+	assert.Equal(t, "New Ticket", tk1.Subject())
+
+	// can lookup a ticket by external ID and ticketer
+	tk2, err := models.LookupTicketByExternalID(ctx, db, models.ZendeskID, "EX7869")
 	assert.NoError(t, err)
-	assert.Equal(t, "New Ticket", tk.Subject())
+	assert.Equal(t, "New Zen Ticket", tk2.Subject())
 
 	// can lookup open tickets by contact
 	org1, _ := models.GetOrgAssets(ctx, db, models.Org1)

services/tickets/mailgun/client.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"mime/multipart"
 	"net/http"
+	"sort"
 
 	"github.com/nyaruka/goflow/utils"
 	"github.com/nyaruka/goflow/utils/httpx"
@@ -50,8 +51,16 @@ func (c *Client) SendMessage(from, to, subject, text string, headers map[string]
 		w.WriteField("to", to)
 		w.WriteField("subject", subject)
 		w.WriteField("text", text)
-		for k, v := range headers {
-			w.WriteField("h:"+k, v)
+
+		// for the sake of tests, we want to output headers in consistent order
+		headerKeys := make([]string, 0, len(headers))
+		for k := range headers {
+			headerKeys = append(headerKeys, k)
+		}
+		sort.Strings(headerKeys)
+
+		for _, k := range headerKeys {
+			w.WriteField("h:"+k, headers[k])
 		}
 	}