Roles #52

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.

Sign up for GitHub

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

Jump to bottom

Merged

mohamed040406 merged 27 commits into fastapi-rewrite from feature/roles

Sep 6, 2021

Contributor

mohamed040406 commented Aug 29, 2021

No description provided.

mohamed040406 added 4 commits

August 29, 2021 02:34


          roles endpoint

77fe850


          fix delete query

6b2b83e


          fix docs

2b9b2a8


          update submodules

a8c09b2

mohamed040406 added the enhancement label

mohamed040406 self-assigned this

mohamed040406 added 7 commits

August 29, 2021 14:21


          fix permissions check

7521bc2


          role members endpoints

495c294


          update docs

6756a13


          update status code

c8aa51b


          fixes

1ffc15c


          tests


          fix test

666d3f2

mohamed040406 marked this pull request as ready for review

September 5, 2021 14:02

mohamed040406 requested review from SylteA and takos22 and removed request for SylteA

September 5, 2021 14:02

SylteA requested changes

View reviewed changes

Member

SylteA left a comment

Documentation needs to be improved (Documenting every response code, updating default response codes, desciptions / summaries for each endpoint)
Any endpoints returning 204 need "" as content, otherwise they will return null

api/app.py Show resolved Hide resolved

api/app.py Show resolved Hide resolved

api/dependencies.py Outdated Show resolved Hide resolved

api/dependencies.py Outdated Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Show resolved Hide resolved

tests/conftest.py Outdated Show resolved Hide resolved

utils/permissions.py Show resolved Hide resolved

mohamed040406 added 11 commits

September 5, 2021 18:29


          update user fixture scope

19ef154


          update auth dependency

6a41544


          reformat queries

8042c94


          Check if role exists after checking permissions

8f54d19

fix

f59a4ff


          update docstring

00f55d9


          docs

0d922dd


          reformat queries

4a8ab17

fix

821bddf


          reorder responses

ced06b8


          remove 422 from responses

4c7dc37

SylteA requested changes

View reviewed changes

api/dependencies.py Outdated Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Outdated Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Outdated Show resolved Hide resolved

api/app.py Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Outdated Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Outdated Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Outdated Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Outdated Show resolved Hide resolved

utils/permissions.py Show resolved Hide resolved

mohamed040406 and others added 3 commits

September 5, 2021 20:50


          update docstring

8aa0a9d


          update dependency

2cc466a


          update this

e11b800

SylteA requested changes

View reviewed changes

api/dependencies.py Outdated Show resolved Hide resolved

api/dependencies.py Outdated Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Outdated Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py Outdated Show resolved Hide resolved

api/versions/v1/routers/roles/routes.py

    
                  user_permissions = 0

                  for role in roles:

                      user_permissions |= role.permissions

Member

SylteA Sep 6, 2021

Just adding a comment here stating what this will do helps a lot.

# Check if the user has administrator permission or all the permissions provided in the role

I don't think we should check all the permissions though, just the permissions that are higher than ManageRoles.

Contributor Author

mohamed040406 Sep 6, 2021

permissions that are "higher"?

mohamed040406 added 2 commits

September 6, 2021 13:24


          commit

403d39c


          Merge branch 'feature/roles' of github.com:Tech-With-Tim/API into fea…

e0534b0

…ture/roles

mohamed040406 added this to the Initial Release milestone

mohamed040406 merged commit 377c298 into fastapi-rewrite

mohamed040406 deleted the feature/roles branch

September 6, 2021 19:43

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels