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

Fix: Implement Date Range Validation for Activity Dates #4515

Merged
merged 2 commits into from
Mar 3, 2025
Merged
Changes from 1 commit
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
48 changes: 33 additions & 15 deletions src/device-registry/validators/activities.validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const { oneOf, query, body, param } = require("express-validator");
const { ObjectId } = require("mongoose").Types;
const { isValidObjectId } = require("mongoose");
const constants = require("@config/constants");
const moment = require("moment");
const { validateNetwork, validateAdminLevels } = require("@validators/common");

const commonValidations = {
Expand Down Expand Up @@ -175,7 +176,21 @@ const commonValidations = {
.trim()
.toDate()
.isISO8601({ strict: true, strictSeparator: true })
.withMessage("date must be a valid datetime."),
.withMessage("date must be a valid datetime.")
.bail()
.custom((date) => {
const now = moment();
const oneMonthAgo = moment().subtract(1, "month");
const inputDate = moment(date);

if (inputDate.isAfter(now)) {
throw new Error("date cannot be in the future");
}
if (inputDate.isBefore(oneMonthAgo)) {
throw new Error("date cannot be more than one month in the past");
}
return true;
}),
],
description: [
body("description")
Expand Down Expand Up @@ -331,12 +346,7 @@ const activitiesValidations = {
.isIn(constants.RECALL_TYPES)
.withMessage("Invalid recallType"),
commonValidations.objectId("user_id", body),
body("date")
.optional()
.trim()
.toDate()
.isISO8601({ strict: true, strictSeparator: true })
.withMessage("Invalid date format"),
...commonValidations.date,
...commonValidations.firstName,
...commonValidations.lastName,
...commonValidations.userName,
Expand Down Expand Up @@ -380,13 +390,7 @@ const activitiesValidations = {
.toLowerCase()
.isIn(["pole", "wall", "faceboard", "rooftop", "suspended"])
.withMessage("Invalid mountType"),
body("date")
.exists()
.withMessage("date is required")
.trim()
.toDate()
.isISO8601({ strict: true, strictSeparator: true })
.withMessage("Invalid date format"),
...commonValidations.date,
//Optional fields validation if provided
body("isPrimaryInLocation")
.optional()
Expand Down Expand Up @@ -506,7 +510,21 @@ const activitiesValidations = {
.trim()
.toDate()
.isISO8601({ strict: true, strictSeparator: true })
.withMessage("Invalid date format"),
.withMessage("Invalid date format")
.bail()
.custom((date) => {
const now = moment();
const oneMonthAgo = moment().subtract(1, "month");
const inputDate = moment(date);

if (inputDate.isAfter(now)) {
throw new Error("date cannot be in the future");
}
if (inputDate.isBefore(oneMonthAgo)) {
throw new Error("date cannot be more than one month in the past");
}
return true;
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Code duplication in batchDeployActivity date validation

The date validation logic here duplicates what's already in commonValidations.date. Consider using a reusable function for this validation to avoid future maintenance issues.

You could potentially refactor this to reuse the validation logic:

- body("*.date")
-   .exists()
-   .withMessage("date is required")
-   .trim()
-   .toDate()
-   .isISO8601({ strict: true, strictSeparator: true })
-   .withMessage("Invalid date format")
-   .bail()
-   .custom((date) => {
-     const now = moment();
-     const oneMonthAgo = moment().subtract(1, "month");
-     const inputDate = moment(date);
-
-     if (inputDate.isAfter(now)) {
-       throw new Error("date cannot be in the future");
-     }
-     if (inputDate.isBefore(oneMonthAgo)) {
-       throw new Error("date cannot be more than one month in the past");
-     }
-     return true;
-   }),
+ body("*.date")
+   .exists()
+   .withMessage("date is required")
+   .trim()
+   .toDate()
+   .isISO8601({ strict: true, strictSeparator: true })
+   .withMessage("Invalid date format")
+   .bail()
+   .custom((date) => {
+     return commonValidations.date[0].builder.custom.func(date);
+   }),

Alternatively, consider creating a reusable function outside the validation definitions that both can use:

+ // Add this near the top of the file, after the imports
+ const validateDateRange = (date) => {
+   const now = moment();
+   const oneMonthAgo = moment().subtract(1, "month");
+   const inputDate = moment(date);
+   
+   if (inputDate.isAfter(now)) {
+     throw new Error("date cannot be in the future");
+   }
+   if (inputDate.isBefore(oneMonthAgo)) {
+     throw new Error("date cannot be more than one month in the past");
+   }
+   return true;
+ };

And then use it in both places:

- .custom((date) => {
-   const now = moment();
-   const oneMonthAgo = moment().subtract(1, "month");
-   const inputDate = moment(date);
-
-   if (inputDate.isAfter(now)) {
-     throw new Error("date cannot be in the future");
-   }
-   if (inputDate.isBefore(oneMonthAgo)) {
-     throw new Error("date cannot be more than one month in the past");
-   }
-   return true;
- }),
+ .custom(validateDateRange),
📝 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
.withMessage("Invalid date format")
.bail()
.custom((date) => {
const now = moment();
const oneMonthAgo = moment().subtract(1, "month");
const inputDate = moment(date);
if (inputDate.isAfter(now)) {
throw new Error("date cannot be in the future");
}
if (inputDate.isBefore(oneMonthAgo)) {
throw new Error("date cannot be more than one month in the past");
}
return true;
}),
.withMessage("Invalid date format")
.bail()
.custom((date) => {
return commonValidations.date[0].builder.custom.func(date);
}),

commonValidations.objectId("*.user_id", body),
commonValidations.objectId("*.host_id", body),
],
Expand Down
Loading