Skip to content

Conversation

@Aakash452
Copy link
Contributor

No description provided.

setSuccessMsg(`Form submitted successfully and sent to ${recipient}`);
setTimeout(() => setSuccessMsg(""), 15000);
setFormData(initialState);
setTimeout(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing cleanup/cancellation of the timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed !, added clearTimeout function.

? "coordinator for manual review!"
: "supervisor!";
setSuccessMsg(`Form submitted successfully and sent to ${recipient}`);
setTimeout(() => setSuccessMsg(""), 15000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing cleanup/cancellation of the timeout. let the team c know about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just copy pasted this handleSubmit() function from the main branch 3 days ago, no other changes are done by our team. Also, for the setTimeout function that navigates the user back to the student dashboard( that I added), fixed that part by adding the clearTimeout function.

Copy link
Collaborator

Choose a reason for hiding this comment

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

let team c know sbout other timeout function, so that they can handle it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Informed them

package.json Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is new package.json added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

those are the existing packages, just the new version updated( bcrypt, dayjs and sweetalert)

Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need a new one? why cant we add it in the client package.json?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed, we actually don't need them.

};
}

// OPTIONAL SAFEGUARD: Only allow sending if student role
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we not allowing other roles to send emails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed ! .It was to prevent Token Request emails sending to the supervisor and coordinator, but removed that part and it still emails are not sending to the supervisor/coordinator.

};
}

// // OPTIONAL SAFEGUARD: Only allow sending if student role
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this not used just remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed!

package.json Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need a new one? why cant we add it in the client package.json?

? "coordinator for manual review!"
: "supervisor!";
setSuccessMsg(`Form submitted successfully and sent to ${recipient}`);
setTimeout(() => setSuccessMsg(""), 15000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

let team c know sbout other timeout function, so that they can handle it.

Remember me
</label>
<Link
{/* <Link
Copy link
Collaborator

Choose a reason for hiding this comment

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

delete if this is not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted!

Copy link
Collaborator

@NNPhaniCharan NNPhaniCharan left a comment

Choose a reason for hiding this comment

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

Resolve conflicts

@NNPhaniCharan NNPhaniCharan merged commit 101975b into main Apr 29, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants