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

Add view to generate invoice for contribution #5331

Merged
merged 7 commits into from
Nov 13, 2019

Conversation

zoek1
Copy link
Contributor

@zoek1 zoek1 commented Oct 18, 2019

Description

When a contribution is made to Gitcoin Grant, an invoice is created using the Gitcoin Bounties template.

  1. Each contribution has an invoice and is displayed only to contribution owner:
    Test Grant _ Gitcoin

  2. Display invoice contribution:
    Grow Open Source _ Gitcoin (1)

Refers/Fixes

#4264

Testing

</tr>
</table>
</div>
</body>
Copy link
Member

Choose a reason for hiding this comment

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

indent this by +1 ?

<meta charset="utf-8">
{% include 'shared/head.html' %}
{% include 'shared/cards.html' %}

Copy link
Member

Choose a reason for hiding this comment

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

^ deindent these by 1

Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

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

@zoek1 could you throw in a screenshot of the invoice page ?

@danlipert
Copy link
Contributor

Hey Miguel - thanks for your PR - next time if you are submitting a PR thats still a WIP, please use the draft PR function - thanks!

@zoek1
Copy link
Contributor Author

zoek1 commented Oct 23, 2019

Sure, I'll add the screenshot and update the PR description.

@codecov
Copy link

codecov bot commented Oct 26, 2019

Codecov Report

Merging #5331 into master will decrease coverage by 0.21%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5331      +/-   ##
==========================================
- Coverage   29.85%   29.63%   -0.22%     
==========================================
  Files         241      242       +1     
  Lines       20406    20819     +413     
  Branches     2918     3009      +91     
==========================================
+ Hits         6092     6170      +78     
- Misses      14063    14397     +334     
- Partials      251      252       +1
Impacted Files Coverage Δ
app/grants/urls.py 100% <ø> (ø) ⬆️
app/grants/models.py 61.51% <0%> (ø) ⬆️
app/grants/views.py 16.32% <20%> (+0.04%) ⬆️
...eting/management/commands/assemble_leaderboards.py 63.78% <0%> (-11.32%) ⬇️
app/quests/views.py 15.89% <0%> (-7.92%) ⬇️
app/quests/admin.py 43.75% <0%> (-1.25%) ⬇️
app/dashboard/admin.py 62.59% <0%> (-1.12%) ⬇️
app/quests/models.py 44.89% <0%> (-0.91%) ⬇️
app/kudos/admin.py 61.72% <0%> (-0.61%) ⬇️
app/quests/helpers.py 20.63% <0%> (-0.61%) ⬇️
... and 23 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 6b95c8f...7e77df0. Read the comment docs.

@zoek1 zoek1 changed the title WIP: Add view to generate invoice for contribution Add view to generate invoice for contribution Oct 26, 2019
@zoek1 zoek1 requested a review from thelostone-mc October 26, 2019 23:57
@danlipert
Copy link
Contributor

@zoek1 is this ready for review now?

@zoek1
Copy link
Contributor Author

zoek1 commented Oct 29, 2019

Yep, it's ready for review!
@danlipert

</td>

<td>
0x2af47a65da8cd66729b4209c22017d6a5c2d2400
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats up with this hardcoded value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I based this invoice on the invoice template for bounties, I took the value from there. As I understand this address represents the gitcoin address to manage such transactions. This value is hardcoded in other places in the codebase, what is recommended in this cases? @danlipert

Copy link
Contributor

Choose a reason for hiding this comment

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

So I took a look and actually its the Bounties Network v1 Contract Address, which doesn't make sense here... lets use the grant contract address for each grant instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! 😀

contribution = Contribution.objects.prefetch_related('subscription', 'subscription__grant').get(pk=contribution_pk)

# only allow invoice viewing if admin or if grant contributor
is_contributor = request.user.username.lower().lstrip('@') == contribution.subscription.contributor_profile.handle.lower().lstrip('@')
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm.. probably can do this logic simpler like has_view_privs = request.user.is_staff or request.user.profile == contribution.subscription.contributor_profile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, i just changed!

@danlipert danlipert merged commit 3137d6d into gitcoinco:master Nov 13, 2019
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.

4 participants