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

Allow crowdfund issues #1597

Closed
wants to merge 12 commits into from
Closed

Allow crowdfund issues #1597

wants to merge 12 commits into from

Conversation

jvmaia
Copy link
Contributor

@jvmaia jvmaia commented Jul 2, 2018

Description

Allow the crowd to increase contributions to a bounty, so we can crowdfund issues.

Checklist
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)

urls, views

Testing

I have tested this as the funder and the collaborator.

Refers/Fixes

Fixes: #1380

if (document.changePayoutAmount) {
$.get('/issue/increase/changepayout/' + amount + '/?pk=' + bountyId + '&network=' + document.web3network)
.fail(function(error) {
console.log(error);

Choose a reason for hiding this comment

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

Unexpected console statement. (no-console)

from django.utils import timezone
from django.utils.translation import gettext as _
from django.urls import reverse

Choose a reason for hiding this comment

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

F811 redefinition of unused 'reverse' from line 28

print(form_link)

response_text = '''
There is someone trying to increase the bounty on {value} on your issue, can you change the payout amount of the bounty? The form to do it: {form_link}. You can increase the payout amount in the value {value}.

Choose a reason for hiding this comment

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

E501 line too long (209 > 120 characters)

@codecov
Copy link

codecov bot commented Jul 2, 2018

Codecov Report

Merging #1597 into master will decrease coverage by 0.08%.
The diff coverage is 13.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1597      +/-   ##
==========================================
- Coverage   29.93%   29.85%   -0.09%     
==========================================
  Files         130      130              
  Lines        9596     9678      +82     
  Branches     1243     1247       +4     
==========================================
+ Hits         2873     2889      +16     
- Misses       6619     6685      +66     
  Partials      104      104
Impacted Files Coverage Δ
app/app/urls.py 89.74% <ø> (ø) ⬆️
app/marketing/mails.py 11.95% <11.11%> (-0.02%) ⬇️
app/retail/emails.py 21.1% <33.33%> (+0.24%) ⬆️
app/dashboard/views.py 14.43% <6.66%> (-0.19%) ⬇️
app/retail/views.py 37.25% <0%> (-1.21%) ⬇️
app/retail/utils.py 20% <0%> (-0.39%) ⬇️
app/dashboard/tokens.py 100% <0%> (ø) ⬆️
app/dashboard/utils.py 26.12% <0%> (+0.21%) ⬆️
app/dashboard/notifications.py 16.84% <0%> (+0.32%) ⬆️

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 3d0c91b...6aa7e69. Read the comment docs.


if (document.changePayoutAmount) {
$.get('/issue/increase/changepayout/' + amount + '/?pk=' + bountyId + '&network=' + document.web3network)
.fail(function(error) {

Choose a reason for hiding this comment

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

Unexpected empty function. (no-empty-function)

subject = 'Change payout amount'
form_link = reverse('request_change_payout', kwargs={'value': value, 'pk': bounty.id, 'network': bounty.network})

response_text = "There is someone trying to increase the bounty on {value} on your issue," +

Choose a reason for hiding this comment

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

E999 SyntaxError: invalid syntax

@@ -111,6 +111,10 @@ $(document).ready(function() {
mixpanel.track('Submit New Bounty Success', {});
document.location.href = '/funding/details/?url=' + issueURL;
}, 1000);

if (document.changePayoutAmount) {
$.get('/issue/increase/changepayout/' + amount + '/?pk=' + bountyId + '&network=' + document.web3network)

Choose a reason for hiding this comment

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

Missing semicolon. (semi)

@@ -588,6 +589,16 @@ def render_new_bounty_roundup(to_email):
return response_html, response_txt, subject


def render_change_payout_amount(bounty, value):
subject = 'Change payout amount'
form_link = reverse('request_change_payout', kwargs={'value': value, 'pk': bounty.id, 'network': bounty.network})

Choose a reason for hiding this comment

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

F841 local variable 'form_link' is assigned to but never used

@jvmaia
Copy link
Contributor Author

jvmaia commented Jul 2, 2018

Does the stickler-ci verify when a variable is used inside a f-string?

@owocki
Copy link
Contributor

owocki commented Jul 3, 2018

i keep getting crazy high gas fees when i try to use this feature ( screenshot : http://bits.owocki.com/2T1M3e2r3Q3v/Screen%20Shot%202018-07-02%20at%205.58.38%20PM.png )

Usually that means theres a smart contract error on thetx i'm about to submit

@jvmaia
Copy link
Contributor Author

jvmaia commented Jul 3, 2018

Ook, I'll try to remove the if/else statement inside do_bounty function and separate it into 3 different functions.

@jvmaia
Copy link
Contributor Author

jvmaia commented Jul 3, 2018

Hey, I didn't got this error here, weird. But I'll make the change because it's more readable.

@owocki
Copy link
Contributor

owocki commented Jul 3, 2018

Hey, I didn't got this error here, weird. But I'll make the change because it's more readable.

Here are the reproduction steps for the issue.

In a browser:

  1. Create a new bounty

In another browser with different metamask seed:

  1. Open the bounty you created.
  2. Click 'Add contribution'.
  3. Submit the 'increase funding form'
  4. Metamask error appears.

@@ -662,9 +661,8 @@ var do_actions = function(result) {
}

if (show_increase_bounty) {
Copy link
Contributor

Choose a reason for hiding this comment

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

won't you need to update show_increase_bounty too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it needed? what I change here is the argument from enabled = increase_bounty_enabled; to enabled = true;, the show_increase_bounty is fine.

@owocki
Copy link
Contributor

owocki commented Jul 3, 2018

debugging further, it seems to call do_bounty_as_funder_increasingPayout even when i am not the funder

@owocki
Copy link
Contributor

owocki commented Jul 3, 2018

when there is no contract error, i get a very high gas price

@ghost ghost assigned owocki Jul 3, 2018
@ghost ghost added the in progress label Jul 3, 2018
@owocki
Copy link
Contributor

owocki commented Jul 3, 2018

9dc7a12 should fix the issues with is_funder

@owocki
Copy link
Contributor

owocki commented Jul 3, 2018

I'm still seeing contract errors when I try to use the flow though. I think this just needs to be testeded end to end in each of these scenarios:

    if (document.isFunder) {
      if (document.changePayoutAmount) {
        do_bounty_as_funder_increasingPayout();
      } else {
        do_bounty_as_funder_changingFulfillment();
      }
    } else {
      do_bounty_as_contributor();
    }

@jvmaia
Copy link
Contributor Author

jvmaia commented Jul 3, 2018

thanks for the fixes, sorry for the delay, I'll test it now!

@jvmaia
Copy link
Contributor Author

jvmaia commented Jul 4, 2018

Here are the reproduction steps for the issue.

ook, I did it exactly as you described, but the error does not appears to me. I'm using firefox quantum to submit the 'increase funding form' as the contributor. screenshot: https://i.imgur.com/Lj7Ix9L.png

@jvmaia
Copy link
Contributor Author

jvmaia commented Jul 4, 2018

my next commits fix the variable document.changePayoutAmount and the if/else to decide which function call.

@jvmaia
Copy link
Contributor Author

jvmaia commented Jul 4, 2018

the error appears only when the do_bounty_as_funder_changingFulfillment is called, I'll dig into it tomorrow.

@jvmaia
Copy link
Contributor Author

jvmaia commented Jul 5, 2018

Ook, I find the problem, the contract.changeBountyFulfillment requires the issue is at stage Draft (doc link), and the contract.issueAndActivateBounty, used to create the bounty, set the issue's stage to Active (doc link).

@owocki
Copy link
Contributor

owocki commented Jul 6, 2018

oh shoot.. i guess that means that we can't do this after all... ? @jvmaia thats my fault, i can still pay out

@mbeylin any workarounds for #1597 (comment) you can think of ?

@owocki
Copy link
Contributor

owocki commented Jul 8, 2018

very sorry we could not merge this..

@mbeylin let us know if you see any potential workarounds for 1.0.. this is a highly requested feature from our community

@owocki
Copy link
Contributor

owocki commented Jul 9, 2018

unfortunately it looks liek transitionToState() is an internal only method right now

@jvmaia
Copy link
Contributor Author

jvmaia commented Jul 9, 2018

ooh sorry @owocki, it would be a pleasure to work on the workaround to this.

@owocki owocki closed this Jul 13, 2018
@ghost ghost removed the in progress label Jul 13, 2018
@owocki owocki mentioned this pull request Jul 26, 2018
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.

3 participants