Skip to content

Commit 68c8418

Browse files
AchoArnoldCopilot
andcommitted
fix: use correct query params for message index endpoint in integration tests
The searchMessages helper was using 'owners' (plural) param and missing 'contact', but the GET /v1/messages endpoint requires 'owner' (singular) and 'contact' fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 10e12a3 commit 68c8418

9 files changed

Lines changed: 91 additions & 51 deletions

File tree

api/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ require (
3333
github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible
3434
github.com/jszwec/csvutil v1.10.0
3535
github.com/lib/pq v1.12.3
36+
github.com/matoous/go-nanoid/v2 v2.1.0
3637
github.com/nyaruka/phonenumbers v1.7.2
3738
github.com/palantir/stacktrace v0.0.0-20161112013806-78658fd2d177
3839
github.com/patrickmn/go-cache v2.1.0+incompatible

api/go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
237237
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
238238
github.com/lib/pq v1.12.3 h1:tTWxr2YLKwIvK90ZXEw8GP7UFHtcbTtty8zsI+YjrfQ=
239239
github.com/lib/pq v1.12.3/go.mod h1:/p+8NSbOcwzAEI7wiMXFlgydTwcgTr3OSKMsD2BitpA=
240+
github.com/matoous/go-nanoid/v2 v2.1.0 h1:P64+dmq21hhWdtvZfEAofnvJULaRR1Yib0+PnU669bE=
241+
github.com/matoous/go-nanoid/v2 v2.1.0/go.mod h1:KlbGNQ+FhrUNIHUxZdL63t7tl4LaPkZNpUULS8H4uVM=
240242
github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE=
241243
github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8=
242244
github.com/mattn/go-isatty v0.0.22 h1:j8l17JJ9i6VGPUFUYoTUKPSgKe/83EYU2zBC7YNKMw4=

api/pkg/entities/bulk_message.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import "time"
44

55
// BulkMessage represents a summary of a bulk message batch
66
type BulkMessage struct {
7-
RequestID string `json:"request_id" example:"bulk-32343a19-da5e-4b1b-a767-3298a73703cb"`
7+
RequestID string `json:"request_id" example:"bulk-csv-a1B2c3D4e5"`
88
Total int64 `json:"total" example:"150"`
99
ScheduledCount int64 `json:"scheduled_count" example:"50"`
1010
PendingCount int64 `json:"pending_count" example:"30"`

api/pkg/handlers/bulk_message_handler.go

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
package handlers
22

33
import (
4+
"crypto/rand"
45
"fmt"
56
"sync"
67
"sync/atomic"
78

89
"github.com/NdoleStudio/httpsms/pkg/requests"
9-
"github.com/google/uuid"
10-
1110
"github.com/NdoleStudio/httpsms/pkg/services"
1211
"github.com/NdoleStudio/httpsms/pkg/telemetry"
1312
"github.com/NdoleStudio/httpsms/pkg/validators"
1413
"github.com/davecgh/go-spew/spew"
1514
"github.com/gofiber/fiber/v2"
15+
gonanoid "github.com/matoous/go-nanoid/v2"
1616
"github.com/palantir/stacktrace"
1717
)
1818

@@ -99,7 +99,7 @@ func (h *BulkMessageHandler) Store(c *fiber.Ctx) error {
9999
return h.responseBadRequest(c, err)
100100
}
101101

102-
messages, validationErrors := h.validator.ValidateStore(ctx, h.userIDFomContext(c), file)
102+
messages, fileType, validationErrors := h.validator.ValidateStore(ctx, h.userIDFomContext(c), file)
103103
if len(validationErrors) != 0 {
104104
msg := fmt.Sprintf("validation errors [%s], while sending bulk sms from CSV file [%s] for [%s]", spew.Sdump(validationErrors), file.Filename, h.userIDFomContext(c))
105105
ctxLogger.Warn(stacktrace.NewError(msg))
@@ -111,7 +111,7 @@ func (h *BulkMessageHandler) Store(c *fiber.Ctx) error {
111111
return h.responsePaymentRequired(c, *msg)
112112
}
113113

114-
requestID := uuid.New()
114+
requestID := h.generateRequestID(fileType, "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz")
115115
wg := sync.WaitGroup{}
116116
count := atomic.Int64{}
117117

@@ -144,3 +144,22 @@ func (h *BulkMessageHandler) Store(c *fiber.Ctx) error {
144144
wg.Wait()
145145
return h.responseAccepted(c, fmt.Sprintf("Added %d out of %d messages to the queue", count.Load(), len(messages)))
146146
}
147+
148+
func (h *BulkMessageHandler) generateRequestID(fileType string, alphabet string) string {
149+
id, err := gonanoid.Generate(alphabet, 10)
150+
if err != nil {
151+
id = h.randomAlphaNum(10, alphabet)
152+
}
153+
return fmt.Sprintf("bulk-%s-%s", fileType, id)
154+
}
155+
156+
func (h *BulkMessageHandler) randomAlphaNum(length int, alphabet string) string {
157+
b := make([]byte, length)
158+
if _, err := rand.Read(b); err != nil {
159+
return alphabet[:length]
160+
}
161+
for i := range b {
162+
b[i] = alphabet[int(b[i])%len(alphabet)]
163+
}
164+
return string(b)
165+
}

api/pkg/requests/bulk_message_request.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,28 @@
11
package requests
22

33
import (
4-
"fmt"
54
"strings"
65
"time"
76

87
"github.com/NdoleStudio/httpsms/pkg/entities"
98
"github.com/NdoleStudio/httpsms/pkg/services"
10-
"github.com/google/uuid"
119
"github.com/nyaruka/phonenumbers"
1210
)
1311

1412
// BulkMessage represents a single message in a bulk SMS request
1513
type BulkMessage struct {
1614
request
15+
FileType string `json:"type"`
1716
FromPhoneNumber string `csv:"FromPhoneNumber"`
1817
ToPhoneNumber string `csv:"ToPhoneNumber"`
1918
Content string `csv:"Content"`
20-
SendTimeRaw string `csv:"SendTime(optional)"`
19+
SendTime string `csv:"SendTime(optional)"`
2120
AttachmentURLs string `csv:"AttachmentURLs(optional)" validate:"optional"` // Comma separated list of URLs
2221
}
2322

2423
// GetSendTime parses the raw SendTime string into a *time.Time
2524
func (input *BulkMessage) GetSendTime() *time.Time {
26-
raw := strings.TrimSpace(input.SendTimeRaw)
25+
raw := strings.TrimSpace(input.SendTime)
2726
if raw == "" {
2827
return nil
2928
}
@@ -61,13 +60,13 @@ func (input *BulkMessage) Sanitize() *BulkMessage {
6160
}
6261

6362
// ToMessageSendParams converts BulkMessage to services.MessageSendParams
64-
func (input *BulkMessage) ToMessageSendParams(userID entities.UserID, requestID uuid.UUID, source string, index int) services.MessageSendParams {
63+
func (input *BulkMessage) ToMessageSendParams(userID entities.UserID, requestID string, source string, index int) services.MessageSendParams {
6564
from, _ := phonenumbers.Parse(input.FromPhoneNumber, phonenumbers.UNKNOWN_REGION)
6665

6766
return services.MessageSendParams{
6867
Source: source,
6968
Owner: from,
70-
RequestID: input.sanitizeStringPointer(fmt.Sprintf("bulk-%s", requestID.String())),
69+
RequestID: input.sanitizeStringPointer(requestID),
7170
UserID: userID,
7271
SendAt: input.GetSendTime(),
7372
RequestReceivedAt: time.Now().UTC(),

api/pkg/validators/bulk_message_handler_validator.go

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func NewBulkMessageHandlerValidator(
5252
}
5353

5454
// ValidateStore validates the requests.BillingUsageHistory request
55-
func (v *BulkMessageHandlerValidator) ValidateStore(ctx context.Context, userID entities.UserID, header *multipart.FileHeader) ([]*requests.BulkMessage, url.Values) {
55+
func (v *BulkMessageHandlerValidator) ValidateStore(ctx context.Context, userID entities.UserID, header *multipart.FileHeader) ([]*requests.BulkMessage, string, url.Values) {
5656
ctx, span, ctxLogger := v.tracer.StartWithLogger(ctx, v.logger)
5757
defer span.End()
5858

@@ -61,22 +61,22 @@ func (v *BulkMessageHandlerValidator) ValidateStore(ctx context.Context, userID
6161
result := url.Values{}
6262
result.Add("document", "Cannot load your account. Please try again later or contact support.")
6363
ctxLogger.Error(v.tracer.WrapErrorSpan(span, stacktrace.Propagate(err, fmt.Sprintf("cannot load user [%s]", userID))))
64-
return nil, result
64+
return nil, "", result
6565
}
6666

67-
messages, result := v.parseFile(ctxLogger, user, header)
67+
messages, fileType, result := v.parseFile(ctxLogger, user, header)
6868
if len(result) != 0 {
69-
return messages, result
69+
return messages, fileType, result
7070
}
7171

7272
if len(messages) == 0 {
7373
result.Add("document", "The uploaded file doesn't contain any valid records. Make sure you are using the official httpSMS template.")
74-
return messages, result
74+
return messages, fileType, result
7575
}
7676

7777
if len(messages) > 1000 {
7878
result.Add("document", "The uploaded file must contain less than 1000 records.")
79-
return messages, result
79+
return messages, fileType, result
8080
}
8181

8282
for index, message := range messages {
@@ -85,30 +85,32 @@ func (v *BulkMessageHandlerValidator) ValidateStore(ctx context.Context, userID
8585

8686
result = v.validateMessages(ctx, messages)
8787
if len(result) != 0 {
88-
return messages, result
88+
return messages, fileType, result
8989
}
9090

9191
result = v.validateOwners(ctx, userID, messages)
9292
if len(result) != 0 {
93-
return messages, result
93+
return messages, fileType, result
9494
}
9595

96-
return messages, result
96+
return messages, fileType, result
9797
}
9898

99-
func (v *BulkMessageHandlerValidator) parseFile(ctxLogger telemetry.Logger, user *entities.User, header *multipart.FileHeader) ([]*requests.BulkMessage, url.Values) {
99+
func (v *BulkMessageHandlerValidator) parseFile(ctxLogger telemetry.Logger, user *entities.User, header *multipart.FileHeader) ([]*requests.BulkMessage, string, url.Values) {
100100
if header.Header.Get("Content-Type") == "text/csv" || strings.HasSuffix(header.Filename, ".csv") {
101-
return v.parseCSV(ctxLogger, user, header)
101+
messages, result := v.parseCSV(ctxLogger, user, header)
102+
return messages, "csv", result
102103
}
103104
if header.Header.Get("Content-Type") == "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" || strings.HasSuffix(header.Filename, ".xlsx") {
104-
return v.parseXlsx(ctxLogger, user, header)
105+
messages, result := v.parseXlsx(ctxLogger, user, header)
106+
return messages, "xls", result
105107
}
106108

107109
ctxLogger.Error(stacktrace.NewError(fmt.Sprintf("cannot parse file [%s] for user [%s] with content type [%s]", header.Filename, user.ID, header.Header.Get("Content-Type"))))
108110

109111
result := url.Values{}
110112
result.Add("document", fmt.Sprintf("The file [%s] is not a valid CSV or Excel file.", header.Filename))
111-
return nil, result
113+
return nil, "", result
112114
}
113115

114116
func (v *BulkMessageHandlerValidator) parseXlsx(ctxLogger telemetry.Logger, user *entities.User, header *multipart.FileHeader) ([]*requests.BulkMessage, url.Values) {
@@ -158,7 +160,7 @@ func (v *BulkMessageHandlerValidator) parseXlsx(ctxLogger telemetry.Logger, user
158160
FromPhoneNumber: strings.TrimSpace(row[0]),
159161
ToPhoneNumber: strings.TrimSpace(row[1]),
160162
Content: row[2],
161-
SendTimeRaw: sendTimeRaw,
163+
SendTime: sendTimeRaw,
162164
AttachmentURLs: attachmentURLs,
163165
})
164166
}
@@ -266,10 +268,10 @@ func (v *BulkMessageHandlerValidator) validateMessages(_ context.Context, messag
266268
result.Add("document", fmt.Sprintf("Row [%d]: The message content must be less than 1024 characters.", index+2))
267269
}
268270

269-
if strings.TrimSpace(message.SendTimeRaw) != "" {
271+
if strings.TrimSpace(message.SendTime) != "" {
270272
sendTime := message.GetSendTime()
271273
if sendTime == nil {
272-
result.Add("document", fmt.Sprintf("Row [%d]: The SendTime [%s] is not a valid date format. Use RFC3339 (e.g. 2023-11-11T02:10:01Z) or YYYY-MM-DDTHH:MM:SS.", index+2, message.SendTimeRaw))
274+
result.Add("document", fmt.Sprintf("Row [%d]: The SendTime [%s] is not a valid date format. Use RFC3339 (e.g. 2023-11-11T02:10:01Z) or YYYY-MM-DDTHH:MM:SS.", index+2, message.SendTime))
273275
} else if sendTime.After(time.Now().Add(420 * time.Hour)) {
274276
result.Add("document", fmt.Sprintf("Row [%d]: The SendTime [%s] cannot be more than 20 days (420 hours) in the future.", index+2, sendTime.Format(time.RFC3339)))
275277
}

tests/helpers_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,10 +381,10 @@ func fetchBulkMessages(ctx context.Context, t *testing.T) []BulkMessageEntry {
381381
return result.Data
382382
}
383383

384-
func searchMessages(ctx context.Context, t *testing.T, query string, owner string) []httpsms.Message {
384+
func searchMessages(ctx context.Context, t *testing.T, contact string, owner string) []httpsms.Message {
385385
t.Helper()
386386

387-
url := fmt.Sprintf("%s/v1/messages?query=%s&owners=%s&limit=10&skip=0", apiBaseURL, query, owner)
387+
url := fmt.Sprintf("%s/v1/messages?contact=%s&owner=%s&limit=20&skip=0", apiBaseURL, contact, owner)
388388
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
389389
require.NoError(t, err)
390390
req.Header.Set("x-api-key", userAPIKey)

tests/integration_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -409,8 +409,9 @@ func TestBulkSMS_CSV(t *testing.T) {
409409
phone := setupPhone(ctx, t, 60)
410410

411411
// Build CSV content with 1 message
412+
contact := randomPhoneNumber()
412413
csvContent := fmt.Sprintf("FromPhoneNumber,ToPhoneNumber,Content,SendTime(optional)\n%s,%s,CSV bulk test message,\n",
413-
phone.PhoneNumber, randomPhoneNumber())
414+
phone.PhoneNumber, contact)
414415

415416
// Upload CSV
416417
statusCode, respBody := uploadBulkFile(ctx, t, "test.csv", []byte(csvContent))
@@ -428,7 +429,7 @@ func TestBulkSMS_CSV(t *testing.T) {
428429
time.Sleep(2 * time.Second)
429430

430431
// Search for the bulk message by owner to get message IDs
431-
messages := searchMessages(ctx, t, "", phone.PhoneNumber)
432+
messages := searchMessages(ctx, t, contact, phone.PhoneNumber)
432433
require.GreaterOrEqual(t, len(messages), 1, "expected at least 1 message for phone %s", phone.PhoneNumber)
433434

434435
// Find the message with bulk- request_id prefix
@@ -510,8 +511,10 @@ func TestBulkSMS_Excel(t *testing.T) {
510511
// Wait for messages to be persisted
511512
time.Sleep(2 * time.Second)
512513

513-
// Search for bulk messages by owner
514-
messages := searchMessages(ctx, t, "", phone.PhoneNumber)
514+
// Search for bulk messages by owner and each contact
515+
messages1 := searchMessages(ctx, t, contact1, phone.PhoneNumber)
516+
messages2 := searchMessages(ctx, t, contact2, phone.PhoneNumber)
517+
messages := append(messages1, messages2...)
515518
require.GreaterOrEqual(t, len(messages), 2, "expected at least 2 messages for phone %s", phone.PhoneNumber)
516519

517520
// Find messages with bulk- request_id prefix

web/pages/bulk-messages/index.vue

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -101,19 +101,19 @@
101101
<h4 class="text-h4 mb-3">Bulk Message History</h4>
102102
<p class="text--secondary">
103103
Your 10 most recent bulk SMS uploads are shown below, including a
104-
delivery status breakdown for each batch. Click
105-
<code>View</code> to see individual messages on the search page
104+
delivery status breakdown for each batch. Click on a row to see
105+
individual messages on the search page.
106106
</p>
107107
<v-progress-linear
108108
v-if="loadingHistory"
109109
indeterminate
110110
class="mb-4"
111111
></v-progress-linear>
112-
<v-simple-table>
112+
<v-simple-table v-else>
113113
<template #default>
114114
<thead>
115115
<tr class="text-uppercase subtitle-2">
116-
<th class="text-left">Name</th>
116+
<th class="text-left">ID</th>
117117
<th class="text-center">Created At</th>
118118
<th class="text-center">Total</th>
119119
<th class="text-center">Pending</th>
@@ -122,13 +122,19 @@
122122
<th class="text-center">Delivered</th>
123123
<th class="text-center">Failed</th>
124124
<th class="text-center">Expired</th>
125-
<th class="text-center">Action</th>
126125
</tr>
127126
</thead>
128127
<tbody>
129-
<tr v-for="order in bulkOrders" :key="order.request_id">
130-
<td class="text-left font-weight-medium">
131-
{{ order.request_id }}
128+
<tr
129+
v-for="order in bulkOrders"
130+
:key="order.request_id"
131+
class="clickable-row"
132+
@click="
133+
$router.push(`/search-messages?query=${order.request_id}`)
134+
"
135+
>
136+
<td class="text-left">
137+
{{ cleanName(order.request_id) }}
132138
</td>
133139
<td class="text-center">
134140
{{ order.created_at | timestamp }}
@@ -140,16 +146,6 @@
140146
<td class="text-center">{{ order.delivered_count }}</td>
141147
<td class="text-center">{{ order.failed_count }}</td>
142148
<td class="text-center">{{ order.expired_count }}</td>
143-
<td class="text-center">
144-
<v-btn
145-
small
146-
color="primary"
147-
:to="`/search-messages?query=${order.request_id}`"
148-
>
149-
<v-icon small left>{{ mdiEye }}</v-icon>
150-
View
151-
</v-btn>
152-
</td>
153149
</tr>
154150
</tbody>
155151
</template>
@@ -224,6 +220,15 @@ export default Vue.extend({
224220
this.fetchBulkOrders()
225221
},
226222
methods: {
223+
cleanName(requestId: string): string {
224+
if (requestId.startsWith('bulk-csv-')) {
225+
return requestId.replace(/^bulk-csv-/, '') + '.csv'
226+
}
227+
if (requestId.startsWith('bulk-xls-')) {
228+
return requestId.replace(/^bulk-xls-/, '') + '.xlsx'
229+
}
230+
return requestId.replace(/^bulk-/, '')
231+
},
227232
fetchBulkOrders() {
228233
this.loadingHistory = true
229234
this.$store
@@ -263,3 +268,12 @@ export default Vue.extend({
263268
},
264269
})
265270
</script>
271+
272+
<style scoped>
273+
.clickable-row {
274+
cursor: pointer;
275+
}
276+
.clickable-row:hover {
277+
background-color: rgba(0, 0, 0, 0.04);
278+
}
279+
</style>

0 commit comments

Comments
 (0)