Skip to content

Commit 6e63df6

Browse files
RishiChaubey31RishiChaubey31
committed
refactor: improve oooRoleCheckMiddleware and oooRequests validation
- Enhanced the oooRoleCheckMiddleware for better error handling and code clarity. - Updated the acknowledgeOooRequestValidator to include specific Joi validation error handling. - Simplified the validateOooAcknowledgeRequest function by removing unnecessary try-catch nesting and improving logging for invalid request types.
1 parent ea2222c commit 6e63df6

File tree

3 files changed

+109
-89
lines changed

3 files changed

+109
-89
lines changed

‎middlewares/oooRoleCheckMiddleware.ts‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,28 +3,28 @@ import { CustomRequest, CustomResponse } from "../types/global";
33
import { REQUEST_TYPE } from "../constants/requests";
44
import { devFlagMiddleware } from "./devFlag";
55
import authorizeRoles from "./authorizeRoles";
6+
67
const { SUPERUSER } = require("../constants/roles");
78

8-
/**
9-
* Conditional middleware that applies devFlag and superuser checks only for OOO requests.
10-
* This allows onboarding requests to bypass these checks while maintaining security for OOO operations.
11-
*
12-
* @param {CustomRequest} req - The request object
13-
* @param {CustomResponse} res - The response object
14-
* @param {NextFunction} next - The next middleware function
15-
*/
16-
export const oooRoleCheckMiddleware = (req: CustomRequest, res: CustomResponse, next: NextFunction) => {
9+
export const oooRoleCheckMiddleware = (
10+
req: CustomRequest,
11+
res: CustomResponse,
12+
next: NextFunction
13+
) => {
1714
const requestType = req.body?.type;
1815

19-
2016
if (requestType === REQUEST_TYPE.OOO) {
21-
//TODO: Remove this middleware once the OOO feature is tested and ready to be used
22-
devFlagMiddleware(req, res, (err: any) => {
17+
// TODO: Remove this middleware once the OOO feature is tested and ready
18+
return devFlagMiddleware(req, res, (err) => {
2319
if (err) return next(err);
24-
authorizeRoles([SUPERUSER])(req, res, next);
25-
});
26-
} else {
2720

28-
next();
21+
try {
22+
return authorizeRoles([SUPERUSER])(req, res, next);
23+
} catch (authErr) {
24+
return next(authErr);
25+
}
26+
});
2927
}
30-
};
28+
29+
return next();
30+
};

‎middlewares/validators/oooRequests.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,19 @@ export const acknowledgeOooRequestValidator = async (
8686
try {
8787
await acknowledgeOooRequestSchema.validateAsync(req.body, { abortEarly: false });
8888
await paramsSchema.validateAsync(req.params, { abortEarly: false });
89-
next();
90-
} catch (error) {
91-
const errorMessages = error.details.map((detail) => detail.message);
92-
logger.error(`${ERROR_WHILE_ACKNOWLEDGING_REQUEST}: ${errorMessages}`);
93-
return res.boom.badRequest(errorMessages);
89+
return next();
90+
} catch (error: unknown) {
91+
const isJoiValidationError = (err: unknown): err is joi.ValidationError => {
92+
return Boolean(err) && typeof (err as any).isJoi === "boolean" && (err as any).isJoi === true && Array.isArray((err as any).details);
93+
};
94+
95+
if (isJoiValidationError(error)) {
96+
const errorMessages = error.details.map((detail) => detail.message);
97+
logger.error(`${ERROR_WHILE_ACKNOWLEDGING_REQUEST}: ${errorMessages}`);
98+
return res.boom.badRequest(errorMessages);
99+
}
100+
const genericMessage = typeof (error as any)?.message === "string" ? (error as any).message : String(error);
101+
logger.error(`${ERROR_WHILE_ACKNOWLEDGING_REQUEST}: ${genericMessage}`);
102+
return res.boom.badRequest([ERROR_WHILE_ACKNOWLEDGING_REQUEST]);
94103
}
95104
};

‎services/oooRequest.ts‎

Lines changed: 78 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -111,25 +111,26 @@ export const createOooRequest = async (
111111
* @throws {Error} Throws an error if an issue occurs during validation.
112112
*/
113113

114-
export const validateOooAcknowledgeRequest = async (
114+
export const validateOooAcknowledgeRequest = (
115115
requestType: string,
116116
requestStatus: string,
117-
) => {
118-
try {
119-
if (requestType !== REQUEST_TYPE.OOO) {
120-
throw new BadRequest(INVALID_REQUEST_TYPE)
121-
}
122-
if (requestStatus === REQUEST_STATE.APPROVED) {
123-
throw new BadRequest(REQUEST_ALREADY_APPROVED)
124-
}
125-
if (requestStatus === REQUEST_STATE.REJECTED) {
126-
throw new BadRequest(REQUEST_ALREADY_REJECTED)
127-
}
128-
} catch (error) {
129-
logger.error("Error while validating OOO acknowledge request", error);
130-
throw error;
117+
) => {
118+
if (requestType !== REQUEST_TYPE.OOO) {
119+
logger.error(`Invalid request type: ${requestType}`);
120+
throw new BadRequest(INVALID_REQUEST_TYPE);
131121
}
132-
}
122+
123+
if (requestStatus === REQUEST_STATE.APPROVED) {
124+
logger.error(`Request already approved`);
125+
throw new BadRequest(REQUEST_ALREADY_APPROVED);
126+
}
127+
128+
if (requestStatus === REQUEST_STATE.REJECTED) {
129+
logger.error(`Request already rejected`);
130+
throw new BadRequest(REQUEST_ALREADY_REJECTED);
131+
}
132+
};
133+
133134

134135
/**
135136
* Acknowledges the OOO request.
@@ -145,56 +146,66 @@ export const acknowledgeOooRequest = async (
145146
requestId: string,
146147
body: AcknowledgeOooRequestBody,
147148
superUserId: string,
148-
) => {
149-
try{
150-
const requestData = await getRequests({ id: requestId }) as OooStatusRequest | oldOooStatusRequest;
151-
if (!requestData) {
152-
throw new NotFound("Request not found");
153-
}
154-
const { type, status, from, until, requestedBy } = requestData as OooStatusRequest;
155-
156-
await validateOooAcknowledgeRequest(type as string, status as string);
157-
const requestResult = await updateRequest(requestId, body, superUserId, REQUEST_TYPE.OOO);
158-
if(requestResult.error){
159-
throw new BadRequest(requestResult.error);
160-
}
161-
const [acknowledgeLogType, returnMessage]=
162-
requestResult.status === REQUEST_STATE.APPROVED ? [REQUEST_LOG_TYPE.REQUEST_APPROVED, REQUEST_APPROVED_SUCCESSFULLY] : [REQUEST_LOG_TYPE.REQUEST_REJECTED, REQUEST_REJECTED_SUCCESSFULLY];
163-
const requestLog = {
164-
type: acknowledgeLogType,
165-
meta: {
166-
requestId: requestId,
167-
action: LOG_ACTION.UPDATE,
168-
userId: superUserId,
169-
},
170-
body: requestResult,
171-
}
172-
await addLog(requestLog.type, requestLog.meta, requestLog.body);
173-
if (requestResult.status === REQUEST_STATE.APPROVED) {
174-
await addFutureStatus({
175-
requestId,
176-
state: REQUEST_TYPE.OOO,
177-
from: from,
178-
endsOn: until,
179-
userId: requestedBy,
180-
message: body.comment,
181-
});
182-
await createUserFutureStatus({
183-
requestId,
184-
status: userState.OOO,
185-
state: statusState.UPCOMING,
186-
from: from,
187-
endsOn: until,
188-
userId: requestedBy,
189-
message: body.comment,
190-
createdAt: Date.now()
191-
});
192-
}
193-
return {
194-
message: returnMessage,
195-
};
149+
) => {
150+
try {
151+
const requestData = await getRequests({ id: requestId }) as OooStatusRequest | oldOooStatusRequest;
152+
if (!requestData) {
153+
throw new NotFound("Request not found");
154+
}
155+
156+
const { type, status, from, until, requestedBy } = requestData as OooStatusRequest;
157+
158+
await validateOooAcknowledgeRequest(type, status);
159+
160+
const requestResult = await updateRequest(requestId, body, superUserId, REQUEST_TYPE.OOO);
161+
if (requestResult.error) {
162+
throw new BadRequest(requestResult.error);
163+
}
164+
165+
const [acknowledgeLogType, returnMessage] =
166+
requestResult.status === REQUEST_STATE.APPROVED
167+
? [REQUEST_LOG_TYPE.REQUEST_APPROVED, REQUEST_APPROVED_SUCCESSFULLY]
168+
: [REQUEST_LOG_TYPE.REQUEST_REJECTED, REQUEST_REJECTED_SUCCESSFULLY];
169+
170+
await addLog(
171+
acknowledgeLogType,
172+
{
173+
requestId,
174+
action: LOG_ACTION.UPDATE,
175+
userId: superUserId,
176+
},
177+
requestResult,
178+
);
179+
180+
if (requestResult.status === REQUEST_STATE.APPROVED) {
181+
await addFutureStatus({
182+
requestId,
183+
state: REQUEST_TYPE.OOO,
184+
from,
185+
endsOn: until,
186+
userId: requestedBy,
187+
message: body.comment ?? "",
188+
});
189+
190+
await createUserFutureStatus({
191+
requestId,
192+
status: userState.OOO,
193+
state: statusState.UPCOMING,
194+
from,
195+
endsOn: until,
196+
userId: requestedBy,
197+
message: body.comment ?? "",
198+
createdAt: Date.now(),
199+
});
200+
}
201+
202+
return {
203+
message: returnMessage,
204+
request: requestResult,
205+
};
196206
} catch (error) {
197-
logger.error(ERROR_WHILE_ACKNOWLEDGING_REQUEST, error);
198-
throw error;
207+
logger.error(ERROR_WHILE_ACKNOWLEDGING_REQUEST, { error });
208+
throw error;
199209
}
200-
}
210+
};
211+

0 commit comments

Comments
 (0)