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

Add user profiles #791

Merged
merged 88 commits into from Aug 14, 2020
Merged

Add user profiles #791

merged 88 commits into from Aug 14, 2020

Conversation

adelavega
Copy link
Collaborator

@adelavega adelavega commented Jun 29, 2020

Merge Summary (8/14/2020):

  • Added user profiles for each user under "My Profile". Displays public profile, with publicly viewable analyses only.
  • Added user_name for all users. Set to default value based on name, but can be changed by user.
  • All profile values can be edited by users under "My Profile"
  • Authors are linked in public profile view to profiles
  • Authors are also linked in analysis summary

WIP PR starting #159

  • Added Institution field to db

Frontend todo:

  • Add Institution to sign up sheet form
  • Re-route Google users to let them set their name and institution when signing up that way.
  • Add a My Account page where users can edit their information
  • Add public profiles for users that can be clicked under an analysis (Show authors for public analysis listing #417). These profiles could summarize their profile info + number of analyses created (or even a listing of their public analyses)

Questions:

  • Should we an add an Anonymous privacy mode. If this is set to true in My Account then their profile would be private to others, and any analyes shared by them would not show their name. I think this is probably unnecessary as users can just keep their analyses private, and theres no listing of profiles
  • Should we add a username? Currently we store google_id to this is a starting point. We could force people to use a username, which for now we could set to the first part of their email. This could be what is displayed instead of their name, and would be the URL for profile pages.

@adelavega
Copy link
Collaborator Author

Should we let users upload a profile pic besides their google one?

@adelavega
Copy link
Collaborator Author

Minor thing but I'm not sure why your update to the user schema broke the tests.

I'm not sure if the test that's failing makes that much sense anyways. Basically, the schema should reject partial updates, unless the mandatory fields are included. But it looks like now it's allowing a single field update.... not sure why that's changed but it looks OK

@rwblair
Copy link
Collaborator

rwblair commented Jul 15, 2020

@adelavega last two commits have public profiles. Currently not linked anywhere. I have them using the user id from the database for both the front end and api routes. Do we want to identify them by some other method? Email is guaranteed to be unique but encoded in the url its a lot more busy looking than just an id. We had also talked about generating another type of id, maybe on like for analyses?

Fixed the clone being visible for non logged in users.

Also right now it only displays fields for which there are values in the user profile, not sure if we want to default to just showing empty entries.

Finally I still need to make the twitter handle and personal site and actual link.

@adelavega
Copy link
Collaborator Author

adelavega commented Jul 15, 2020

Looking pretty good.

I say we have the dropdown menu have a link to "Your Profile" and if you're viewing your own profile, there's a link to edit your profile. Maybe we also add a link back from the profile editor to your public profile.

I'd be nice to see a bit more visual structure, like maybe some lines between the fields, or little emojis preceding each field (again I'm totally open to ripping off github)

For now, I'd be OK with sticking to these obscure IDs, we can tackle migrating to user names in a separate PR.

We should soon also add references to people in the public analysis listing (and review tab somewhere), which links to people's profiles.

adelavega and others added 24 commits August 13, 2020 15:54
…egrityerror, display username in use error on profile edit form
@codecov-commenter
Copy link

codecov-commenter commented Aug 13, 2020

Codecov Report

Merging #791 into master will increase coverage by 0.11%.
The diff coverage is 83.52%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #791      +/-   ##
==========================================
+ Coverage   83.13%   83.25%   +0.11%     
==========================================
  Files          63       63              
  Lines        2870     2902      +32     
==========================================
+ Hits         2386     2416      +30     
- Misses        484      486       +2     
Impacted Files Coverage Δ
neuroscout/core.py 67.74% <ø> (ø)
neuroscout/models/analysis.py 97.18% <ø> (+12.73%) ⬆️
neuroscout/resources/__init__.py 100.00% <ø> (ø)
neuroscout/utils/db.py 86.36% <20.00%> (-5.58%) ⬇️
neuroscout/resources/user.py 74.02% <80.00%> (+0.44%) ⬆️
neuroscout/schemas/user.py 87.17% <80.00%> (-5.42%) ⬇️
neuroscout/auth.py 57.97% <83.33%> (+2.41%) ⬆️
neuroscout/models/auth.py 96.77% <100.00%> (+0.77%) ⬆️
neuroscout/schemas/analysis.py 97.70% <100.00%> (ø)
neuroscout/tasks/report.py 76.00% <100.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5369fe5...01abe56. Read the comment docs.

@adelavega adelavega merged commit 5bcbcee into master Aug 14, 2020
@adelavega adelavega deleted the add_profiles branch August 14, 2020 18:49
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.

None yet

4 participants