Fixing advanced payout - Default to Bountied Amount for Payment, etc. #2365

Anish-Agnihotri · 2018-10-05T20:22:23Z

Description

This PR resolves #2294 by introducing:

Alerts upon advanced payment > 100%.
Automatic table altering upon X click.
Other misc. payout preview cleaning (removing mentions of bounty stake, etc.).

See it in action!

Checklist

Remove Return Bounty Stake Checkbox
Default all payments to pay from the Initial Funded Bounty Amount
Add an alert (bg color #E66700, #FFF for copy) when user exceeds 100% or Bountied value in ETH.
Add wallet information and overage amount to the alert.
Display the overage amount on the payout preview in #E66700
Remove refunded Eth
Remove mention of bounty stake
If multiple users have been added to the pay list, and a user is removed, the payout preview should reflect the removal.
This issue is tested

Affected core subsystem(s)

UI is the only core system affected, since all changes are aesthetic.

Testing

Tested across two environments across two platforms (W10, Mojave). Feel free to test yourself as well.

Refers/Fixes

Fixes #2294.

…Guidance on Overage

stickler-ci · 2018-10-05T20:22:33Z

app/assets/v2/js/pages/bulk_payout.js

@@ -68,8 +68,11 @@ $(document).ready(function($) {

  $(document).on('click', '.remove', function(event) {
    event.preventDefault();
+    var position = ($(this).parents('tr').index()+2).toString();


Expected blank line after variable declarations. (newline-after-var)
Infix operators must be spaced. (space-infix-ops)

stickler-ci · 2018-10-05T20:22:33Z

app/assets/v2/js/pages/bulk_payout.js

@@ -68,8 +68,11 @@ $(document).ready(function($) {

  $(document).on('click', '.remove', function(event) {
    event.preventDefault();
+    var position = ($(this).parents('tr').index()+2).toString();
+    $("#transaction_registry tr:nth-child(" + position + ")").remove();


Strings must use singlequote. (quotes)

stickler-ci · 2018-10-05T20:22:33Z

app/assets/v2/js/pages/bulk_payout.js

@@ -212,11 +224,26 @@ $(document).ready(function($) {
    var denomination = $('#token_name').text();
    var original_amount = $('#original_amount').val();
    var net = round(original_amount - tc, 2);
+    var over = round(((original_amount) - (get_total_cost()))*-1, 4);


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

stickler-ci · 2018-10-05T20:22:34Z

app/assets/v2/js/pages/bulk_payout.js

+    if (over > 0) {
+      $('.overageAlert').css('display', 'inline-block');
+      $('.overagePreview').css('display', 'inline-block');
+    }


Closing curly brace does not appear on the same line as the subsequent block. (brace-style)

mbeacom

Can you please run make fix-eslint on this to resolve stickler comments?

mbeacom · 2018-10-05T20:35:26Z

Additionally, can you resolve the merge conflict?

Anish-Agnihotri · 2018-10-05T20:37:42Z

@mbeacom

Edit: All issues have been resolved. Thanks!

stickler-ci · 2018-10-05T20:37:48Z

app/assets/v2/js/pages/bulk_payout.js

@@ -68,8 +68,12 @@ $(document).ready(function($) {

  $(document).on('click', '.remove', function(event) {
    event.preventDefault();
+    var position = ($(this).parents('tr').index()+2).toString();


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

stickler-ci · 2018-10-05T20:37:48Z

app/assets/v2/js/pages/bulk_payout.js

@@ -212,11 +225,25 @@ $(document).ready(function($) {
    var denomination = $('#token_name').text();
    var original_amount = $('#original_amount').val();
    var net = round(original_amount - tc, 2);
+    var over = round((original_amount - get_total_cost())*-1, 4);


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

codecov · 2018-10-05T20:52:04Z

Codecov Report

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

@@            Coverage Diff             @@
##           master    #2365      +/-   ##
==========================================
- Coverage   29.48%   29.47%   -0.02%     
==========================================
  Files         146      146              
  Lines       11771    11777       +6     
  Branches     1597     1599       +2     
==========================================
  Hits         3471     3471              
- Misses       8182     8188       +6     
  Partials      118      118

Impacted Files	Coverage Δ
app/retail/views.py	`30% <0%> (-0.69%)`	⬇️
app/dashboard/gas_views.py	`23.15% <0%> (ø)`	⬆️

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 f2ebc34...c388e52. Read the comment docs.

pinkiebell · 2018-10-06T11:00:23Z

app/dashboard/templates/bulk_payout_bounty.html

+        float: left;
+        padding-right: 10px;
+      }
+      .overageAlert> div:last-child {


> - cosmetic whitespace missing 😄

pinkiebell · 2018-10-06T11:03:06Z

LGTM; Just that one cosmetic one-liner to fix 😄

@pixiebell

Recommended by @pixiebell. Thanks for the review!

Anish-Agnihotri · 2018-10-07T02:51:35Z

LGTM; Just that one cosmetic one-liner to fix 😄

Thanks for the review! Just pushed out the fix in the new commit :).

Edit: Apologies for misspelling your Github username in the commit log.

Edit 2: Codecov errors are not related to these commits. Branch is safe to merge.

thelostone-mc · 2018-10-07T04:42:48Z

app/assets/v2/js/pages/bulk_payout.js

@@ -166,7 +170,16 @@ $(document).ready(function($) {
      _alert('You do not have any transactions to payout.  Please add payees to the form.', 'error');
      return;
    }
+    var usernames = $('.username');


encourage moving to let / const as opposed to var ?
cause we've got the ES6 support coming in this week

thelostone-mc · 2018-10-07T04:43:06Z

app/assets/v2/js/pages/bulk_payout.js

+    var usernames = $('.username');
+
+    for (var i = 0; i < usernames.length; i++) {
+      var username = usernames[i].textContent.trim();


thelostone-mc · 2018-10-07T04:43:52Z

app/dashboard/templates/bulk_payout_bounty.html

@@ -120,6 +153,15 @@ <h5>{% trans 'Payout Preview' %}</h5>
                    </a>
                  </div>
                </div>
+


remove the extra line ?

thelostone-mc · 2018-10-07T12:10:26Z

Hmm I see changes in the codebase aka ( autcomplete and tooltip PR ) repeating here. Any idea why that's happening ? 🤔 here, here and a few other places.

Could you double check to make sure only your changes are preset in the PR ?

Anish-Agnihotri · 2018-10-07T20:47:26Z

@thelostone-mc I had to add those since there seemed to be a merge error otherwise. I can rebase from default, restructure, and push out another commit tonight.

And, sure, I'll change variable declaration to follow ES6 standards as well.

Fixing advanced payout - Default to Bountied Amount for Payment, Add …

37a439e

…Guidance on Overage

stickler-ci reviewed Oct 5, 2018

View reviewed changes

mbeacom suggested changes Oct 5, 2018

View reviewed changes

mbeacom requested review from SaptakS and thelostone-mc October 5, 2018 20:33

mbeacom added frontend This needs frontend expertise. bounties community member labels Oct 5, 2018

Fixing advanced payout - lint errors, and merge conflicts.

1bd6864

stickler-ci reviewed Oct 5, 2018

View reviewed changes

Fixing advanced payout - autolinting.

278847e

pinkiebell reviewed Oct 6, 2018

View reviewed changes

Fixing cosmetic white spacing.

c388e52

Recommended by @pixiebell. Thanks for the review!

gitcoinbot mentioned this pull request Oct 7, 2018

Advanced Payout - Default to Bountied Amount for Payment, Add Guidance on Overage #2294

Closed

9 tasks

thelostone-mc reviewed Oct 7, 2018

View reviewed changes

This was referenced Oct 8, 2018

bug: advanced payout #2383

Merged

Adding overage alert to advanced payout. #2386

Merged

Anish-Agnihotri closed this Oct 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing advanced payout - Default to Bountied Amount for Payment, etc. #2365

Fixing advanced payout - Default to Bountied Amount for Payment, etc. #2365

Anish-Agnihotri commented Oct 5, 2018 •

edited

Loading

stickler-ci Oct 5, 2018

stickler-ci Oct 5, 2018

stickler-ci Oct 5, 2018

stickler-ci Oct 5, 2018

mbeacom left a comment

mbeacom commented Oct 5, 2018

Anish-Agnihotri commented Oct 5, 2018 •

edited

Loading

stickler-ci Oct 5, 2018

stickler-ci Oct 5, 2018

codecov bot commented Oct 5, 2018 •

edited

Loading

pinkiebell Oct 6, 2018

pinkiebell commented Oct 6, 2018

Anish-Agnihotri commented Oct 7, 2018 •

edited

Loading

thelostone-mc Oct 7, 2018

thelostone-mc Oct 7, 2018

thelostone-mc Oct 7, 2018

thelostone-mc commented Oct 7, 2018 •

edited

Loading

Anish-Agnihotri commented Oct 7, 2018

@@ @@ -120,6 +153,15 @@ <h5>{% trans 'Payout Preview' %}</h5> @@
                                   </a>
                                 </div>
                               </div>

Fixing advanced payout - Default to Bountied Amount for Payment, etc. #2365

Fixing advanced payout - Default to Bountied Amount for Payment, etc. #2365

Conversation

Anish-Agnihotri commented Oct 5, 2018 • edited Loading

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

mbeacom left a comment

Choose a reason for hiding this comment

mbeacom commented Oct 5, 2018

Anish-Agnihotri commented Oct 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 5, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

pinkiebell commented Oct 6, 2018

Anish-Agnihotri commented Oct 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thelostone-mc commented Oct 7, 2018 • edited Loading

Anish-Agnihotri commented Oct 7, 2018

Anish-Agnihotri commented Oct 5, 2018 •

edited

Loading

Anish-Agnihotri commented Oct 5, 2018 •

edited

Loading

codecov bot commented Oct 5, 2018 •

edited

Loading

Anish-Agnihotri commented Oct 7, 2018 •

edited

Loading

thelostone-mc commented Oct 7, 2018 •

edited

Loading