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

consolidated payment wizard flow #1789

Merged
merged 5 commits into from
Jul 20, 2018
Merged

consolidated payment wizard flow #1789

merged 5 commits into from
Jul 20, 2018

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Jul 19, 2018

This PR has a vastly improved payout flow, in my opinioin.
UX:

  1. One button on the bounty details page
  2. Choose between basic payout and advanced payout. Explains the diff right there in the UI
  3. can switch between them

Check it out: http://bits.owocki.com/0P0q3R3d1B38/Screen%20Recording%202018-07-18%20at%2010.54%20PM.gif

@ghost ghost assigned owocki Jul 19, 2018
@ghost ghost added the in progress label Jul 19, 2018
@owocki owocki requested review from mbeacom and SaptakS July 19, 2018 04:55
@codecov
Copy link

codecov bot commented Jul 19, 2018

Codecov Report

Merging #1789 into master will increase coverage by 2.09%.
The diff coverage is 18.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1789      +/-   ##
==========================================
+ Coverage   28.59%   30.68%   +2.09%     
==========================================
  Files         128      130       +2     
  Lines       10047    11441    +1394     
  Branches     1324     1678     +354     
==========================================
+ Hits         2873     3511     +638     
- Misses       7073     7780     +707     
- Partials      101      150      +49
Impacted Files Coverage Δ
app/app/urls.py 86.04% <ø> (-3.96%) ⬇️
app/dashboard/models.py 61.34% <0%> (+5.95%) ⬆️
app/dashboard/helpers.py 23.4% <0%> (+5.83%) ⬆️
app/dashboard/views.py 18.6% <25%> (+3.37%) ⬆️
app/dashboard/gas_views.py 32.35% <0%> (-1.61%) ⬇️
app/retail/views.py 31.29% <0%> (-0.32%) ⬇️
app/dashboard/router.py 34.16% <0%> (-0.05%) ⬇️
...itcoinbot/management/commands/get_notifications.py 0% <0%> (ø) ⬆️
app/github/utils.py 58.67% <0%> (ø)
... and 8 more

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 3a9e243...69d9e92. Read the comment docs.

@PixelantDesign
Copy link
Contributor

very cool

@thelostone-mc
Copy link
Member

thelostone-mc commented Jul 19, 2018

This is neattttt! Good for review ?

@SaptakS
Copy link
Contributor

SaptakS commented Jul 20, 2018

Just a question. Are we adding this color to our pallette of colors. Since I saw you using this also in the admin actions PR so curious. @PixelantDesign .I was wondering if we could stick to one of our blues.

@ghost ghost assigned mbeacom Jul 20, 2018
Copy link
Contributor

@mbeacom mbeacom left a comment

Choose a reason for hiding this comment

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

Other than my comment, lgtm. - Should we throw this on staging to start QA?

@@ -938,7 +938,7 @@ def receive_url(self):
if self.web3_type == 'yge':
return self.url

pk = self.metadata['priv_key']
pk = self.metadata.get('priv_key')
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we default to something in this get or should we use pk = self.metadata['priv_key'] wrapped in try/except on KeyError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change actually shouldnt get in this PR. ill remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@owocki
Copy link
Contributor Author

owocki commented Jul 20, 2018

@SaptakS its probably a better idea to stick to one of the palette colors. ill give the blue a try unless @PixelantDesign has another direction she wants to go

@owocki
Copy link
Contributor Author

owocki commented Jul 20, 2018

just turned around the feedback

@owocki owocki merged commit eca1314 into master Jul 20, 2018
@ghost ghost removed the in progress label Jul 20, 2018
@mbeacom mbeacom deleted the kevin/payout_flow branch July 23, 2018 13:31
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.

5 participants