Skip to content
4 changes: 4 additions & 0 deletions constants/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ export const REQUEST_LOG_TYPE = {
REQUEST_CANCELLED: "REQUEST_CANCELLED",
REQUEST_UPDATED: "REQUEST_UPDATED",
PENDING_REQUEST_FOUND: "PENDING_REQUEST_FOUND",
REQUEST_ALREADY_APPROVED: "REQUEST_ALREADY_APPROVED",
REQUEST_ALREADY_REJECTED: "REQUEST_ALREADY_REJECTED",
};

export const REQUEST_CREATED_SUCCESSFULLY = "Request created successfully";
Expand All @@ -39,7 +41,9 @@ export const REQUEST_ALREADY_REJECTED = "Request already rejected";
export const ERROR_WHILE_FETCHING_REQUEST = "Error while fetching request";
export const ERROR_WHILE_CREATING_REQUEST = "Error while creating request";
export const ERROR_WHILE_UPDATING_REQUEST = "Error while updating request";
export const ERROR_WHILE_ACKNOWLEDGING_REQUEST = "Error while acknowledging request";

export const REQUEST_ID_REQUIRED = "Request id is required";
export const REQUEST_DOES_NOT_EXIST = "Request does not exist";
export const REQUEST_ALREADY_PENDING = "Request already exists please wait for approval or rejection";
export const UNAUTHORIZED_TO_CREATE_OOO_REQUEST = "Unauthorized to create OOO request";
Expand Down
50 changes: 49 additions & 1 deletion controllers/oooRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import {
REQUEST_ALREADY_PENDING,
USER_STATUS_NOT_FOUND,
OOO_STATUS_ALREADY_EXIST,
UNAUTHORIZED_TO_UPDATE_REQUEST,
ERROR_WHILE_ACKNOWLEDGING_REQUEST,
REQUEST_ID_REQUIRED,
} from "../constants/requests";
import { statusState } from "../constants/userStatus";
import { logType } from "../constants/logs";
Expand All @@ -20,9 +23,11 @@ import { getRequestByKeyValues, getRequests, updateRequest } from "../models/req
import { createUserFutureStatus } from "../models/userFutureStatus";
import { getUserStatus, addFutureStatus } from "../models/userStatus";
import { createOooRequest, validateUserStatus } from "../services/oooRequest";
import * as oooRequestService from "../services/oooRequest";
import { CustomResponse } from "../typeDefinitions/global";
import { OooRequestCreateRequest, OooRequestResponse, OooStatusRequest } from "../types/oooRequest";
import { AcknowledgeOooRequest, OooRequestCreateRequest, OooRequestResponse, OooStatusRequest } from "../types/oooRequest";
import { UpdateRequest } from "../types/requests";
import { NextFunction } from "express";

/**
* Controller to handle the creation of OOO requests.
Expand Down Expand Up @@ -148,3 +153,46 @@ export const updateOooRequestController = async (req: UpdateRequest, res: Custom
return res.boom.badImplementation(ERROR_WHILE_UPDATING_REQUEST);
}
};

/**
* Acknowledges an Out-of-Office (OOO) request
*
* @param {AcknowledgeOooRequest} req - The request object.
* @param {OooRequestResponse} res - The response object.
* @returns {Promise<OooRequestResponse>} Resolves with success or failure.
*/
export const acknowledgeOooRequest = async (
req: AcknowledgeOooRequest,
res: OooRequestResponse,
next: NextFunction
)
: Promise<OooRequestResponse> => {
try {
const dev = req.query.dev === "true";
if(!dev) return res.boom.notImplemented("Feature not implemented");

Comment on lines +164 to +173
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Avoid feature-flag sprawl in controller layer

The early return gated by dev query param (?dev=true) spreads flag logic into controllers. Prefer feature-flag middleware (or config switch) so the controller focuses only on domain logic:

-      const dev = req.query.dev === "true";
-      if(!dev) return res.boom.notImplemented("Feature not implemented");

Moving this behind a dedicated middleware keeps controllers clean and prevents accidental flag omission on other routes.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const acknowledgeOooRequest = async (
req: AcknowledgeOooRequest,
res: OooRequestResponse,
next: NextFunction
)
: Promise<OooRequestResponse> => {
try {
const dev = req.query.dev === "true";
if(!dev) return res.boom.notImplemented("Feature not implemented");
export const acknowledgeOooRequest = async (
req: AcknowledgeOooRequest,
res: OooRequestResponse,
next: NextFunction
)
: Promise<OooRequestResponse> => {
try {
// Controller logic continues here; feature-flag checks moved to middleware

const isSuperuser = req.userData?.roles?.super_user;
if (!isSuperuser) {
return res.boom.forbidden(UNAUTHORIZED_TO_UPDATE_REQUEST);
}

const requestBody = req.body;
const superUserId = req.userData.id;
const requestId = req.params.id;

if (!requestId) {
return res.boom.badRequest(REQUEST_ID_REQUIRED);
}

const response = await oooRequestService.acknowledgeOooRequest(requestId, requestBody, superUserId);

return res.status(200).json({
message: response.message,
});
}
catch(error){
logger.error(ERROR_WHILE_ACKNOWLEDGING_REQUEST, error);
next(error);
return res;
}
Comment on lines +193 to +197
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Return the error response, not the stale res

In the catch block you next(error) but still return res;, which is just the Express response object in its current state. Returning nothing (or next(error) directly) avoids confusion and the theoretical risk of double-sending.

-      next(error);
-      return res;
+      return next(error);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
catch(error){
logger.error(ERROR_WHILE_ACKNOWLEDGING_REQUEST, error);
next(error);
return res;
}
catch(error){
logger.error(ERROR_WHILE_ACKNOWLEDGING_REQUEST, error);
return next(error);
}

};
13 changes: 8 additions & 5 deletions controllers/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {
} from "../constants/requests";
import { getRequests } from "../models/requests";
import { getPaginatedLink } from "../utils/helper";
import { createOooRequestController, updateOooRequestController } from "./oooRequests";
import { OooRequestCreateRequest, OooRequestResponse } from "../types/oooRequest";
import { acknowledgeOooRequest, createOooRequestController, updateOooRequestController } from "./oooRequests";
import { AcknowledgeOooRequest, OooRequestCreateRequest, OooRequestResponse } from "../types/oooRequest";
import { CustomResponse } from "../typeDefinitions/global";
import { ExtensionRequestRequest, ExtensionRequestResponse } from "../types/extensionRequests";
import { createTaskExtensionRequest, updateTaskExtensionRequest } from "./extensionRequestsv2";
Expand All @@ -16,8 +16,7 @@ import { createTaskRequestController } from "./taskRequestsv2";
import { OnboardingExtensionCreateRequest, OnboardingExtensionResponse, UpdateOnboardingExtensionStateRequest } from "../types/onboardingExtension";
import { createOnboardingExtensionRequestController, updateOnboardingExtensionRequestController, updateOnboardingExtensionRequestState } from "./onboardingExtension";
import { UpdateOnboardingExtensionRequest } from "../types/onboardingExtension";

import { Request } from "express";
import { NextFunction, Request } from "express";

export const createRequestController = async (
req: OooRequestCreateRequest | ExtensionRequestRequest | TaskRequestRequest | OnboardingExtensionCreateRequest,
Expand Down Expand Up @@ -121,9 +120,13 @@ export const getRequestsController = async (req: any, res: any) => {
* @param {CustomResponse} res - The response object.
* @returns {Promise<void>} Resolves or sends an error for invalid types.
*/
export const updateRequestBeforeAcknowledgedController = async (req: Request, res: CustomResponse) => {
export const updateRequestBeforeAcknowledgedController = async (req: Request, res: CustomResponse, next: NextFunction) => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Updated function signature with missing JSDoc.

The function signature has been updated to include the next parameter needed for middleware chaining, but the JSDoc comment doesn't reflect this change.

Update the JSDoc comment to include the next parameter:

/**
 * Processes update requests before acknowledgment based on type.
 * 
 * @param {Request} req - The request object.
 * @param {CustomResponse} res - The response object.
+ * @param {NextFunction} next - The next middleware function.
 * @returns {Promise<void>} Resolves or sends an error for invalid types.
 */
export const updateRequestBeforeAcknowledgedController = async (req: Request, res: CustomResponse, next: NextFunction) => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const updateRequestBeforeAcknowledgedController = async (req: Request, res: CustomResponse, next: NextFunction) => {
/**
* Processes update requests before acknowledgment based on type.
*
* @param {Request} req - The request object.
* @param {CustomResponse} res - The response object.
* @param {NextFunction} next - The next middleware function.
* @returns {Promise<void>} Resolves or sends an error for invalid types.
*/
export const updateRequestBeforeAcknowledgedController = async (req: Request, res: CustomResponse, next: NextFunction) => {

const type = req.body.type;

switch(type){
case REQUEST_TYPE.OOO:
await acknowledgeOooRequest(req as AcknowledgeOooRequest, res as OooRequestResponse, next);
break;
case REQUEST_TYPE.ONBOARDING:
await updateOnboardingExtensionRequestController(req as UpdateOnboardingExtensionRequest, res as OnboardingExtensionResponse);
break;
Expand Down
45 changes: 44 additions & 1 deletion middlewares/validators/oooRequests.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import joi from "joi";
import { NextFunction } from "express";
import { REQUEST_STATE, REQUEST_TYPE } from "../../constants/requests";
import { OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";
import { AcknowledgeOooRequest, OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";

export const createOooStatusRequestValidator = async (
req: OooRequestCreateRequest,
Expand Down Expand Up @@ -38,3 +38,46 @@ export const createOooStatusRequestValidator = async (

await schema.validateAsync(req.body, { abortEarly: false });
};

const schema = joi
.object()
.strict()
.keys({
comment: joi.string().optional()
.messages({
"string.empty": "comment cannot be empty",
}),
status: joi
.string()
.valid(REQUEST_STATE.APPROVED, REQUEST_STATE.REJECTED)
.required()
.messages({
"any.only": "status must be APPROVED or REJECTED",
}),
type: joi.string().equal(REQUEST_TYPE.OOO).required().messages({
"type.any": "type is required",
})
});

/**
* Middleware to validate the acknowledge Out-Of-Office (OOO) request payload.
*
* @param {AcknowledgeOooRequest} req - The request object containing the body to be validated.
* @param {OooRequestResponse} res - The response object used to send error responses if validation fails.
* @param {NextFunction} next - The next middleware function to call if validation succeeds.
* @returns {Promise<void>} Resolves or sends errors.
*/
export const acknowledgeOooRequest = async (
req: AcknowledgeOooRequest,
res: OooRequestResponse,
next: NextFunction
): Promise<void> => {
try {
await schema.validateAsync(req.body, { abortEarly: false });
next();
} catch (error) {
const errorMessages = error.details.map((detail:{message: string}) => detail.message);
logger.error(`Error while validating request payload : ${errorMessages}`);
return res.boom.badRequest(errorMessages);
}
};
Comment on lines +70 to +83
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing logger import.

The middleware uses logger.error() on line 80, but there's no import statement for the logger.

Add the missing import at the top of the file:

import joi from "joi";
import { NextFunction } from "express";
import { REQUEST_STATE, REQUEST_TYPE } from "../../constants/requests";
import { AcknowledgeOooRequest, OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";
+import logger from "../../utils/logger";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const acknowledgeOooRequest = async (
req: AcknowledgeOooRequest,
res: OooRequestResponse,
next: NextFunction
): Promise<void> => {
try {
await schema.validateAsync(req.body, { abortEarly: false });
next();
} catch (error) {
const errorMessages = error.details.map((detail:{message: string}) => detail.message);
logger.error(`Error while validating request payload : ${errorMessages}`);
return res.boom.badRequest(errorMessages);
}
};
// middlewares/validators/oooRequests.ts
import joi from "joi";
import { NextFunction } from "express";
import { REQUEST_STATE, REQUEST_TYPE } from "../../constants/requests";
import { AcknowledgeOooRequest, OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";
+import logger from "../../utils/logger";
export const acknowledgeOooRequest = async (
req: AcknowledgeOooRequest,
res: OooRequestResponse,
next: NextFunction
): Promise<void> => {
try {
await schema.validateAsync(req.body, { abortEarly: false });
next();
} catch (error) {
const errorMessages = error.details.map((detail:{message: string}) => detail.message);
logger.error(`Error while validating request payload : ${errorMessages}`);
return res.boom.badRequest(errorMessages);
}
};

16 changes: 11 additions & 5 deletions middlewares/validators/requests.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import joi from "joi";
import { NextFunction } from "express";
import { REQUEST_STATE, REQUEST_TYPE } from "../../constants/requests";
import { OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";
import { createOooStatusRequestValidator } from "./oooRequests";
import { AcknowledgeOooRequest, OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";
import { acknowledgeOooRequest, createOooStatusRequestValidator } from "./oooRequests";
import { createExtensionRequestValidator } from "./extensionRequestsv2";
import {createTaskRequestValidator} from "./taskRequests";
import { ExtensionRequestRequest, ExtensionRequestResponse } from "../../types/extensionRequests";
Expand Down Expand Up @@ -125,18 +125,24 @@ export const getRequestsMiddleware = async (req: OooRequestCreateRequest, res: O
/**
* Validates update requests based on their type.
*
* @param {UpdateOnboardingExtensionRequest} req - Request object.
* @param {UpdateOnboardingExtensionRequest | AcknowledgeOooRequest} req - Request object.
* @param {CustomResponse} res - Response object.
* @param {NextFunction} next - Next middleware if valid.
* @returns {Promise<void>} Resolves or sends errors.
*/
export const updateRequestValidator = async (
req: UpdateOnboardingExtensionRequest,
req: UpdateOnboardingExtensionRequest | AcknowledgeOooRequest,
res: CustomResponse,
next: NextFunction
): Promise<void> => {
const type = req.body.type;

switch (type) {
case REQUEST_TYPE.OOO:
await acknowledgeOooRequest(
req,
res as OooRequestResponse, next);
break;
Comment on lines +141 to +145
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

OOO request handling integration looks good, minor formatting issue.

The OOO request case is properly integrated into the validator switch statement, but there's inconsistent spacing after the comma.

Improve formatting consistency:

      case REQUEST_TYPE.OOO:
          await acknowledgeOooRequest(
            req,
-            res as OooRequestResponse, next);
+            res as OooRequestResponse, 
+            next);
          break;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
case REQUEST_TYPE.OOO:
await acknowledgeOooRequest(
req,
res as OooRequestResponse, next);
break;
case REQUEST_TYPE.OOO:
await acknowledgeOooRequest(
req,
res as OooRequestResponse,
next);
break;

case REQUEST_TYPE.ONBOARDING:
await updateOnboardingExtensionRequestValidator(
req,
Expand All @@ -145,4 +151,4 @@ export const updateRequestValidator = async (
default:
return res.boom.badRequest("Invalid type");
}
};
};
16 changes: 16 additions & 0 deletions models/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
REQUEST_DOES_NOT_EXIST,
} from "../constants/requests";
import { getUserId } from "../utils/users";
import { NotFound } from "http-errors";
const SIZE = 5;

export const createRequest = async (body: any) => {
Expand Down Expand Up @@ -69,6 +70,21 @@ export const updateRequest = async (id: string, body: any, lastModifiedBy: strin
}
};

export const getRequestById = async (id: string) => {
try {
const requestDoc = await requestModel.doc(id).get();

if (!requestDoc.exists) {
throw new NotFound(REQUEST_DOES_NOT_EXIST);
}

return requestDoc.data();
} catch (error) {
logger.error(ERROR_WHILE_FETCHING_REQUEST, error);
throw error;
}
};
Comment on lines +73 to +86
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Well-implemented getRequestById function.

The function correctly retrieves a request by ID and returns appropriate errors when the document doesn't exist. The error handling follows the established pattern in the codebase.

Consider adding JSDoc documentation to clarify the function's purpose and parameters:

+/**
+ * Retrieves a request document by its ID
+ * @param {string} id - The ID of the request to retrieve
+ * @returns {Promise<any>} The request document data
+ * @throws {NotFound} When the request document doesn't exist
+ */
export const getRequestById = async (id: string) => {
  try {
    const requestDoc = await requestModel.doc(id).get();

    if (!requestDoc.exists) {
      throw new NotFound(REQUEST_DOES_NOT_EXIST);
    }

    return requestDoc.data();
  } catch (error) {
    logger.error(ERROR_WHILE_FETCHING_REQUEST, error);
    throw error;
  }
};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const getRequestById = async (id: string) => {
try {
const requestDoc = await requestModel.doc(id).get();
if (!requestDoc.exists) {
throw new NotFound(REQUEST_DOES_NOT_EXIST);
}
return requestDoc.data();
} catch (error) {
logger.error(ERROR_WHILE_FETCHING_REQUEST, error);
throw error;
}
};
/**
* Retrieves a request document by its ID
* @param {string} id - The ID of the request to retrieve
* @returns {Promise<any>} The request document data
* @throws {NotFound} When the request document doesn't exist
*/
export const getRequestById = async (id: string) => {
try {
const requestDoc = await requestModel.doc(id).get();
if (!requestDoc.exists) {
throw new NotFound(REQUEST_DOES_NOT_EXIST);
}
return requestDoc.data();
} catch (error) {
logger.error(ERROR_WHILE_FETCHING_REQUEST, error);
throw error;
}
};


export const getRequests = async (query: any) => {
let { id, type, requestedBy, state, prev, next, page, size = SIZE } = query;
const dev = query.dev === "true";
Expand Down
110 changes: 109 additions & 1 deletion services/oooRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,22 @@ import {
REQUEST_LOG_TYPE,
REQUEST_STATE,
USER_STATUS_NOT_FOUND,
REQUEST_TYPE,
REQUEST_ALREADY_APPROVED,
REQUEST_ALREADY_REJECTED,
REQUEST_APPROVED_SUCCESSFULLY,
REQUEST_REJECTED_SUCCESSFULLY,
INVALID_REQUEST_TYPE,
} from "../constants/requests";
import { userState } from "../constants/userStatus";
import { createRequest } from "../models/requests";
import { createRequest, getRequestById } from "../models/requests";
import { OooStatusRequest, OooStatusRequestBody } from "../types/oooRequest";
import { UserStatus } from "../types/userStatus";
import { addLog } from "./logService";
import { BadRequest, Conflict } from "http-errors";
import { updateRequest } from "../models/requests";
import { AcknowledgeOooRequestBody } from "../types/oooRequest";
import { addFutureStatus } from "../models/userStatus";

/**
* Validates the user status.
Expand Down Expand Up @@ -93,3 +103,101 @@ export const createOooRequest = async (
throw error;
}
}

/**
* Validates an Out-Of-Office (OOO) acknowledge request
*
* @param {string} requestId - The unique identifier of the request.
* @param {string} requestType - The type of the request (expected to be 'OOO').
* @param {string} requestStatus - The current status of the request.
* @throws {Error} Throws an error if an issue occurs during validation.
*/
export const validateOooAcknowledgeRequest = async (
requestType: string,
requestStatus: string,
) => {
Comment on lines +115 to +118
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

async keyword unnecessary

validateOooAcknowledgeRequest performs only synchronous checks. Dropping async avoids wrapping the return value in an unnecessary promise and makes intent clearer.

-export const validateOooAcknowledgeRequest = async (
+export const validateOooAcknowledgeRequest = (
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const validateOooAcknowledgeRequest = async (
requestType: string,
requestStatus: string,
) => {
export const validateOooAcknowledgeRequest = (
requestType: string,
requestStatus: string,
) => {


try {

if (requestType !== REQUEST_TYPE.OOO) {
throw new BadRequest(INVALID_REQUEST_TYPE);
}

if (requestStatus === REQUEST_STATE.APPROVED) {
throw new Conflict(REQUEST_ALREADY_APPROVED);
}

if (requestStatus === REQUEST_STATE.REJECTED) {
throw new Conflict(REQUEST_ALREADY_REJECTED);
}
} catch (error) {
logger.error("Error while validating OOO acknowledge request", error);
throw error;
}
}
Comment on lines +107 to +137
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

JSDoc parameters don’t match implementation

The comment lists @param {string} requestId, requestType, requestStatus, but the function actually receives only requestType & requestStatus.

Keeping docs accurate prevents false assumptions:

- * @param {string} requestId - The unique identifier of the request.
- * @param {string} requestType - The type of the request (expected to be 'OOO').
- * @param {string} requestStatus - The current status of the request.
+ * @param {string} requestType - The type of the request (expected to be 'OOO').
+ * @param {string} requestStatus - The current status of the request.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Validates an Out-Of-Office (OOO) acknowledge request
*
* @param {string} requestId - The unique identifier of the request.
* @param {string} requestType - The type of the request (expected to be 'OOO').
* @param {string} requestStatus - The current status of the request.
* @throws {Error} Throws an error if an issue occurs during validation.
*/
export const validateOooAcknowledgeRequest = async (
requestType: string,
requestStatus: string,
) => {
try {
if (requestType !== REQUEST_TYPE.OOO) {
throw new BadRequest(INVALID_REQUEST_TYPE);
}
if (requestStatus === REQUEST_STATE.APPROVED) {
throw new Conflict(REQUEST_ALREADY_APPROVED);
}
if (requestStatus === REQUEST_STATE.REJECTED) {
throw new Conflict(REQUEST_ALREADY_REJECTED);
}
} catch (error) {
logger.error("Error while validating OOO acknowledge request", error);
throw error;
}
}
/**
* Validates an Out-Of-Office (OOO) acknowledge request
*
* @param {string} requestType - The type of the request (expected to be 'OOO').
* @param {string} requestStatus - The current status of the request.
* @throws {Error} Throws an error if an issue occurs during validation.
*/
export const validateOooAcknowledgeRequest = async (
requestType: string,
requestStatus: string,
) => {
try {
if (requestType !== REQUEST_TYPE.OOO) {
throw new BadRequest(INVALID_REQUEST_TYPE);
}
if (requestStatus === REQUEST_STATE.APPROVED) {
throw new Conflict(REQUEST_ALREADY_APPROVED);
}
if (requestStatus === REQUEST_STATE.REJECTED) {
throw new Conflict(REQUEST_ALREADY_REJECTED);
}
} catch (error) {
logger.error("Error while validating OOO acknowledge request", error);
throw error;
}
}


/**
* Acknowledges an Out-of-Office (OOO) request
*
* @param {string} requestId - The ID of the OOO request to acknowledge.
* @param {AcknowledgeOooRequestBody} body - The acknowledgement body containing acknowledging details.
* @param {string} superUserId - The unique identifier of the superuser user.
* @returns {Promise<object>} The acknowledged OOO request.
* @throws {Error} Throws an error if an issue occurs during acknowledgment process.
*/
export const acknowledgeOooRequest = async (
requestId: string,
body: AcknowledgeOooRequestBody,
superUserId: string,
) => {
try {
const requestData = await getRequestById(requestId);

await validateOooAcknowledgeRequest(requestData.type, requestData.status);

const requestResult = await updateRequest(requestId, body, superUserId, REQUEST_TYPE.OOO);

if ("error" in requestResult) {
throw new BadRequest(requestResult.error);
}

const [acknowledgeLogType, returnMessage] =
requestResult.status === REQUEST_STATE.APPROVED
? [REQUEST_LOG_TYPE.REQUEST_APPROVED, REQUEST_APPROVED_SUCCESSFULLY]
: [REQUEST_LOG_TYPE.REQUEST_REJECTED, REQUEST_REJECTED_SUCCESSFULLY];

const requestLog = {
type: acknowledgeLogType,
meta: {
requestId,
action: LOG_ACTION.UPDATE,
userId: superUserId,
},
body: requestResult,
};

await addLog(requestLog.type, requestLog.meta, requestLog.body);

if (requestResult.status === REQUEST_STATE.APPROVED) {
await addFutureStatus({
requestId,
state: REQUEST_TYPE.OOO,
from: requestData.from,
endsOn: requestData.until,
userId: requestData.userId,
message: body.comment,
});
}

return {
message: returnMessage,
data: {
id: requestResult.id,
...requestResult,
},
};
} catch (error) {
Comment on lines +148 to +199
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Potential race: request data may change between fetch and update

Between calling getRequestById(requestId) and updateRequest(...), another concurrent process could alter the request’s status, re-opening the race you just guarded against. To fully guarantee consistency, add a status precondition in the update layer (e.g., conditional write or transaction).

If the underlying data store supports atomic conditional updates (Firestore transaction, SQL WHERE status = 'PENDING'), leverage it to ensure you don’t approve an already-approved request due to a race.

logger.error("Error while acknowledging OOO request", error);
throw error;
}
}
2 changes: 1 addition & 1 deletion test/fixtures/oooRequest/oooRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ export const createOooRequests3 = {
status: REQUEST_STATE.PENDING
};

export const acknowledgeOooRequest = {
export const testAcknowledgeOooRequest = {
type: REQUEST_TYPE.OOO,
status: REQUEST_STATE.APPROVED,
comment: "OOO request approved as it's emergency."
Expand Down
Loading
Loading