diff --git a/server/controllers/AutomationController.js b/server/controllers/AutomationController.js index 110351019..025c05aab 100644 --- a/server/controllers/AutomationController.js +++ b/server/controllers/AutomationController.js @@ -1,7 +1,8 @@ const axios = require('axios'); const { getCollectionJobById, - updateCollectionJobById + updateCollectionJobById, + updateCollectionJobTestStatusByQuery } = require('../models/services/CollectionJobService'); const { findOrCreateTestResult @@ -24,7 +25,7 @@ const { } = require('../models/services/TestResultReadService'); const http = require('http'); const { NO_OUTPUT_STRING } = require('../util/constants'); -const getTests = require('../models/services/TestsService'); +const runnableTestsResolver = require('../resolvers/TestPlanReport/runnableTestsResolver'); const getGraphQLContext = require('../graphql-context'); const httpAgent = new http.Agent({ family: 4 }); @@ -99,6 +100,23 @@ const updateJobStatus = async (req, res) => { ...(externalLogsUrl != null && { externalLogsUrl }) }; + // When new status is 'COMPLETED' or 'ERROR' or 'CANCELLED' + // update any CollectionJobTestStatus children still 'QUEUED' to be 'CANCELLED' + if ( + status === COLLECTION_JOB_STATUS.COMPLETED || + status === COLLECTION_JOB_STATUS.CANCELLED || + status === COLLECTION_JOB_STATUS.ERROR + ) { + await updateCollectionJobTestStatusByQuery({ + where: { + collectionJobId: req.params.jobID, + status: COLLECTION_JOB_STATUS.QUEUED + }, + values: { status: COLLECTION_JOB_STATUS.CANCELLED }, + transaction: req.transaction + }); + } + const graphqlResponse = await updateCollectionJobById({ id: req.params.jobID, values: updatePayload, @@ -137,32 +155,29 @@ const getApprovedFinalizedTestResults = async (testPlanRun, context) => { return getFinalizedTestResults({ testPlanReport, context }); }; -const updateOrCreateTestResultWithResponses = async ({ +const getTestByRowIdentifer = async ({ + testPlanRun, testRowIdentifier, + context +}) => { + const tests = await runnableTestsResolver( + testPlanRun.testPlanReport, + null, + context + ); + return tests.find( + test => parseInt(test.rowNumber, 10) === testRowIdentifier + ); +}; + +const updateOrCreateTestResultWithResponses = async ({ + testId, testPlanRun, responses, atVersionId, browserVersionId, context }) => { - const allTestsForTestPlanVersion = await getTests( - testPlanRun.testPlanReport.testPlanVersion - ); - - const isV2 = - testPlanRun.testPlanReport.testPlanVersion.metadata - .testFormatVersion === 2; - - const testId = allTestsForTestPlanVersion.find( - test => - (!isV2 || test.at?.name === 'NVDA') && - parseInt(test.rowNumber, 10) === testRowIdentifier - )?.id; - - if (testId === undefined) { - throwNoTestFoundError(testRowIdentifier); - } - const { testResult } = await findOrCreateTestResult({ testId, testPlanRunId: testPlanRun.id, @@ -243,9 +258,12 @@ const updateJobResults = async (req, res) => { const context = getGraphQLContext({ req }); const { transaction } = context; const { + // Old way - testCsvRow and presentationNumber can be removed + // once all requests from automation are using the new URL parameter testCsvRow, presentationNumber, responses, + status, capabilities: { atName, atVersion: atVersionName, @@ -263,35 +281,69 @@ const updateJobResults = async (req, res) => { `Job with id ${id} is not running, cannot update results` ); } - - /* TODO: Change this to use a better key based lookup system after gh-958 */ - const [at] = await getAts({ search: atName, transaction }); - const [browser] = await getBrowsers({ search: browserName, transaction }); - - const [atVersion, browserVersion] = await Promise.all([ - findOrCreateAtVersion({ - where: { atId: at.id, name: atVersionName }, - transaction - }), - findOrCreateBrowserVersion({ - where: { browserId: browser.id, name: browserVersionName }, - transaction + if (status && !Object.values(COLLECTION_JOB_STATUS).includes(status)) { + throw new HttpQueryError(400, `Invalid status: ${status}`, true); + } + const { testPlanRun } = job; + + // New way: testRowNumber is now a URL request param + // Old way: v1 tests store testCsvRow in rowNumber, v2 tests store presentationNumber in rowNumber + const testRowIdentifier = + req.params.testRowNumber ?? presentationNumber ?? testCsvRow; + const testId = ( + await getTestByRowIdentifer({ + testPlanRun, + testRowIdentifier, + context }) - ]); + )?.id; + + if (testId === undefined) { + throwNoTestFoundError(testRowIdentifier); + } - const processedResponses = convertEmptyStringsToNoOutputMessages(responses); + // status only update, or responses were provided (default to complete) + if (status || responses) { + await updateCollectionJobTestStatusByQuery({ + where: { collectionJobId: id, testId }, + // default to completed if not specified (when results are present) + values: { status: status ?? COLLECTION_JOB_STATUS.COMPLETED }, + transaction: req.transaction + }); + } - // v1 tests store testCsvRow in rowNumber, v2 tests store presentationNumber in rowNumber - const testRowIdentifier = presentationNumber ?? testCsvRow; + // responses were provided + if (responses) { + /* TODO: Change this to use a better key based lookup system after gh-958 */ + const [at] = await getAts({ search: atName, transaction }); + const [browser] = await getBrowsers({ + search: browserName, + transaction + }); - await updateOrCreateTestResultWithResponses({ - testRowIdentifier, - responses: processedResponses, - testPlanRun: job.testPlanRun, - atVersionId: atVersion.id, - browserVersionId: browserVersion.id, - context - }); + const [atVersion, browserVersion] = await Promise.all([ + findOrCreateAtVersion({ + where: { atId: at.id, name: atVersionName }, + transaction + }), + findOrCreateBrowserVersion({ + where: { browserId: browser.id, name: browserVersionName }, + transaction + }) + ]); + + const processedResponses = + convertEmptyStringsToNoOutputMessages(responses); + + await updateOrCreateTestResultWithResponses({ + testId, + responses: processedResponses, + testPlanRun, + atVersionId: atVersion.id, + browserVersionId: browserVersion.id, + context + }); + } res.json({ success: true }); }; diff --git a/server/graphql-schema.js b/server/graphql-schema.js index 92cf9d25a..e060abfa3 100644 --- a/server/graphql-schema.js +++ b/server/graphql-schema.js @@ -102,6 +102,25 @@ const graphqlSchema = gql` The URL where the logs for the job can be found. """ externalLogsUrl: String + """ + An array of individual test status for every runnable test in the Job. + """ + testStatus: [CollectionJobTestStatus] + } + + """ + A status for a specific Test on a specific CollectionJob. + """ + type CollectionJobTestStatus { + """ + The test this status reflects. + """ + test: Test! + """ + The status of the test, which can be "QUEUED", "RUNNING", "COMPLETED", + "ERROR", or "CANCELLED" + """ + status: CollectionJobStatus! } type Browser { diff --git a/server/migrations/20240404171101-addCollectionJobTestStatus.js b/server/migrations/20240404171101-addCollectionJobTestStatus.js new file mode 100644 index 000000000..7abf682ce --- /dev/null +++ b/server/migrations/20240404171101-addCollectionJobTestStatus.js @@ -0,0 +1,58 @@ +'use strict'; + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up(queryInterface, Sequelize) { + return queryInterface.sequelize.transaction(async transaction => { + await queryInterface.createTable( + 'CollectionJobTestStatus', + { + id: { + type: Sequelize.INTEGER, + primaryKey: true, + autoIncrement: true + }, + testId: { + type: Sequelize.STRING, + allowNull: false + }, + collectionJobId: { + type: Sequelize.INTEGER, + allowNull: false, + onDelete: 'CASCADE', + onUpdate: 'CASCADE', + references: { + model: 'CollectionJob', + key: 'id' + } + }, + status: { + type: Sequelize.STRING, + allowNull: false, + defaultValue: 'QUEUED' + } + }, + { transaction } + ); + await queryInterface.addConstraint('CollectionJobTestStatus', { + type: 'unique', + name: 'CollectionJob_Test_unique', + fields: ['collectionJobId', 'testId'], + transaction + }); + }); + }, + + async down(queryInterface) { + return queryInterface.sequelize.transaction(async transaction => { + await queryInterface.removeConstraint( + 'CollectionJobTestStatus', + 'CollectionJob_Test_unique', + { transaction } + ); + await queryInterface.dropTable('CollectionJobTestStatus', { + transaction + }); + }); + } +}; diff --git a/server/models/CollectionJob.js b/server/models/CollectionJob.js index c66499a80..dc57b229d 100644 --- a/server/models/CollectionJob.js +++ b/server/models/CollectionJob.js @@ -45,6 +45,12 @@ module.exports = function (sequelize, DataTypes) { sourceKey: 'testPlanRunId', as: 'testPlanRun' }); + + Model.hasMany(models.CollectionJobTestStatus, { + as: 'testStatus', + foreignKey: 'collectionJobId', + sourceKey: 'id' + }); }; Model.QUEUED = COLLECTION_JOB_STATUS.QUEUED; diff --git a/server/models/CollectionJobTestStatus.js b/server/models/CollectionJobTestStatus.js new file mode 100644 index 000000000..88427a1dd --- /dev/null +++ b/server/models/CollectionJobTestStatus.js @@ -0,0 +1,56 @@ +const { COLLECTION_JOB_STATUS } = require('../util/enums'); + +const MODEL_NAME = 'CollectionJobTestStatus'; + +module.exports = function (sequelize, DataTypes) { + const Model = sequelize.define( + MODEL_NAME, + { + id: { + type: DataTypes.INTEGER, + allowNull: false, + primaryKey: true, + autoIncrement: true + }, + collectionJobId: { + type: DataTypes.INTEGER, + allowNull: false, + references: { + model: 'CollectionJob', + key: 'id' + } + }, + testId: { + type: DataTypes.STRING, + allowNull: null + }, + status: { + type: DataTypes.STRING, + allowNull: false, + defaultValue: COLLECTION_JOB_STATUS.QUEUED + } + }, + { + timestamps: false, + tableName: MODEL_NAME + } + ); + + Model.associate = function (models) { + Model.belongsTo(models.CollectionJob, { + foreignKey: 'collectionJobId', + targetKey: 'id', + as: 'collectionJob', + onDelete: 'CASCADE', + onUpdate: 'CASCADE' + }); + }; + + Model.QUEUED = COLLECTION_JOB_STATUS.QUEUED; + Model.RUNNING = COLLECTION_JOB_STATUS.RUNNING; + Model.COMPLETED = COLLECTION_JOB_STATUS.COMPLETED; + Model.CANCELLED = COLLECTION_JOB_STATUS.CANCELLED; + Model.ERROR = COLLECTION_JOB_STATUS.ERROR; + + return Model; +}; diff --git a/server/models/services/CollectionJobService.js b/server/models/services/CollectionJobService.js index a73dbd20e..bbe756bd9 100644 --- a/server/models/services/CollectionJobService.js +++ b/server/models/services/CollectionJobService.js @@ -1,5 +1,5 @@ const ModelService = require('./ModelService'); -const { CollectionJob } = require('../'); +const { CollectionJob, CollectionJobTestStatus } = require('../'); const { COLLECTION_JOB_ATTRIBUTES, TEST_PLAN_ATTRIBUTES, @@ -8,7 +8,8 @@ const { TEST_PLAN_VERSION_ATTRIBUTES, AT_ATTRIBUTES, BROWSER_ATTRIBUTES, - USER_ATTRIBUTES + USER_ATTRIBUTES, + COLLECTION_JOB_TEST_STATUS_ATTRIBUTES } = require('./helpers'); const { COLLECTION_JOB_STATUS } = require('../../util/enums'); const { Op } = require('sequelize'); @@ -175,10 +176,21 @@ const userAssociation = userAttributes => ({ attributes: userAttributes }); +/** + * @param {string[]} collectionJobTestStatusAttributes - attributes to be returned in the result + * @returns {{association: string, attributes: string[]}} + */ +const collectionJobTestStatusAssociation = + collectionJobTestStatusAttributes => ({ + association: 'testStatus', + attributes: collectionJobTestStatusAttributes + }); + /** * @param {object} options * @param {object} options.values - CollectionJob to be created - * @param {string[]} options.collectionJobAttributes - TestPlanRun attributes to be returned in the result + * @param {string[]} options.collectionJobAttributes - CollectionJob attributes to be returned in the result + * @param {string[]} options.collectionJobTestStatusAttributes - CollectionJobTestStatus attributes to be returned in the result * @param {string[]} options.testPlanReportAttributes - TestPlanReport attributes to be returned in the result * @param {string[]} options.testPlanVersionAttributes - TestPlanVersion attributes to be returned in the result * @param {string[]} options.testPlanAttributes - TestPlanVersion attributes to be returned in the result @@ -195,6 +207,7 @@ const createCollectionJob = async ({ testPlanReportId }, collectionJobAttributes = COLLECTION_JOB_ATTRIBUTES, + collectionJobTestStatusAttributes = COLLECTION_JOB_TEST_STATUS_ATTRIBUTES, testPlanReportAttributes = TEST_PLAN_REPORT_ATTRIBUTES, testPlanRunAttributes = TEST_PLAN_RUN_ATTRIBUTES, testPlanVersionAttributes = TEST_PLAN_VERSION_ATTRIBUTES, @@ -222,10 +235,29 @@ const createCollectionJob = async ({ transaction }); + // create QUEUED status entries for each test in the test plan run + const context = getGraphQLContext({ req: { transaction } }); + const tests = await runnableTestsResolver( + testPlanRun.testPlanReport, + null, + context + ); + await ModelService.bulkCreate(CollectionJobTestStatus, { + valuesList: tests.map(test => ({ + testId: test.id, + collectionJobId: collectionJobResult.id, + status: COLLECTION_JOB_STATUS.QUEUED + })), + transaction + }); + return ModelService.getById(CollectionJob, { id: collectionJobResult.id, attributes: collectionJobAttributes, include: [ + collectionJobTestStatusAssociation( + collectionJobTestStatusAttributes + ), testPlanRunAssociation( testPlanRunAttributes, userAttributes, @@ -243,7 +275,8 @@ const createCollectionJob = async ({ /** * @param {object} options * @param {string} options.id - id for the CollectionJob - * @param {string[]} options.collectionJobAttributes - TestPlanRun attributes to be returned in the result + * @param {string[]} options.collectionJobAttributes - CollectionJob attributes to be returned in the result + * @param {string[]} options.collectionJobTestStatusAttributes - CollectionJobTestStatus attributes to be returned in the result * @param {string[]} options.testPlanReportAttributes - TestPlanReport attributes to be returned in the result * @param {string[]} options.testPlanVersionAttributes - TestPlanVersion attributes to be returned in the result * @param {string[]} options.testPlanAttributes - TestPlanVersion attributes to be returned in the result @@ -256,6 +289,7 @@ const createCollectionJob = async ({ const getCollectionJobById = async ({ id, collectionJobAttributes = COLLECTION_JOB_ATTRIBUTES, + collectionJobTestStatusAttributes = COLLECTION_JOB_TEST_STATUS_ATTRIBUTES, testPlanReportAttributes = TEST_PLAN_REPORT_ATTRIBUTES, testPlanRunAttributes = TEST_PLAN_RUN_ATTRIBUTES, testPlanVersionAttributes = TEST_PLAN_VERSION_ATTRIBUTES, @@ -269,6 +303,9 @@ const getCollectionJobById = async ({ id, attributes: collectionJobAttributes, include: [ + collectionJobTestStatusAssociation( + collectionJobTestStatusAttributes + ), testPlanRunAssociation( testPlanRunAttributes, userAttributes, @@ -287,7 +324,8 @@ const getCollectionJobById = async ({ * @param {object} options * @param {string|any} options.search - use this to combine with {@param filter} to be passed to Sequelize's where clause * @param {object} options.where - use this define conditions to be passed to Sequelize's where clause - * @param {string[]} options.collectionJobAttributes - Browser attributes to be returned in the result + * @param {string[]} options.collectionJobAttributes - CollectionJob attributes to be returned in the result + * @param {string[]} options.collectionJobTestStatusAttributes - CollectionJobTestStatus attributes to be returned in the result * @param {string[]} options.testPlanReportAttributes - TestPlanReport attributes to be returned in the result * @param {string[]} options.testPlanVersionAttributes - TestPlanVersion attributes to be returned in the result * @param {string[]} options.testPlanAttributes - TestPlanVersion attributes to be returned in the result @@ -306,6 +344,7 @@ const getCollectionJobs = async ({ search, where = {}, collectionJobAttributes = COLLECTION_JOB_ATTRIBUTES, + collectionJobTestStatusAttributes = COLLECTION_JOB_TEST_STATUS_ATTRIBUTES, testPlanReportAttributes = TEST_PLAN_REPORT_ATTRIBUTES, testPlanRunAttributes = TEST_PLAN_RUN_ATTRIBUTES, testPlanVersionAttributes = TEST_PLAN_VERSION_ATTRIBUTES, @@ -324,6 +363,9 @@ const getCollectionJobs = async ({ where, attributes: collectionJobAttributes, include: [ + collectionJobTestStatusAssociation( + collectionJobTestStatusAttributes + ), testPlanRunAssociation( testPlanRunAttributes, userAttributes, @@ -382,7 +424,8 @@ const triggerWorkflow = async (job, testIds, { transaction }) => { * @param {object} options * @param {string} options.id - id of the CollectionJob to be updated * @param {object} options.values - values to be used to update columns for the record being referenced for {@param id} - * @param {string[]} options.collectionJobAttributes - Browser attributes to be returned in the result + * @param {string[]} options.collectionJobAttributes - CollectionJob attributes to be returned in the result + * @param {string[]} options.collectionJobTestStatusAttributes - CollectionJobTestStatus attributes to be returned in the result * @param {string[]} options.testPlanReportAttributes - TestPlanReport attributes to be returned in the result * @param {string[]} options.testPlanVersionAttributes - TestPlanVersion attributes to be returned in the result * @param {string[]} options.testPlanAttributes - TestPlanVersion attributes to be returned in the result @@ -396,6 +439,7 @@ const updateCollectionJobById = async ({ id, values = {}, collectionJobAttributes = COLLECTION_JOB_ATTRIBUTES, + collectionJobTestStatusAttributes = COLLECTION_JOB_TEST_STATUS_ATTRIBUTES, testPlanReportAttributes = TEST_PLAN_REPORT_ATTRIBUTES, testPlanRunAttributes = TEST_PLAN_RUN_ATTRIBUTES, testPlanVersionAttributes = TEST_PLAN_VERSION_ATTRIBUTES, @@ -415,6 +459,9 @@ const updateCollectionJobById = async ({ id, attributes: collectionJobAttributes, include: [ + collectionJobTestStatusAssociation( + collectionJobTestStatusAttributes + ), testPlanRunAssociation( testPlanRunAttributes, userAttributes, @@ -592,6 +639,30 @@ const restartCollectionJob = async ({ id }, { transaction }) => { return triggerWorkflow(job, [], { transaction }); }; +/** + * CollectionJobTestStatus updates + */ + +/** + * Update CollectionJobTestStatus entries in bulk via query. + * @param {object} options + * @param {object} options.where - values of the CollectionJobTestStatus record to be updated + * @param {object} options.values - values to be used to update columns for the record being referenced + * @param {*} options.transaction - Sequelize transaction + * @returns {Promise<*>} + */ +const updateCollectionJobTestStatusByQuery = ({ + where, + values = {}, + transaction +}) => { + return ModelService.update(CollectionJobTestStatus, { + values, + where, + transaction + }); +}; + module.exports = { // Basic CRUD createCollectionJob, @@ -603,5 +674,7 @@ module.exports = { scheduleCollectionJob, restartCollectionJob, cancelCollectionJob, - retryCanceledCollections + retryCanceledCollections, + // Basic CRUD for CollectionJobTestStatus + updateCollectionJobTestStatusByQuery }; diff --git a/server/models/services/helpers.js b/server/models/services/helpers.js index 32c75b7cb..f7f188c71 100644 --- a/server/models/services/helpers.js +++ b/server/models/services/helpers.js @@ -11,7 +11,8 @@ const { User, UserRoles, UserAts, - CollectionJob + CollectionJob, + CollectionJobTestStatus } = require('../index'); /** @@ -39,5 +40,8 @@ module.exports = { USER_ATTRIBUTES: getSequelizeModelAttributes(User), USER_ROLES_ATTRIBUTES: getSequelizeModelAttributes(UserRoles), USER_ATS_ATTRIBUTES: getSequelizeModelAttributes(UserAts), - COLLECTION_JOB_ATTRIBUTES: getSequelizeModelAttributes(CollectionJob) + COLLECTION_JOB_ATTRIBUTES: getSequelizeModelAttributes(CollectionJob), + COLLECTION_JOB_TEST_STATUS_ATTRIBUTES: getSequelizeModelAttributes( + CollectionJobTestStatus + ) }; diff --git a/server/resolvers/CollectionJob/index.js b/server/resolvers/CollectionJob/index.js new file mode 100644 index 000000000..fa8c5525d --- /dev/null +++ b/server/resolvers/CollectionJob/index.js @@ -0,0 +1,2 @@ +const testStatus = require('./testStatusResolver'); +module.exports = { testStatus }; diff --git a/server/resolvers/CollectionJob/testStatusResolver.js b/server/resolvers/CollectionJob/testStatusResolver.js new file mode 100644 index 000000000..1c46a5d2b --- /dev/null +++ b/server/resolvers/CollectionJob/testStatusResolver.js @@ -0,0 +1,15 @@ +// eslint-disable-next-line no-unused-vars +const testStatusResolver = (collectionJob, _, context) => { + // resolve testStatus.test for each test + // testPlanRun "might" be null if the testPlanRun had been deleted + const testPlanTests = + collectionJob.testPlanRun?.testPlanReport.testPlanVersion.tests ?? []; + const tests = new Map(testPlanTests.map(test => [test.id, test])); + + return collectionJob.testStatus.map(status => ({ + ...status.dataValues, + // if not found, at least return the test id + test: tests.get(status.testId) ?? { id: status.testId } + })); +}; +module.exports = testStatusResolver; diff --git a/server/resolvers/index.js b/server/resolvers/index.js index 5e097b81d..5cf7dece6 100644 --- a/server/resolvers/index.js +++ b/server/resolvers/index.js @@ -41,6 +41,7 @@ const TestPlanRunOperations = require('./TestPlanRunOperations'); const TestResultOperations = require('./TestResultOperations'); const TestPlanVersionOperations = require('./TestPlanVersionOperations'); const CollectionJobOperations = require('./CollectionJobOperations'); +const CollectionJob = require('./CollectionJob'); const TestPlanRun = require('./TestPlanRun'); const Test = require('./Test'); const ScenarioResult = require('./ScenarioResult'); @@ -84,6 +85,7 @@ const resolvers = { AtOperations, AtVersionOperations, BrowserOperations, + CollectionJob, User, TestPlan, TestPlanVersion, diff --git a/server/routes/automation.js b/server/routes/automation.js index 460516af7..50bbe7605 100644 --- a/server/routes/automation.js +++ b/server/routes/automation.js @@ -10,10 +10,14 @@ const { handleError } = require('../middleware/handleError'); const router = Router(); +// Old Way (backwards compat) router.post('/:jobID/update', verifyAutomationScheduler, updateJobStatus); - router.post('/:jobID/result', verifyAutomationScheduler, updateJobResults); +// New way +router.post('/:jobID', verifyAutomationScheduler, updateJobStatus); +router.post('/:jobID/test/:testRowNumber', verifyAutomationScheduler, updateJobResults); + router.use(handleError); module.exports = router;