Skip to content

Commit

Permalink
Feature/45933 bulk upload schools (#2002)
Browse files Browse the repository at this point in the history
* [admin] add nodemon package back in

* [admin] add view page to upload the organisations file

* [admin] - util update

clean up local files when destroying the dev local env

* [admin] bugfix auto-upload files

Clarify the tmp folder for uploaded files using full path

* [admin] comment out azure-upload as probably not needed

* [load-test] update reset script to drop results schema tables

* [admin] add initial upload work

* [admin] debounce form submit

* [load-test] update reset script to clear out new schools

* [admin] improve form

* [admin] add validation error reporting

* [db] add new job type

* [admin] add initial job life-cycle tracking on the school upload

* [load-test] add back in removed line for users

* [load-test] security upgrade

relock lockfie to upgrade [email protected] to [email protected]

* [admin] refactor; add test

* [admin] add tests

* [tslib] add test

* remove debug line

* [tslib] add test

* [tslib/school-import] refactor to clean code and add tests

* Simplify

* [admin] add feature to download organisation job results

* bump minor versions

* bump versions

* [admin/pupil] remove sinon

* convert to jest

* [admin] build fix

* [admin] optimise sql as per review comment

* Update upload-pupil-census.ejs

Add csrf token back in

Co-authored-by: Mohsen Qureshi <[email protected]>
  • Loading branch information
jon-shipley and activemq authored Oct 18, 2021
1 parent dc4f0da commit a90b87f
Show file tree
Hide file tree
Showing 27 changed files with 2,098 additions and 1,167 deletions.
15 changes: 9 additions & 6 deletions admin/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ app.set('view engine', 'ejs')
app.use(partials())
busboy.extend(app, {
upload: true,
path: 'data/files',
path: path.join(__dirname, 'data', 'files'),
allowedPath: (url) => allowedPath(url),
mimeTypeLimit: [
'text/csv', // correct
Expand All @@ -151,10 +151,12 @@ busboy.extend(app, {
]
})

const allowedPath = (url) => (/^\/pupil-register\/pupil\/add-batch-pupils$/).test(url) ||
const allowedPath = (url) =>
(/^\/pupil-register\/pupil\/add-batch-pupils$/).test(url) ||
(/^\/test-developer\/upload-new-form$/).test(url) ||
(/^\/test-developer\/upload$/).test(url) ||
(/^\/service-manager\/upload-pupil-census\/upload$/).test(url) ||
(/^\/test-developer\/upload$/).test(url)
(/^\/service-manager\/organisations\/upload$/).test(url)

// as we run in container over http, we must set up proxy trust for secure cookies
let secureCookie = false
Expand Down Expand Up @@ -243,9 +245,10 @@ if (config.Auth.mode === authModes.dfeSignIn) {

// Middleware to upload all files uploaded to Azure Blob storage
// Should be configured after busboy
if (config.AZURE_STORAGE_CONNECTION_STRING) {
app.use(require('./lib/azure-upload'))
}
// Commented out 2021-09-23 as this has never been used.
// if (config.AZURE_STORAGE_CONNECTION_STRING) {
// app.use(require('./lib/azure-upload'))
// }

app.use(function (req, res, next) {
// make the user and isAuthenticated vars available in the view templates
Expand Down
8 changes: 7 additions & 1 deletion admin/assets/javascripts/custom-file-upload.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@ $(function () {
if (isSubmitted) {
return false
}
$formElement.val('Sending...')
// Update the form submit button to say it's uploading
var submitButton = $('#upload-form-submit')
if (submitButton.text) {
$('#upload-form-submit').text('Sending...')
} else if (submitButton.val) {
$formElement.val('Sending...')
}
isSubmitted = true
return true
})
Expand Down
52 changes: 52 additions & 0 deletions admin/controllers/service-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const scePresenter = require('../helpers/sce')
const schoolService = require('../services/school.service')
const featureToggles = require('feature-toggles')
const { formUtil, formUtilTypes } = require('../lib/form-util')
const organisationBulkUploadService = require('../services/organisation-bulk-upload.service')

const controller = {

Expand Down Expand Up @@ -434,6 +435,57 @@ const controller = {
}
return next(error)
}
},

getUploadOrganisations: async function getUploadOrganisations (req, res, next, error = new ValidationError()) {
req.breadcrumbs('Manage organisations', '/service-manager/organisations')
res.locals.pageTitle = 'Bulk upload organisations'
req.breadcrumbs(res.locals.pageTitle)
const jobSlug = req.params.jobSlug
let jobStatus
try {
if (jobSlug !== undefined) {
jobStatus = await organisationBulkUploadService.getUploadStatus(jobSlug)
}

res.render('service-manager/bulk-upload-organisations', {
breadcrumbs: req.breadcrumbs(),
fileErrors: error,
jobStatus: jobStatus
})
} catch (error) {
return next(error)
}
},

postUploadOrganisations: async function postUploadOrganisations (req, res, next) {
const uploadFile = req.files?.fileOrganisations
try {
const validationError = await organisationBulkUploadService.validate(uploadFile)
if (validationError.hasError()) {
return controller.getUploadOrganisations(req, res, next, validationError)
}
const jobSlug = await organisationBulkUploadService.upload(uploadFile)
req.flash('info', 'File has been uploaded')
res.redirect(`/service-manager/organisations/upload/${jobSlug.toLowerCase()}`)
} catch (error) {
return next(error)
}
},

downloadJobOutput: async function downloadJobOutput (req, res, next) {
const slug = req.params.slug
try {
const zipResults = await organisationBulkUploadService.getZipResults(slug)
res.set({
'Content-Disposition': 'attachment; filename="job-output.zip"',
'Content-type': 'application/octet-stream',
'Content-Length': zipResults.length // Buffer.length (bytes)
})
res.send(zipResults)
} catch (error) {
next(error)
}
}
}

Expand Down
31 changes: 20 additions & 11 deletions admin/lib/azure-upload.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict'

const path = require('path')
const moment = require('moment')
const blobService = require('../services/data-access/azure-blob.data.service')

Expand All @@ -14,19 +13,29 @@ module.exports = async function (req, res, next) {
return next()
}

await blobService.createContainerIfNotExistsAsync(container)
try {
await blobService.createContainerIfNotExistsAsync(container)
} catch (error) {
console.error(`Failed to create container ${container}: ${error.message}`)
}

// Container exists and is private
Object.getOwnPropertyNames(req.files).forEach(field => {
// TODO: add _userid to the filename
const files = Object.getOwnPropertyNames(req.files)
for (const field of files) {
const files = req.files[field]
// If only 1 file is being upload created an array with a single file object
const submittedFilesObj = Array.isArray(files) ? files : [files]
submittedFilesObj.map(async (fileObj) => {
const remoteFilename = moment().format('YYYYMMDDHHmmss') + '-' + fileObj.field + '-' + fileObj.uuid
const localFilename = path.join(__dirname, '/../', fileObj.file)
await blobService.createBlockBlobFromLocalFileAsync(container, remoteFilename, localFilename)
})
next()
})
for (const fileObj of submittedFilesObj) {
// Create a safe derivative of the user name for use in the filename
const userId = req.user?.UserName?.replace(/[^A-Za-z0-9]/, '')
const remoteFilename = moment().format('YYYYMMDDHHmmss') + '-' + userId + '-' + fileObj.field + '-' + fileObj.uuid
const localFilename = fileObj.file
try {
await blobService.createBlockBlobFromLocalFileAsync(container, remoteFilename, localFilename)
} catch (error) {
console.error(`ERROR: Failed to upload file ${fileObj.filename} to ${container}`)
}
}
}
next()
}
9 changes: 5 additions & 4 deletions admin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"watch:integration": "yarn jest --watch --coverage=no --config ./tests-integration/jest.integration.config.js"
},
"mtc": {
"assets-version": "cf97ab52a7b50fb51fd6f2673c4ec147"
"assets-version": "e93c2fae4e22b00c9e51494b9880f452"
},
"engines": {
"node": ">= 12"
Expand All @@ -40,8 +40,9 @@
},
"dependencies": {
"@azure/service-bus": "^7.0.5",
"adm-zip": "^0.5.7",
"applicationinsights": "^2.1.5",
"axios": "^0.21.3",
"axios": "^0.22.0",
"azure-storage": "^2.10.4",
"bcryptjs": "^2.4.3",
"bluebird": "^3.5.1",
Expand All @@ -62,15 +63,15 @@
"fs-extra": "^10.0.0",
"govuk-frontend": "^3.2.0",
"helmet": "^4.3.1",
"ioredis": "4.27.8",
"ioredis": "4.27.10",
"jsonwebtoken": "^8.1.1",
"login.dfe.async-retry": "git+https://github.com/DFE-Digital/login.dfe.async-retry.git#v1.0",
"lz-string": "^1.4.4",
"md5": "^2.2.1",
"merge-stream": "^2.0.0",
"moment-timezone": "^0.5.23",
"morgan": "~1.10.0",
"mssql": "7.2.0",
"mssql": "7.2.1",
"nocache": "^3.0.1",
"node-dir": "^0.1.17",
"openid-client": "^4.2.2",
Expand Down
16 changes: 16 additions & 0 deletions admin/routes/service-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ router.post('/organisations/search',
serviceManagerController.postSearch
)

router.get('/organisations/upload/:jobSlug?',
isAuthenticated(roles.serviceManager),
serviceManagerController.getUploadOrganisations
)

router.post('/organisations/upload',
isAuthenticated(roles.serviceManager),
serviceManagerController.postUploadOrganisations
)

router.get(
'/organisations/:slug',
isAuthenticated([roles.serviceManager]),
Expand All @@ -102,4 +112,10 @@ router.post('/organisations/:slug/edit',
isAuthenticated(roles.serviceManager),
serviceManagerController.postEditOrganisation
)

router.get('/job/:slug',
isAuthenticated(roles.serviceManager),
serviceManagerController.downloadJobOutput
)

module.exports = router
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const service = {
school_id: schoolId
})
if (response.status === 200) {
// @ts-ignore - inferred type of {schoolId: any} seems speculative.
return response.data.pin
} else {
throw new Error(`non 200 status code returned. ${response.status}:${response.statusText}`)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
const sqlService = require('./sql.service')
const uuid = require('uuid')
const R = require('ramda')

const organisationBulkUploadDataService = {
/**
* Determine if an existing organisation upload job is running.
* @returns {Promise<boolean>}
*/
isExistingJob: async function isExistingJob () {
const sql = `SELECT COUNT(*) as count
FROM mtc_admin.job j
join mtc_admin.jobType jt on (j.jobType_id = jt.id)
JOIN mtc_admin.jobStatus js on (j.jobStatus_id = js.id)
WHERE js.jobStatusCode IN ('SUB', 'PRC')
AND jt.jobTypeCode = 'ORG'`
const res = await sqlService.query(sql)
const count = R.propOr(0, 'count', res[0])
return count > 0
},

/**
* Create a new job record for Organisation file upload
* @returns {Promise<undefined | string>} The newly inserted ID of the job record
*/
createJobRecord: async function createJobRecord () {
if (await this.isExistingJob()) {
throw new Error('A job is already running')
}

const sql = `
INSERT INTO mtc_admin.job (jobStatus_id, jobType_id, jobInput)
OUTPUT INSERTED.urlSlug
VALUES ((SELECT id from mtc_admin.jobStatus where jobStatusCode = 'SUB'),
(SELECT id from mtc_admin.jobType where jobTypeCode = 'ORG'),
'File upload')
`
const retVal = await sqlService.query(sql)
return retVal[0].urlSlug
},

/**
* Return the job record by urlSlug
* @param jobSlug
* @returns {Promise<Object>}
*/
sqlGetJobData: async function sqlGetJobData (jobSlug) {
if (!uuid.validate(jobSlug)) {
throw new Error(`Invalid UUID for jobSlug: ${jobSlug}`)
}
const sql = `
SELECT j.*,
jt.description as jobTypeDescription,
jt.jobTypeCode,
js.description as jobStatusDescription,
js.jobStatusCode
FROM mtc_admin.job j
JOIN mtc_admin.jobType jt on (j.jobType_id = jt.id)
JOIN mtc_admin.jobStatus js on (j.jobStatus_id = js.id)
AND jt.jobTypeCode = 'ORG'
AND j.urlSlug = @slug
`
const params = [
{ name: 'slug', value: jobSlug, type: sqlService.TYPES.UniqueIdentifier }
]
const data = await sqlService.query(sql, params)
return R.head(data)
}
}

module.exports = organisationBulkUploadDataService
73 changes: 73 additions & 0 deletions admin/services/organisation-bulk-upload.service.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
const azureBlobDataService = require('./data-access/azure-blob.data.service')
const fileValidator = require('../lib/validator/file-validator.js')
const organisationBulkUploadDataService = require('./data-access/organisation-bulk-upload.data.service')
const AdmZip = require('adm-zip')

// Files get uploaded to this container. dns naming conventions.
const container = 'school-import'

const organisationBulkUploadService = {
/**
* Validate an upload file for basic errors
* @param uploadFile
* @returns {Promise<*>}
*/
validate: function validate (uploadFile) {
return fileValidator.validate(uploadFile, 'file-upload')
},

/**
*
* @param {{ file: string, filename: string }} uploadFile
* @returns {Promise<string>} job slug UUID
*/
upload: async function upload (uploadFile) {
const remoteFilename = uploadFile.filename
await azureBlobDataService.createContainerIfNotExistsAsync(container)

// Create the job record first as it acts as a singleton - we only want one file upload at a time. If we can't
// create the job record, then we abort.
const jobSlug = await organisationBulkUploadDataService.createJobRecord()
await azureBlobDataService.createBlockBlobFromLocalFileAsync(container, remoteFilename, uploadFile.file)
return jobSlug
},

/**
* Get the status of the file upload
* @param jobSlug uuid
* @returns {Promise<{urlSlug: (string|*), code: (string|*), description: (string|*), errorOutput: (string|*), jobOutput: any}>}
*/
getUploadStatus: async function getUploadStatus (jobSlug) {
const jobData = await organisationBulkUploadDataService.sqlGetJobData(jobSlug)
if (jobData === undefined) {
throw new Error('Job ID not found')
}
return {
description: jobData.jobStatusDescription,
code: jobData.jobStatusCode,
errorOutput: jobData.errorOutput,
jobOutput: JSON.parse(jobData.jobOutput),
urlSlug: jobData.urlSlug
}
},

/**
* Return a buffer containing the Zipped data (error.txt, output.txt) for the service-manager to download.
* @param jobSlug
* @returns {Promise<Buffer>}
*/
getZipResults: async function getZipResults (jobSlug) {
if (jobSlug === undefined) {
throw new Error('Missing job ID')
}
const jobData = await this.getUploadStatus(jobSlug)
const zip = new AdmZip()
// noinspection JSCheckFunctionSignatures - 3rd and 4th args are optional
zip.addFile('error.txt', jobData.jobOutput.stderr.join('\n'))
// noinspection JSCheckFunctionSignatures - 3rd and 4th args are optional
zip.addFile('output.txt', jobData.jobOutput.stdout.join('\n'))
return zip.toBuffer()
}
}

module.exports = organisationBulkUploadService
Loading

0 comments on commit a90b87f

Please sign in to comment.