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 itemized payments #27

Merged
merged 9 commits into from
Jul 28, 2023
Merged

Conversation

apromessi
Copy link
Collaborator

@apromessi apromessi commented Jul 10, 2023

Summary of Changes

Adds the concept of an ItemizedPayment for recording how much of an incoming payment went towards fees (instruments) and how much went to the project (attributions). We need this historical data when calculating the total amount that a person has paid into the project so that we can compare the total amount accurately with the price and the correct investment amount. Only the portion of a payment that went towards the project should count towards price/investment.

Public Domain Dedication

  • In contributing, I relinquish any copyright claims on my contribution and freely release it into the public domain in the simple hope that it will provide value.

(Why: The freely released, copyright-free work in this repository represents an investment in a better way of doing things called attribution-based economics. Attribution-based economics is based on the simple idea that we gain more by giving more, not by holding on to things that, truly, we could only create because we, in our turn, received from others. As it turns out, an economic system based on attribution -- where those who give more are more empowered -- is significantly more efficient than capitalism while also being stable and fair (unlike capitalism, on both counts), giving it transformative power to elevate the human condition and address the problems that face us today along with a host of others that have been intractable since the beginning. You can help make this a reality by releasing your work in the same way -- freely into the public domain in the simple hope of providing value. Learn more about attribution-based economics at drym.org, tell your friends, do your part.)

@apromessi apromessi changed the title Add payment allocations Add itemized payments Jul 10, 2023
@countvajhula
Copy link
Contributor

LGTM

Copy link
Collaborator

@jairtrejo jairtrejo left a comment

Choose a reason for hiding this comment

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

Looks good to me!

I left a comment about using all (itemized) payments vs only new, but I think it works the same as long as payments are processed correctly every time.

total_payments = total_amount_paid(payment.email)
previous_total = total_payments - payment.amount
total_payments = total_amount_paid_to_project(
payment.email, new_itemized_payments
Copy link
Collaborator

Choose a reason for hiding this comment

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

Going from all payments to only the new ones is a significant change. It introduces the assumption that previous payments you are no longer looking at were already accounted for.

Is there a way to make this work with all previous itemized payments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call out! So we actually do use all itemized payments when we tally the total amount paid. Inside total_amount_paid_to_project, we combine the sums of existing itemized payments that have already been recorded in the file and these new_itemized_payments.

The reason we have to pass in the new itemized payments is because we haven't actually recorded them yet since we wait until payments have been processed without errors before recording anything (new valuation, renormalized attributions, and now, itemized payments). If we switch to a sqlite database at some point, this issue will be solved. We will be able to write to the itemized payments table within a transaction and query the whole table to get the correct total instead of passing new itemized payments down the chain.

@apromessi apromessi merged commit d68a1be into drym-org:main Jul 28, 2023
4 checks passed
@coveralls
Copy link

Pull Request Test Coverage Report for Build 5549650281

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 33 of 42 (78.57%) changed or added relevant lines in 2 files are covered.
  • 9 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.6%) to 61.017%

Changes Missing Coverage Covered Lines Changed/Added Lines %
oldabe/money_in.py 26 35 74.29%
Files with Coverage Reduction New Missed Lines %
oldabe/money_in.py 9 58.65%
Totals Coverage Status
Change from base Build 5459039338: -0.6%
Covered Lines: 203
Relevant Lines: 328

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 5549650281

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 33 of 42 (78.57%) changed or added relevant lines in 2 files are covered.
  • 9 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.6%) to 61.017%

Changes Missing Coverage Covered Lines Changed/Added Lines %
oldabe/money_in.py 26 35 74.29%
Files with Coverage Reduction New Missed Lines %
oldabe/money_in.py 9 58.65%
Totals Coverage Status
Change from base Build 5459039338: -0.6%
Covered Lines: 203
Relevant Lines: 328

💛 - Coveralls

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