Alert user if insufficient tokens for tip #3058

dylanjw · 2018-12-03T22:53:39Z

Description

See #2250

Checklist

linter status: 100% pass
changes don't break existing behavior
commit message follows commit guidelines

Affected core subsystem(s)

ui

Testing

I tested locally with docker. I both test the validation message on sending a tip, and on submitting a bounty.

Refers/Fixes

Fixes #2250

stickler-ci · 2018-12-03T22:53:54Z

app/assets/onepager/js/send.js

+    denomFactor = Math.pow(10, tokenDetails.decimals);
+
+    check_balance_and_alert_user_if_not_enough(
+      tokenAddress, 


Trailing spaces not allowed. (no-trailing-spaces)

stickler-ci · 2018-12-03T22:53:54Z

app/assets/onepager/js/send.js

+
+    check_balance_and_alert_user_if_not_enough(
+      tokenAddress, 
+      amount, 


Trailing spaces not allowed. (no-trailing-spaces)

stickler-ci · 2018-12-03T22:53:55Z

app/assets/v2/js/pages/new_bounty.js

      if (!isETH) {
-        check_balance_and_alert_user_if_not_enough(tokenAddress, amount);
+        check_balance_and_alert_user_if_not_enough(
+          tokenAddress, 


Trailing spaces not allowed. (no-trailing-spaces)

stickler-ci · 2018-12-03T22:53:55Z

app/assets/v2/js/pages/new_bounty.js

-        check_balance_and_alert_user_if_not_enough(tokenAddress, amount);
+        check_balance_and_alert_user_if_not_enough(
+          tokenAddress, 
+          amount, 


Trailing spaces not allowed. (no-trailing-spaces)

stickler-ci · 2018-12-03T22:53:55Z

app/assets/v2/js/shared.js

@@ -1336,3 +1336,29 @@ function shuffleArray(array) {
  }
  return array;
 }
+
+function check_balance_and_alert_user_if_not_enough(
+    tokenAddress, 


Expected indentation of 2 spaces but found 4. (indent)
Trailing spaces not allowed. (no-trailing-spaces)

stickler-ci · 2018-12-03T22:53:55Z

app/assets/v2/js/shared.js

+
+function check_balance_and_alert_user_if_not_enough(
+    tokenAddress, 
+    amount, 


Expected indentation of 2 spaces but found 4. (indent)
Trailing spaces not allowed. (no-trailing-spaces)

stickler-ci · 2018-12-03T22:53:55Z

app/assets/v2/js/shared.js

+function check_balance_and_alert_user_if_not_enough(
+    tokenAddress, 
+    amount, 
+    msg='You do not have enough tokens to perform this action.') {


Expected indentation of 2 spaces but found 4. (indent)
Infix operators must be spaced. (space-infix-ops)

stickler-ci · 2018-12-03T22:53:56Z

app/assets/v2/js/shared.js

+    var balance_rounded = Math.round(balance * 10) / 10;
+
+    if (parseFloat(amount) > balance) {
+      var msg1 = gettext(msg)


Missing semicolon. (semi)

stickler-ci · 2018-12-03T22:53:56Z

app/assets/v2/js/shared.js

+    }
+  });
+
+};


Unnecessary semicolon. (no-extra-semi)

stickler-ci · 2018-12-03T22:58:16Z

app/assets/v2/js/shared.js

+function check_balance_and_alert_user_if_not_enough(
+  tokenAddress,
+  amount,
+  msg='You do not have enough tokens to perform this action.') {


Infix operators must be spaced. (space-infix-ops)

codecov · 2018-12-03T22:59:00Z

Codecov Report

Merging #3058 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3058      +/-   ##
==========================================
- Coverage   29.77%   29.72%   -0.05%     
==========================================
  Files         180      180              
  Lines       13734    13734              
  Branches     1842     1842              
==========================================
- Hits         4089     4083       -6     
- Misses       9507     9513       +6     
  Partials      138      138

Impacted Files	Coverage Δ
app/dashboard/embed.py	`28.16% <0%> (-3.45%)`	⬇️

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 d417fa4...7843701. Read the comment docs.

codecov · 2018-12-03T22:59:00Z

Codecov Report

Merging #3058 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3058      +/-   ##
==========================================
- Coverage   30.37%   30.33%   -0.04%     
==========================================
  Files         206      206              
  Lines       16404    16404              
  Branches     2156     2156              
==========================================
- Hits         4982     4976       -6     
- Misses      11242    11248       +6     
  Partials      180      180

Impacted Files	Coverage Δ
app/dashboard/embed.py	`28.16% <0%> (-3.45%)`	⬇️

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 dc2c030...f2f4a80. Read the comment docs.

thelostone-mc

@dylanjw could you

~~fix travis CI failures~~
throw in a recording of the fix

And if you are up for making the code base a lil better
L81 - L96. Update var with let / const

dylanjw · 2019-02-25T13:06:29Z

@thelostone-mc The CI is throwing the following error:

django.core.management.base.CommandError: Conflicting migrations detected; multiple leaf nodes in the migration graph: (0012_merge_20190221_2246, 0014_matchpledge in grants).
E           To fix them run 'python manage.py makemigrations --merge'

I dont think this is related to the changes in the PR.

thelostone-mc · 2019-02-25T14:43:28Z

@dylanjw My bad ! I'll fix up travis ^_^
Could you throw in a recording of the PR ?

NA

owocki · 2019-02-25T16:52:50Z

migration issues have been resolved upstream

dylanjw · 2019-02-27T01:03:11Z

After testing this again recently it looks like there has been a regression with Eth based tips. When I first worked on this, I think insufficient Eth had a warning already, and I just needed to implement the alternative token warning. Now Im only getting the warning for the alternative token. I will include a fix for the insufficient Eth in this PR.

dylanjw · 2019-03-05T05:24:49Z

The regression when sending an ETH tip looks a little deeper than I anticipated. I opened this issue: #3877.

Im surprised by that issue, because there looks like a lot of areas where that issue could be breaking things.

dylanjw · 2019-03-05T05:46:49Z

While it is decided if #3877 is a bug or not, I have updated this PR to normalize 0x0 to the full length address.

Here are screenshots of the tip error message displaying with Eth and with an alternate token:

thelostone-mc · 2019-03-05T15:15:41Z

@dylanjw will test this out but could you fix the CI errors ?

dylanjw · 2019-03-07T05:57:34Z

@thelostone-mc Im getting a CI error from the code coverage calculation. The file with the change in coverage looks unrelated to my changes.

Also, normalize 0x0 to full legnth

dylanjw · 2019-04-01T22:42:23Z

@thelostone-mc Ready to merge

stickler-ci reviewed Dec 3, 2018

View reviewed changes

dylanjw force-pushed the alert-underfunded-tip branch from 79cf7dc to 88c9817 Compare December 3, 2018 22:57

stickler-ci reviewed Dec 3, 2018

View reviewed changes

dylanjw force-pushed the alert-underfunded-tip branch from 88c9817 to 7843701 Compare December 3, 2018 22:58

dylanjw force-pushed the alert-underfunded-tip branch 2 times, most recently from 2e67def to f9898d0 Compare December 3, 2018 23:09

dylanjw force-pushed the alert-underfunded-tip branch from f9898d0 to b679fbb Compare February 25, 2019 09:42

dylanjw mentioned this pull request Feb 25, 2019

bug (er... UX thing) -- tips flow should warn you when u dont ahve enough ERC20 tokens to send tip #2250

Closed

thelostone-mc previously requested changes Feb 25, 2019

View reviewed changes

dylanjw force-pushed the alert-underfunded-tip branch from 93d5026 to 4c3506c Compare February 26, 2019 02:25

dylanjw force-pushed the alert-underfunded-tip branch from 6aace8f to 34a9ccb Compare March 5, 2019 05:45

dylanjw force-pushed the alert-underfunded-tip branch from ee6f0fc to 69a9ae0 Compare March 6, 2019 07:26

dylanjw added 4 commits April 1, 2019 15:18

Alert user if insufficient tokens for tip

dc27306

Change var declarations to

d9f495b

Run balance check on Eth when tipping

391684d

Also, normalize 0x0 to full legnth

Fix js lint errors

978207a

dylanjw force-pushed the alert-underfunded-tip branch from 69a9ae0 to 978207a Compare April 1, 2019 22:21

thelostone-mc approved these changes Apr 12, 2019

View reviewed changes

Merge branch 'master' into alert-underfunded-tip

67dfaf6

thelostone-mc requested review from owocki and danlipert April 12, 2019 09:43

Merge branch 'master' into alert-underfunded-tip

f2f4a80

SaptakS merged commit 80d30f3 into gitcoinco:master Apr 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alert user if insufficient tokens for tip #3058

Alert user if insufficient tokens for tip #3058

dylanjw commented Dec 3, 2018

stickler-ci Dec 3, 2018

stickler-ci Dec 3, 2018

stickler-ci Dec 3, 2018

stickler-ci Dec 3, 2018

stickler-ci Dec 3, 2018

stickler-ci Dec 3, 2018

stickler-ci Dec 3, 2018

stickler-ci Dec 3, 2018

stickler-ci Dec 3, 2018

stickler-ci Dec 3, 2018

codecov bot commented Dec 3, 2018

codecov bot commented Dec 3, 2018 •

edited

Loading

thelostone-mc left a comment •

edited

Loading

dylanjw commented Feb 25, 2019

thelostone-mc commented Feb 25, 2019

owocki commented Feb 25, 2019

dylanjw commented Feb 27, 2019

dylanjw commented Mar 5, 2019

dylanjw commented Mar 5, 2019

thelostone-mc commented Mar 5, 2019

dylanjw commented Mar 7, 2019 •

edited

Loading

dylanjw commented Apr 1, 2019

+                  }
+                });
+              };

Alert user if insufficient tokens for tip #3058

Alert user if insufficient tokens for tip #3058

Conversation

dylanjw commented Dec 3, 2018

Description

Checklist

Affected core subsystem(s)

Testing

Refers/Fixes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 3, 2018

Codecov Report

codecov bot commented Dec 3, 2018 • edited Loading

Codecov Report

thelostone-mc left a comment • edited Loading

Choose a reason for hiding this comment

dylanjw commented Feb 25, 2019

thelostone-mc commented Feb 25, 2019

owocki commented Feb 25, 2019

dylanjw commented Feb 27, 2019

dylanjw commented Mar 5, 2019

dylanjw commented Mar 5, 2019

thelostone-mc commented Mar 5, 2019

dylanjw commented Mar 7, 2019 • edited Loading

dylanjw commented Apr 1, 2019

codecov bot commented Dec 3, 2018 •

edited

Loading

thelostone-mc left a comment •

edited

Loading

dylanjw commented Mar 7, 2019 •

edited

Loading