-
Notifications
You must be signed in to change notification settings - Fork 45
Port /v2/user/<user-uuid>/active-signature to golang #4714
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
Port /v2/user/<user-uuid>/active-signature to golang #4714
Conversation
Signed-off-by: Lukasz Gryglicki <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a new /user/{userID}/active-signature endpoint to the Go-based API spec, along with its shared response schema.
- Introduce
user-active-signature.yamldefining the response object. - Add the
/user/{userID}/active-signatureGET path with parameters and responses. - Reference the new schema in
cla.v2.yamldefinitions.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| swagger/common/user-active-signature.yaml | New schema for “User Active Signature” response |
| swagger/cla.v2.yaml | Added /user/{userID}/active-signature endpoint and wired the schema |
Comments suppressed due to low confidence (4)
cla-backend-go/swagger/cla.v2.yaml:2458
- The summary field includes a JSON example block; consider moving that example into the description and keeping the summary to a brief one-line overview.
summary: |
cla-backend-go/swagger/cla.v2.yaml:2451
- [nitpick] The path parameter is named
userID(camelCase) while the response schema usesuser_id(snake_case). Consider standardizing naming conventions across parameters and payload fields.
- name: userID
cla-backend-go/swagger/cla.v2.yaml:2469
- The new
getUserActiveSignatureendpoint does not appear to have accompanying tests; please add unit/integration tests to cover its behavior.
operationId: getUserActiveSignature
cla-backend-go/swagger/cla.v2.yaml:2465
- There's an extra trailing apostrophe at the end of this line which will break YAML parsing; please remove it.
'return_url': <url-where-user-initiated-signature-from>'
Signed-off-by: Lukasz Gryglicki <[email protected]>
Signed-off-by: Lukasz Gryglicki <[email protected]>
Signed-off-by: Lukasz Gryglicki <[email protected]>
Signed-off-by: Lukasz Gryglicki <[email protected]>
Signed-off-by: Lukasz Gryglicki <[email protected]>
|
@lukaszgryglicki, LGTM, just make sure Python's API responses are the same as this Golang version. For example Python version also returned 200 with a null body for no signature and returns BadRequest on error, and JSON null on no result. Any change in status code could affect UI. I assume you checked if this model matches what the Python API returned (field names, types, nesting). I recommend to use AI/CoPilot to compare these 2 implementations (Python vs. Golang) which should be able to catch any differences. |
|
I will also add test coverage that check exactly for this on Tue. |
|
Sounds good, thanks! |
… old v2 one Signed-off-by: Lukasz Gryglicki <[email protected]>
|
I've added test coverage for the new API - PTAL again @mlehotskylf - ty. |
Signed-off-by: Lukasz Gryglicki <[email protected]>
cc @mlehotskylf