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

Fix/bulk invite api error response #6247

Merged
merged 3 commits into from
Oct 23, 2024

Conversation

makeavish
Copy link
Member

@makeavish makeavish commented Oct 23, 2024

Summary

Respond with 400 in case of failures. Since it is a bulk API which can have multiple types of failures, so according to discussion with frontend team, we have decided to use 400 as common failure code with failure details in json


Important

Change bulk invite API to return 400 status code for failures.

  • Behavior:
    • Change HTTP status code from 500 to 400 for failure responses in inviteUsers() in http_handler.go.
  • Misc:
    • Update comments to reflect the new status code behavior in http_handler.go.

This description was created by Ellipsis for 5a7e597. It will automatically update as commits are pushed.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions github-actions bot added the bug Something isn't working label Oct 23, 2024
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 5a7e597 in 49 seconds

More details
  • Looked at 28 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. pkg/query-service/app/parser.go:752
  • Draft comment:
    Removing the name validation might cause issues if the name is required elsewhere. Ensure this change aligns with application requirements.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. pkg/query-service/app/parser.go:752
  • Draft comment:
    Ensure all necessary fields are validated in parseInviteUsersRequest. The removal of name validation is fine, but other fields should still be checked.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The removal of the name validation in the parseInviteUsersRequest function is correct as it aligns with the PR's goal to handle bulk invite errors more gracefully. However, the function should still ensure that all necessary fields are validated.

Workflow ID: wflow_Ol0QeRYQph8aa1Sf


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

1 similar comment
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@makeavish makeavish merged commit 0e2b670 into develop Oct 23, 2024
13 of 16 checks passed
@makeavish makeavish deleted the fix/bulk-invite-api-error-response branch October 23, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants