Skip to content

Commit

Permalink
fix(callback): update status at GovsgOp table, use timestamp, enforce…
Browse files Browse the repository at this point in the history
… update order (#2085)

* fix: update status at GovsgOp table

* fix: throw error to trigger retry

* fix: GovsgOps table model

* fix: timestamp and enforce update order

* fix: stick to 500 error

* fix: only throw error for Sent status

* refactor(backend): extend govsgop from govsg-message directly

* fix(frontend): change otp input type to alphanumeric (#2086)

* feat(backend): duplicate govsg choice for recipient type (#2087)

* test(e2e): fix getting otp

* feat(frontend): enable inline attachment (#2088)

* fix(copywriting): update language

* refactor: move govsg.class.ts up one level

* fix(worker-logger): add missing timestamp consolidation

---------

Co-authored-by: Stanley Nguyen <hung.ngn.the@gmail.com>
Co-authored-by: Stanley Nguyen <stanleynguyen@users.noreply.github.com>
  • Loading branch information
3 people committed Jul 7, 2023
1 parent d556dfd commit 1a8601f
Show file tree
Hide file tree
Showing 17 changed files with 283 additions and 50 deletions.
48 changes: 48 additions & 0 deletions backend/src/core/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,54 @@ export const govsgMessageStatusMapper = (
}
}

// ensure status updates never overrides a less "updated" status
// there is still a non-zero chance of race condition happening, but ignore for now
export const shouldUpdateStatus = (
status: GovsgMessageStatus,
prevStatus: GovsgMessageStatus
): boolean => {
switch (status) {
case GovsgMessageStatus.Error:
return true // always update error status
case GovsgMessageStatus.InvalidRecipient:
return true // always update invalid recipient status
case GovsgMessageStatus.Unsent:
return false // will never be occur in practice
case GovsgMessageStatus.Accepted:
return prevStatus === GovsgMessageStatus.Unsent
case GovsgMessageStatus.Sent:
return (
prevStatus === GovsgMessageStatus.Unsent ||
prevStatus === GovsgMessageStatus.Accepted
)
case GovsgMessageStatus.Delivered:
return (
prevStatus === GovsgMessageStatus.Sent ||
prevStatus === GovsgMessageStatus.Unsent ||
prevStatus === GovsgMessageStatus.Accepted
)
case GovsgMessageStatus.Read:
return (
prevStatus === GovsgMessageStatus.Delivered ||
prevStatus === GovsgMessageStatus.Sent ||
prevStatus === GovsgMessageStatus.Unsent ||
prevStatus === GovsgMessageStatus.Accepted
)
case GovsgMessageStatus.Deleted:
return (
prevStatus === GovsgMessageStatus.Read ||
prevStatus === GovsgMessageStatus.Delivered ||
prevStatus === GovsgMessageStatus.Sent ||
prevStatus === GovsgMessageStatus.Unsent ||
prevStatus === GovsgMessageStatus.Accepted
)
default: {
const exhaustiveCheck: never = status
throw new Error(`Unhandled status: ${exhaustiveCheck}`)
}
}
}

export enum DefaultCredentialName {
Email = 'EMAIL_DEFAULT',
SMS = 'Postman_SMS_Demo',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict'

module.exports = {
up: async (queryInterface, _) => {
await queryInterface.addIndex(
'govsg_ops',
['service_provider_message_id'],
{
name: 'govsg_ops_service_provider_message_id_key',
unique: true,
}
)
},

down: async (queryInterface, _) => {
await queryInterface.removeIndex(
'govsg_ops',
['service_provider_message_id'],
{
name: 'govsg_ops_service_provider_message_id_key',
unique: true,
}
)
},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
'use strict'

module.exports = {
up: async (queryInterface, Sequelize) => {
await queryInterface.addColumn('govsg_messages', 'deleted_by_user_at', {
type: Sequelize.DataTypes.DATE,
allowNull: true,
})
await queryInterface.addColumn(
'govsg_messages_transactional',
'deleted_by_user_at',
{
type: Sequelize.DataTypes.DATE,
allowNull: true,
}
)
await queryInterface.addColumn('govsg_ops', 'deleted_by_user_at', {
type: Sequelize.DataTypes.DATE,
allowNull: true,
})
await queryInterface.createFunction(
'log_job_govsg',
[{ type: 'integer', name: 'selected_campaign_id' }],
'void',
'plpgsql',
`
UPDATE govsg_messages m
-- setting dequeued_at to null so it can be retried if needed
SET dequeued_at = NULL,
-- coalesced fields should prioritise message table over ops table
-- because callbacks might arrive before logging
status = CASE
WHEN m.status = 'UNSENT' THEN COALESCE(p.status, m.status)
ELSE COALESCE(m.status, p.status)
END,
error_code = COALESCE(m.error_code, p.error_code),
error_description = COALESCE(m.error_description, p.error_description),
service_provider_message_id = p.service_provider_message_id,
send_attempted_at = p.send_attempted_at,
sent_at = COALESCE(m.sent_at, p.sent_at),
delivered_at = COALESCE(m.delivered_at, p.delivered_at),
read_at = COALESCE(m.read_at, p.read_at),
errored_at = COALESCE(m.errored_at, p.errored_at),
deleted_by_user_at = COALESCE(m.deleted_by_user_at, p.deleted_by_user_at),
accepted_at = p.accepted_at,
updated_at = clock_timestamp()
FROM govsg_ops p WHERE
m.id = p.id;
DELETE FROM govsg_ops p WHERE p.campaign_id = selected_campaign_id;
PERFORM update_stats_govsg(selected_campaign_id);
`,
[],
{ force: true }
)
},

down: async (queryInterface, Sequelize) => {
await queryInterface.createFunction(
'log_job_govsg',
[{ type: 'integer', name: 'selected_campaign_id' }],
'void',
'plpgsql',
`
UPDATE govsg_messages m
-- setting dequeued_at to null so it can be retried if needed
SET dequeued_at = NULL,
-- coalesced fields should prioritise message table over ops table
-- because callbacks might arrive before logging
status = CASE
WHEN m.status = 'UNSENT' THEN COALESCE(p.status, m.status)
ELSE COALESCE(m.status, p.status)
END,
error_code = COALESCE(m.error_code, p.error_code),
error_description = COALESCE(m.error_description, p.error_description),
service_provider_message_id = p.service_provider_message_id,
send_attempted_at = p.send_attempted_at,
accepted_at = p.accepted_at,
updated_at = clock_timestamp()
FROM govsg_ops p WHERE
m.id = p.id;
DELETE FROM govsg_ops p WHERE p.campaign_id = selected_campaign_id;
PERFORM update_stats_govsg(selected_campaign_id);
`,
[],
{ force: true }
)
await queryInterface.removeColumn('govsg_messages', 'deleted_by_user_at')
await queryInterface.removeColumn(
'govsg_messages_transactional',
'deleted_by_user_at'
)
await queryInterface.removeColumn('govsg_ops', 'deleted_by_user_at')
},
}
9 changes: 8 additions & 1 deletion backend/src/govsg/middlewares/govsg-callback.middleware.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { loggerWithLabel } from '@core/logger'
import { Request, Response, NextFunction } from 'express'
import { GovsgCallbackService } from '@govsg/services/govsg-callback.service'
import { UnexpectedWebhookError } from '@shared/clients/whatsapp-client.class/errors'
import {
MessageIdNotFoundWebhookError,
UnexpectedWebhookError,
} from '@shared/clients/whatsapp-client.class/errors'
import { WhatsAppApiClient } from '@shared/clients/whatsapp-client.class/types'

const logger = loggerWithLabel(module)
Expand Down Expand Up @@ -68,6 +71,10 @@ const parseWebhook = async (
res.sendStatus(500)
return
}
if (err instanceof MessageIdNotFoundWebhookError) {
res.sendStatus(400)
return
}
next(err)
}
}
Expand Down
3 changes: 3 additions & 0 deletions backend/src/govsg/models/govsg-message-transactional.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,7 @@ export class GovsgMessageTransactional extends Model<GovsgMessageTransactional>

@Column({ type: DataType.DATE, allowNull: true })
erroredAt: Date | null

@Column({ type: DataType.DATE, allowNull: true })
deletedByUserAt: Date | null
}
3 changes: 3 additions & 0 deletions backend/src/govsg/models/govsg-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,7 @@ export class GovsgMessage extends Model<GovsgMessage> {

@Column({ type: DataType.DATE, allowNull: true })
erroredAt: Date | null

@Column({ type: DataType.DATE, allowNull: true })
deletedByUserAt: Date | null
}
4 changes: 2 additions & 2 deletions backend/src/govsg/models/govsg-op.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Model, Table } from 'sequelize-typescript'
import { Table } from 'sequelize-typescript'
import { GovsgMessage } from './govsg-message'

@Table({ tableName: 'govsg_ops', underscored: true, timestamps: true })
export class GovsgOp extends Model<GovsgMessage> {}
export class GovsgOp extends GovsgMessage {}
Loading

0 comments on commit 1a8601f

Please sign in to comment.