Skip to content

Conversation

Pavangbhat
Copy link
Contributor

@Pavangbhat Pavangbhat commented Mar 23, 2024

Date: 23-03-2024

Developer Name: Pavan Bhat


Issue Ticket Number

Real-Dev-Squad/cron-jobs#60

Description

Created a new route that can support nickname sync action

Documentation Updated?

  • Yes
  • No

Under Feature Flag

  • Yes
  • No

Database Changes

  • Yes
  • No

Breaking Changes

  • Yes
  • No

Development Tested?

  • Yes
  • No

Screenshots

Screenshot 1

Test Coverage

Screenshot 1

Additional Notes

Updated API contracts here: Real-Dev-Squad/website-api-contracts#180

@Pavangbhat Pavangbhat marked this pull request as draft March 23, 2024 19:58
@Pavangbhat Pavangbhat self-assigned this Mar 26, 2024
@Pavangbhat Pavangbhat changed the title created new route for sync Created new route for nickname sync actions Mar 26, 2024
@Pavangbhat Pavangbhat force-pushed the feat/user-status-sync branch from bb3d10c to 690d0a1 Compare March 30, 2024 16:48
router.get("/", validateGetQueryParams, getUserStatusControllers);
router.get("/self", authenticate, getUserStatus);
router.get("/:userId", getUserStatus);
router.patch("/sync", authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]), syncUserStatus);

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited. This route handler performs [authorization](2), but is not rate-limited. This route handler performs [authorization](3), but is not rate-limited. This route handler performs [authorization](4), but is not rate-limited. This route handler performs [authorization](5), but is not rate-limited. This route handler performs [authorization](6), but is not rate-limited. This route handler performs [authorization](7), but is not rate-limited.
@Pavangbhat Pavangbhat force-pushed the feat/user-status-sync branch from e735491 to beac0a5 Compare March 30, 2024 18:17
@Pavangbhat Pavangbhat marked this pull request as ready for review March 30, 2024 18:18
@Pavangbhat Pavangbhat force-pushed the feat/user-status-sync branch from e16e29d to ef37641 Compare March 30, 2024 18:44
Comment on lines +245 to +246
await updateAllUserStatus(req, res, next);
const usersData = await getTaskBasedUsersStatus(req, res, next);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use this two fx as middleware?
also, these functions seem to be returning res, which will result in an error, because in that case we are calling res/json 3 times.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO logic that should be part of controllers should not be used as middleware

also, these functions seem to be returning res, which will result in an error, because in that case we are calling res/json 3 times.

Appologies did not understand

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.

2 participants