Updated src/main, because on changing the FastAPI version on_event(st… #100
Conversation
…artup) was depecated in the new version. Incorporated ragtech_commons_api and deleted the old ones
nargis-sultani
requested review from
hkeeler,
guffee23,
jcadam14 and
lchen-2101
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
nargis-sultani
changed the title
| from regtech_api_commons.models.auth import AuthenticatedUser | ||
| from regtech_api_commons.oauth2 import oauth2_admin | ||
|
|
||
| oauth2_admin.OAuth2Admin.update_user | ||
|
|
There was a problem hiding this comment.
Is this line supposed to be here?
There was a problem hiding this comment.
Ooops, nope. Removing it now.
src/routers/admin.py
Outdated
| @@ -21,12 +22,12 @@ def get_me(request: Request): | |||
| @router.put("/me/", status_code=HTTPStatus.ACCEPTED, dependencies=[Depends(check_domain)]) | |||
| @requires("manage-account") | |||
| def update_me(request: Request, user: UserProfile): | |||
| oauth2_admin.update_user(request.user.id, user.to_keycloak_user()) | |||
| OAuth2Admin.update_user(request.user.id, user.to_keycloak_user()) | |||
There was a problem hiding this comment.
you should be creating an instance of the class, not use the class methods in a static fashion, not quite sure how this is working with the _admin attribute not constructed within the instance...
tests/app/test_config.py
Outdated
| @@ -20,7 +21,7 @@ def test_jwt_opts_valid_values(): | |||
| "jwt_opts_test2": "true", | |||
| "jwt_opts_test3": "12", | |||
| } | |||
| settings = Settings(**mock_config) | |||
| settings = KeycloakSettings(**mock_config) | |||
There was a problem hiding this comment.
the tests that are associated with Keycloak portions should be removed from here now; and added to the api-commons tests
src/routers/institutions.py
Outdated
| @@ -49,7 +49,7 @@ async def create_institution( | |||
| fi: FinancialInstitutionDto, | |||
| ): | |||
| db_fi = await repo.upsert_institution(request.state.db_session, fi) | |||
| kc_id = oauth2_admin.upsert_group(fi.lei, fi.name) | |||
| kc_id = OAuth2Admin.upsert_group(fi.lei, fi.name) | |||
tests/app/test_api_commons.py
Outdated
There was a problem hiding this comment.
I meant removing the tests related to KeycloakSettings from regtech-user-fi-management repo completely, and add these tests to the regtech-api-commons repo at a later time (create a ticket for it, I think there might be a ticket already about adding tests and coverage to that repo)
lchen-2101
deleted the
features/88_incorporate_regtech_api_commons_library
branch
…artup) was depecated in the new version.
Incorporated ragtech_commons_api and deleted the old ones