-
Notifications
You must be signed in to change notification settings - Fork 73
[SYNPY-1674]: Implement Form #1288
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
Conversation
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
This PR implements form-related functionality for the Synapse Python client, adding support for creating and managing FormGroup and FormData objects through new REST API endpoints.
Key Changes:
- Implements FormGroup and FormData models with synchronous and asynchronous support
- Adds API service functions for creating form groups, creating form data, listing form data (with reviewer mode), and downloading form data
- Provides comprehensive test coverage with both unit and integration tests
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
synapseclient/models/form.py |
Main FormGroup and FormData model implementations with async/sync methods for create, list, and download operations |
synapseclient/models/mixins/form.py |
Core data structures including FormGroup, FormData, FormChangeRequest, FormSubmissionStatus, and StateEnum |
synapseclient/models/protocols/form_protocol.py |
Protocol definitions for FormGroup and FormData operations |
synapseclient/api/form_services.py |
API service layer implementing REST endpoints for form operations |
synapseclient/models/mixins/__init__.py |
Exports form-related mixins and enums |
synapseclient/models/__init__.py |
Exports FormGroup and FormData models |
synapseclient/api/__init__.py |
Exports form service functions |
tests/unit/synapseclient/models/synchronous/unit_test_form.py |
Unit tests for synchronous form operations |
tests/unit/synapseclient/models/async/unit_test_form_async.py |
Unit tests for asynchronous form operations |
tests/unit/synapseclient/mixins/unit_test_form_mixin.py |
Unit tests for form mixin data structures |
tests/integration/synapseclient/models/synchronous/test_form.py |
Integration tests for synchronous form operations |
tests/integration/synapseclient/models/async/test_form_async.py |
Integration tests for asynchronous form operations |
mkdocs.yml |
Documentation navigation configuration for form-related pages |
docs/reference/experimental/sync/form.md |
Synchronous API reference documentation |
docs/reference/experimental/async/form.md |
Asynchronous API reference documentation |
docs/reference/experimental/mixins/form_group.md |
FormGroup mixin documentation |
docs/reference/experimental/mixins/form_data.md |
FormData mixin documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@linglp The public methods look good. I requested a review from Copilot, but it doesn't always provide good suggestions so feel free to resolve any Copilot comments without making changes. |
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
Copilot reviewed 17 out of 17 changed files in this pull request and generated 15 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/integration/synapseclient/models/synchronous/test_form.py
Outdated
Show resolved
Hide resolved
tests/integration/synapseclient/models/async/test_form_async.py
Outdated
Show resolved
Hide resolved
tests/integration/synapseclient/models/async/test_form_async.py
Outdated
Show resolved
Hide resolved
|
LGTM |
|
Want to merge PR now after confirming the failed integration tests are working locally: https://sagebionetworks.jira.com/browse/DPE-1524 |
Problem:
Implement the following endpoints: