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

Personal Tokens #7212

Merged
merged 178 commits into from
Sep 29, 2020
Merged

Personal Tokens #7212

merged 178 commits into from
Sep 29, 2020

Conversation

proofoftom
Copy link
Contributor

Description

The merge of all things Gitcoin personal tokens.

Refers/Fixes

This adds the ability for one to create a personal token, and for other Gitcoin members to purchase and redeem those tokens for one's time.

Testing

Running QA document here: https://hackmd.io/K3DRZVwCSsuneF29wdCxvg?view

@mds1
Copy link
Contributor

mds1 commented Sep 3, 2020

Interesting, let me know! We didn't change base.txt at all so it seems those may be unrelated to this PR?

@thelostone-mc
Copy link
Member

thelostone-mc commented Sep 4, 2020

@mds1 I've fixed it up on master !

  • Could you rebase it against master
  • resolve conflicts

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.

just left an small comment. Looking nice guys

Comment on lines 112 to 116
if (data.profile.has_ptoken) {
mount_graph = [ tips_total_percent, token_total_percent, bounties_total_percent, grants_total_percent ];
} else {
mount_graph = [ tips_total_percent, bounties_total_percent, grants_total_percent ];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe something more cleaner like
if (data.profile.has_ptoken) { mount_graph.push(token_total_percent) }

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll defer to @zoek1 on this one

Copy link
Contributor

Choose a reason for hiding this comment

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

done

@@ -62,7 +62,7 @@ def handle(self, *args, **options):
network = 'mainnet' if not settings.DEBUG else 'rinkeby'
from_address = settings.MINICLR_ADDRESS
from_pk = settings.MINICLR_PRIVATE_KEY
DAI_ADDRESS = '0x6b175474e89094c44da98b954eedeac495271d0f' if network=='mainnet' else '0x6A9865aDE2B6207dAAC49f8bCba9705dEB0B0e6D'
DAI_ADDRESS = '0x6b175474e89094c44da98b954eedeac495271d0f' if network=='mainnet' else '0x5592EC0cfb4dbc12D3aB100b257153436a1f0FEa'
Copy link
Member

Choose a reason for hiding this comment

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

^ Was this update intended ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea this was intended.

The first address 0x6A9865aDE2B6207dAAC49f8bCba9705dEB0B0e6D is the official Rinkeby DAI address. But Oasis on Rinkeby has been down for a while now, so it's very difficult to mint DAI on RInkeby.

We switched to 0x5592EC0cfb4dbc12D3aB100b257153436a1f0FEa, which is the address for Rinkeby DAI given out when using the faucet available on https://app.compound.finance/. This makes it a lot easier to get Rinkeby DAI for development/testing

@@ -213,7 +213,7 @@
"real_period_seconds": "2592000",
"frequency_unit": "days",
"frequency": "30",
"token_address": "0x6A9865aDE2B6207dAAC49f8bCba9705dEB0B0e6D",
"token_address": "0x5592EC0cfb4dbc12D3aB100b257153436a1f0FEa",
"token_symbol": "DAI",
"gas_price": "25000.0000",
"new_approve_tx_id": "0x980ce738853f454c83d5551dc086a29d2d89781fb041a9ce581474ffdf639de4",
Copy link
Member

Choose a reason for hiding this comment

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

^ could we undo all these changes

Copy link
Contributor

Choose a reason for hiding this comment

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

See above comment #7212 (comment) — I think we should keep these changes.

Copy link
Member

Choose a reason for hiding this comment

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

gotcha

app/economy/models.py Outdated Show resolved Hide resolved
@mds1
Copy link
Contributor

mds1 commented Sep 4, 2020

Hey @thelostone-mc and @octavioamu — just add one more commit which updates the Terms of Service to add a new section based on what @owocki provided. Section 12 is the new section. Can you quickly review to confirm that all re-numberings of sections 13 through 30 were done properly? Just want to make sure there's none I missed. Thanks!

@codecov
Copy link

codecov bot commented Sep 4, 2020

Codecov Report

Merging #7212 into master will increase coverage by 0.51%.
The diff coverage is 27.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7212      +/-   ##
==========================================
+ Coverage   26.00%   26.51%   +0.51%     
==========================================
  Files         310      311       +1     
  Lines       31226    30724     -502     
  Branches     4622     4546      -76     
==========================================
+ Hits         8119     8146      +27     
+ Misses      22828    22299     -529     
  Partials      279      279              
Impacted Files Coverage Δ
app/app/context.py 0.00% <0.00%> (ø)
app/dashboard/notifications.py 17.38% <ø> (ø)
app/inbox/signals.py 17.89% <0.00%> (-6.75%) ⬇️
app/ptokens/apps.py 0.00% <0.00%> (ø)
app/retail/views.py 20.38% <0.00%> (-0.39%) ⬇️
app/ptokens/mails.py 12.26% <12.26%> (ø)
app/dashboard/views.py 10.42% <13.33%> (-0.04%) ⬇️
app/ptokens/views.py 15.27% <15.27%> (ø)
app/ptokens/emails.py 23.91% <23.91%> (ø)
app/ptokens/helpers.py 42.85% <42.85%> (ø)
... and 60 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 9a9492f...587aab5. Read the comment docs.

@thelostone-mc thelostone-mc merged commit 58836c3 into gitcoinco:master Sep 29, 2020
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.

10 participants