From abfc8cdb427fe0791a1742622ae262e69e30c038 Mon Sep 17 00:00:00 2001 From: Kevin Berry <41717340+51ngul4r1ty@users.noreply.github.com> Date: Mon, 30 Nov 2020 00:39:03 -0500 Subject: [PATCH] Story/000150/story complete status (#192) * added atoll scripts etc. * added backlog item status to UI * added ability to patch backlogitem via api * make sture status has default "N" value * updated data model docs * added reference for checkbox component * bump ver * drop as any Co-authored-by: Kevin --- .vscode/settings.json | 59 +++++----- atoll-core-main.code-workspace | 14 ++- docs/HISTORY.md | 6 + docs/dataModel/DATA_MODEL.md | 5 +- docs/dataModel/DATA_MODEL_BACKLOG_ITEMS.md | 54 +++++++++ .../{ISSUES.md => DATA_MODEL_ISSUES.md} | 0 package-lock.json | 8 +- package.json | 4 +- src/database/model/upgrade.sql | 2 + src/server/api/handlers/backlogItems.ts | 44 +++++++- src/server/api/routes.ts | 4 +- .../api/utils/__tests__/patcher.test.ts | 105 ++++++++++++++++++ src/server/api/utils/patcher.ts | 62 +++++++++++ src/server/api/utils/routerHelper.ts | 5 + src/server/dataaccess/mappers.ts | 31 +++--- src/server/dataaccess/models/BacklogItem.ts | 6 +- 16 files changed, 345 insertions(+), 64 deletions(-) create mode 100644 docs/dataModel/DATA_MODEL_BACKLOG_ITEMS.md rename docs/dataModel/{ISSUES.md => DATA_MODEL_ISSUES.md} (100%) create mode 100644 src/server/api/utils/__tests__/patcher.test.ts create mode 100644 src/server/api/utils/patcher.ts diff --git a/.vscode/settings.json b/.vscode/settings.json index 5ccb292f..00281c44 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,32 +1,29 @@ { - "editor.rulers": [ - 132 - ], - "files.exclude": { - "**/.git": true, - "**/.svn": true, - "**/.hg": true, - "**/CVS": true, - "**/.DS_Store": true, - "node_modules": true, - ".yalc": true - }, - "search.exclude": { - "**/node_modules": true, - "**/bower_components": true, - "**/*.code-search": true, - "build": true, - "coverage": true - }, - "bookmarks.navigateThroughAllFiles": true, - "bookmarks.backgroundLineColor": "#102030", - "editor.formatOnSave": true, - "editor.defaultFormatter": "esbenp.prettier-vscode", - "coverage-gutters.showGutterCoverage": true, - "coverage-gutters.coverageFileNames": [ - "lcov.info" - ], - "coverage-gutters.lcovname": "lcov.info", - "coverage-gutters.coverageReportFileName": "coverage/**/index.html", - "coverage-gutters.showLineCoverage": false -} \ No newline at end of file + "editor.rulers": [132], + "files.exclude": { + "**/.git": true, + "**/.svn": true, + "**/.hg": true, + "**/CVS": true, + "**/.DS_Store": true, + "node_modules": true, + ".yalc": true + }, + "search.exclude": { + "**/node_modules": true, + "**/bower_components": true, + "**/*.code-search": true, + "build": true, + "coverage": true + }, + "bookmarks.navigateThroughAllFiles": true, + "bookmarks.backgroundLineColor": "#102030", + "editor.formatOnSave": true, + "editor.defaultFormatter": "esbenp.prettier-vscode", + "coverage-gutters.showGutterCoverage": true, + "coverage-gutters.coverageFileNames": ["lcov.info"], + "coverage-gutters.lcovname": "lcov.info", + "coverage-gutters.coverageReportFileName": "coverage/**/index.html", + "coverage-gutters.showLineCoverage": false, + "deno.enable": false +} diff --git a/atoll-core-main.code-workspace b/atoll-core-main.code-workspace index c5778fbc..488d197f 100644 --- a/atoll-core-main.code-workspace +++ b/atoll-core-main.code-workspace @@ -5,18 +5,22 @@ }, { "path": "../atoll-shared" + }, + { + "path": "../atoll-scripts" } ], "settings": { "bookmarks.navigateThroughAllFiles": true, "bookmarks.backgroundLineColor": "#102030", "coverage-gutters.showGutterCoverage": true, - "coverage-gutters.coverageFileNames": [ - "lcov.info" - ], + "coverage-gutters.coverageFileNames": ["lcov.info"], "coverage-gutters.lcovname": "lcov.info", "coverage-gutters.coverageReportFileName": "coverage/**/index.html", "coverage-gutters.showLineCoverage": false, - "typescript.tsdk": "atoll-core\\node_modules\\typescript\\lib" + "typescript.tsdk": "atoll-core\\node_modules\\typescript\\lib", + "deno.import_intellisense_origins": { + "https://deno.land": true + } } -} \ No newline at end of file +} diff --git a/docs/HISTORY.md b/docs/HISTORY.md index 838c4695..c7abd425 100644 --- a/docs/HISTORY.md +++ b/docs/HISTORY.md @@ -33,3 +33,9 @@ Atomic Design A quick overview of Atomic Design terminology: https://www.youtube.com/watch?v=aMtnGeiWTyU + +Miscellaneous Component References +---------------------------------- + +How to build the checkbox: +https://webdesign.tutsplus.com/tutorials/how-to-make-custom-accessible-checkboxes-and-radio-buttons--cms-32074 diff --git a/docs/dataModel/DATA_MODEL.md b/docs/dataModel/DATA_MODEL.md index 862724f0..30edda25 100644 --- a/docs/dataModel/DATA_MODEL.md +++ b/docs/dataModel/DATA_MODEL.md @@ -59,8 +59,9 @@ Planned vs Unplanned Splitting vs Continuing a Story ------------------------------- -A story may not be completed in a sprint so it can be continued. Also, if the team recognizes that a story can be done in multiple -parts then it can be split. These are not the same thing and should be treated differently: +It is possible that a story is not be completed in a sprint. In that case it will need to span multiple sprints so that it can be +continued. Also, if the team recognizes that a story can be done in multiple parts then it can be split. These are not the same +thing and should be treated differently: - any part of a story (i.e. a task) can be allocated to a sprint individually (2+ sprints containing same story) - multiple stories can relate to an originating story (there's an inherent hierachy) diff --git a/docs/dataModel/DATA_MODEL_BACKLOG_ITEMS.md b/docs/dataModel/DATA_MODEL_BACKLOG_ITEMS.md new file mode 100644 index 00000000..ec5aedd0 --- /dev/null +++ b/docs/dataModel/DATA_MODEL_BACKLOG_ITEMS.md @@ -0,0 +1,54 @@ +Backlog Items +============= + +Statuses +-------- + +1. Not Started +2. In Progress +3. Completed + +Sub-statuses +------------ + +Within each of the above statuses we have smaller "steps" that are defined below. These +steps could actually be done in parallel and indepently and depend on a team's process and +maturity with CI/CD etc. For example, someone performing manual testing could be doing it +using a branch that the developer has provided for code review before the code review is +complete. To support this, these sub-statuses are treated as independent checks on a +checklist - once all have been completed the issue/story can "exit" its current status and +"enter" the next one. + +**1. Not Started** +1.1. Not Started - Idea +1.2. Not Started - Defined + +**2. In Progress** +2.1. In Progress - In Development +2.2. In Progress - Code Review +2.3. In Progress - Testing +2.4. In Progress - Acceptance + +**3. Ready to Release** +3.1. Ready to Release + +**4. Released** +4.1. Released + +Feature Toggles and "Release" Status +------------------------------------ + +A story could be implemented behind a feature toggle. The code itself could then be +released to production and toggled on later. For the purpose of Atoll's data model this +toggled-on state is the true "Released" status. + + +Blocked "Status" +---------------- + +Unlike other statuses, "blocked" simply means that a work item has been paused because something is preventing it from being +worked on. The workflow in and out of the blocked state can follow a few paths: + +1. Not Started --> Blocked --> In Progress +2. Not Started --> Blocked --> Not Started +3. In Progress --> Blocked --> In Progress diff --git a/docs/dataModel/ISSUES.md b/docs/dataModel/DATA_MODEL_ISSUES.md similarity index 100% rename from docs/dataModel/ISSUES.md rename to docs/dataModel/DATA_MODEL_ISSUES.md diff --git a/package-lock.json b/package-lock.json index a949b4b9..14a63079 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,13 +1,13 @@ { "name": "atoll", - "version": "0.22.5", + "version": "0.23.0", "lockfileVersion": 1, "requires": true, "dependencies": { "@atoll/shared": { - "version": "0.22.5", - "resolved": "https://registry.npmjs.org/@atoll/shared/-/shared-0.22.5.tgz", - "integrity": "sha512-EPholroEDwGNhxjAp1umZflIQwxnWlsfiXmdJ4LPyQUhbcMSE7M1tdlMGQdIy+aPUp6oeGyg1h8HoLwUZ5j0lA==", + "version": "0.23.0", + "resolved": "https://registry.npmjs.org/@atoll/shared/-/shared-0.23.0.tgz", + "integrity": "sha512-4qdoxnJSZkJKNPBUn9XKqpjuiPonXzD1mkxm0VnbaVLp9lg+mKUhYv6lZQ1TlORrfa9iW7A2IEDQ2YDxskOvxA==", "requires": { "@csstools/normalize.css": "10.1.0", "@flopflip/react-broadcast": "10.1.11", diff --git a/package.json b/package.json index 4d44a6f0..63670fb0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "atoll", - "version": "0.22.5", + "version": "0.23.0", "author": { "name": "Kevin Berry", "email": "41717340+51ngul4r1ty@users.noreply.github.com" @@ -57,7 +57,7 @@ "setup": "ts-node ./scripts/setup.ts" }, "dependencies": { - "@atoll/shared": "0.22.5", + "@atoll/shared": "0.23.0", "@flopflip/memory-adapter": "1.6.0", "@flopflip/react-broadcast": "10.1.11", "axios": "0.19.2", diff --git a/src/database/model/upgrade.sql b/src/database/model/upgrade.sql index 2be033de..66aefbfc 100644 --- a/src/database/model/upgrade.sql +++ b/src/database/model/upgrade.sql @@ -21,3 +21,5 @@ alter table sprintbacklogitem add column "status" char(1); update sprintbacklogitem set "status" = 'D'; alter table backlogitem alter column estimate type decimal(10, 2); + +alter table backlogitem add column status char(1); diff --git a/src/server/api/handlers/backlogItems.ts b/src/server/api/handlers/backlogItems.ts index d12d2c52..852785be 100644 --- a/src/server/api/handlers/backlogItems.ts +++ b/src/server/api/handlers/backlogItems.ts @@ -17,6 +17,9 @@ import { } from "../utils/responder"; import { getParamsFromRequest } from "../utils/filterHelper"; import { backlogItemFetcher } from "./fetchers/backlogItemFetcher"; +import { addIdToBody } from "../utils/uuidHelper"; +import { getInvalidPatchMessage, getPatchedItem } from "../utils/patcher"; +import { backlogItemRankFirstItemInserter } from "./inserters/backlogItemRankInserter"; // data access import { @@ -30,10 +33,6 @@ import { } from "../../dataaccess"; import { sequelize } from "../../dataaccess/connection"; -// interfaces/types -import { addIdToBody } from "../utils/uuidHelper"; -import { backlogItemRankFirstItemInserter } from "./inserters/backlogItemRankInserter"; - export const backlogItemsGetHandler = async (req: Request, res: Response) => { const params = getParamsFromRequest(req); const result = await backlogItemFetcher(params.projectId); @@ -325,6 +324,43 @@ export const backlogItemPutHandler = async (req: Request, res: Response) => { } }; +export const backlogItemPatchHandler = async (req: Request, res: Response) => { + const queryParamItemId = req.params.itemId; + if (!queryParamItemId) { + respondWithFailedValidation(res, "Item ID is required in URI path for this operation"); + return; + } + const bodyItemId = req.body.id; + if (bodyItemId) { + respondWithFailedValidation( + res, + `Item ID should only be provided in URI path - Item ID was found in payload: ${bodyItemId}` + ); + return; + } + try { + const backlogItem = await BacklogItemModel.findOne({ + where: { id: queryParamItemId } + }); + if (!backlogItem) { + respondWithNotFound(res, `Unable to find backlogitem to patch with ID ${queryParamItemId}`); + } else { + const originalBacklogItem = mapToBacklogItem(backlogItem); + const invalidPatchMessage = getInvalidPatchMessage(originalBacklogItem, req.body); + if (invalidPatchMessage) { + respondWithFailedValidation(res, `Unable to patch: ${invalidPatchMessage}`); + } else { + const newItem = getPatchedItem(originalBacklogItem, req.body); + + await backlogItem.update(newItem); + respondWithItem(res, backlogItem, originalBacklogItem); + } + } + } catch (err) { + respondWithError(res, err); + } +}; + export const backlogItemsReorderPostHandler = async (req: Request, res: Response) => { const sourceItemId = req.body.sourceItemId; const targetItemId = req.body.targetItemId; diff --git a/src/server/api/routes.ts b/src/server/api/routes.ts index 89b7460b..370c00f5 100644 --- a/src/server/api/routes.ts +++ b/src/server/api/routes.ts @@ -20,7 +20,8 @@ import { backlogItemsPostHandler, backlogItemsReorderPostHandler, backlogItemGetHandler, - backlogItemPutHandler + backlogItemPutHandler, + backlogItemPatchHandler } from "./handlers/backlogItems"; import { sprintPostHandler, sprintsGetHandler, sprintDeleteHandler } from "./handlers/sprints"; import { backlogItemRanksGetHandler, backlogItemRankGetHandler } from "./handlers/backlogItemRanks"; @@ -75,6 +76,7 @@ setupRoutes(router, `/${BACKLOG_ITEM_RESOURCE_NAME}`, { get: backlogItemsGetHand setupRoutes(router, `/${BACKLOG_ITEM_RESOURCE_NAME}/:itemId`, { get: backlogItemGetHandler, put: backlogItemPutHandler, + patch: backlogItemPatchHandler, delete: backlogItemsDeleteHandler }); diff --git a/src/server/api/utils/__tests__/patcher.test.ts b/src/server/api/utils/__tests__/patcher.test.ts new file mode 100644 index 00000000..2e1a889d --- /dev/null +++ b/src/server/api/utils/__tests__/patcher.test.ts @@ -0,0 +1,105 @@ +// test related +import "jest"; + +// code under test +import { getValidationFailureMessage, validateBaseKeys, validatePatchObjects } from "../patcher"; + +describe("Patcher", () => { + describe("validateBaseKeys", () => { + it("should handle empty objects correctly", () => { + const actual = validateBaseKeys({}, {}); + expect(actual.valid).toBeTruthy(); + }); + it("should treat null and empty objects the same", () => { + const actual = validateBaseKeys(null, {}); + expect(actual.valid).toBeTruthy(); + }); + it("should treat empty and null objects the same", () => { + const actual = validateBaseKeys({}, null); + expect(actual.valid).toBeTruthy(); + }); + it("should handle flat object structure correctly", () => { + const actual = validateBaseKeys({ a: 1, b: 2, c: 3 }, { a: 10, b: 20, c: 30 }); + expect(actual.valid).toBeTruthy(); + }); + it("should return invalid when extra fields found", () => { + const actual = validateBaseKeys({ a: 1, b: 2 }, { a: 10, b: 20, c: 30 }); + expect(actual.valid).toBeFalsy(); + expect(actual.extraFields).toStrictEqual(["c"]); + }); + it("should ignore complex object fields in target node", () => { + const actual = validateBaseKeys({ a: 1, complex: { x: 5, y: 9 } }, { a: 10 }); + expect(actual.valid).toBeTruthy(); + }); + it("should treat extra complex object fields in source node as invalid", () => { + const actual = validateBaseKeys({ a: 1 }, { a: 10, complex: { x: 5, y: 9 } }); + expect(actual.valid).toBeFalsy(); + expect(actual.extraFields).toStrictEqual(["complex"]); + }); + it("should ignore complex object extra fields in source node", () => { + const actual = validateBaseKeys({ a: 1, complex: { x: 5, y: 9 } }, { a: 10, complex: { x: 5, y: 9, z: 7 } }); + expect(actual.valid).toBeTruthy(); + }); + }); + describe("validatePatchObjects", () => { + it("should handle unpatchable nested objects correctly", () => { + const actual = validatePatchObjects( + { + a: 1, + b: { + ba: 2, + c: { + ca: 3 + } + } + }, + { + a: 1, + b: { + ba: 2, + c: { + ca: 3, + cb: "invalid" + } + } + } + ); + expect(actual.valid).toBeFalsy(); + expect(actual.extraFields).toStrictEqual(["b.c.cb"]); + }); + it("should handle patchable nested objects correctly", () => { + const actual = validatePatchObjects( + { + a: 1, + b: { + ba: 2, + c: { + ca: 3, + cb: "valid" + } + } + }, + { + a: 1, + b: { + ba: 2, + c: { + ca: 3 + } + } + } + ); + expect(actual.valid).toBeTruthy(); + }); + }); + describe("getValidationFailureMessage", () => { + it("should handle a single invalid nested field", () => { + const actual = getValidationFailureMessage({ valid: false, extraFields: ["b.c.cb"] }); + expect(actual).toEqual("extra fields found in new object: b.c.cb"); + }); + it("should handle valid scenario", () => { + const actual = getValidationFailureMessage({ valid: true, extraFields: [] }); + expect(actual).toEqual("patch object is valid"); + }); + }); +}); diff --git a/src/server/api/utils/patcher.ts b/src/server/api/utils/patcher.ts new file mode 100644 index 00000000..dc19c284 --- /dev/null +++ b/src/server/api/utils/patcher.ts @@ -0,0 +1,62 @@ +export interface PatchValidationResult { + valid: boolean; + extraFields: string[]; +} + +export const validateBaseKeys = (targetNode: any, sourceNode: any): PatchValidationResult => { + const sourceKeys = Object.keys(sourceNode || {}); + const extraFields = []; + sourceKeys.forEach((key) => { + if (!targetNode[key]) { + extraFields.push(key); + } + }); + return { + valid: extraFields.length === 0, + extraFields + }; +}; + +export const validatePatchObjects = (targetNode: any, sourceNode: any): PatchValidationResult => { + const validationResult = validateBaseKeys(targetNode, sourceNode); + const sourceKeys = Object.keys(sourceNode || {}); + sourceKeys.forEach((key) => { + const newSourceNode = sourceNode[key]; + if (typeof newSourceNode === "object") { + const newTargetNode = targetNode[key]; + if (newTargetNode) { + const childValidationResult = validatePatchObjects(newTargetNode, newSourceNode); + childValidationResult.extraFields.forEach((field) => { + validationResult.extraFields.push(key + "." + field); + }); + } + } + }); + return { + valid: validationResult.extraFields.length === 0, + extraFields: validationResult.extraFields + }; +}; + +export const getValidationFailureMessage = (validationResult: PatchValidationResult): string => { + if (validationResult.valid) { + return "patch object is valid"; + } + return "extra fields found in new object: " + validationResult.extraFields.join(", "); +}; + +export const getInvalidPatchMessage = (obj: any, fields: any) => { + const validationResult = validatePatchObjects(obj, fields); + if (!validationResult.valid) { + return getValidationFailureMessage(validationResult); + } + return null; +}; + +export const getPatchedItem = (obj: any, fields: any) => { + const validationResult = validatePatchObjects(obj, fields); + if (!validationResult.valid) { + throw new Error(getValidationFailureMessage(validationResult)); + } + return { ...obj, ...fields }; +}; diff --git a/src/server/api/utils/routerHelper.ts b/src/server/api/utils/routerHelper.ts index ccd9a71c..a50d2fe4 100644 --- a/src/server/api/utils/routerHelper.ts +++ b/src/server/api/utils/routerHelper.ts @@ -7,6 +7,7 @@ export type RouteMethodHandler = RequestHandler; export interface RouteHandlers { get?: RouteMethodHandler; + patch?: RouteMethodHandler; put?: RouteMethodHandler; post?: RouteMethodHandler; delete?: RouteMethodHandler; @@ -18,6 +19,10 @@ export const setupRoutes = (router: Router, path: string, handlers: RouteHandler allowMethods.push("GET"); router.get(path, auth, handlers.get); } + if (handlers.patch) { + allowMethods.push("POST"); + router.patch(path, auth, handlers.patch); + } if (handlers.post) { allowMethods.push("POST"); router.post(path, auth, handlers.post); diff --git a/src/server/dataaccess/mappers.ts b/src/server/dataaccess/mappers.ts index 3f7a16d9..d45eba2a 100644 --- a/src/server/dataaccess/mappers.ts +++ b/src/server/dataaccess/mappers.ts @@ -16,7 +16,8 @@ import { convertDbFloatToNumber } from "./conversionUtils"; export const mapToBacklogItem = (item: any): ApiBacklogItem => { return { ...item.dataValues, - estimate: convertDbFloatToNumber(item.dataValues.estimate) + estimate: convertDbFloatToNumber(item.dataValues.estimate), + status: item.dataValues.status || "N" }; }; @@ -32,20 +33,22 @@ export const mapSprintBacklogToBacklogItem = (item: any): ApiBacklogItemInSprint const sprintBacklogWithItems = { ...item.dataValues }; + const backlogitem = sprintBacklogWithItems.backlogitem; const result = { - id: sprintBacklogWithItems.backlogitem.id, - projectId: sprintBacklogWithItems.backlogitem.projectId, - friendlyId: sprintBacklogWithItems.backlogitem.friendlyId, - externalId: sprintBacklogWithItems.backlogitem.externalId, - rolePhrase: sprintBacklogWithItems.backlogitem.rolePhrase, - storyPhrase: sprintBacklogWithItems.backlogitem.storyPhrase, - reasonPhrase: sprintBacklogWithItems.backlogitem.reasonPhrase, - estimate: convertDbFloatToNumber(sprintBacklogWithItems.backlogitem.estimate), - type: sprintBacklogWithItems.backlogitem.type, - createdAt: sprintBacklogWithItems.backlogitem.createdAt, - updatedAt: sprintBacklogWithItems.backlogitem.updatedAt, - version: sprintBacklogWithItems.backlogitem.version, - displayindex: sprintBacklogWithItems.displayindex + id: backlogitem.id, + projectId: backlogitem.projectId, + friendlyId: backlogitem.friendlyId, + externalId: backlogitem.externalId, + rolePhrase: backlogitem.rolePhrase, + storyPhrase: backlogitem.storyPhrase, + reasonPhrase: backlogitem.reasonPhrase, + estimate: convertDbFloatToNumber(backlogitem.estimate), + type: backlogitem.type, + createdAt: backlogitem.createdAt, + updatedAt: backlogitem.updatedAt, + version: backlogitem.version, + displayindex: sprintBacklogWithItems.displayindex, + status: backlogitem.status }; return result; }; diff --git a/src/server/dataaccess/models/BacklogItem.ts b/src/server/dataaccess/models/BacklogItem.ts index ee49c86b..247de8c9 100644 --- a/src/server/dataaccess/models/BacklogItem.ts +++ b/src/server/dataaccess/models/BacklogItem.ts @@ -46,7 +46,11 @@ BacklogItemModel.init( type: DataTypes.INTEGER, allowNull: true }, - type: DataTypes.STRING(50) + type: DataTypes.STRING(50), + status: { + type: DataTypes.CHAR(1), + allowNull: true + } }, { modelName: "backlogitem",