Merged
Conversation
Added endpoints to get, update, and delete polls
This route returns list of all permissions a member has in a workspace and permissions for each group in the workspace that a member is a part of
Fixed issues with permissions fetching and formatted the output
Patched poll actions
The validator check that published polls have questions, and that questions' ids are in correct format
Added validators to check if question id is positive, correct options are postitive, unique, and in the possible range
Since polls do not have members there needs to be a condition to get member from workspace insted
When updating poll the response would return backling objet to the workspace instead of the workspace schema, the issue was caused by saving the poll and subsequently replacing mongo document, to resolve the issue the workspace is stored before updating the poll
Added an endpoint to get member permissions in a workspace and a group
When user has no permissions, the function should return empty list
If a user does not have a policy, the function would raise HTTP Exception, so an additional try-excpept block is required
When getting poll policies, we should use workspace for getting member list, since poll does not have members
Changed resource type
Raise UserNotAuthorized if user is not a member
User now is able to get polls with get_poll or get_polls(workspace) permission
Added name and description field validation
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces API version 2 enhancements to the unipoll API, focusing on improving group and poll management, standardizing exception handling, and adding comprehensive permission management. The changes enhance workspace inclusion capabilities, refine validation logic, and restructure routing for better organization.
- Improved group and poll management with enhanced query parameters and validation
- Standardized exception handling with more descriptive error messages and proper HTTP status codes
- Enhanced permission management system with new member permission endpoints
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/unipoll_api/utils/permissions.py | Enhanced permission checking with better error handling and resource-specific logic |
| src/unipoll_api/schemas/workspace.py | Added field validation constraints for workspace creation and update requests |
| src/unipoll_api/schemas/question.py | Implemented comprehensive validation for question schemas with type constraints |
| src/unipoll_api/schemas/poll.py | Added poll validation logic and workspace field updates |
| src/unipoll_api/routes/v2/polls.py | Fixed parameter initialization bug in poll retrieval |
| src/unipoll_api/routes/v2/permissions.py | Added new member permissions endpoint with comprehensive permission listing |
| src/unipoll_api/routes/v2/groups.py | Extended group query parameters to include workspace information |
| src/unipoll_api/routes/v1/workspace.py | Refactored workspace routes with new permission endpoint |
| src/unipoll_api/routes/v1/poll.py | New poll management routes with workspace-scoped endpoints |
| src/unipoll_api/routes/v1/group.py | Updated group routes with workspace dependencies and new permission endpoints |
| src/unipoll_api/routes/v1/init.py | Added poll routes to main router configuration |
| src/unipoll_api/exceptions/workspace.py | Updated exception handling to use Member instead of Account |
| src/unipoll_api/exceptions/resource.py | Improved exception messages and changed status codes for better clarity |
| src/unipoll_api/exceptions/poll.py | Standardized poll exception handling |
| src/unipoll_api/exceptions/group.py | Updated group exception handling consistency |
| src/unipoll_api/documents.py | Modified Poll document structure with BackLink for workspace relationship |
| src/unipoll_api/dependencies.py | Fixed exception types and improved error handling |
| src/unipoll_api/actions/poll.py | Enhanced poll creation and update logic with better workspace handling |
| src/unipoll_api/actions/policy.py | Added resource-specific member retrieval logic |
| src/unipoll_api/actions/group.py | Added workspace inclusion support for group operations |
| params = {"include_questions": True, | ||
| "include_policies": True} | ||
| else: | ||
| if "groups" in include: |
There was a problem hiding this comment.
The condition checks for "groups" but should check for "questions" based on the context and the parameter assignment on line 58.
Suggested change
| if "groups" in include: | |
| if "questions" in include: |
Comment on lines
+65
to
+66
| if "workspaces" in include: | ||
| params["include_workspaces"] = True |
There was a problem hiding this comment.
The condition checks for "workspaces" (plural) but should check for "workspace" (singular) to match the query parameter definition and the parameter assignment on line 66.
Suggested change
| if "workspaces" in include: | |
| params["include_workspaces"] = True | |
| if "workspace" in include: | |
| params["include_workspace"] = True |
| if "policies" in include: | ||
| params["include_policies"] = True | ||
| if "workspaces" in include: | ||
| params["include_workspaces"] = True |
There was a problem hiding this comment.
The parameter name should be "include_workspace" (singular) to match the function parameter in the GroupActions.get_group method.
Suggested change
| params["include_workspaces"] = True | |
| params["include_workspace"] = True |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several updates across the
unipoll_apicodebase, focusing on enhancing group and poll management, improving exception handling, and refining dependencies and routing. The most significant changes include adding support for workspace inclusion in group queries, improving poll creation and update workflows, and standardizing exception handling for better error reporting.Enhancements to Group and Poll Management:
include_workspaceparameter to theget_groupfunction, allowing workspace details to be optionally included in the response. Updated the workspace serialization to only include specific fields (id,name,description). ([[1]](https://github.com/csed-ucm/psephos/pull/94/files#diff-dafacf886695f5ded37150101af1be3e3eaefe9562182fb92003050ac4d9ea41R85),[[2]](https://github.com/csed-ucm/psephos/pull/94/files#diff-dafacf886695f5ded37150101af1be3e3eaefe9562182fb92003050ac4d9ea41L92-R96))[[1]](https://github.com/csed-ucm/psephos/pull/94/files#diff-393f0167ffc02c3865cf57eaef625a206bb5582b78f4346360418f2ea85eb9b4R40-R49),[[2]](https://github.com/csed-ucm/psephos/pull/94/files#diff-393f0167ffc02c3865cf57eaef625a206bb5582b78f4346360418f2ea85eb9b4R61-R91))update_pollwhere the workspace turned into a link after updating. Added a workaround to save the workspace before updating the poll and adjusted the response format. ([[1]](https://github.com/csed-ucm/psephos/pull/94/files#diff-393f0167ffc02c3865cf57eaef625a206bb5582b78f4346360418f2ea85eb9b4L104-R128),[[2]](https://github.com/csed-ucm/psephos/pull/94/files#diff-393f0167ffc02c3865cf57eaef625a206bb5582b78f4346360418f2ea85eb9b4L119-R151))Improvements to Exception Handling:
UserNotAuthorizedexception across various modules to provide more detailed error messages, including the resource type and name. ([[1]](https://github.com/csed-ucm/psephos/pull/94/files#diff-1e8b4b8d122438e7353e075faa822301eae17d32f5b0cad06bc0edd42d5540fbL26-R26),[[2]](https://github.com/csed-ucm/psephos/pull/94/files#diff-97c1cbf5172089ef1a5aa111a40e5a58a33ea8f9eac23dae7f6f25303d23cab4L44-R44),[[3]](https://github.com/csed-ucm/psephos/pull/94/files#diff-cd5d188f559d03db25c276cb5c415e1d87bf97884e9be1d57dbcb0e475df8f55L49-R51))409 Conflictstatus code instead of400 Bad Request. ([src/unipoll_api/exceptions/resource.pyL25-R25](https://github.com/csed-ucm/psephos/pull/94/files#diff-cd5d188f559d03db25c276cb5c415e1d87bf97884e9be1d57dbcb0e475df8f55L25-R25))Memberinstead ofAccountfor better clarity. ([[1]](https://github.com/csed-ucm/psephos/pull/94/files#diff-e03beb28c2b49fefb15a90fa9c708b2e1c508a57afd6a8c2ec6b5ef386f117c5L55-R56),[[2]](https://github.com/csed-ucm/psephos/pull/94/files#diff-97c1cbf5172089ef1a5aa111a40e5a58a33ea8f9eac23dae7f6f25303d23cab4L55-R56))Dependency and Routing Updates:
PollRoutesto the main router and simplified dependencies for group and poll routes. ([[1]](https://github.com/csed-ucm/psephos/pull/94/files#diff-6213aa2dc7904bee716b9ca3823c29275ebb5f333e3fe5d33edaa122c807931aR9),[[2]](https://github.com/csed-ucm/psephos/pull/94/files#diff-6213aa2dc7904bee716b9ca3823c29275ebb5f333e3fe5d33edaa122c807931aL25-R28))[src/unipoll_api/routes/v1/group.pyL8-R80](https://github.com/csed-ucm/psephos/pull/94/files#diff-7681ab21657eddc8645d7d308f57b83c727ed507df239491209de5e8ec134603L8-R80))Other Notable Changes:
Linktype withBackLinkfor theworkspacefield in thePolldocument to better represent relationships. ([src/unipoll_api/documents.pyL172-L177](https://github.com/csed-ucm/psephos/pull/94/files#diff-9436de0bdd228d52bac6a9e652a53b49bf48b90044ffd098203ef31595faaf42L172-L177))[src/unipoll_api/dependencies.pyL49-R52](https://github.com/csed-ucm/psephos/pull/94/files#diff-bfce4882dc04f49a73a81580802630710122d01e4c8c35b2580c7a5bcf73a3eaL49-R52))get_pollto usePollNotFoundinstead ofGroupNotFound. ([src/unipoll_api/dependencies.pyL104-R103](https://github.com/csed-ucm/psephos/pull/94/files#diff-bfce4882dc04f49a73a81580802630710122d01e4c8c35b2580c7a5bcf73a3eaL104-R103))