From be00f49d760634a0d1d6212284f1455724f2c8bf Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Fri, 19 Mar 2021 14:44:31 -0400 Subject: [PATCH 1/9] Allow legacy AR comments to be updated --- package.json | 2 +- src/routes/activityReports/handlers.js | 20 +++++++++ src/routes/activityReports/handlers.test.js | 45 +++++++++++++++++---- src/routes/activityReports/index.js | 3 ++ src/services/users.test.js | 18 ++++----- 5 files changed, 70 insertions(+), 18 deletions(-) diff --git a/package.json b/package.json index 51b676328f..87b77b0f1c 100644 --- a/package.json +++ b/package.json @@ -105,7 +105,7 @@ "statements": 90, "functions": 90, "branches": 81, - "lines": 90 + "lines": 90 } } }, diff --git a/src/routes/activityReports/handlers.js b/src/routes/activityReports/handlers.js index 4284804d10..668ded142c 100644 --- a/src/routes/activityReports/handlers.js +++ b/src/routes/activityReports/handlers.js @@ -16,6 +16,7 @@ import { goalsForGrants } from '../../services/goals'; import { userById, usersWithPermissions } from '../../services/users'; import { REPORT_STATUSES, DECIMAL_BASE } from '../../constants'; import { getUserReadRegions } from '../../services/accessValidation'; +import { logger } from '../../logger'; const { APPROVE_REPORTS } = SCOPES; @@ -25,6 +26,25 @@ const logContext = { namespace, }; +export async function updateLegacyFields(req, res) { + try { + const { legacyReportId } = req.params; + const report = await activityReportByLegacyId(legacyReportId); + if (!report) { + res.sendStatus(404); + return; + } + const user = await userById(req.session.userId); + const imported = { ...report.imported, ...req.body }; + logger.debug(`Saving new data: ${JSON.stringify(imported, null, 2)}`); + + const savedReport = await createOrUpdate({ imported }, report); + res.json(savedReport); + } catch (error) { + handleErrors(req, res, error, logContext); + } +} + export async function getLegacyReport(req, res) { try { const { legacyReportId } = req.params; diff --git a/src/routes/activityReports/handlers.test.js b/src/routes/activityReports/handlers.test.js index b39125de54..38e739ae8f 100644 --- a/src/routes/activityReports/handlers.test.js +++ b/src/routes/activityReports/handlers.test.js @@ -10,6 +10,7 @@ import { getReports, getReportAlerts, getLegacyReport, + updateLegacyFields, } from './handlers'; import { activityReportById, @@ -21,6 +22,7 @@ import { activityReportAlerts, activityReportByLegacyId, } from '../../services/activityReports'; +import { getUserReadRegions } from '../../services/accessValidation'; import { userById, usersWithPermissions } from '../../services/users'; import ActivityReport from '../../policies/activityReport'; import User from '../../policies/user'; @@ -36,6 +38,8 @@ jest.mock('../../services/activityReports', () => ({ activityReportByLegacyId: jest.fn(), })); +jest.mock('../../services/accessValidation'); + jest.mock('../../services/users', () => ({ userById: jest.fn(), usersWithPermissions: jest.fn(), @@ -69,7 +73,7 @@ describe('Activity Report handlers', () => { jest.clearAllMocks(); }); - describe('activityReportByLegacyId', () => { + describe('getLegacyReport', () => { const request = { ...mockRequest, params: { legacyReportId: 1 }, @@ -100,6 +104,35 @@ describe('Activity Report handlers', () => { }); }); + describe('updateLegacyFields', () => { + const discussions = [{ + comments: [{ + text: 'My comment', + author: 'smartsheet.user@tta.com', + timestamp: '2021-03-22T12:30:00Z', + }], + }]; + const request = { + ...mockRequest, + params: { legacyReportId: 1 }, + body: { discussions }, + }; + + it('updates the import data with the discussion', async () => { + activityReportByLegacyId.mockResolvedValue(report); + createOrUpdate.mockResolvedValue(report); + await updateLegacyFields(request, mockResponse); + expect(createOrUpdate).toHaveBeenCalledWith({ imported: { discussions } }, report); + expect(mockResponse.json).toHaveBeenCalledWith(report); + }); + + it('handles a report not being found', async () => { + activityReportByLegacyId.mockResolvedValue(null); + await updateLegacyFields(request, mockResponse); + expect(mockResponse.sendStatus).toHaveBeenCalledWith(404); + }); + }); + describe('reviewReport', () => { const request = { ...mockRequest, @@ -354,9 +387,7 @@ describe('Activity Report handlers', () => { it('returns the reports', async () => { activityReports.mockResolvedValue({ count: 1, rows: [report] }); - userById.mockResolvedValue({ - id: 1, - }); + getUserReadRegions.mockResolvedValue([1]); await getReports(request, mockResponse); expect(mockResponse.json).toHaveBeenCalledWith({ count: 1, rows: [report] }); @@ -364,6 +395,8 @@ describe('Activity Report handlers', () => { it('handles a list of reports that are not found', async () => { activityReports.mockResolvedValue(null); + getUserReadRegions.mockResolvedValue([1]); + await getReports(request, mockResponse); expect(mockResponse.sendStatus).toHaveBeenCalledWith(404); }); @@ -377,10 +410,6 @@ describe('Activity Report handlers', () => { it('returns my alerts', async () => { activityReportAlerts.mockResolvedValue({ count: 1, rows: [report] }); - userById.mockResolvedValue({ - id: 1, - }); - await getReportAlerts(request, mockResponse); expect(mockResponse.json).toHaveBeenCalledWith({ alertsCount: 1, alerts: [report] }); }); diff --git a/src/routes/activityReports/index.js b/src/routes/activityReports/index.js index f47aee2039..33bb264fb6 100644 --- a/src/routes/activityReports/index.js +++ b/src/routes/activityReports/index.js @@ -12,7 +12,9 @@ import { reviewReport, resetToDraft, getLegacyReport, + updateLegacyFields, } from './handlers'; +import userAdminAccessMiddleware from '../../middleware/userAdminAccessMiddleware'; const router = express.Router(); @@ -26,6 +28,7 @@ router.get('/activity-recipients', getActivityRecipients); router.get('/goals', getGoals); router.get('/alerts', getReportAlerts); router.get('/legacy/:legacyReportId', getLegacyReport); +router.put('/legacy/:legacyReportId', userAdminAccessMiddleware, updateLegacyFields); router.get('/:activityReportId', getReport); router.get('/', getReports); router.put('/:activityReportId', saveReport); diff --git a/src/services/users.test.js b/src/services/users.test.js index 945164ea38..82155d462a 100644 --- a/src/services/users.test.js +++ b/src/services/users.test.js @@ -13,33 +13,33 @@ describe('Users DB service', () => { jest.clearAllMocks(); }); - afterAll(() => { - db.sequelize.close(); + afterAll(async () => { + await db.sequelize.close(); }); describe('userById', () => { beforeEach(async () => { await User.create({ - id: 50, + id: 54, name: 'user 1', hsesUsername: 'user.1', - hsesUserId: '50', + hsesUserId: '54', }); await User.create({ - id: 51, + id: 55, name: 'user 2', hsesUsername: 'user.2', - hsesUserId: '51', + hsesUserId: '55', }); }); afterEach(async () => { - await User.destroy({ where: { id: [50, 51] } }); + await User.destroy({ where: { id: [54, 55] } }); }); it('retrieves the correct user', async () => { - const user = await userById(50); - expect(user.id).toBe(50); + const user = await userById(54); + expect(user.id).toBe(54); expect(user.name).toBe('user 1'); }); }); From 48561954815f57fdae1663368d841a2a0d7a0277 Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Mon, 22 Mar 2021 16:19:36 -0400 Subject: [PATCH 2/9] Allow admins to upload files on any report --- src/middleware/userAdminAccessMiddleware.js | 4 +- src/routes/files/handlers.js | 21 +++---- src/routes/files/handlers.test.js | 65 +++++++++++---------- src/services/accessValidation.js | 7 ++- 4 files changed, 51 insertions(+), 46 deletions(-) diff --git a/src/middleware/userAdminAccessMiddleware.js b/src/middleware/userAdminAccessMiddleware.js index 8790dd8011..5a94db2912 100644 --- a/src/middleware/userAdminAccessMiddleware.js +++ b/src/middleware/userAdminAccessMiddleware.js @@ -16,9 +16,7 @@ import handleErrors from '../lib/apiErrorHandler'; export default async function userAdminAccessMiddleware(req, res, next) { try { const { userId } = req.session; - if ((await validateUserAuthForAdmin(userId))) { - auditLogger.info(`User ${userId} successfully checked ADMIN access`); - } else { + if (!(await validateUserAuthForAdmin(userId))) { auditLogger.error(`User ${userId} attempted to access an ADMIN route without permission`); // consider sending a 404 rather than a 403 (Forbidden) to avoid confirming route return res.sendStatus(httpCodes.FORBIDDEN); diff --git a/src/routes/files/handlers.js b/src/routes/files/handlers.js index 3c94bd3291..86d725c318 100644 --- a/src/routes/files/handlers.js +++ b/src/routes/files/handlers.js @@ -9,6 +9,7 @@ import createFileMetaData, { import ActivityReportPolicy from '../../policies/activityReport'; import { activityReportById } from '../../services/activityReports'; import { userById } from '../../services/users'; +import { validateUserAuthForAdmin } from '../../services/accessValidation'; import { auditLogger } from '../../logger'; import { FILE_STATUSES } from '../../constants'; @@ -53,10 +54,10 @@ export const deleteHandler = async (req, res) => { export default async function uploadHandler(req, res) { const form = new multiparty.Form(); - form.parse(req, async (error, fields, files) => { + await form.parse(req, async (error, fields, files) => { const { reportId } = fields; if (error) { - res.status(500).send(error); + return res.status(500).send(error); } let buffer; let metadata; @@ -67,29 +68,25 @@ export default async function uploadHandler(req, res) { const report = await activityReportById(reportId); const authorization = new ActivityReportPolicy(user, report); - if (!authorization.canUpdate()) { - res.sendStatus(403); - return; + if (!(authorization.canUpdate() || (await validateUserAuthForAdmin(req.session.userId)))) { + return res.sendStatus(403); } try { if (!files.file) { - res.status(400).send({ error: 'file required' }); - return; + return res.status(400).send({ error: 'file required' }); } const { path, originalFilename, size } = files.file[0]; if (!size) { - res.status(400).send({ error: 'fileSize required' }); + return res.status(400).send({ error: 'fileSize required' }); } if (!reportId) { - res.status(400).send({ error: 'reportId required' }); - return; + return res.status(400).send({ error: 'reportId required' }); } buffer = fs.readFileSync(path); type = await fileType.fromFile(path); if (!type) { - res.status(400).send('Could not determine file type'); - return; + return res.status(400).send('Could not determine file type'); } fileName = `${uuidv4()}.${type.ext}`; metadata = await createFileMetaData( diff --git a/src/routes/files/handlers.test.js b/src/routes/files/handlers.test.js index 0b9fcb3d60..ee7fbdc259 100644 --- a/src/routes/files/handlers.test.js +++ b/src/routes/files/handlers.test.js @@ -3,7 +3,6 @@ import db, { File, ActivityReport, User, - Permission, } from '../../models'; import app from '../../app'; import { uploadFile, deleteFileFromS3, getPresignedURL } from '../../lib/s3'; @@ -12,8 +11,13 @@ import SCOPES from '../../middleware/scopeConstants'; import { REPORT_STATUSES, FILE_STATUSES } from '../../constants'; import ActivityReportPolicy from '../../policies/activityReport'; import * as Files from '../../services/files'; +import { validateUserAuthForAdmin, validateUserAuthForAccess } from '../../services/accessValidation'; jest.mock('../../policies/activityReport'); +jest.mock('../../services/accessValidation', () => ({ + validateUserAuthForAdmin: jest.fn().mockResolvedValue(false), + validateUserAuthForAccess: jest.fn().mockResolvedValue(true), +})); const request = require('supertest'); @@ -26,23 +30,6 @@ const mockUser = { hsesUserId: '2046', hsesUsername: '2046', homeRegionId: 1, - permissions: [ - { - userId: 2046, - regionId: 5, - scopeId: SCOPES.READ_WRITE_REPORTS, - }, - { - userId: 2046, - regionId: 6, - scopeId: SCOPES.READ_WRITE_REPORTS, - }, - { - userId: 2046, - regionId: 14, - scopeId: SCOPES.SITE_ACCESS, - }, - ], }; const mockSession = jest.fn(); @@ -63,7 +50,7 @@ describe('File Upload', () => { let report; let fileId; beforeAll(async () => { - user = await User.create(mockUser, { include: [{ model: Permission, as: 'permissions' }] }); + user = await User.create(mockUser); report = await ActivityReport.create(reportObject); process.env.NODE_ENV = 'test'; process.env.BYPASS_AUTH = 'true'; @@ -76,7 +63,7 @@ describe('File Upload', () => { process.env = ORIGINAL_ENV; // restore original env await db.sequelize.close(); }); - afterEach(() => { + beforeEach(() => { jest.clearAllMocks(); }); @@ -90,15 +77,35 @@ describe('File Upload', () => { canUpdate: () => true, })); uploadFile.mockResolvedValue({ key: 'key' }); - await request(app) + const response = await request(app) .post('/api/files') .field('reportId', report.dataValues.id) .attach('file', `${__dirname}/testfiles/testfile.pdf`) - .expect(200) - .then((res) => { - fileId = res.body.id; - expect(uploadFile).toHaveBeenCalled(); - }); + .expect(200); + fileId = response.body.id; + expect(uploadFile).toHaveBeenCalled(); + expect(mockAddToScanQueue).toHaveBeenCalled(); + const file = await File.findOne({ where: { id: fileId } }); + const uuid = file.dataValues.key.slice(0, -4); + expect(file.dataValues.id).toBe(fileId); + expect(file.dataValues.status).not.toBe(null); + expect(file.dataValues.originalFileName).toBe('testfile.pdf'); + expect(file.dataValues.activityReportId).toBe(report.dataValues.id); + expect(validate(uuid)).toBe(true); + }); + it('allows an admin to upload a file', async () => { + ActivityReportPolicy.mockImplementation(() => ({ + canUpdate: () => false, + })); + validateUserAuthForAdmin.mockResolvedValue(true); + uploadFile.mockResolvedValue({ key: 'key' }); + const response = await request(app) + .post('/api/files') + .field('reportId', report.dataValues.id) + .attach('file', `${__dirname}/testfiles/testfile.pdf`) + .expect(200); + fileId = response.body.id; + expect(uploadFile).toHaveBeenCalled(); expect(mockAddToScanQueue).toHaveBeenCalled(); const file = await File.findOne({ where: { id: fileId } }); const uuid = file.dataValues.key.slice(0, -4); @@ -155,9 +162,7 @@ describe('File Upload', () => { .post('/api/files') .attach('file', `${__dirname}/testfiles/testfile.pdf`) .expect(400, { error: 'reportId required' }) - .then(() => { - expect(uploadFile).not.toHaveBeenCalled(); - }); + await expect(uploadFile).not.toHaveBeenCalled(); }); it('tests a file upload without a file', async () => { ActivityReportPolicy.mockImplementation(() => ({ @@ -172,7 +177,7 @@ describe('File Upload', () => { }); }); it('tests an unauthorized upload', async () => { - jest.clearAllMocks(); + validateUserAuthForAdmin.mockResolvedValue(false); ActivityReportPolicy.mockImplementation(() => ({ canUpdate: () => false, })); diff --git a/src/services/accessValidation.js b/src/services/accessValidation.js index 51611de6fd..689cdf2d1e 100644 --- a/src/services/accessValidation.js +++ b/src/services/accessValidation.js @@ -65,7 +65,12 @@ export async function validateUserAuthForAdmin(userId) { scopeId: ADMIN, }, }); - return userPermission !== null; + if (userPermission !== null) { + logger.info(`User ${userId} successfully checked ADMIN access`); + return true; + } + logger.warn(`User ${userId} unsuccessfully checked ADMIN access`); + return false; } catch (error) { logger.error(`${JSON.stringify({ ...logContext })} - ADMIN Access error - ${error}`); throw error; From 803ba2b7ee33dbecdc11ed59e67537203e32c295 Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Tue, 23 Mar 2021 10:48:24 -0400 Subject: [PATCH 3/9] Display comments and attachments in legacy reports --- frontend/src/pages/LegacyReport/index.js | 28 +++++++++++++++++-- .../src/pages/LegacyReport/reportColumns.js | 1 + src/routes/activityReports/handlers.test.js | 14 +++------- src/services/activityReports.js | 12 ++++++++ 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/frontend/src/pages/LegacyReport/index.js b/frontend/src/pages/LegacyReport/index.js index f521e542e0..5a5881ebc4 100644 --- a/frontend/src/pages/LegacyReport/index.js +++ b/frontend/src/pages/LegacyReport/index.js @@ -5,6 +5,7 @@ import { Alert, Table } from '@trussworks/react-uswds'; import { map } from 'lodash'; import Container from '../../components/Container'; +import FileReviewItem from '../ActivityReport/Pages/Review/FileReviewItem'; import { legacyReportById } from '../../fetchers/activityReports'; import reportColumns from './reportColumns'; @@ -45,7 +46,7 @@ function LegacyReport({ match }) { ); } - const { imported } = legacyReport; + const { imported, attachments } = legacyReport; const entries = map(reportColumns, (display, field) => { const value = imported[field]; return { @@ -57,9 +58,9 @@ function LegacyReport({ match }) { const tableEntries = entries.filter((item) => item.value).map(({ field, display, value }) => ( - + {display} - + {value.split('\n').map((string) =>
{string}
)} @@ -90,6 +91,27 @@ function LegacyReport({ match }) { {tableEntries} + {attachments && attachments.length > 0 + && ( + + Attachments + + {attachments.map(({ + id, + originalFileName, + url: { url }, + status, + }) => ( + + ))} + + + )} diff --git a/frontend/src/pages/LegacyReport/reportColumns.js b/frontend/src/pages/LegacyReport/reportColumns.js index eaeaadd2e3..3077b39637 100644 --- a/frontend/src/pages/LegacyReport/reportColumns.js +++ b/frontend/src/pages/LegacyReport/reportColumns.js @@ -40,6 +40,7 @@ const reportFields = { additionalNotesForThisActivity: 'Additional notes for this activity', manager: 'Manager', managerApproval: 'Manager approval', + comments: 'Comments', created: 'Time Created', createdBy: 'Created By', modified: 'Time Modified', diff --git a/src/routes/activityReports/handlers.test.js b/src/routes/activityReports/handlers.test.js index 38e739ae8f..4819085a73 100644 --- a/src/routes/activityReports/handlers.test.js +++ b/src/routes/activityReports/handlers.test.js @@ -105,24 +105,18 @@ describe('Activity Report handlers', () => { }); describe('updateLegacyFields', () => { - const discussions = [{ - comments: [{ - text: 'My comment', - author: 'smartsheet.user@tta.com', - timestamp: '2021-03-22T12:30:00Z', - }], - }]; + const comments = 'smartsheet.user@tta.com: My comment'; const request = { ...mockRequest, params: { legacyReportId: 1 }, - body: { discussions }, + body: { comments }, }; - it('updates the import data with the discussion', async () => { + it('updates the import data with the comments', async () => { activityReportByLegacyId.mockResolvedValue(report); createOrUpdate.mockResolvedValue(report); await updateLegacyFields(request, mockResponse); - expect(createOrUpdate).toHaveBeenCalledWith({ imported: { discussions } }, report); + expect(createOrUpdate).toHaveBeenCalledWith({ imported: { comments } }, report); expect(mockResponse.json).toHaveBeenCalledWith(report); }); diff --git a/src/services/activityReports.js b/src/services/activityReports.js index 4455b1af52..398f6732e5 100644 --- a/src/services/activityReports.js +++ b/src/services/activityReports.js @@ -175,6 +175,18 @@ export function activityReportByLegacyId(legacyId) { where: { legacyId, }, + include: [ + { + model: File, + where: { + status: { + [Op.ne]: 'UPLOAD_FAILED', + }, + }, + as: 'attachments', + required: false, + }, + ], }); } From f867c8e460d05a6c83581f7ac7bf8239d8f1d7f5 Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Tue, 23 Mar 2021 11:53:20 -0400 Subject: [PATCH 4/9] Add general SCANNING_FAILED status to capture other failure reasons Such as: "[ERR_FR_MAX_BODY_LENGTH_EXCEEDED]: Request body larger than maxBodyLength" --- src/constants.js | 1 + .../20210323155040-add-scanning-failed-enum.js | 9 +++++++++ src/models/file.js | 1 + src/workers/files.js | 6 +++++- 4 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 src/migrations/20210323155040-add-scanning-failed-enum.js diff --git a/src/constants.js b/src/constants.js index d33a364555..30ce10de03 100644 --- a/src/constants.js +++ b/src/constants.js @@ -12,6 +12,7 @@ export const FILE_STATUSES = { QUEUED: 'SCANNING_QUEUED', QUEUEING_FAILED: 'QUEUEING_FAILED', SCANNING: 'SCANNING', + SCANNING_FAILED: 'SCANNING_FAILED', APPROVED: 'APPROVED', REJECTED: 'REJECTED', }; diff --git a/src/migrations/20210323155040-add-scanning-failed-enum.js b/src/migrations/20210323155040-add-scanning-failed-enum.js new file mode 100644 index 0000000000..55fd27cc4f --- /dev/null +++ b/src/migrations/20210323155040-add-scanning-failed-enum.js @@ -0,0 +1,9 @@ +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.sequelize.query('ALTER TYPE "enum_Files_status" ADD VALUE \'SCANNING_FAILED\';'); + }, + down: async (queryInterface, Sequelize) => { + let query = 'DELETE FROM pg_enum WHERE enumlabel = \'SCANNING_FAILED\' AND enumtypid = ( SELECT oid FROM pg_type WHERE typname = \'enum_Files_status\')'; + await queryInterface.sequelize.query(query); + }, +}; diff --git a/src/models/file.js b/src/models/file.js index 44099a5c9a..6df1bf045a 100644 --- a/src/models/file.js +++ b/src/models/file.js @@ -33,6 +33,7 @@ module.exports = (sequelize, DataTypes) => { 'QUEUEING_FAILED', 'SCANNING_QUEUED', 'SCANNING', + 'SCANNING_FAILED', 'APPROVED', 'REJECTED', ), diff --git a/src/workers/files.js b/src/workers/files.js index 24f56e6e85..30bc405626 100644 --- a/src/workers/files.js +++ b/src/workers/files.js @@ -6,6 +6,8 @@ import { File } from '../models'; import { FILE_STATUSES } from '../constants'; const { + SCANNING, + SCANNING_FAILED, APPROVED, REJECTED, } = FILE_STATUSES; @@ -39,6 +41,7 @@ const updateFileStatus = async (key, fileStatus) => { const processFile = async (key) => { let res; try { + await updateFileStatus(key, SCANNING); const data = await downloadFile(key); const form = new FormData(); form.append('name', key); @@ -47,10 +50,11 @@ const processFile = async (key) => { res = await axios.post(`${process.env.CLAMAV_ENDPOINT}/scan`, form, { httpsAgent: agent, headers: { ...form.getHeaders() } }); await updateFileStatus(key, APPROVED); } catch (error) { - if (error.response.status === 406) { + if (error.response && error.response.status === 406) { await updateFileStatus(key, REJECTED); return { status: error.response.status, data: error.response.data }; } + await updateFileStatus(key, SCANNING_FAILED); throw error; } return ({ status: res.status, data: res.data }); From 44b789ef7aefbd7411b9e4f18cf905613cb18990 Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Tue, 23 Mar 2021 12:00:29 -0400 Subject: [PATCH 5/9] Linter fixes --- .../20210323155040-add-scanning-failed-enum.js | 6 +++--- src/routes/activityReports/handlers.js | 2 +- src/routes/files/handlers.js | 14 +++++--------- src/routes/files/handlers.test.js | 5 ++--- 4 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/migrations/20210323155040-add-scanning-failed-enum.js b/src/migrations/20210323155040-add-scanning-failed-enum.js index 55fd27cc4f..c8fbf7f7a0 100644 --- a/src/migrations/20210323155040-add-scanning-failed-enum.js +++ b/src/migrations/20210323155040-add-scanning-failed-enum.js @@ -1,9 +1,9 @@ module.exports = { - up: async (queryInterface, Sequelize) => { + up: async (queryInterface) => { await queryInterface.sequelize.query('ALTER TYPE "enum_Files_status" ADD VALUE \'SCANNING_FAILED\';'); }, - down: async (queryInterface, Sequelize) => { - let query = 'DELETE FROM pg_enum WHERE enumlabel = \'SCANNING_FAILED\' AND enumtypid = ( SELECT oid FROM pg_type WHERE typname = \'enum_Files_status\')'; + down: async (queryInterface) => { + const query = 'DELETE FROM pg_enum WHERE enumlabel = \'SCANNING_FAILED\' AND enumtypid = ( SELECT oid FROM pg_type WHERE typname = \'enum_Files_status\')'; await queryInterface.sequelize.query(query); }, }; diff --git a/src/routes/activityReports/handlers.js b/src/routes/activityReports/handlers.js index 668ded142c..22d2951692 100644 --- a/src/routes/activityReports/handlers.js +++ b/src/routes/activityReports/handlers.js @@ -34,7 +34,7 @@ export async function updateLegacyFields(req, res) { res.sendStatus(404); return; } - const user = await userById(req.session.userId); + // no authorization here because the entire route is only available to admins const imported = { ...report.imported, ...req.body }; logger.debug(`Saving new data: ${JSON.stringify(imported, null, 2)}`); diff --git a/src/routes/files/handlers.js b/src/routes/files/handlers.js index 86d725c318..c746087cfb 100644 --- a/src/routes/files/handlers.js +++ b/src/routes/files/handlers.js @@ -96,8 +96,7 @@ export default async function uploadHandler(req, res) { size, ); } catch (err) { - await handleErrors(req, res, err, logContext); - return; + return handleErrors(req, res, err, logContext); } try { const uploadedFile = await uploadFile(buffer, fileName, type); @@ -108,17 +107,14 @@ export default async function uploadHandler(req, res) { if (metadata) { await updateStatus(metadata.id, UPLOAD_FAILED); } - await handleErrors(req, res, err, logContext); - return; + return handleErrors(req, res, err, logContext); } try { await addToScanQueue({ key: metadata.key }); - await updateStatus(metadata.id, QUEUED); + return updateStatus(metadata.id, QUEUED); } catch (err) { - if (metadata) { - await updateStatus(metadata.id, QUEUEING_FAILED); - auditLogger.error(`${logContext} Failed to queue ${metadata.originalFileName}. Error: ${err}`); - } + auditLogger.error(`${logContext} Failed to queue ${metadata.originalFileName}. Error: ${err}`); + return updateStatus(metadata.id, QUEUEING_FAILED); } }); } diff --git a/src/routes/files/handlers.test.js b/src/routes/files/handlers.test.js index ee7fbdc259..bffa861033 100644 --- a/src/routes/files/handlers.test.js +++ b/src/routes/files/handlers.test.js @@ -7,11 +7,10 @@ import db, { import app from '../../app'; import { uploadFile, deleteFileFromS3, getPresignedURL } from '../../lib/s3'; import * as queue from '../../services/scanQueue'; -import SCOPES from '../../middleware/scopeConstants'; import { REPORT_STATUSES, FILE_STATUSES } from '../../constants'; import ActivityReportPolicy from '../../policies/activityReport'; import * as Files from '../../services/files'; -import { validateUserAuthForAdmin, validateUserAuthForAccess } from '../../services/accessValidation'; +import { validateUserAuthForAdmin } from '../../services/accessValidation'; jest.mock('../../policies/activityReport'); jest.mock('../../services/accessValidation', () => ({ @@ -161,7 +160,7 @@ describe('File Upload', () => { await request(app) .post('/api/files') .attach('file', `${__dirname}/testfiles/testfile.pdf`) - .expect(400, { error: 'reportId required' }) + .expect(400, { error: 'reportId required' }); await expect(uploadFile).not.toHaveBeenCalled(); }); it('tests a file upload without a file', async () => { From dfc5633f29c9ecf908bdc3dbb48931a626213bfc Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Tue, 23 Mar 2021 16:03:34 -0400 Subject: [PATCH 6/9] Ensure legacy AR ids have a 2 digit region number --- .../20210323195745-fix-legacy-report-region-id.js | 10 ++++++++++ src/tools/importActivityReports.js | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 src/migrations/20210323195745-fix-legacy-report-region-id.js diff --git a/src/migrations/20210323195745-fix-legacy-report-region-id.js b/src/migrations/20210323195745-fix-legacy-report-region-id.js new file mode 100644 index 0000000000..7694f45df9 --- /dev/null +++ b/src/migrations/20210323195745-fix-legacy-report-region-id.js @@ -0,0 +1,10 @@ +module.exports = { + up: (queryInterface, Sequelize) => ( + queryInterface.sequelize.query('UPDATE "ActivityReports" SET "legacyId" = regexp_replace("legacyId", \'R(\d)-\', \'R0\1-\') WHERE "legacyId" ~ \'R\d-\'') + ), + down: async () => { + /** + * Non-reversible + */ + }, +}; diff --git a/src/tools/importActivityReports.js b/src/tools/importActivityReports.js index 7efadff326..c381520b0f 100644 --- a/src/tools/importActivityReports.js +++ b/src/tools/importActivityReports.js @@ -187,7 +187,7 @@ function coerceInt(value) { function coerceReportId(value, region) { if (!value) return null; // R14 is a test region, and shouldn't be used in actual reportIds - return value.replace(invalidRegionRE, `R${region}`); + return value.replace(invalidRegionRE, `R${region.padStart(2, '0')}`); } function parseGrantNumbers(value) { From 1b31acfa9fffe61affa94c3a907dd3d696361a18 Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Wed, 24 Mar 2021 09:59:04 -0400 Subject: [PATCH 7/9] Include attchment transition script in full repo --- .../legacy_report_attachments/.ruby-version | 1 + .../legacy_report_attachments/Gemfile | 12 ++ .../legacy_report_attachments/Gemfile.lock | 57 +++++++ .../legacy_report_attachments/README.md | 41 +++++ .../legacy_report_attachments/Rakefile | 6 + .../lib/attachment.rb | 152 ++++++++++++++++++ .../legacy_report_attachments/lib/config.rb | 17 ++ .../legacy_report_attachments/smartsheet.yml | 25 +++ 8 files changed, 311 insertions(+) create mode 100644 smartsheet_scripts/legacy_report_attachments/.ruby-version create mode 100644 smartsheet_scripts/legacy_report_attachments/Gemfile create mode 100644 smartsheet_scripts/legacy_report_attachments/Gemfile.lock create mode 100644 smartsheet_scripts/legacy_report_attachments/README.md create mode 100644 smartsheet_scripts/legacy_report_attachments/Rakefile create mode 100644 smartsheet_scripts/legacy_report_attachments/lib/attachment.rb create mode 100644 smartsheet_scripts/legacy_report_attachments/lib/config.rb create mode 100644 smartsheet_scripts/legacy_report_attachments/smartsheet.yml diff --git a/smartsheet_scripts/legacy_report_attachments/.ruby-version b/smartsheet_scripts/legacy_report_attachments/.ruby-version new file mode 100644 index 0000000000..37c2961c24 --- /dev/null +++ b/smartsheet_scripts/legacy_report_attachments/.ruby-version @@ -0,0 +1 @@ +2.7.2 diff --git a/smartsheet_scripts/legacy_report_attachments/Gemfile b/smartsheet_scripts/legacy_report_attachments/Gemfile new file mode 100644 index 0000000000..d2612639e2 --- /dev/null +++ b/smartsheet_scripts/legacy_report_attachments/Gemfile @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +source "https://rubygems.org" + +git_source(:github) {|repo_name| "https://github.com/#{repo_name}" } + +gem "activesupport", "~> 6.1" +gem "smartsheet", "~> 2.101" +gem "rake", "~> 13.0" +gem "faraday", "~> 1.3" +gem "faraday_middleware", "~> 1.0" +gem "faraday-cookie_jar" diff --git a/smartsheet_scripts/legacy_report_attachments/Gemfile.lock b/smartsheet_scripts/legacy_report_attachments/Gemfile.lock new file mode 100644 index 0000000000..e743f81071 --- /dev/null +++ b/smartsheet_scripts/legacy_report_attachments/Gemfile.lock @@ -0,0 +1,57 @@ +GEM + remote: https://rubygems.org/ + specs: + activesupport (6.1.3) + concurrent-ruby (~> 1.0, >= 1.0.2) + i18n (>= 1.6, < 2) + minitest (>= 5.1) + tzinfo (~> 2.0) + zeitwerk (~> 2.3) + awrence (1.2.1) + concurrent-ruby (1.1.8) + domain_name (0.5.20190701) + unf (>= 0.0.5, < 1.0.0) + faraday (1.3.0) + faraday-net_http (~> 1.0) + multipart-post (>= 1.2, < 3) + ruby2_keywords + faraday-cookie_jar (0.0.7) + faraday (>= 0.8.0) + http-cookie (~> 1.0.0) + faraday-net_http (1.0.1) + faraday_middleware (1.0.0) + faraday (~> 1.0) + http-cookie (1.0.3) + domain_name (~> 0.5) + i18n (1.8.9) + concurrent-ruby (~> 1.0) + minitest (5.14.4) + multipart-post (2.1.1) + plissken (1.4.1) + rake (13.0.3) + ruby2_keywords (0.0.4) + smartsheet (2.101.1) + awrence (~> 1.0) + faraday (>= 0.13.1, < 2) + faraday_middleware (>= 0.10.0, < 2) + plissken (~> 1.2) + tzinfo (2.0.4) + concurrent-ruby (~> 1.0) + unf (0.1.4) + unf_ext + unf_ext (0.0.7.7) + zeitwerk (2.4.2) + +PLATFORMS + ruby + +DEPENDENCIES + activesupport (~> 6.1) + faraday (~> 1.3) + faraday-cookie_jar + faraday_middleware (~> 1.0) + rake (~> 13.0) + smartsheet (~> 2.101) + +BUNDLED WITH + 2.1.4 diff --git a/smartsheet_scripts/legacy_report_attachments/README.md b/smartsheet_scripts/legacy_report_attachments/README.md new file mode 100644 index 0000000000..96af308b5b --- /dev/null +++ b/smartsheet_scripts/legacy_report_attachments/README.md @@ -0,0 +1,41 @@ +Legacy Report Attachments +========================= + +Script to import comments and file attachments from Smartsheet ARs. + +Configuration +------------- + +### Environment Variables + +To run, the following three ENV vars must be set: + +* `SMARTSHEET_API_TOKEN` api token for smartsheet, must have access to all 12 region AR sheets +* `SMARTHUB_SESSION_COOKIE` the session cookie for a smarthub admin with read access to all 12 regions +* `SMARTHUB_SESSION_SIG` the session.sig cooke for the same smarthub admin + +### smartsheet.yml + +This file contains the sheet ids for smartsheet activity report sheets + +### Attachment::SMARTHUB_URI_BASE + +This constant (set in `lib/attachment.rb`) is the base url of the smarthub + +Running +------- + +* Ensure ruby 2.7.2 is installed +* `bundle install` +* Set ENV vars +* `bundle exec rake run[REGION]` where `REGION` is the region number + + +example: `bundle exec rake run[1]` + + +Disclaimers +----------- + +There are several attachments stored in onedrive that are behind additional login layers. +These will need to be imported by hand diff --git a/smartsheet_scripts/legacy_report_attachments/Rakefile b/smartsheet_scripts/legacy_report_attachments/Rakefile new file mode 100644 index 0000000000..df35c70d32 --- /dev/null +++ b/smartsheet_scripts/legacy_report_attachments/Rakefile @@ -0,0 +1,6 @@ +require_relative "lib/attachment" + +desc "Transfer attachments by region" +task :run, [:region] do |task, args| + Attachment.new(args[:region]).call +end diff --git a/smartsheet_scripts/legacy_report_attachments/lib/attachment.rb b/smartsheet_scripts/legacy_report_attachments/lib/attachment.rb new file mode 100644 index 0000000000..a572b1320a --- /dev/null +++ b/smartsheet_scripts/legacy_report_attachments/lib/attachment.rb @@ -0,0 +1,152 @@ +require "active_support/all" +require "logger" +require "smartsheet" +require "faraday" +require "faraday_middleware" +require "faraday-cookie_jar" +require "tempfile" +require_relative "config" + +class Attachment + + SMARTHUB_URI_BASE = "https://ttahub.ohs.acf.hhs.gov" + + attr_reader :region, :client, :cookie + def initialize(region) + @region = region.to_s + # session cookies taken out of chrome dev tools for an active admin session + fail "Session cookie not provided" if ENV["SMARTHUB_SESSION_COOKIE"].blank? || ENV["SMARTHUB_SESSION_SIG"].blank? + @cookie = "session=#{ENV["SMARTHUB_SESSION_COOKIE"]}; session.sig=#{ENV["SMARTHUB_SESSION_SIG"]}" + token = ENV["SMARTSHEET_API_TOKEN"] + fail "No API Token provided" if token.blank? + logger = Logger.new($stdout) + logger.level = :warn + @client = Smartsheet::Client.new( + token: token, + logger: logger, + base_url: Smartsheet::Constants::GOV_API_URL + ) + end + + def call + sheet = client.sheets.get(sheet_id: sheet_id, params: { + include: "attachments,discussions", + level: "2", + columnIds: report_id_column_id + }) + sheet[:rows].each do |row| + next if row[:discussions].blank? && row[:attachments].blank? + process_row(row) + end + sheet + end + + def process_row(row) + legacy_id = row[:cells].first[:value].gsub(/^R14/, "R#{"%02d" % region}") + puts "Processing #{legacy_id}" + activity_report = if row[:discussions].present? + discussions = row[:discussions].flat_map { |disc| retrieve_discussion(disc[:id]) }.join("\n") + update_comments legacy_id, discussions + else + activity_report legacy_id + end + (row[:attachments] || []).each do |attachment| + process_attachment activity_report, attachment + end + end + + def process_attachment(activity_report, attachment_meta) + if activity_report["attachments"].none? { |attach| attach["originalFileName"] == attachment_meta[:name] } + attach = client.sheets.attachments.get(sheet_id: sheet_id, attachment_id: attachment_meta[:id]) + if attach[:url].blank? + fail "No URL to download file: #{attach.inspect}" + end + Tempfile.create do |file| + success = download_file(file, attach[:url]) + if success + post_file(activity_report["id"], file, attachment_meta) + else + puts "Failed to download file for: #{activity_report["displayId"]}, #{attach.inspect}" + end + end + end + end + + def update_comments(legacy_id, discussions) + response = put("/api/activity-reports/legacy/#{legacy_id}", {comments: discussions}) + if response.success? + JSON.parse(response.body) + else + fail "#{response.status} #{response.body}" + end + end + + def activity_report(legacy_id) + response = get("/api/activity-reports/legacy/#{legacy_id}") + if response.success? + JSON.parse(response.body) + else + fail "#{response.status} #{response.body}" + end + end + + def download_file(file, url) + conn = Faraday.new(url) do |c| + c.use FaradayMiddleware::FollowRedirects + c.use :cookie_jar + end + response = conn.get + if response.success? + file.write(response.body) + file.flush + true + else + false + end + end + + def post_file(report_id, file, attachment_meta) + filepart = Faraday::FilePart.new(file.path, attachment_meta[:mime_type], attachment_meta[:name]) + params = { + "reportId": report_id, + file: filepart + } + multipart_faraday.post("/api/files", params) + end + + def put(url, params) + faraday.put(url, params.to_json, "Content-Type" => "application/json") + end + + def get(url) + faraday.get(url) + end + + def multipart_faraday + @multipart_faraday ||= Faraday.new(url: SMARTHUB_URI_BASE, headers: {'Cookie' => cookie}) do |f| + f.request :multipart + end + end + + def faraday + @faraday ||= Faraday.new(url: SMARTHUB_URI_BASE, headers: {'Cookie' => cookie}) + end + + def retrieve_discussion(disc_id) + client.sheets.discussions.get(sheet_id: sheet_id, discussion_id: disc_id)[:comments].map do |comment| + "#{comment[:created_by][:email]}: #{comment[:text]}" + end + end + + def report_id_column_id + @report_id_column_id ||= sheet_columns.find { |c| c[:title] == "ReportID" }[:id] + end + + def sheet_columns + @sheet_columns ||= client.sheets.columns.list(sheet_id: sheet_id)[:data] + end + + def sheet_id + @sheet_id ||= Config.load[region][:ar_sheet_id] + end +end diff --git a/smartsheet_scripts/legacy_report_attachments/lib/config.rb b/smartsheet_scripts/legacy_report_attachments/lib/config.rb new file mode 100644 index 0000000000..c19d8c12a9 --- /dev/null +++ b/smartsheet_scripts/legacy_report_attachments/lib/config.rb @@ -0,0 +1,17 @@ +require "yaml" +require "erb" +require "active_support/core_ext/hash" + +class Config + CONFIG_FILE = File.expand_path(File.join(__dir__, "..", "smartsheet.yml")).freeze + + def self.load + @config ||= yaml.with_indifferent_access[:regions] + end + + private + + def self.yaml + @yaml ||= YAML.load(ERB.new(File.read(CONFIG_FILE)).result) + end +end diff --git a/smartsheet_scripts/legacy_report_attachments/smartsheet.yml b/smartsheet_scripts/legacy_report_attachments/smartsheet.yml new file mode 100644 index 0000000000..32897fbe18 --- /dev/null +++ b/smartsheet_scripts/legacy_report_attachments/smartsheet.yml @@ -0,0 +1,25 @@ +regions: + "1": + ar_sheet_id: "1515864581675124" + "2": + ar_sheet_id: "5139786187348084" + "3": + ar_sheet_id: "2184092773455988" + "4": + ar_sheet_id: "2458901960923252" + "5": + ar_sheet_id: "1333002054080628" + "6": + ar_sheet_id: "6793245517092980" + "7": + ar_sheet_id: "5175863912634484" + "8": + ar_sheet_id: "4964482802194548" + "9": + ar_sheet_id: "2924064098949236" + "10": + ar_sheet_id: "7427663726319732" + "11": + ar_sheet_id: "354711583266932" + "12": + ar_sheet_id: "2712682988509300" From ce3be5f1027c9bb3854f581c020c22b5a75da12a Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Wed, 24 Mar 2021 11:11:28 -0400 Subject: [PATCH 8/9] Linter fixes --- src/migrations/20210323195745-fix-legacy-report-region-id.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/migrations/20210323195745-fix-legacy-report-region-id.js b/src/migrations/20210323195745-fix-legacy-report-region-id.js index 7694f45df9..79a1a0aadd 100644 --- a/src/migrations/20210323195745-fix-legacy-report-region-id.js +++ b/src/migrations/20210323195745-fix-legacy-report-region-id.js @@ -1,6 +1,6 @@ module.exports = { - up: (queryInterface, Sequelize) => ( - queryInterface.sequelize.query('UPDATE "ActivityReports" SET "legacyId" = regexp_replace("legacyId", \'R(\d)-\', \'R0\1-\') WHERE "legacyId" ~ \'R\d-\'') + up: (queryInterface) => ( + queryInterface.sequelize.query('UPDATE "ActivityReports" SET "legacyId" = regexp_replace("legacyId", \'R(\\d)-\', \'R0\\1-\') WHERE "legacyId" ~ \'R\\d-\'') ), down: async () => { /** From aeec8a043a452fb5eb2d0d869f3b8bdbea85f80a Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Wed, 24 Mar 2021 11:22:20 -0400 Subject: [PATCH 9/9] coerceReportId now works with either string or number regions --- src/tools/importActivityReports.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/importActivityReports.js b/src/tools/importActivityReports.js index c381520b0f..13d278211d 100644 --- a/src/tools/importActivityReports.js +++ b/src/tools/importActivityReports.js @@ -187,7 +187,8 @@ function coerceInt(value) { function coerceReportId(value, region) { if (!value) return null; // R14 is a test region, and shouldn't be used in actual reportIds - return value.replace(invalidRegionRE, `R${region.padStart(2, '0')}`); + const paddedRegion = `${region}`.padStart(2, '0'); + return value.replace(invalidRegionRE, `R${paddedRegion}`); } function parseGrantNumbers(value) {