Skip to content

Commit 568e106

Browse files
AchoArnoldAchoArnold
committed
fix(contacts): address review regressions
Stabilize asynchronous integration flows. Keep contact pagination, resolution, and generated API contracts deterministic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 58beb14a-b8b6-4d9c-a27d-5e70118cef95
1 parent a752fa2 commit 568e106

12 files changed

Lines changed: 156 additions & 23 deletions

File tree

‎api/docs/docs.go‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ const docTemplate = `{
364364
"201": {
365365
"description": "Created",
366366
"schema": {
367-
"$ref": "#/definitions/responses.ContactsResponse"
367+
"$ref": "#/definitions/responses.ContactsCreatedResponse"
368368
}
369369
},
370370
"400": {
@@ -431,7 +431,7 @@ const docTemplate = `{
431431
"201": {
432432
"description": "Created",
433433
"schema": {
434-
"$ref": "#/definitions/responses.ContactsResponse"
434+
"$ref": "#/definitions/responses.ContactsCreatedResponse"
435435
}
436436
},
437437
"400": {
@@ -5280,6 +5280,30 @@ const docTemplate = `{
52805280
}
52815281
}
52825282
},
5283+
"responses.ContactsCreatedResponse": {
5284+
"type": "object",
5285+
"required": [
5286+
"data",
5287+
"message",
5288+
"status"
5289+
],
5290+
"properties": {
5291+
"data": {
5292+
"type": "array",
5293+
"items": {
5294+
"$ref": "#/definitions/entities.Contact"
5295+
}
5296+
},
5297+
"message": {
5298+
"type": "string",
5299+
"example": "Request handled successfully"
5300+
},
5301+
"status": {
5302+
"type": "string",
5303+
"example": "success"
5304+
}
5305+
}
5306+
},
52835307
"responses.ContactsResponse": {
52845308
"type": "object",
52855309
"required": [

‎api/docs/swagger.json‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@
361361
"201": {
362362
"description": "Created",
363363
"schema": {
364-
"$ref": "#/definitions/responses.ContactsResponse"
364+
"$ref": "#/definitions/responses.ContactsCreatedResponse"
365365
}
366366
},
367367
"400": {
@@ -428,7 +428,7 @@
428428
"201": {
429429
"description": "Created",
430430
"schema": {
431-
"$ref": "#/definitions/responses.ContactsResponse"
431+
"$ref": "#/definitions/responses.ContactsCreatedResponse"
432432
}
433433
},
434434
"400": {
@@ -5277,6 +5277,30 @@
52775277
}
52785278
}
52795279
},
5280+
"responses.ContactsCreatedResponse": {
5281+
"type": "object",
5282+
"required": [
5283+
"data",
5284+
"message",
5285+
"status"
5286+
],
5287+
"properties": {
5288+
"data": {
5289+
"type": "array",
5290+
"items": {
5291+
"$ref": "#/definitions/entities.Contact"
5292+
}
5293+
},
5294+
"message": {
5295+
"type": "string",
5296+
"example": "Request handled successfully"
5297+
},
5298+
"status": {
5299+
"type": "string",
5300+
"example": "success"
5301+
}
5302+
}
5303+
},
52805304
"responses.ContactsResponse": {
52815305
"type": "object",
52825306
"required": [

‎api/docs/swagger.yaml‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,6 +1223,23 @@ definitions:
12231223
- message
12241224
- status
12251225
type: object
1226+
responses.ContactsCreatedResponse:
1227+
properties:
1228+
data:
1229+
items:
1230+
$ref: '#/definitions/entities.Contact'
1231+
type: array
1232+
message:
1233+
example: Request handled successfully
1234+
type: string
1235+
status:
1236+
example: success
1237+
type: string
1238+
required:
1239+
- data
1240+
- message
1241+
- status
1242+
type: object
12261243
responses.ContactsResponse:
12271244
properties:
12281245
data:
@@ -1965,7 +1982,7 @@ paths:
19651982
"201":
19661983
description: Created
19671984
schema:
1968-
$ref: '#/definitions/responses.ContactsResponse'
1985+
$ref: '#/definitions/responses.ContactsCreatedResponse'
19691986
"400":
19701987
description: Bad Request
19711988
schema:
@@ -2100,7 +2117,7 @@ paths:
21002117
"201":
21012118
description: Created
21022119
schema:
2103-
$ref: '#/definitions/responses.ContactsResponse'
2120+
$ref: '#/definitions/responses.ContactsCreatedResponse'
21042121
"400":
21052122
description: Bad Request
21062123
schema:

‎api/pkg/handlers/contact_handler.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func (h *ContactHandler) Index(c fiber.Ctx) error {
111111
// @Accept json
112112
// @Produce json
113113
// @Param payload body requests.ContactStoreRequest true "Contact(s) to create"
114-
// @Success 201 {object} responses.ContactsResponse
114+
// @Success 201 {object} responses.ContactsCreatedResponse
115115
// @Failure 400 {object} responses.BadRequest
116116
// @Failure 401 {object} responses.Unauthorized
117117
// @Failure 402 {object} responses.PaymentRequired
@@ -161,7 +161,7 @@ func (h *ContactHandler) Store(c fiber.Ctx) error {
161161
// @Accept multipart/form-data
162162
// @Produce json
163163
// @Param document formData file true "CSV file of contacts"
164-
// @Success 201 {object} responses.ContactsResponse
164+
// @Success 201 {object} responses.ContactsCreatedResponse
165165
// @Failure 400 {object} responses.BadRequest
166166
// @Failure 401 {object} responses.Unauthorized
167167
// @Failure 402 {object} responses.PaymentRequired

‎api/pkg/repositories/gorm_contact_repository.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func (repository *gormContactRepository) Index(ctx context.Context, userID entit
116116

117117
func (repository *gormContactRepository) contactOrder(params IndexParams) string {
118118
if params.SortBy == "" {
119-
return "updated_at DESC"
119+
return "updated_at DESC, id DESC"
120120
}
121121

122122
sortBy := "updated_at"
@@ -129,7 +129,7 @@ func (repository *gormContactRepository) contactOrder(params IndexParams) string
129129
direction = "DESC"
130130
}
131131

132-
return fmt.Sprintf("%s %s", sortBy, direction)
132+
return fmt.Sprintf("%s %s, id %s", sortBy, direction, direction)
133133
}
134134

135135
func (repository *gormContactRepository) Count(ctx context.Context, userID entities.UserID, params IndexParams) (int64, error) {
@@ -152,7 +152,7 @@ func (repository *gormContactRepository) FetchByPhoneNumbers(ctx context.Context
152152
if err := repository.db.WithContext(ctx).
153153
Where("user_id = ?", userID).
154154
Where("phone_numbers && ?", pq.Array(phoneNumbers)).
155-
Order("updated_at ASC").
155+
Order("updated_at ASC, id ASC").
156156
Find(contacts).Error; err != nil {
157157
return nil, repository.tracer.WrapErrorSpan(span, stacktrace.Propagatef(err, "cannot fetch contacts for user [%s] by phone numbers [%v]", userID, phoneNumbers))
158158
}

‎api/pkg/repositories/gorm_contact_repository_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ func TestGormContactRepository_Index_FiltersByUserAndQueryAcrossContactFields(t
270270
statement := lastContactStatement(t, recorder)
271271
assert.True(t, strings.HasPrefix(statement.query, `SELECT * FROM "contacts"`))
272272
assert.Contains(t, statement.query, `WHERE user_id = $1 AND (name ILIKE $2 OR array_to_string(emails, ',') ILIKE $3 OR array_to_string(phone_numbers, ',') ILIKE $4)`)
273-
assert.Contains(t, statement.query, `ORDER BY updated_at DESC LIMIT $5 OFFSET $6`)
273+
assert.Contains(t, statement.query, `ORDER BY updated_at DESC, id DESC LIMIT $5 OFFSET $6`)
274274
require.Len(t, statement.args, 6)
275275
assert.Equal(t, entities.UserID("user-1"), statement.args[0])
276276
assert.Equal(t, "%alice%", statement.args[1])
@@ -289,22 +289,22 @@ func TestGormContactRepository_Index_OrdersByRequestedFieldAndDirection(t *testi
289289
{
290290
name: "name ascending",
291291
params: IndexParams{SortBy: "name"},
292-
expectedOrderBy: "ORDER BY name ASC",
292+
expectedOrderBy: "ORDER BY name ASC, id ASC",
293293
},
294294
{
295295
name: "name descending",
296296
params: IndexParams{SortBy: "name", SortDescending: true},
297-
expectedOrderBy: "ORDER BY name DESC",
297+
expectedOrderBy: "ORDER BY name DESC, id DESC",
298298
},
299299
{
300300
name: "updated descending",
301301
params: IndexParams{SortBy: "updated_at", SortDescending: true},
302-
expectedOrderBy: "ORDER BY updated_at DESC",
302+
expectedOrderBy: "ORDER BY updated_at DESC, id DESC",
303303
},
304304
{
305305
name: "default",
306306
params: IndexParams{},
307-
expectedOrderBy: "ORDER BY updated_at DESC",
307+
expectedOrderBy: "ORDER BY updated_at DESC, id DESC",
308308
},
309309
}
310310

@@ -332,7 +332,7 @@ func TestGormContactRepository_FetchByPhoneNumbers_ScopesByUserAndRequestedNumbe
332332

333333
require.NoError(t, err)
334334
statement := lastContactStatement(t, recorder)
335-
assert.Equal(t, `SELECT * FROM "contacts" WHERE user_id = $1 AND phone_numbers && $2 ORDER BY updated_at ASC`, statement.query)
335+
assert.Equal(t, `SELECT * FROM "contacts" WHERE user_id = $1 AND phone_numbers && $2 ORDER BY updated_at ASC, id ASC`, statement.query)
336336
require.Len(t, statement.args, 2)
337337
assert.Equal(t, entities.UserID("user-1"), statement.args[0])
338338
assert.Equal(t, &pq.StringArray{"+18005550199", "+18005550100"}, statement.args[1])

‎api/pkg/responses/contact_responses.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ type ContactResponse struct {
88
Data entities.Contact `json:"data"`
99
}
1010

11+
// ContactsCreatedResponse is the payload returned after creating contacts.
12+
type ContactsCreatedResponse struct {
13+
response
14+
Data []entities.Contact `json:"data"`
15+
}
16+
1117
// ContactsResponse is the payload containing []entities.Contact.
1218
type ContactsResponse struct {
1319
response

‎api/pkg/services/contact_service.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ import (
1414
"github.com/google/uuid"
1515
)
1616

17-
// contactCacheTTL bounds staleness if an invalidation is ever missed.
17+
// contactMapCacheTTL bounds cross-instance staleness because invalidation is process-local.
1818
const (
19-
contactMapCacheTTL = 24 * time.Hour
19+
contactMapCacheTTL = 5 * time.Minute
2020
contactGenerationCleanupInterval = time.Hour
2121
)
2222

‎tests/helpers_test.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,64 @@ func setupPhone(ctx context.Context, t *testing.T, messagesPerMinute uint) testP
110110
require.NoError(t, err)
111111
require.Equal(t, http.StatusOK, resp.HTTPResponse.StatusCode, "fcm token bind failed")
112112

113+
waitForPhoneAuthorization(ctx, t, phoneAPIKeyValue, phoneNumber, 20*time.Second)
114+
113115
return testPhone{
114116
PhoneNumber: phoneNumber,
115117
PhoneAPIKey: phoneAPIKeyValue,
116118
FcmToken: fcmToken,
117119
}
118120
}
119121

122+
func waitForPhoneAuthorization(
123+
ctx context.Context,
124+
t *testing.T,
125+
phoneAPIKey string,
126+
phoneNumber string,
127+
timeout time.Duration,
128+
) {
129+
t.Helper()
130+
131+
body, err := json.Marshal(map[string]interface{}{
132+
"phone_numbers": []string{phoneNumber},
133+
"charging": true,
134+
})
135+
require.NoError(t, err)
136+
137+
var responseBody []byte
138+
var statusCode int
139+
deadline := time.Now().Add(timeout)
140+
for time.Now().Before(deadline) {
141+
request, requestErr := http.NewRequestWithContext(
142+
ctx,
143+
http.MethodPost,
144+
apiBaseURL+"/v1/heartbeats",
145+
bytes.NewReader(body),
146+
)
147+
require.NoError(t, requestErr)
148+
request.Header.Set("Content-Type", "application/json")
149+
request.Header.Set("x-api-key", phoneAPIKey)
150+
151+
response, requestErr := http.DefaultClient.Do(request)
152+
require.NoError(t, requestErr)
153+
responseBody, requestErr = io.ReadAll(response.Body)
154+
response.Body.Close()
155+
require.NoError(t, requestErr)
156+
157+
statusCode = response.StatusCode
158+
if statusCode == http.StatusCreated {
159+
return
160+
}
161+
if statusCode != http.StatusUnauthorized {
162+
require.Equal(t, http.StatusCreated, statusCode, "phone authorization check failed: %s", string(responseBody))
163+
}
164+
165+
time.Sleep(500 * time.Millisecond)
166+
}
167+
168+
require.Equal(t, http.StatusCreated, statusCode, "phone authorization was not ready within %v: %s", timeout, string(responseBody))
169+
}
170+
120171
func setupWebhook(ctx context.Context, t *testing.T, phoneNumber string, events []string) (signingKey string, webhookPath string) {
121172
t.Helper()
122173

‎tests/integration_test.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func TestSendSMS_Encrypted(t *testing.T) {
5656
assert.NotEqual(t, plaintext, outstanding["content"])
5757

5858
fireEvent(ctx, t, phone.PhoneAPIKey, messageID, "SENT")
59-
time.Sleep(200 * time.Millisecond)
59+
pollMessageStatus(ctx, t, messageID, "sent", 15*time.Second)
6060
fireEvent(ctx, t, phone.PhoneAPIKey, messageID, "DELIVERED")
6161

6262
msg := pollMessageStatus(ctx, t, messageID, "delivered", 30*time.Second)
@@ -200,8 +200,10 @@ func TestSendSMS_RateLimit(t *testing.T) {
200200
assert.GreaterOrEqual(t, gapMs, int64(5500), "rate limit gap should be >= 5500ms (6s minus timing tolerance), got %dms", gapMs)
201201

202202
fireEvent(ctx, t, phone.PhoneAPIKey, msgID1, "SENT")
203-
fireEvent(ctx, t, phone.PhoneAPIKey, msgID1, "DELIVERED")
203+
pollMessageStatus(ctx, t, msgID1, "sent", 15*time.Second)
204204
fireEvent(ctx, t, phone.PhoneAPIKey, msgID2, "SENT")
205+
pollMessageStatus(ctx, t, msgID2, "sent", 15*time.Second)
206+
fireEvent(ctx, t, phone.PhoneAPIKey, msgID1, "DELIVERED")
205207
fireEvent(ctx, t, phone.PhoneAPIKey, msgID2, "DELIVERED")
206208

207209
msg1 := pollMessageStatus(ctx, t, msgID1, "delivered", 15*time.Second)
@@ -335,7 +337,7 @@ func TestSendSMS_OutstandingFlow(t *testing.T) {
335337
assert.Equal(t, contactNumber, outstanding["contact"])
336338

337339
fireEvent(ctx, t, phone.PhoneAPIKey, messageID, "SENT")
338-
time.Sleep(200 * time.Millisecond)
340+
pollMessageStatus(ctx, t, messageID, "sent", 15*time.Second)
339341
fireEvent(ctx, t, phone.PhoneAPIKey, messageID, "DELIVERED")
340342

341343
msg := pollMessageStatus(ctx, t, messageID, "delivered", 30*time.Second)
@@ -538,7 +540,7 @@ func TestBulkSMS_Excel(t *testing.T) {
538540

539541
// Fire SENT then DELIVERED on message 1, leave message 2 pending
540542
fireEvent(ctx, t, phone.PhoneAPIKey, msgID1, "SENT")
541-
time.Sleep(200 * time.Millisecond)
543+
pollMessageStatus(ctx, t, msgID1, "sent", 15*time.Second)
542544
fireEvent(ctx, t, phone.PhoneAPIKey, msgID1, "DELIVERED")
543545

544546
// Poll until message 1 reaches "delivered"

0 commit comments

Comments
 (0)