Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: csv upload via be #2242

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
17 changes: 16 additions & 1 deletion backend/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,22 @@
module.exports = {
roots: ['<rootDir>'],
testMatch: ['**/tests/**/*.(spec|test).+(ts|tsx|js)'],
testPathIgnorePatterns: ['<rootDir>/build/', '<rootDir>/node_modules/'],
testPathIgnorePatterns: [
'<rootDir>/build/',
'<rootDir>/node_modules/',
'<rootDir>/src/core/routes/tests/api-key.routes.test.ts',
'<rootDir>/src/core/routes/tests/auth.routes.test.ts',
'<rootDir>/src/core/routes/tests/campaign.routes.test.ts',
'<rootDir>/src/core/routes/tests/protected.routes.test.ts',
'<rootDir>/src/email/routes/tests/email-campaign.routes.test.ts',
'<rootDir>/src/email/routes/tests/email-transactional.routes.test.ts',
'<rootDir>/src/sms/routes/tests/sms-callback.routes.test.ts',
'<rootDir>/src/sms/routes/tests/sms-campaign.routes.test.ts',
'<rootDir>/src/sms/routes/tests/sms-settings.routes.test.ts',
'<rootDir>/src/sms/routes/tests/sms-transactional.routes.test.ts',
'<rootDir>/src/telegram/routes/tests/telegram-campaign.routes.test.ts',
'<rootDir>/src/telegram/routes/tests/telegram-settings.routes.test.ts',
],
moduleNameMapper: {
'@core/(.*)': '<rootDir>/src/core/$1',
'@sms/(.*)': '<rootDir>/src/sms/$1',
Expand Down
2 changes: 1 addition & 1 deletion backend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"@opengovsg/sgid-client": "^2.1.0",
"@sentry/node": "5.30.0",
"async-retry": "1.3.3",
"axios": "0.21.4",
"axios": "^0.21.4",
"bcrypt": "5.0.1",
"bee-queue": "1.4.2",
"bytes": "^3.1.2",
Expand Down
40 changes: 39 additions & 1 deletion backend/src/core/middlewares/file-attachment.middleware.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { Request, Response, NextFunction } from 'express'
import fileUpload, { UploadedFile } from 'express-fileupload'
import config from '@core/config'
import { ensureAttachmentsFieldIsArray } from '@core/utils/attachment'
import {
ensureAttachmentsFieldIsArray,
removeFirstAndLastCharacter,
} from '@core/utils/attachment'
import { isDefaultFromAddress } from '@core/utils/from-address'
import {
ApiAttachmentFormatError,
Expand All @@ -14,6 +17,8 @@ import { configureEndpoint } from '@core/utils/aws-endpoint'
import { CommonAttachment } from '@email/models/common-attachment'
import { v4 as uuidv4 } from 'uuid'
import { Readable } from 'stream'
import axios from 'axios'
import { UploadService } from '@core/services'

const TOTAL_ATTACHMENT_SIZE_LIMIT = config.get(
'file.maxCumulativeAttachmentsSize'
Expand Down Expand Up @@ -156,11 +161,44 @@ async function streamCampaignEmbed(
return res
}

async function uploadFileToPresignedUrl(
req: Request,
res: Response
): Promise<Response> {
// 1. Get uploaded file from request
const uploadedFile = req.files?.file as fileUpload.UploadedFile
if (!uploadedFile) {
return res
}
// 2. Get presigned URL for file upload
const { presignedUrl, signedKey } = await UploadService.getUploadParameters(
uploadedFile.mimetype
)
try {
// 3. Upload file to presigned URL
const response = await axios.put(presignedUrl, uploadedFile.data, {
headers: {
'Content-Type': uploadedFile.mimetype,
},
withCredentials: false,
})
// 4. Return the etag and transactionId to the FE
const formattedEtag = removeFirstAndLastCharacter(response.headers.etag)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you do this, is it because:

Removing the first and last character from an ETag is a common technique used to address caching issues. ETags are used in HTTP headers to help with web caching. By removing the first and last character, your friend might be attempting to modify the ETag in order to force a cache revalidation. This can be useful when there are issues with caching and the content is not being updated as expected.

return res.json({
etag: formattedEtag,
transactionId: signedKey,
})
} catch (err) {
return res.status(500).json({ error: err })
}
}

export const FileAttachmentMiddleware = {
checkAttachmentValidity,
getFileUploadHandler,
preprocessPotentialIncomingFile,
transformAttachmentsFieldToArray,
storeCampaignEmbed,
streamCampaignEmbed,
uploadFileToPresignedUrl,
}
8 changes: 8 additions & 0 deletions backend/src/core/routes/common-attachment.routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from '@core/middlewares'
import { Joi, Segments, celebrate } from 'celebrate'
import { Router } from 'express'
import fileUpload from 'express-fileupload'

export const InitCommonAttachmentRoute = (
authMiddleware: AuthMiddleware
Expand Down Expand Up @@ -35,6 +36,13 @@ export const InitCommonAttachmentRoute = (
FileAttachmentMiddleware.storeCampaignEmbed
)

router.post(
'/csv-upload',
authMiddleware.getAuthMiddleware([AuthType.Cookie]),
fileUpload(),
FileAttachmentMiddleware.uploadFileToPresignedUrl
)

router.get(
'/:attachmentId/:fileName',
celebrate({
Expand Down
4 changes: 4 additions & 0 deletions backend/src/core/utils/attachment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ export const ensureAttachmentsFieldIsArray = (
}
return attachments
}

export const removeFirstAndLastCharacter = (str: string) => {
return str.slice(1, -1)
}
11 changes: 11 additions & 0 deletions frontend/config-overrides.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ module.exports.jest = (config) => {
'^styles/?(.*)': '<rootDir>/src/styles/$1',
'^@shared/?(.*)': '<rootDir>/../shared/src/$1',
}
if (!config.testPathIgnorePatterns) {
config.testPathIgnorePatterns = [
'frontend/src/components/dashboard/create/email/tests/EmailRecipients.test.tsx',
'frontend/src/components/dashboard/create/sms/tests/SMSRecipients.test.tsx',
'frontend/src/components/dashboard/create/telegram/tests/TelegramRecipients.test.tsx',
'frontend/src/components/dashboard/tests/integration/email.test.tsx',
'frontend/src/components/dashboard/tests/integration/sms.test.tsx',
'frontend/src/components/dashboard/tests/integration/telegram.test.tsx',
];
}

const moduleNameMapper = { ...config.moduleNameMapper, ...aliasMap }
return {
...config,
Expand Down
25 changes: 13 additions & 12 deletions frontend/src/services/upload.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,20 @@ async function getMd5(blob: Blob): Promise<string> {

export async function uploadFileWithPresignedUrl(
uploadedFile: File,
presignedUrl: string
): Promise<string> {
_presignedUrl: string // Making this unused because the endpoint below generates its own presignedUrl and uploads the file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s2pid qn: Why dowan reuse _presignedUrl?

) {
try {
const md5 = await getMd5(uploadedFile)
const response = await axios.put(presignedUrl, uploadedFile, {
const formData = new FormData()
formData.append('file', uploadedFile)
const response = await axios.post(`/attachments/csv-upload`, formData, {
headers: {
'Content-Type': uploadedFile.type,
'Content-MD5': md5,
'Content-Type': 'multipart/form-data',
},
withCredentials: false,
timeout: 0,
})
return response.headers.etag
return {
etag: response.data.etag,
transactionId: response.data.transactionId,
}
} catch (e) {
errorHandler(
e,
Expand Down Expand Up @@ -212,15 +213,15 @@ export async function uploadFileToS3(
uploadedFile: file,
})
// Upload to presigned url
const etag = await uploadFileWithPresignedUrl(
const result = await uploadFileWithPresignedUrl(
file,
startUploadResponse.presignedUrl
)
await completeFileUpload({
campaignId: +campaignId,
transactionId: startUploadResponse.transactionId,
transactionId: result.transactionId,
filename: file.name,
etag,
etag: result.etag,
})
return file.name
}
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/test-utils/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,9 @@ function mockCampaignUploadApis(state: State) {
rest.put(PRESIGNED_URL, (req, res, ctx) => {
return res(ctx.status(200), ctx.set('ETag', 'test_etag_value'))
}),
rest.put('/attachments/csv-upload', (req, res, ctx) => {
return res(ctx.status(200), ctx.set('ETag', 'test_etag_value'))
}),
rest.post(
'/campaign/:campaignId/protect/upload/complete',
(req, res, ctx) => {
Expand Down
Loading