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.

Sign up for GitHub

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

refactoring the createValidateSelectedSitesField middleware function #3658

Merged
merged 3 commits into from
Oct 15, 2024
Merged

refactoring the createValidateSelectedSitesField middleware function #3658

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
84acb71
refactoring the createValidateSelectedSitesField middleware function
Baalmart Oct 15, 2024
c53a557
refactored validateUniqueFieldsInSelectedSites to effectively ignore …
Baalmart Oct 15, 2024
76b925c
To prevent potential runtime errors, adding a check to ensure that va…
Baalmart Oct 15, 2024
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
24 changes: 17 additions & 7 deletions src/auth-service/routes/v2/preferences.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,14 @@ const headers = (req, res, next) => {
next();
};

function createValidateSelectedSitesField(requiredFields) {
function createValidateSelectedSitesField(requiredFields, allowId = false) {
return function (value) {
// Edge case: Check if _id field is present
if (!allowId && "_id" in value) {
throw new Error("_id field is not allowed");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure 'value' is not null or undefined before checking '_id' field

To prevent potential runtime errors, consider adding a check to ensure that value is not null or undefined before accessing its properties. This will safeguard against situations where value might be missing.

Apply this diff to include the null check:

function createValidateSelectedSitesField(requiredFields, allowId = false) {
  return function (value) {
+   if (!value) {
+     throw new Error("Value must not be null or undefined");
+   }
    // Edge case: Check if _id field is present
    if (!allowId && "_id" in value) {
      throw new Error("_id field is not allowed");
    }
📝 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
function createValidateSelectedSitesField(requiredFields, allowId = false) {
return function (value) {
// Edge case: Check if _id field is present
if (!allowId && "_id" in value) {
throw new Error("_id field is not allowed");
}
function createValidateSelectedSitesField(requiredFields, allowId = false) {
return function (value) {
if (!value) {
throw new Error("Value must not be null or undefined");
}
// Edge case: Check if _id field is present
if (!allowId && "_id" in value) {
throw new Error("_id field is not allowed");
}


// Check for required fields
if (!requiredFields.every((field) => field in value)) {
throw new Error(
`Missing required fields: ${requiredFields
Expand Down Expand Up @@ -110,6 +116,7 @@ function createValidateSelectedSitesField(requiredFields) {
return numericValid && stringValid && tagValid && isValidSiteId;
};
}

const validateUniqueFieldsInSelectedSites = (req, res, next) => {
const selectedSites = req.body.selected_sites;

Expand Down Expand Up @@ -372,7 +379,7 @@ router.post(
body("selected_sites.*")
.optional()
.custom(
createValidateSelectedSitesField(["_id", "search_name", "name"])
createValidateSelectedSitesField(["_id", "search_name", "name"], true)
)
.withMessage(
"Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
Expand Down Expand Up @@ -574,7 +581,7 @@ router.patch(
body("selected_sites.*")
.optional()
.custom(
createValidateSelectedSitesField(["_id", "search_name", "name"])
createValidateSelectedSitesField(["_id", "search_name", "name"], true)
)
.withMessage(
"Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
Expand Down Expand Up @@ -777,7 +784,7 @@ router.put(
body("selected_sites.*")
.optional()
.custom(
createValidateSelectedSitesField(["_id", "search_name", "name"])
createValidateSelectedSitesField(["_id", "search_name", "name"], true)
)
.withMessage(
"Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
Expand Down Expand Up @@ -947,7 +954,7 @@ router.post(
body("selected_sites.*")
.optional()
.custom(
createValidateSelectedSitesField(["_id", "search_name", "name"])
createValidateSelectedSitesField(["_id", "search_name", "name"], true)
)
.withMessage(
"Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
Expand Down Expand Up @@ -1178,7 +1185,10 @@ router.post(
.notEmpty()
.withMessage("selected_sites should not be empty"),
body("selected_sites.*").custom(
createValidateSelectedSitesField(["site_id", "search_name", "name"])
createValidateSelectedSitesField(
["site_id", "search_name", "name"],
false
)
),
],
]),
Expand Down Expand Up @@ -1214,7 +1224,7 @@ router.put(
return ObjectId(value);
}),
body("selected_site")
.custom(createValidateSelectedSitesField([]))
.custom(createValidateSelectedSitesField([], false))
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Specify required fields when using createValidateSelectedSitesField

Currently, createValidateSelectedSitesField is called with an empty array for requiredFields. This may lead to unintended behavior since no fields are being validated as required. Please specify the required fields to ensure proper validation.

For example:

.custom(createValidateSelectedSitesField(
-  [],
+  ['search_name', 'name'],
  false
))

Committable suggestion was skipped due to low confidence.

.withMessage(
"Invalid selected site data. Verify required fields and data types."
),
Expand Down
Loading