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

feat: implements a new GET route /users?profile=true #2201

Merged
merged 12 commits into from
Oct 16, 2024

Conversation

vikasosmium
Copy link
Contributor

@vikasosmium vikasosmium commented Oct 7, 2024

Date: 08-Oct-2024

Developer Name: Vikas Singh

Design Doc: Deprecate /users/self GET Route and Implement New Replacement Route #2126


Issue Ticket Number

Description

Implements a new route for fetching user data as /users?profile=true

Documentation Updated?

Under Feature Flag

  • Yes
  • No

Database Changes

  • Yes
  • No

Breaking Changes

  • Yes
  • No

Development Tested?

  • Yes
  • No

Screenshots

Screenshot 1

Test Coverage

1. Unit Test

Screenshot 2024-10-09 071621

Screenshot 2024-10-16 003454

Screenshot 2024-10-16 003407

Screenshot 2024-10-16 003529

  1. Integration Test

Screenshot 2024-10-16 003651

Screenshot 2024-10-16 003706

Screenshot 2024-10-16 003734

@shubhdevelop

This comment has been minimized.

@shubhdevelop

This comment has been minimized.

controllers/users.js Outdated Show resolved Hide resolved
controllers/users.js Outdated Show resolved Hide resolved
@Mir-SA
Copy link

Mir-SA commented Oct 8, 2024

@vikasosmium can you please add test coverage?
some of the tests in this pr seem to fail in my local build...
image
image

@vikasosmium vikasosmium changed the title Deprecate users/self get route Implements a new route for fetching user data as /users?profile=true Oct 9, 2024
@vikasosmium vikasosmium changed the title Implements a new route for fetching user data as /users?profile=true Implements a new route for fetching user data Oct 9, 2024
@vikasosmium
Copy link
Contributor Author

vikasosmium commented Oct 9, 2024

I have added test coverage screenshots.

@vikasosmium vikasosmium changed the title Implements a new route for fetching user data Implements a new route /users?profile for fetching user data Oct 9, 2024
@vikasosmium vikasosmium changed the title Implements a new route /users?profile for fetching user data Implements a new GET route /users?profile=true Oct 9, 2024
@vikasosmium vikasosmium marked this pull request as ready for review October 9, 2024 02:19
@shubhdevelop

This comment has been minimized.

@shubhdevelop

This comment has been minimized.

controllers/users.js Outdated Show resolved Hide resolved
controllers/users.js Outdated Show resolved Hide resolved
routes/users.js Fixed Show fixed Hide fixed
test/unit/middlewares/authCondition.test.js Fixed Show fixed Hide fixed
routes/users.js Fixed Show fixed Hide fixed
@vikasosmium vikasosmium requested a review from yesyash October 14, 2024 21:02
Comment on lines 119 to 137
if (dev) {
let user;

try {
if (req.userData.id) {
const result = await dataAccess.retrieveUsers({ id: req.userData.id });
user = result.user;
} else {
return res.boom.badRequest("User ID not provided.");
}
} catch (error) {
logger.error(`Error while fetching user: ${error}`);
return res.boom.serverUnavailable(INTERNAL_SERVER_ERROR);
}

return res.send(user);
} else {
return res.boom.badRequest("Route not found");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (dev) {
let user;
try {
if (req.userData.id) {
const result = await dataAccess.retrieveUsers({ id: req.userData.id });
user = result.user;
} else {
return res.boom.badRequest("User ID not provided.");
}
} catch (error) {
logger.error(`Error while fetching user: ${error}`);
return res.boom.serverUnavailable(INTERNAL_SERVER_ERROR);
}
return res.send(user);
} else {
return res.boom.badRequest("Route not found");
}
if (dev) {
if (!req.userData.id) {
return res.boom.badRequest("User ID not provided.");
}
try {
const result = await dataAccess.retrieveUsers({ id: req.userData.id });
return res.send(result.user);
} catch (error) {
logger.error(`Error while fetching user: ${error}`);
return res.boom.serverUnavailable(INTERNAL_SERVER_ERROR);
}
} else {
return res.boom.badRequest("Route not found");
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will check 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 have made the suggested changes.

@vikasosmium vikasosmium changed the title Implements a new GET route /users?profile=true feat: implements a new GET route /users?profile=true Oct 14, 2024
controllers/users.js Outdated Show resolved Hide resolved
middlewares/validators/user.js Show resolved Hide resolved
test/integration/users.test.js Show resolved Hide resolved
controllers/users.js Outdated Show resolved Hide resolved
controllers/users.js Outdated Show resolved Hide resolved
controllers/users.js Show resolved Hide resolved
controllers/users.js Outdated Show resolved Hide resolved
controllers/users.js Outdated Show resolved Hide resolved
Comment on lines 119 to 137
if (dev) {
let user;

try {
if (req.userData.id) {
const result = await dataAccess.retrieveUsers({ id: req.userData.id });
user = result.user;
} else {
return res.boom.badRequest("User ID not provided.");
}
} catch (error) {
logger.error(`Error while fetching user: ${error}`);
return res.boom.serverUnavailable(INTERNAL_SERVER_ERROR);
}

return res.send(user);
} else {
return res.boom.badRequest("Route not found");
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will check this!

middlewares/authCondition.js Outdated Show resolved Hide resolved
routes/users.js Dismissed Show dismissed Hide dismissed
@iamitprakash iamitprakash merged commit ba269bd into Real-Dev-Squad:develop Oct 16, 2024
3 checks passed
@Achintya-Chatterjee Achintya-Chatterjee mentioned this pull request Oct 16, 2024
10 tasks
@vikasosmium vikasosmium added feature task Feature that has to be built hacktoberfest hacktoberfest celebration to contribution to open source labels Oct 28, 2024
@vikasosmium vikasosmium added enhancement New feature or request backend labels Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend enhancement New feature or request feature task Feature that has to be built hacktoberfest hacktoberfest celebration to contribution to open source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate /users/self GET Route and Implement New Replacement Route
7 participants