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

Roles refactor: New public API with reworked project roles #337

Merged
merged 6 commits into from
Dec 10, 2024

Conversation

varmar05
Copy link
Contributor

Added new public /v2 endpoints for project members CRUD and new endpoint for registration of users.
Also small refactoring was done (e.g. moving workspace role definition from EE).

@coveralls
Copy link

coveralls commented Nov 29, 2024

Pull Request Test Coverage Report for Build 12238117740

Details

  • 231 of 236 (97.88%) changed or added relevant lines in 15 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 94.071%

Changes Missing Coverage Covered Lines Changed/Added Lines %
server/mergin/sync/public_api_v2_controller.py 54 56 96.43%
server/mergin/sync/interfaces.py 17 20 85.0%
Totals Coverage Status
Change from base Build 12007064691: 0.1%
Covered Lines: 6584
Relevant Lines: 6999

💛 - Coveralls

@varmar05 varmar05 requested a review from MarcelGeo November 29, 2024 12:13
Copy link
Contributor

@MarcelGeo MarcelGeo 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. Just minor suggestions and one question:

  • do we want to use ProjectPermissions.Upload and other methods from project permissions class? I think we could rewrite it or remove.

server/mergin/app.py Show resolved Hide resolved
server/mergin/auth/app.py Show resolved Hide resolved
server/mergin/auth/models.py Outdated Show resolved Hide resolved
server/mergin/sync/public_api_v2_controller.py Outdated Show resolved Hide resolved
server/mergin/sync/public_api_v2_controller.py Outdated Show resolved Hide resolved
@varmar05
Copy link
Contributor Author

varmar05 commented Dec 9, 2024

  • do we want to use ProjectPermissions.Upload and other methods from project permissions class? I think we could rewrite it or remove.

I did not touch this one, as it is widely used. If we want further refactoring then I'd probably vote for removing it completely.

So far it is hanging there if we one day decide to introduce permissions besides roles.

Copy link
Contributor

@harminius harminius left a comment

Choose a reason for hiding this comment

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

Just curious why we split that project access PUT.

@MarcelGeo MarcelGeo merged commit 1fbdb12 into develop-roles-refactor Dec 10, 2024
4 checks passed
@MarcelGeo MarcelGeo deleted the new_members_api branch December 10, 2024 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants