Skip to content

fix: change state to status #2407

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

Open
ZendeAditya wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

fix: change state to status #2407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
df7cee5
fix: change state to status in onboarding files
ZendeAditya Apr 29, 2025
640635c
state to status change done
ZendeAditya May 2, 2025
c92a7ac
database migration script
ZendeAditya May 2, 2025
db5e50c
removed unwanted files
ZendeAditya May 2, 2025
af83196
chore: rename state to status
ZendeAditya May 8, 2025
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
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,5 +124,4 @@ dist

# Local config file
config/local.js

package-lock.json
1 change: 0 additions & 1 deletion config/default.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*
* Documentation: https://github.com/lorenwest/node-config/wiki/Configuration-Files
*/

const NODE_ENV = process.env.NODE_ENV;
module.exports = {
port: 3000,
Expand Down
2 changes: 1 addition & 1 deletion constants/requests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export const REQUEST_STATE = {
export const REQUEST_STATUS = {
APPROVED: "APPROVED",
PENDING: "PENDING",
REJECTED: "REJECTED",
Expand Down
28 changes: 14 additions & 14 deletions controllers/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,20 +172,20 @@ const githubAuthCallback = (req, res, next) => {
updated_at: null,
};

if (!userData.email) {
const githubBaseUrl = config.get("githubApi.baseUrl");
const res = await fetch(`${githubBaseUrl}/user/emails`, {
headers: {
Authorization: `token ${accessToken}`,
},
});
const emails = await res.json();
const primaryEmails = emails.filter((item) => item.primary);

if (primaryEmails.length > 0) {
userData.email = primaryEmails[0].email;
}
}
// if (!userData.email) {
// const githubBaseUrl = config.get("githubApi.baseUrl");
// const res = await fetch(`${githubBaseUrl}/user/emails`, {
// headers: {
// Authorization: `token ${accessToken}`,
// },
// });
// const emails = await res.json();
// const primaryEmails = emails.filter((item) => item.primary);

// if (primaryEmails.length > 0) {
// userData.email = primaryEmails[0].email;
// }
// }
Comment on lines +175 to +188
Copy link

@coderabbitai coderabbitai bot Apr 29, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Commenting out email fallback mechanism doesn't align with PR objectives

This change removes a fallback mechanism that retrieves a user's primary email when not provided in the initial GitHub response. This modification doesn't relate to the PR's stated objective of renaming "state" to "status" in onboarding-related files.

Removing this fallback could potentially cause authentication failures when users have private email settings on GitHub, as userData.email might be undefined when passed to users.addOrUpdate() on line 190.

🏁 Script executed:

#!/bin/bash
# Check if the addOrUpdate function requires an email field
rg -A 5 "addOrUpdate" models/users.js

Length of output: 391

I want to confirm whether addOrUpdate enforces or uses the email field. Let’s pull in its full implementation and see any email-related logic:

🏁 Script executed:

#!/bin/bash
# Show the full addOrUpdate implementation
rg -A 200 -n "const addOrUpdate" models/users.js

# Highlight any usage of userData.email within that function
rg -n "userData.email" -A 5 models/users.js

Length of output: 7631

To confirm how addOrUpdate is invoked and how an undefined email propagates, let’s locate its usage in controllers/auth.js and inspect the surrounding logic:

🏁 Script executed:

#!/bin/bash
# Find where addOrUpdate is called in the auth controller
rg -n "addOrUpdate" controllers/auth.js -A5

# Show the context around line 190 of controllers/auth.js
sed -n '170,210p' controllers/auth.js

Length of output: 2025

Let’s inspect how userData is initialized in the GitHub callback to see if email is ever set:

🏁 Script executed:

#!/bin/bash
# Show the initialization of userData in controllers/auth.js
rg -n "const userData" -A15 controllers/auth.js

Length of output: 861

Let’s inspect the entire GitHub callback handler (lines 130–260) to see how userData is constructed:

🏁 Script executed:

#!/bin/bash
# Dump lines 130–260 of the auth controller to view GitHub callback and userData init
sed -n '130,260p' controllers/auth.js

Length of output: 4307

Re-enable GitHub email fallback or properly handle profile.emails

By commenting out the block that fetches a user’s private email via /user/emails, any GitHub user without a public email will arrive with userData.email === undefined. That undefined value then flows into users.addOrUpdate() and:

  • Skips the email‐based lookup altogether (so new users get created every time if their GitHub IDs don’t match)
  • Or, when updating an existing user, spreads email: undefined over the existing record (potentially wiping out the stored email or even throwing a Firestore error)

This change is unrelated to renaming “state”→“status” and introduces a critical break in both user‐matching and data integrity. Please restore or replace this fallback (e.g. pull from profile.emails or re-enable the /user/emails call).

Locations to fix:

  • controllers/auth.js, lines ~174–188 (the commented-out email fetch block)


const { userId, incompleteUserDetails, role } = await users.addOrUpdate(userData);

Expand Down
6 changes: 3 additions & 3 deletions controllers/extensionRequestsv2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
REQUEST_APPROVED_SUCCESSFULLY,
REQUEST_LOG_TYPE,
REQUEST_REJECTED_SUCCESSFULLY,
REQUEST_STATE,
REQUEST_STATUS,
Copy link

@coderabbitai coderabbitai bot Apr 29, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Property naming inconsistency with PR intent

The PR objective is to rename the field "state" to "status", but in this file, you've updated the constant from REQUEST_STATE to REQUEST_STATUS while keeping the property name as state. This creates inconsistency in the codebase.

Update all property references from state to status for consistency:

-if (latestExtensionRequest && latestExtensionRequest.state === REQUEST_STATUS.PENDING) {
+if (latestExtensionRequest && latestExtensionRequest.status === REQUEST_STATUS.PENDING) {
-requestResult.state === REQUEST_STATUS.APPROVED
+requestResult.status === REQUEST_STATUS.APPROVED

Also applies to: 67-67, 120-120

REQUEST_TYPE,
} from "../constants/requests";
import { addLog } from "../models/logs";
Expand Down Expand Up @@ -64,7 +64,7 @@ export const createTaskExtensionRequest = async (req: ExtensionRequestRequest, r
type: REQUEST_TYPE.EXTENSION,
});

if (latestExtensionRequest && latestExtensionRequest.state === REQUEST_STATE.PENDING) {
if (latestExtensionRequest && latestExtensionRequest.state === REQUEST_STATUS.PENDING) {
return res.boom.badRequest("An extension request for this task already exists.");
}
const requestNumber: number =
Expand Down Expand Up @@ -117,7 +117,7 @@ export const updateTaskExtensionRequest = async (req: any, res: any) => {
return res.boom.badRequest(requestResult.error);
}
const [logType, returnMessage] =
requestResult.state === REQUEST_STATE.APPROVED
requestResult.status === REQUEST_STATUS.APPROVED
? [REQUEST_LOG_TYPE.REQUEST_APPROVED, REQUEST_APPROVED_SUCCESSFULLY]
: [REQUEST_LOG_TYPE.REQUEST_REJECTED, REQUEST_REJECTED_SUCCESSFULLY];

Expand Down
10 changes: 5 additions & 5 deletions controllers/onboardingExtension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
REQUEST_DOES_NOT_EXIST,
REQUEST_LOG_TYPE,
REQUEST_REJECTED_SUCCESSFULLY,
REQUEST_STATE,
REQUEST_STATUS,
REQUEST_TYPE,
REQUEST_UPDATED_SUCCESSFULLY,
UNAUTHORIZED_TO_CREATE_ONBOARDING_EXTENSION_REQUEST,
Expand Down Expand Up @@ -73,7 +73,7 @@ export const createOnboardingExtensionRequestController = async (
type: REQUEST_TYPE.ONBOARDING
});

if(latestExtensionRequest && latestExtensionRequest.state === REQUEST_STATE.PENDING){
if(latestExtensionRequest && latestExtensionRequest.status === REQUEST_STATUS.PENDING){
return res.boom.conflict(REQUEST_ALREADY_PENDING);
}

Expand All @@ -93,7 +93,7 @@ export const createOnboardingExtensionRequestController = async (
if(!latestExtensionRequest){
requestNumber = 1;
oldEndsOn = discordJoinedDateInMillisecond + millisecondsInThirtyOneDays;
}else if(latestExtensionRequest.state === REQUEST_STATE.REJECTED) {
}else if(latestExtensionRequest.status === REQUEST_STATUS.REJECTED) {
requestNumber = latestExtensionRequest.requestNumber + 1;
oldEndsOn = latestExtensionRequest.oldEndsOn;
}else{
Expand All @@ -105,7 +105,7 @@ export const createOnboardingExtensionRequestController = async (

const onboardingExtension = await createRequest({
type: REQUEST_TYPE.ONBOARDING,
state: REQUEST_STATE.PENDING,
status: REQUEST_STATUS.PENDING,
userId: userId,
requestedBy: username,
oldEndsOn: oldEndsOn,
Expand Down Expand Up @@ -179,7 +179,7 @@ export const updateOnboardingExtensionRequestState = async (
return res.boom.badRequest(response.error);
}

const [logType, returnMessage] = response.state === REQUEST_STATE.APPROVED
const [logType, returnMessage] = response.state === REQUEST_STATUS.APPROVED
? [REQUEST_LOG_TYPE.REQUEST_APPROVED, REQUEST_APPROVED_SUCCESSFULLY]
: [REQUEST_LOG_TYPE.REQUEST_REJECTED, REQUEST_REJECTED_SUCCESSFULLY];

Expand Down
10 changes: 5 additions & 5 deletions controllers/oooRequests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
REQUEST_CREATED_SUCCESSFULLY,
ERROR_WHILE_CREATING_REQUEST,
REQUEST_ALREADY_PENDING,
REQUEST_STATE,
REQUEST_STATUS,
REQUEST_TYPE,
ERROR_WHILE_UPDATING_REQUEST,
REQUEST_APPROVED_SUCCESSFULLY,
Expand All @@ -28,9 +28,9 @@ export const createOooRequestController = async (req: OooRequestCreateRequest, r
}

try {
const latestOooRequest:OooStatusRequest = await getRequestByKeyValues({ requestedBy: userId, type: REQUEST_TYPE.OOO , state: REQUEST_STATE.PENDING });
const latestOooRequest: OooStatusRequest = await getRequestByKeyValues({ requestedBy: userId, type: REQUEST_TYPE.OOO, status: REQUEST_STATUS.PENDING });

if (latestOooRequest && latestOooRequest.state === REQUEST_STATE.PENDING) {
if (latestOooRequest && latestOooRequest.status === REQUEST_STATUS.PENDING) {
return res.boom.badRequest(REQUEST_ALREADY_PENDING);
}

Expand Down Expand Up @@ -75,7 +75,7 @@ export const updateOooRequestController = async (req: UpdateRequest, res: Custom
return res.boom.badRequest(requestResult.error);
}
const [logType, returnMessage] =
requestResult.state === REQUEST_STATE.APPROVED
requestResult.status === REQUEST_STATUS.APPROVED
? [REQUEST_LOG_TYPE.REQUEST_APPROVED, REQUEST_APPROVED_SUCCESSFULLY]
: [REQUEST_LOG_TYPE.REQUEST_REJECTED, REQUEST_REJECTED_SUCCESSFULLY];

Expand All @@ -90,7 +90,7 @@ export const updateOooRequestController = async (req: UpdateRequest, res: Custom
body: requestResult,
};
await addLog(requestLog.type, requestLog.meta, requestLog.body);
if (requestResult.state === REQUEST_STATE.APPROVED) {
if (requestResult.status === REQUEST_STATUS.APPROVED) {
const requestData = await getRequests({ id: requestId });

if (requestData) {
Expand Down
52 changes: 43 additions & 9 deletions controllers/requests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
REQUEST_FETCHED_SUCCESSFULLY,
REQUEST_TYPE,
} from "../constants/requests";
import { getRequests } from "../models/requests";
import { getRequests, updateRequestStateToStatus } from "../models/requests";
import { getPaginatedLink } from "../utils/helper";
import { createOooRequestController, updateOooRequestController } from "./oooRequests";
import { OooRequestCreateRequest, OooRequestResponse } from "../types/oooRequest";
Expand All @@ -24,6 +24,7 @@ export const createRequestController = async (
res: CustomResponse
) => {
const type = req.body.type;
console.log("type", type);
switch (type) {
case REQUEST_TYPE.OOO:
return await createOooRequestController(req as OooRequestCreateRequest, res as OooRequestResponse);
Expand Down Expand Up @@ -60,12 +61,12 @@ export const getRequestsController = async (req: any, res: any) => {
return res.status(204).send();
}

if (query.id) {
return res.status(200).json({
message: REQUEST_FETCHED_SUCCESSFULLY,
data: requests,
});
}
if (query.id) {
return res.status(200).json({
message: REQUEST_FETCHED_SUCCESSFULLY,
data: requests,
});
}

const { allRequests, next, prev, page } = requests;
if (allRequests.length === 0) {
Expand Down Expand Up @@ -123,11 +124,44 @@ export const getRequestsController = async (req: any, res: any) => {
*/
export const updateRequestBeforeAcknowledgedController = async (req: Request, res: CustomResponse) => {
const type = req.body.type;
switch(type){
switch (type) {
case REQUEST_TYPE.ONBOARDING:
await updateOnboardingExtensionRequestController(req as UpdateOnboardingExtensionRequest, res as OnboardingExtensionResponse);
break;
default:
return res.boom.badRequest("Invalid request");
}
}
};
interface RequestWithParams extends Request {
params: {
id: string;
};
}

export const migrateRequestStateToStatus = async (req: RequestWithParams, res: CustomResponse) => {
try {
const docId = req.params.id;
console.log(docId);
const result = await updateRequestStateToStatus(docId);

if (result.success) {
return res.status(200).json({
success: true,
message: 'Successfully migrated state to status',
data: result.data
});
} else {
return res.status(404).json({
success: false,
message: result.message
});
}
} catch (error) {
console.error('Migration error:', error);
return res.status(500).json({
success: false,
message: 'Failed to migrate document',
error: error.message
});
}
};
13 changes: 7 additions & 6 deletions controllers/taskRequestsv2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { REQUEST_STATE, TASK_REQUEST_MESSAGES } from "../constants/requests";
import { REQUEST_STATUS, TASK_REQUEST_MESSAGES } from "../constants/requests";
import { TASK_REQUEST_TYPE } from "../constants/taskRequests";
import { addLog } from "../models/logs";
import { createRequest, getRequestByKeyValues } from "../models/requests";
Expand All @@ -24,6 +24,7 @@ export const createTaskRequestController = async (req: TaskRequestRequest, res:
const userPromise: any = await fetchUser({ userId: taskRequestData.userId });
const userData: userData = userPromise.user;
if (!userData.id || !userData.username) {
console.log("userData", userData);
return res.boom.notFound(TASK_REQUEST_MESSAGES.USER_NOT_FOUND);
}
try {
Expand Down Expand Up @@ -64,13 +65,13 @@ export const createTaskRequestController = async (req: TaskRequestRequest, res:

if (
existingRequest &&
existingRequest.state === REQUEST_STATE.PENDING &&
existingRequest.state === REQUEST_STATUS.PENDING &&
existingRequest.requestors.includes(requestedBy)
) {
return res.boom.badRequest(TASK_REQUEST_MESSAGES.TASK_REQUEST_EXISTS);
} else if (
existingRequest &&
existingRequest.state === REQUEST_STATE.PENDING &&
existingRequest.state === REQUEST_STATUS.PENDING &&
!existingRequest.requestors.includes(requestedBy)
) {
existingRequest.requestors.push(requestedBy);
Expand All @@ -83,7 +84,7 @@ export const createTaskRequestController = async (req: TaskRequestRequest, res:
markdownEnabled: taskRequestData.markdownEnabled,
firstName: userData.first_name,
lastName: userData.last_name,
state: REQUEST_STATE.PENDING,
status: REQUEST_STATUS.PENDING,
requestedAt: Date.now(),
});
const updatedRequest = await createRequest(existingRequest);
Expand Down Expand Up @@ -116,7 +117,7 @@ export const createTaskRequestController = async (req: TaskRequestRequest, res:
externalIssueHtmlUrl: taskRequestData.externalIssueHtmlUrl,
requestType: taskRequestData.requestType,
type: taskRequestData.type,
state: REQUEST_STATE.PENDING,
status: REQUEST_STATUS.PENDING,
requestedBy: requestedBy,
taskTitle: taskRequestData.taskTitle,
users: [
Expand All @@ -129,7 +130,7 @@ export const createTaskRequestController = async (req: TaskRequestRequest, res:
markdownEnabled: taskRequestData.markdownEnabled,
firstName: userData.first_name,
lastName: userData.last_name,
state: REQUEST_STATE.PENDING,
status: REQUEST_STATUS.PENDING,
requestedAt: Date.now(),
},
],
Expand Down
7 changes: 3 additions & 4 deletions controllers/tasksRequests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ const addTaskRequests = async (req, res) => {
if (!username) {
return res.boom.badRequest("User not found");
}
if (!issueData) {
return res.boom.badRequest("Issue does not exist");
}
// if (!issueData) {
// return res.boom.badRequest("Issue does not exist");
// }
break;
}
}
Expand All @@ -112,7 +112,6 @@ const addTaskRequests = async (req, res) => {
body: newTaskRequest.taskRequest,
};
await addLog(taskRequestLog.type, taskRequestLog.meta, taskRequestLog.body);

const statusCode = newTaskRequest.isCreate ? 201 : 200;
return res.status(statusCode).json({
message: "Task request successful.",
Expand Down
8 changes: 4 additions & 4 deletions middlewares/validators/extensionRequestsv2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import joi from "joi";
import { ExtensionRequestRequest, ExtensionRequestResponse } from "../../types/extensionRequests";
import { NextFunction } from "express";
import { REQUEST_TYPE,REQUEST_STATE } from "../../constants/requests";
import { REQUEST_TYPE,REQUEST_STATUS } from "../../constants/requests";

export const createExtensionRequestValidator = async (
req: ExtensionRequestRequest,
Expand Down Expand Up @@ -32,9 +32,9 @@ export const createExtensionRequestValidator = async (
message: joi.string().required().messages({
"string.empty": "message cannot be empty",
}),
state: joi.string().valid(REQUEST_STATE.PENDING).required().messages({
"string.empty": "state cannot be empty",
"any.required": "state is required",
status: joi.string().valid(REQUEST_STATUS.PENDING).required().messages({
"string.empty": "status cannot be empty",
"any.required": "status is required",
}),
type: joi.string().valid(REQUEST_TYPE.EXTENSION).required().messages({
"string.empty": "type cannot be empty",
Expand Down
6 changes: 3 additions & 3 deletions middlewares/validators/oooRequests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import joi from "joi";
import { NextFunction } from "express";
import { REQUEST_STATE, REQUEST_TYPE } from "../../constants/requests";
import { REQUEST_STATUS, REQUEST_TYPE } from "../../constants/requests";
import { OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";

export const createOooStatusRequestValidator = async (
Expand Down Expand Up @@ -30,8 +30,8 @@ export const createOooStatusRequestValidator = async (
"any.required": "message is required",
"string.empty": "message cannot be empty",
}),
state: joi.string().valid(REQUEST_STATE.PENDING).required().messages({
"any.only": "state must be PENDING",
state: joi.string().valid(REQUEST_STATUS.PENDING).required().messages({
Copy link

@coderabbitai coderabbitai bot Apr 29, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Constant reference updated but property name remains unchanged

The validation schema now correctly uses REQUEST_STATUS.PENDING instead of the previous REQUEST_STATE.PENDING. However, the property being validated is still named state rather than status, which seems inconsistent with the PR objective of renaming "state" to "status".

Should the property name in the schema also be updated to status? This would require updating:

  1. The schema definition
  2. The error message on line 34
  3. Any dependent code that accesses this property

🏁 Script executed:

#!/bin/bash
# Check for instances of the validation error message mentioning "state"
rg -t ts '"state must be PENDING"'

Length of output: 341

Rename the state field to status in the validator and update tests

To fully align with the PR’s intent of renaming “state” → “status,” you’ll need to:

• middlewares/validators/oooRequests.ts
– Change the schema key from state to status
– Update the validation message from "state must be PENDING" to "status must be PENDING"

• test/integration/requests.test.ts
– Update the assertion to expect "status must be PENDING"

• test/unit/middlewares/oooRequests.test.ts
– Update the assertion to expect "status must be PENDING"

• Anywhere else in the codebase that reads or writes request.state, adjust to request.status

Example diff in oooRequests.ts:

-  state: joi.string().valid(REQUEST_STATUS.PENDING).required().messages({
-    "any.only": "state must be PENDING",
+  status: joi.string().valid(REQUEST_STATUS.PENDING).required().messages({
+    "any.only": "status must be PENDING",

After making these changes, rerun your test suite to confirm everything passes.

📝 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
state: joi.string().valid(REQUEST_STATUS.PENDING).required().messages({
status: joi.string().valid(REQUEST_STATUS.PENDING).required().messages({
"any.only": "status must be PENDING",
}),

"any.only": "status must be PENDING",
}),
type: joi.string().valid(REQUEST_TYPE.OOO).required(),
});
Expand Down
Loading