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 Trust Bonus Tab With BrightID Integration #7333

Merged
merged 39 commits into from
Sep 9, 2020

Conversation

apbendi
Copy link
Contributor

@apbendi apbendi commented Sep 4, 2020

This PR adds a "Trust Bonus" tab to the user's profile and integrates BrightID as the first element in the Trust Bonus, per #7125

Screen Shot 2020-09-04 at 8 01 28 AM

  • A new column is added to the User's profile for their BrightID identifier, and this is automatically populated for all user's with a new UUID on migration. This identifier will be used to tie the Gitcoin user to their BrightID profile.

  • On their Trust Bonus tab, a user who has not yet connected their BrightID sees a button to "Connect". It brings up a modal which gives them instructions to install the app, and a QR code which they can scan from the app to link their assigned identifier to their BrightID profile.

Screen Shot 2020-09-04 at 8 03 01 AM

  • When a user connects their BrightID to Gitcoin, if they have not yet been sponsored, a Gitcoin sponsorship is automatically applied to them. This requires a Gitcoin specific private key, which much be set in the env vars. Message me to get this, which should be treated equivalently to an API key.

  • Once connected, the user now sees a new modal with instructions for how to get verified. They are exhorted to attend community calls, which are listed.

Screen Shot 2020-09-04 at 8 07 21 AM

  • BrightID calls can be added via the marketing UpcomingDate model. That model has a new field, called context_tag. Any UpcomingDate that is in the future, and has a context_tag of brightid will be shown on this list.

  • Once Verified, the user sees as much on their Trust Bonus screen

Screen Shot 2020-09-04 at 8 11 07 AM

  • The BrightID network is the source of truth on whether as user is validated or not. The Trust Bonus screen pulls this data from their servers.

  • For purposes of calculating CLR matching, we add a boolean column to the profile called is_brightid_verified. This property is updated hourly by a cron job that fetches all verified users from BrightID's servers and updates their records in the database.

Note: this PR does not add the existing SMS verification flow to the Trust Bonus tab, nor does it add new Twitter validation features. Those will be implemented in a separate PR.

@codecov
Copy link

codecov bot commented Sep 4, 2020

Codecov Report

Merging #7333 into master will decrease coverage by 0.25%.
The diff coverage is 30.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7333      +/-   ##
==========================================
- Coverage   26.66%   26.41%   -0.26%     
==========================================
  Files         294      306      +12     
  Lines       28118    30129    +2011     
  Branches     4135     4453     +318     
==========================================
+ Hits         7498     7958     +460     
- Misses      20349    21896    +1547     
- Partials      271      275       +4     
Impacted Files Coverage Δ
app/app/context.py 0.00% <0.00%> (ø)
app/app/urls.py 85.96% <ø> (-0.25%) ⬇️
app/app/utils.py 23.70% <ø> (-0.84%) ⬇️
app/avatar/views_3d.py 9.38% <0.00%> (-0.14%) ⬇️
app/chat/management/commands/get_user_presence.py 0.00% <0.00%> (ø)
.../management/commands/setup_hackathon_event_chat.py 0.00% <0.00%> (ø)
...management/commands/sync_event_bounties_to_chat.py 0.00% <0.00%> (ø)
app/chat/management/commands/sync_users_to_chat.py 0.00% <0.00%> (ø)
...p/dashboard/management/commands/activity_report.py 0.00% <0.00%> (ø)
.../dashboard/management/commands/cleanup_db_space.py 0.00% <ø> (ø)
... and 104 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 288ad71...55b084c. Read the comment docs.

Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

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

Code looks sane ! Will have to pull it in and check
Left some minor comments

app/assets/v2/images/app_stores/apple_app_store.svg Outdated Show resolved Hide resolved
app/dashboard/templates/profiles/tabs.html Outdated Show resolved Hide resolved
app/assets/v2/js/pages/profile-trust.js Show resolved Hide resolved
app/dashboard/brightid_utils.py Outdated Show resolved Hide resolved
* Move BrightID status call into a utility function called from
  the view
* Create function for sponsorship application and import required
  crypto dependencies to do the signing, plus keys in env for signing
* If a user is detected as connected, but not sponsored, automatically
  apply a sponsorhip from Gitcoin to their user
* Add BrightID logo and explanatory copy
* Show app store buttons with links to download the app
* Deep link to connection URL and render the appropriate
  QR code for connection with the app
* When the user is connected by not verified, show an appropriate
  modal when the user clicks the Verify button
* Load data on upcoming BrightID verification calls from the database
  and display them on the modal
* Resolve migration conflicts and add a context tag to the UpcomingDates
  model; use tag to filter appropriate BrightID related calls
@apbendi
Copy link
Contributor Author

apbendi commented Sep 7, 2020

ok @thelostone-mc, all feedback should be addressed! Thanks for the review :)

@thelostone-mc thelostone-mc requested a review from owocki September 8, 2020 05:23
Copy link
Contributor

@octavioamu octavioamu left a comment

Choose a reason for hiding this comment

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

look solid!

@frankchen07
Copy link
Contributor

looks good, some quick comments:

  • are these screens accurate? Perhaps we can use them in the blog post.
  • dope on the ability to add marketing calls. after a user registers though (or chooses not the register) is there the ability for them to go back and see them again? We'll probably be providing the links elsewhere as well, but we want to make sure to be able to expose those links as much as possible

operations = [
migrations.AddField(
model_name='profile',
name='is_brightid_verified',
Copy link
Member

@thelostone-mc thelostone-mc Sep 8, 2020

Choose a reason for hiding this comment

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

while Profile.objects.filter(brightid_uuid__isnull=True).exists():
with transaction.atomic():

for profile in Profile.objects.filter(brightid_uuid__isnull=True)[:1000]:
Copy link
Contributor

Choose a reason for hiding this comment

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

hardcoded limit is gonna get us in trtouble in prod where there are 40k profiles

also whats the plan for new profiles, how are they going to get a brightid uuid?

Copy link
Member

@thelostone-mc thelostone-mc Sep 9, 2020

Choose a reason for hiding this comment

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

I assume it would be auto-assigned one on creation which is unique (as defined in the model)
Is my understanding right @apbendi ?

  • turn the uuid field as blank=True
  • write up a mgmt command which duplicates the logic for assigning uuid & run every 3 hours or so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, if I've done this right, a new UUID should be generated when a user is created, per this migration:

https://github.com/gitcoinco/web/pull/7333/files#diff-f2afa72d3c42b2cac75651dd47a6c961R14

Can you sanity check that looks correct.

I'm not sure about the 1K limit @thelostone-mc, why did you have that in there? Did it take 2 minutes to run for only 1K records or for the full 40K?

Copy link
Member

Choose a reason for hiding this comment

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

2 min for the whole 40k !

@owocki
Copy link
Contributor

owocki commented Sep 8, 2020

https://bits.owocki.com/qGuNnmmv

this looks great :) and works well (though i could only test with my brightid profile, which is verified - what is the user experience like if you connect a brightid that is not verified?).

also - can we stub in the SMS verifiication module and the twitter connect module, per the design? or is that on another PR? even if it not hooked up yet, i think having it stubbed in will have value in articulating our trustscore approach.

@owocki
Copy link
Contributor

owocki commented Sep 8, 2020

left a comment or two. excited for this!

@apbendi
Copy link
Contributor Author

apbendi commented Sep 9, 2020

@frankchen07

are these screens accurate? Perhaps we can use them in the blog post.

Yep, the screens are accurate, though the events shown in the verification modal are not.

dope on the ability to add marketing calls. after a user registers though (or chooses not the register) is there the ability for them to go back and see them again? We'll probably be providing the links elsewhere as well, but we want to make sure to be able to expose those links as much as possible

So, the "Register" button just links the user to the URL included on the marketing model. That's why I think we should make those links that populate the event in the user's calendar, with whatever Zoom/Airmeet info for the call included in the calendar description. Does that make sense? Regardless, the events will continue to show until the datetime is past, or the context_tag is changed to anything other than brightid

@apbendi
Copy link
Contributor Author

apbendi commented Sep 9, 2020

@owocki

what is the user experience like if you connect a brightid that is not verified?

Here's what those user's see:

image

@apbendi
Copy link
Contributor Author

apbendi commented Sep 9, 2020

also - can we stub in the SMS verifiication module and the twitter connect module, per the design? or is that on another PR? even if it not hooked up yet, i think having it stubbed in will have value in articulating our trustscore approach.

@owocki I have the SMS portion in-work in another branch, and Twitter is next on my list to try to tackle. We should have at least the SMS done by the grants round, and I can stub out a placeholder for Twitter with a "coming soon" message or something similar. My plan was to get those both merged into the main Grants integration branch and deployed with the rest of the Grants updates. Is that alright with you, or do you want something also included in this PR?

@octavioamu octavioamu merged commit 4ecca77 into gitcoinco:master Sep 9, 2020
zoek1 pushed a commit to zoek1/web-1 that referenced this pull request Sep 14, 2020
* add constant pot changing mechanism, add analytics script

* remove current designation, add a couple print test statements

* add constant pot changing mechanism, add analytics script

* remove current designation, add a couple print test statements

* add pot dealer, add analytics function & script, add additional calculation data

* update clr estimate increases using a log function, fix adityas comments

* add updated verification status

* fix up adityas 2nd round of requests

* moved analytics function to analytics_clr.py, import methods

* add brightId verfication into clr

* Add a Trust Score tab to the users profile

* Scaffold UI for connecting to BrightID

* Migrations to add, populate and require a brightid identifier on Profile

* Add BrightID UUID to Profile and display on Trust tab as proof-of-concept

* Get status of the users bright id connection when loading the Trust tab

* Display different bright id interface based on auth status & connection

* Only show Trust tab for logged in users own profile

* Get BrightID Sponsorship creation working

* Move BrightID status call into a utility function called from
  the view
* Create function for sponsorship application and import required
  crypto dependencies to do the signing, plus keys in env for signing
* If a user is detected as connected, but not sponsored, automatically
  apply a sponsorhip from Gitcoin to their user

* Create modal poppup for Connect phase of BrightID integration

* Add BrightID logo and explanatory copy
* Show app store buttons with links to download the app
* Deep link to connection URL and render the appropriate
  QR code for connection with the app

* Add modal for verification phase of BrightID Integrations

* When the user is connected by not verified, show an appropriate
  modal when the user clicks the Verify button
* Load data on upcoming BrightID verification calls from the database
  and display them on the modal
* Resolve migration conflicts and add a context tag to the UpcomingDates
  model; use tag to filter appropriate BrightID related calls

* Handle the case when the user is verified

* Clean up copy & implement full Trust Bonus UI design

* Responsive design tweaks for Trust Bonus tab

* Fix migration conflicts & add column for BrightID verification status

* Write command for fetching and updated verified users in local DB

* Schedule BrightID status updater to run at 55th minute of every hour

* Cleanup URLs in BrightID Utils

* Avoid duplicating objectifySerialized utility JS function

* Resolve javascript linting errors in BrightID modals

* Add root level package-lock.json file for dev environment commands

* Compress app store logo svg file

* Fix various spacing an indentation issues in BrightID integration

* Cleanup return condition for brightid sponsorhip assignment

* recreate migration + run pylint

* uuid migration fix

* clr tweaks

Co-authored-by: frankchen07 <[email protected]>
Co-authored-by: Aditya Anand M C <[email protected]>
zoek1 pushed a commit to zoek1/web-1 that referenced this pull request Sep 14, 2020
* add constant pot changing mechanism, add analytics script

* remove current designation, add a couple print test statements

* add constant pot changing mechanism, add analytics script

* remove current designation, add a couple print test statements

* add pot dealer, add analytics function & script, add additional calculation data

* update clr estimate increases using a log function, fix adityas comments

* add updated verification status

* fix up adityas 2nd round of requests

* moved analytics function to analytics_clr.py, import methods

* add brightId verfication into clr

* Add a Trust Score tab to the users profile

* Scaffold UI for connecting to BrightID

* Migrations to add, populate and require a brightid identifier on Profile

* Add BrightID UUID to Profile and display on Trust tab as proof-of-concept

* Get status of the users bright id connection when loading the Trust tab

* Display different bright id interface based on auth status & connection

* Only show Trust tab for logged in users own profile

* Get BrightID Sponsorship creation working

* Move BrightID status call into a utility function called from
  the view
* Create function for sponsorship application and import required
  crypto dependencies to do the signing, plus keys in env for signing
* If a user is detected as connected, but not sponsored, automatically
  apply a sponsorhip from Gitcoin to their user

* Create modal poppup for Connect phase of BrightID integration

* Add BrightID logo and explanatory copy
* Show app store buttons with links to download the app
* Deep link to connection URL and render the appropriate
  QR code for connection with the app

* Add modal for verification phase of BrightID Integrations

* When the user is connected by not verified, show an appropriate
  modal when the user clicks the Verify button
* Load data on upcoming BrightID verification calls from the database
  and display them on the modal
* Resolve migration conflicts and add a context tag to the UpcomingDates
  model; use tag to filter appropriate BrightID related calls

* Handle the case when the user is verified

* Clean up copy & implement full Trust Bonus UI design

* Responsive design tweaks for Trust Bonus tab

* Fix migration conflicts & add column for BrightID verification status

* Write command for fetching and updated verified users in local DB

* Schedule BrightID status updater to run at 55th minute of every hour

* Cleanup URLs in BrightID Utils

* Avoid duplicating objectifySerialized utility JS function

* Resolve javascript linting errors in BrightID modals

* Add root level package-lock.json file for dev environment commands

* Compress app store logo svg file

* Fix various spacing an indentation issues in BrightID integration

* Cleanup return condition for brightid sponsorhip assignment

* recreate migration + run pylint

* uuid migration fix

* clr tweaks

Co-authored-by: frankchen07 <[email protected]>
Co-authored-by: Aditya Anand M C <[email protected]>
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.

6 participants