Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

TODO comments #377

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/server/api/handlers/backlogItemParts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ export const backlogItemPartPatchHandler = async (req: Request, res: Response) =
const queryParamItemId = req.params.itemId;

if (!queryParamItemId) {
// TODO: Introduce a new pattern where the handlerContext handles all responses like this
// then it can also handle 1) closing open transactions, 2) calling `finish`
respondWithFailedValidation(res, "Item ID is required in URI path for this operation");
return;
}
Expand Down Expand Up @@ -122,6 +124,7 @@ export const backlogItemPartPatchHandler = async (req: Request, res: Response) =
}
}
}
// TODO: eliminate direct calls to finish - all responses should call finish
finish(handlerContext);
} catch (err) {
await handleUnexpectedErrorResponse(handlerContext, err);
Expand Down
11 changes: 11 additions & 0 deletions src/server/api/handlers/backlogItems.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import { fetchBacklogItemParts } from "./fetchers/backlogItemPartFetcher";
import { ApiBacklogItemPartWithSprintId, fetchAllocatedAndUnallocatedBacklogItemParts } from "./helpers/sprintBacklogItemHelper";

export const backlogItemsGetHandler = async (req: Request, res: Response) => {
// TODO: Convert this to use the standard patterns with transaction (if there isn't just a single query)
const params = getParamsFromRequest(req);
let result: BacklogItemsResult | RestApiStatusAndMessageOnly;
if (params.projectId && params.backlogItemDisplayId) {
Expand All @@ -92,6 +93,7 @@ export interface BacklogItemGetParams extends core.ParamsDictionary {
export const backlogItemGetHandler = async (req: Request<BacklogItemGetParams>, res: Response) => {
try {
const id = req.params.itemId;
// TODO: Convert this to use the standard patterns (using a transaction)
const itemWithSprintInfoResult = await fetchBacklogItemWithSprintAllocationInfo(id);
if (isRestApiItemResult(itemWithSprintInfoResult)) {
respondWithObj(res, itemWithSprintInfoResult);
Expand All @@ -108,6 +110,7 @@ export const backlogItemsDeleteHandler = async (req: Request, res: Response) =>
let committing = false;
let transaction: Transaction;
try {
// TODO: Convert this to use the standard patterns
transaction = await sequelize.transaction({ isolationLevel: Transaction.ISOLATION_LEVELS.SERIALIZABLE });
await sequelize.query('SET CONSTRAINTS "backlogitemrank_backlogitemId_fkey" DEFERRED;', { transaction });
await sequelize.query('SET CONSTRAINTS "backlogitemrank_nextbacklogitemId_fkey" DEFERRED;', { transaction });
Expand Down Expand Up @@ -197,6 +200,7 @@ const getNewCounterValue = async (projectId: string, backlogItemType: string) =>
let transaction: Transaction;
try {
let rolledBack = false;
// TODO: Convert this to use the standard patterns
transaction = await sequelize.transaction({ isolationLevel: Transaction.ISOLATION_LEVELS.SERIALIZABLE });
let projectSettingsItem: any = await ProjectSettingsDataModel.findOne({
where: { projectId: projectId },
Expand Down Expand Up @@ -254,13 +258,15 @@ export const backlogItemsPostHandler = async (req: Request, res: Response) => {
let transaction: Transaction;
try {
let rolledBack = false;
// TODO: Convert this to use the standard patterns
transaction = await sequelize.transaction({ isolationLevel: Transaction.ISOLATION_LEVELS.SERIALIZABLE });
await sequelize.query('SET CONSTRAINTS "backlogitemrank_backlogitemId_fkey" DEFERRED;', { transaction });
await sequelize.query('SET CONSTRAINTS "backlogitemrank_nextbacklogitemId_fkey" DEFERRED;', { transaction });
const updateBacklogItemPartResult = getUpdatedBacklogItemWhenStatusChanges(null, bodyWithId);
const newItem = updateBacklogItemPartResult.backlogItem;
const addedBacklogItem = await BacklogItemDataModel.create(newItem, { transaction } as CreateOptions);
if (!prevBacklogItemId) {
// TODO: Change name of this and investigate why Postman collection POST returns a "backlogitempart.version cannot be null error"
await backlogItemRankFirstItemInserter(newItem, transaction);
} else {
const result = await backlogItemRankSubsequentItemInserter(newItem, transaction, prevBacklogItemId);
Expand Down Expand Up @@ -330,6 +336,7 @@ export const backlogItemPutHandler = async (req: Request, res: Response) => {
}
let transaction: Transaction;
try {
// TODO: Convert this to use the standard patterns
transaction = await sequelize.transaction({ isolationLevel: Transaction.ISOLATION_LEVELS.SERIALIZABLE });
const backlogItem = await BacklogItemDataModel.findOne({
where: { id: bodyItemId },
Expand Down Expand Up @@ -376,6 +383,7 @@ export const backlogItemsReorderPostHandler = async (req: Request, res: Response
let transaction: Transaction;
try {
let rolledBack = false;
// TODO: Convert this to use the standard patterns
transaction = await sequelize.transaction({ isolationLevel: Transaction.ISOLATION_LEVELS.SERIALIZABLE });

// 1. Unlink source item from old location
Expand Down Expand Up @@ -427,6 +435,7 @@ export const backlogItemJoinUnallocatedPartsPostHandler = async (req: Request, r
if (!queryParamBacklogItemId) {
return await handleFailedValidationResponse(handlerContext, "Item ID is required in URI path for this operation");
}
// TODO: validate that ID is a properly formatted ID

try {
await beginSerializableTransaction(handlerContext);
Expand All @@ -447,6 +456,8 @@ export const backlogItemJoinUnallocatedPartsPostHandler = async (req: Request, r
const unallocatedBacklogItemParts = partsWithAllocationInfo.filter((item) => !item.sprintId);
const unallocatedBacklogItemPartsSorted = unallocatedBacklogItemParts.sort((a, b) => a.partIndex - b.partIndex);

// TODO: Refactor out each of these blocks that have comments into their own functions

// Keep part with lowest partIndex value and remove all the other unallocated parts
// (for example, unallocated parts could be: 1, 5, 6 and 2, 3, 4 could be allocated to sprints)
const partIndexesRemoved: number[] = [];
Expand Down
1 change: 1 addition & 0 deletions src/server/api/handlers/sprintBacklogItemParts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ export const sprintBacklogItemPartsPostHandler = async (req: Request, res: Respo
sprintBacklogItem: addedSprintBacklogItem,
sprintStats
});
// TODO: eliminate direct calls to finish - all responses should call finish
finish(handlerContext);
} catch (err) {
await handleUnexpectedErrorResponse(handlerContext, err);
Expand Down
2 changes: 2 additions & 0 deletions src/server/api/handlers/sprintBacklogItems.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ export const sprintBacklogItemPostHandler = async (req: Request, res: Response)
await commitWithCreatedResponseIfNotAborted(handlerContext, addedSprintBacklogItem, extra);
}
}
// TODO: eliminate direct calls to finish - all responses should call finish
finish(handlerContext);
} catch (err) {
await handleUnexpectedErrorResponse(handlerContext, err);
Expand Down Expand Up @@ -328,6 +329,7 @@ export const sprintBacklogItemDeleteHandler = async (req: Request, res: Response
};
await commitWithOkResponseIfNotAborted(handlerContext, apiBacklogItemTyped, extra);
}
// TODO: eliminate direct calls to finish - all responses should call finish
finish(handlerContext);
} catch (err) {
await handleUnexpectedErrorResponse(handlerContext, err);
Expand Down
1 change: 1 addition & 0 deletions src/server/api/handlers/sprints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ export const sprintGetHandler = async (req: Request, res: Response) => {
await rollbackWithMessageAndStatus(handlerContext, result.message, result.status);
console.log(`Unable to fetch sprint: ${result.message}`);
}
// TODO: eliminate direct calls to finish - all responses should call finish
finish(handlerContext);
} catch (err) {
await handleUnexpectedErrorResponse(handlerContext, err);
Expand Down
2 changes: 2 additions & 0 deletions src/server/api/handlers/utils/handlerContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ export interface HandlerContext {
transactionContext?: HandlerTransactionContext;
}

// TODO: Rename this from start so that it doesn't seem like you have to call `finish`
export const start = (functionTag: string, res: Response): HandlerContext => {
const logContext = logger.info("starting call", [functionTag]);
return { logContext, functionTag, expressContext: { res } };
};

// TODO: Stop exporting finish - it should not be called directly
export const finish = (handlerContext: HandlerContext) => {
logger.info("finishing call", [handlerContext.functionTag]);
};
Expand Down