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

Delete my profile implemented #54

Merged
merged 14 commits into from
Aug 22, 2023

Conversation

Danishkar
Copy link
Contributor

Purpose

The purpose of this PR is to fix #11

Goals

Approach

Screenshots

Checklist

  • This PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.
  • I have read and understood the development best practices guidelines ( http://bit.ly/sef-best-practices )
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

Related PRs

Test environment

Learning

Copy link
Member

@anjula-sack anjula-sack left a comment

Choose a reason for hiding this comment

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

can you write tests as well? @Danishkar

if (!user || user.uuid !== userId) {
res.status(404).json({ message: 'Profile not found' })
} else {
await deleteProfile(user)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
await deleteProfile(user)
await deleteProfile(user.uuid)

@@ -39,3 +39,14 @@ export const updateProfile = async (

return savedProfile
}

export const deleteProfile = async (user: Profile): Promise<void> => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export const deleteProfile = async (user: Profile): Promise<void> => {
export const deleteProfile = async (userId: string): Promise<void> => {

): Promise<void> => {
try {
const userId = req.params.uuid
const user = await getProfile(req)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const user = await getProfile(req)
const user = req.user

@anjula-sack
Copy link
Member

Hi @Danishkar, can you fix the conflicts?

@Danishkar
Copy link
Contributor Author

Hi @Danishkar, can you fix the conflicts?

yea sure

@Danishkar
Copy link
Contributor Author

can you write tests as well? @Danishkar

i will work on that

@anjula-sack
Copy link
Member

Hi @Danishkar, can you fix the conflicts?

yea sure

Can you fix the lint errors as well? Run npm run lint:fix

@Danishkar
Copy link
Contributor Author

npm run lint:fix

done

Copy link
Member

@anjula-sack anjula-sack left a comment

Choose a reason for hiding this comment

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

@Danishkar can you run lint fix again? The CI is failing.

@anjula-sack
Copy link
Member

@Danishkar the tests are failing

@Danishkar
Copy link
Contributor Author

@Danishkar the tests are failing

yea bro I am new to this jest testing I will check on that.

Copy link
Member

@anjula-sack anjula-sack left a comment

Choose a reason for hiding this comment

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

Requested some changes

res: Response
): Promise<void> => {
try {
const userId = req.params.uuid
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const userId = req.params.uuid

try {
const userId = req.params.uuid
const user = req.user as Profile
if (!user || user.uuid !== userId) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!user || user.uuid !== userId) {
if (!user) {

@@ -9,5 +10,6 @@ const profileRouter = express.Router()

profileRouter.get('/profile', requireAuth, getProfileHandler)
profileRouter.put('/profile', requireAuth, updateProfileHandler)
profileRouter.delete('/profile/:uuid', requireAuth, deleteProfileHandler)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
profileRouter.delete('/profile/:uuid', requireAuth, deleteProfileHandler)
profileRouter.delete('/profile', requireAuth, deleteProfileHandler)

@anjula-sack
Copy link
Member

@Danishkar can you take a pull and fix the lint issues @Danishkar

Copy link
Member

@anjula-sack anjula-sack left a comment

Choose a reason for hiding this comment

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

Good job! @Danishkar

@anjula-sack anjula-sack merged commit a5a2e70 into sef-global:main Aug 22, 2023
1 check passed
anjula-sack pushed a commit to anjula-sack/scholarx-backend that referenced this pull request Jul 23, 2024
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.

Implement delete my profile endpoint
2 participants