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

Cleanup getProjectsForUser function #2320

Merged
merged 3 commits into from
Jan 25, 2024

Conversation

lindapaiste
Copy link
Collaborator

Progress on issue #521

Changes:

  • Combine common logic from getProjectsForUser (internal API) and apiGetProjectsForUser (public API).
  • Add comments explaining the difference between the two handlers.
  • Fix the "callback hell" by converting to async/await syntax.
  • Update the test file to support async user lookup.
  • Behavior change - internal API returns an error if there is no req.params.username instead of returning an empty array.

Note: there is a lot of discussion in PR #973 about wrapping an entire Express handler in a try/catch like I am doing here and whether there are better alternatives. We may want to revisit this as I'm not sure what the current best practice is.

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

Copy link
Contributor

@PiyushChandra17 PiyushChandra17 left a comment

Choose a reason for hiding this comment

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

Nicely Done

@raclim raclim added this to the MINOR Release for 2.12.0 milestone Jan 19, 2024
Copy link
Collaborator

@raclim raclim left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this!

@raclim raclim merged commit 1e218f4 into processing:develop Jan 25, 2024
1 check passed
@lindapaiste lindapaiste mentioned this pull request Feb 19, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants