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

New As a user, I would like to be able to request money from someon, so I can get paid and remind a forgetful funder. #6411

Merged
merged 15 commits into from
Apr 29, 2020

Conversation

zoek1
Copy link
Contributor

@zoek1 zoek1 commented Apr 9, 2020

Description
Refers/Fixes

As a user, I would like to be able to request money from someon, so I can get paid and remind a forgetful funder. #6193

Testing

Demo: https://www.dropbox.com/s/3jmejmc052hyjfy/2020-03-24%2011-56-28.flv?dl=0

image

Email template
DeepinScreenshot_20200330010618

@owocki
Copy link
Contributor

owocki commented Apr 9, 2020

thanks; i'm very excited about this. just put it on the @gitcoinco/engineers PR review board

@danlipert
Copy link
Contributor

@zoek1 Does this work with DAI? it seems that it only works for Ether right now

comments = request.POST.get('comments')
network = request.POST.get('network')
address = request.POST.get('address')
token_address = request.POST.get('tokenAddress')
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, i forgot delete this unused variable

@zoek1
Copy link
Contributor Author

zoek1 commented Apr 17, 2020

Updated @danlipert

@zoek1 Does this work with DAI? it seems that it only works for Ether right now

Yes, it works with DAI even with other networks like SIA, or ETC. The 'request payment view' only requires the token name, so the tip view can retrieve the token address.

@zoek1 zoek1 requested a review from danlipert April 17, 2020 02:32
@codecov
Copy link

codecov bot commented Apr 28, 2020

Codecov Report

Merging #6411 into master will increase coverage by 0.03%.
The diff coverage is 31.16%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6411      +/-   ##
==========================================
+ Coverage   27.25%   27.29%   +0.03%     
==========================================
  Files         287      287              
  Lines       26644    26712      +68     
  Branches     3936     3945       +9     
==========================================
+ Hits         7262     7290      +28     
- Misses      19115    19155      +40     
  Partials      267      267              
Impacted Files Coverage Δ
app/app/urls.py 86.20% <ø> (ø)
app/dashboard/tip_views.py 14.39% <7.89%> (-1.03%) ⬇️
app/marketing/mails.py 11.52% <8.33%> (-0.04%) ⬇️
app/retail/emails.py 22.41% <28.57%> (+0.06%) ⬆️
app/dashboard/models.py 50.26% <85.71%> (+0.17%) ⬆️
app/dashboard/admin.py 66.46% <100.00%> (+0.62%) ⬆️
app/dashboard/embed.py 31.60% <0.00%> (+3.44%) ⬆️

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 9b48b55...91c7952. Read the comment docs.

@zoek1
Copy link
Contributor Author

zoek1 commented Apr 28, 2020

@thelostone-mc could you check this PR please? 😃

_alert('Something goes wrong, try later.', 'error');
failure_callback();
});
}
Copy link
Member

Choose a reason for hiding this comment

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

^ could we update all var to const/let?

@thelostone-mc thelostone-mc merged commit 24f9052 into gitcoinco:master Apr 29, 2020
Comment on lines +88 to +90
<select style="width:100px; margin-bottom: 10px; display: inline;">
<option value="{{ fund_request.network }}">{{ fund_request.network }}</option>
</select>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @zoek1 just to let you know here was missing the id="token" so the token select wasn't getting fill.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry @octavioamu, I'm not sure why the id was missing. I really tested 😧

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.

6 participants