8000 record proper usd value of a bounty fulfillment by owocki · Pull Request #8488 · gitcoinco/web · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

record proper usd value of a bounty fulfillment #8488

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 11, 2021
Merged

record proper usd value of a bounty fulfillment #8488

merged 1 commit into from
Mar 11, 2021

Conversation

owocki
Copy link
Contributor
@owocki owocki commented Mar 4, 2021
Description

https://gitcoin.co/nance2uiuc

that user paid out a $60k bounty, to 20 diff users. and instead of giving credit to the user for one $60k bounty the system did it 20 times.

this PR fixes that by taking the bounty fulfillment amount + creating the earning from that.

but this PR is blocked from being truly useful by the fact that bountyfulfillment.payout_amount is null for most objects in production ( #8486 )

Refers/Fixes

see above

Testing

tested locally

@thelostone-mc
Copy link
Contributor

but this PR is blocked from being truly useful by the fact that bountyfulfillment.payout_amount is null for most objects in production ( #8486 )

When a bounty fulfillment is created AKA user submits work to a bounty -> the payout_amount is null
When the funder pays out -> only then is the payout_amount is set (as this may differ from bounty amount)

thelostone-mc
thelostone-mc approved these changes Mar 5, 2021
View reviewed changes
Copy link
Contributor
@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 wise LGTM. we can figure out the DRY code later

@owocki
Copy link
Contributor Author
owocki commented Mar 5, 2021 via email

@thelostone-mc
Copy link
Contributor

got it; then we will need to make the usd value on the earning update when

the "value_usd":instance.value_in_usdt_then, change would take care of this right ?

@owocki
Copy link
Contributor Author
owocki commented Mar 7, 2021

i think so. based on how the defaults dict in Earning.objects.update_or_create performs, it looks like yes ( https://docs.djangoproject.com/en/3.1/ref/models/querysets/#update-or-create )

@thelostone-mc thelostone-mc merged commit efc260e into master Mar 11, 2021
@gdixon gdixon mentioned this pull request Mar 11, 2021
Merged
thelostone-mc
thelostone-mc reviewed Mar 12, 2021
View reviewed changes
app/dashboard/models.py
return self.get_natural_value()


def value_in_usdt_at_time(self, at_time):
Copy link
Contributor

Choose a reason for hiding this comment

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

@gdixon so wrapping thus in a try-catch and returning None in the catch might help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thelostone-mc thelostone-mc thelostone-mc approved these changes

@chibie chibie Awaiting requested review from chibie

@frankchen07 frankchen07 Awaiting requested review from frankchen07

@gdixon gdixon Awaiting requested review from gdixon

@octavioamu octavioamu Awaiting requested review from octavioamu

@PixelantDesign PixelantDesign Awaiting requested review from PixelantDesign

Assignees
No one assigned
Labels
None yet
Projects
42D7 None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@owocki @thelostone-mc
0