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

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

Merged
merged 4 commits into from
Apr 6, 2020

Conversation

zoek1
Copy link
Contributor

@zoek1 zoek1 commented Mar 13, 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

@zoek1
Copy link
Contributor Author

zoek1 commented Mar 13, 2020

What happens when a user isn't logged? Should be redirected to the login page? @owocki

@owocki
Copy link
Contributor

owocki commented Mar 13, 2020 via email

@zoek1 zoek1 force-pushed the feature/request-money branch from ee16b8d to 511a31b Compare March 24, 2020 17:20
@codecov
Copy link

codecov bot commented Mar 24, 2020

Codecov Report

Merging #6220 into master will increase coverage by 0.65%.
The diff coverage is 24.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6220      +/-   ##
==========================================
+ Coverage   27.58%   28.23%   +0.65%     
==========================================
  Files         282      278       -4     
  Lines       26116    25459     -657     
  Branches     3847     3726     -121     
==========================================
- Hits         7203     7189      -14     
+ Misses      18649    17989     -660     
- Partials      264      281      +17
Impacted Files Coverage Δ
app/app/urls.py 91.07% <ø> (+4.63%) ⬆️
app/retail/emails.py 22.48% <28.57%> (+0.14%) ⬆️
app/dashboard/tip_views.py 14.56% <8.57%> (-0.86%) ⬇️
app/dashboard/models.py 50.57% <83.33%> (+0.76%) ⬆️
app/marketing/mails.py 11.98% <9.09%> (+0.19%) ⬆️
app/grants/clr.py 0% <0%> (-13.7%) ⬇️
app/avatar/models.py 38.55% <0%> (-2.59%) ⬇️
app/avatar/admin.py 78.57% <0%> (-1.87%) ⬇️
app/grants/models.py 47.69% <0%> (-1.85%) ⬇️
app/marketing/models.py 62.71% <0%> (-1.34%) ⬇️
... and 42 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 850bd78...c6ffa0b. Read the comment docs.

@zoek1 zoek1 changed the title WIP: As a user, I would like to be able to request money from someon, so I can get paid and remind a forgetful funder. As a user, I would like to be able to request money from someon, so I can get paid and remind a forgetful funder. Mar 24, 2020
$('#request').on('click', function(e) {
e.preventDefault();
if ($(this).hasClass('disabled'))
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to validate that the user has accepted the TOS

@@ -1773,6 +1773,26 @@ def __str__(self):
return f"tip: {self.tip.pk} profile: {self.profile.handle}"


class FundRequest(SuperModel):
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add this model to dashboard/admin

import economy.models


class Migration(migrations.Migration):
Copy link
Contributor

Choose a reason for hiding this comment

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

CommandError: Conflicting migrations detected; multiple leaf nodes in the migration graph: (0093_auto_20200324_1653, 0095_hackathonevent_visible in dashboard).
To fix them run 'python manage.py makemigrations --merge'

Copy link
Contributor

Choose a reason for hiding this comment

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

we are avoiding "merge" on this cases where the migration is on a Pr need to be deleted and recreated.

@owocki
Copy link
Contributor

owocki commented Apr 6, 2020

@zoek1 just left a few small comments. looking really good! this will save a lot of time and heartache and reduce miscommunication i think

@danlipert @gitcoinco/engineers any chance this can get an 👀 pls?

@owocki
Copy link
Contributor

owocki commented Apr 6, 2020

looks great; very impressive art AND you read the instructions and delivered an svg that matches the spec :) yay!!!

anything i should put in your bio on-site for delivering this avatar?

@owocki
Copy link
Contributor

owocki commented Apr 6, 2020

shit i hit merge on the wrong PR (this one). here is a PR that reverts my merge #6386

@octavioamu
Copy link
Contributor

@zoek1 now this was reverted, can you recreate this PR ?

@zoek1
Copy link
Contributor Author

zoek1 commented Apr 6, 2020

Sure @Octavio, also i'll add the requests changes 🙂

@owocki
Copy link
Contributor

owocki commented Apr 6, 2020

ok heres a new PR we can use #6220

unless you want to do it again off ur PR @zoek1 ? sorry again about this.

@zoek1
Copy link
Contributor Author

zoek1 commented Apr 7, 2020

No problem :) I'm finishing another task for the hackathon view and after that, I'll add the requested changes here

@owocki
Copy link
Contributor

owocki commented Apr 8, 2020

@zoek1 can you link the new PR here? im keen to get this live sono!

@owocki
Copy link
Contributor

owocki commented Apr 8, 2020

*soon

@zoek1
Copy link
Contributor Author

zoek1 commented Apr 9, 2020

Working on it right now, I think in a couple of hours it will be finished 😄

@zoek1
Copy link
Contributor Author

zoek1 commented Apr 9, 2020

New PR @owocki #6411

@owocki
Copy link
Contributor

owocki commented Apr 9, 2020 via email

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