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

bounties: fee integration #3961

Merged
merged 23 commits into from
Mar 27, 2019
Merged

bounties: fee integration #3961

merged 23 commits into from
Mar 27, 2019

Conversation

thelostone-mc
Copy link
Member

@thelostone-mc thelostone-mc commented Mar 13, 2019

Description
  • Bounty Fee Integration
  • Refund Fee Flow
  • Metamask notification
  • Ensure db isn't updated when user selects featured bounty and then rejects bounty

Check out gist for info

closes #3891
closes #3910
closes #3909

Pending: @frankchen07

  • email wiring (designs needed)
  • enabling fee for increase bounty fee (design needed)

@codecov
Copy link

codecov bot commented Mar 15, 2019

Codecov Report

Merging #3961 into master will increase coverage by 0.02%.
The diff coverage is 35.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3961      +/-   ##
==========================================
+ Coverage      30%   30.02%   +0.02%     
==========================================
  Files         205      205              
  Lines       15950    16025      +75     
  Branches     2108     2116       +8     
==========================================
+ Hits         4786     4812      +26     
- Misses      10992    11041      +49     
  Partials      172      172
Impacted Files Coverage Δ
app/app/urls.py 90% <ø> (ø) ⬆️
app/dashboard/helpers.py 16.03% <0%> (-0.29%) ⬇️
app/dashboard/views.py 12.13% <13.04%> (+0.05%) ⬆️
app/dashboard/admin.py 72.09% <70%> (-0.18%) ⬇️
app/dashboard/models.py 53.74% <93.33%> (+0.39%) ⬆️
app/grants/views.py 12.41% <0%> (-0.29%) ⬇️

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 fb901de...7eb034e. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 15, 2019

Codecov Report

Merging #3961 into master will increase coverage by 0.02%.
The diff coverage is 35.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3961      +/-   ##
==========================================
+ Coverage   30.19%   30.22%   +0.02%     
==========================================
  Files         205      205              
  Lines       16150    16240      +90     
  Branches     2129     2139      +10     
==========================================
+ Hits         4876     4908      +32     
- Misses      11100    11158      +58     
  Partials      174      174
Impacted Files Coverage Δ
app/app/urls.py 90% <ø> (ø) ⬆️
app/dashboard/helpers.py 15.44% <0%> (-0.27%) ⬇️
app/dashboard/views.py 12.13% <11.36%> (-0.04%) ⬇️
app/dashboard/admin.py 68.09% <52%> (-2.92%) ⬇️
app/dashboard/models.py 54.2% <93.33%> (+0.38%) ⬆️

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 d5482e1...b5f32af. Read the comment docs.

@thelostone-mc thelostone-mc force-pushed the blue-fee branch 3 times, most recently from e28a919 to 36a84e3 Compare March 18, 2019 15:57
@thelostone-mc thelostone-mc marked this pull request as ready for review March 18, 2019 18:01
@thelostone-mc thelostone-mc self-assigned this Mar 18, 2019
@thelostone-mc thelostone-mc added the Gitcoin Bounties Gitcoin Bounties label Mar 18, 2019
@thelostone-mc thelostone-mc changed the title WIP fee integration bounties: fee integration Mar 19, 2019
@frankchen07
Copy link
Contributor

@danlipert @thelostone-mc

First off, great work. Thanks for the amazing teamwork on this, and working through the decisions on contracts and stuff. +1 for Dan putting it up on staging 5 minutes after I asked for it too. Just finished looking at this on staging, and the functioning looks smooth. Some comments below from Aditya's asks.

  1. We'll hold off on the email designs. Because this is a lightweight test, I would say we can remove the "request fee refund" option on the issue details page. If they decide to cancel, they can contact us via email / feedback for refunds and we can deal with it ad-hoc. Since we have the code for it already, we can always reactivate it when necessary. Our goal here is to gauge reactions and see what happens.

Screen Shot 2019-03-20 at 10 47 15

  1. For the fee charge when increasing the bounty amount, I actually don't see an option to increase the bounty amount (example below). That being said, this is also an edge case that we don't need to worry about as of right now for the purposes of this revenue test. My hypothesis is that the majority of folks posting on Gitcoin aren't trying to game the system.

Screen Shot 2019-03-20 at 11 07 53

  1. I know we store featured bounty revenue in our DB, is the % fee also stored in our DB, and does it have a corresponding bounty_id that we can link to? This is mainly for data reporting purposes, as well as having the proper information to process refunds if necessary from our admin.

@thelostone-mc
Copy link
Member Author

thelostone-mc commented Mar 21, 2019

@frankchen07

actually don't see an option to increase the bounty amount (example below).

The increase bounty is the contribute button & I've seen it used by a few funders.
Click on contribute -> and then increase funding.
Every bounty has this option available to the bounty funder.
I've got the code nearly done but I can revert it if we still feel it's not needed

is the % fee also stored in our DB, and does it have a corresponding bounty_id that we can link to?

we store the % fee directly in the bounty table as shown here

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.

Didn't test it in my local to understand deeply but left some comments

@thelostone-mc thelostone-mc dismissed octavioamu’s stale review March 27, 2019 12:28

threw in comments to address review comments

@SaptakS SaptakS merged commit e2a6f81 into master Mar 27, 2019
@thelostone-mc thelostone-mc deleted the blue-fee branch March 27, 2019 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gitcoin Bounties Gitcoin Bounties
Projects
None yet
5 participants