-
Notifications
You must be signed in to change notification settings - Fork 4
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
Feature/data manager user fix #514
Feature/data manager user fix #514
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## feature/data_manager #514 +/- ##
=====================================================
Coverage 81.87% 81.88%
=====================================================
Files 125 125
Lines 5562 5563 +1
=====================================================
+ Hits 4554 4555 +1
Misses 1008 1008
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -20,7 +20,8 @@ class User(StrictBaseModel): | |||
email: EmailStr | |||
createdAt: datetime | |||
role: str | |||
applications: List[str] | |||
provider: str | |||
providerId: str |
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.
I don't see providerId listed in https://resource-watch.github.io/doc-api/reference.html#get-lt-base-api-url-gt-auth-check-logged . Is that documentation out of date? If it is not being returned by 'check-logged', can you put a comment in on why you're adding it?
Also, it looks like 'updatedAt' is returned by 'checked-logged'. Do you want to include that for completeness?
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.
I had one unpushed commit that makes providerId optional as it's only present when provider
represents third party auth (facebook, google, etc). Thanks for catching it.
Also updatedAt
is not present in the actual response.
The documentation looks incorrect/incomplete in both cases.
Pull request checklist
Please check if your PR fulfills the following requirements:
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
The pydantic model for the user doesn't match the response of
{rw-api}/auth/check-logged
responseWhat is the new behavior?
Does this introduce a breaking change?
Other information