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

Payment Requests #3476

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Payment Requests #3476

wants to merge 10 commits into from

Conversation

mark-boute
Copy link
Contributor

@mark-boute mark-boute commented Nov 9, 2023

Closes #3453 and #941.

Requirements

Implementation

  • We will extend the payment models with a PaymentRequest class, this model will store information about the request and the payment itself, it is also a Payable object. Each payed request will become immutable.

  • We will add an AdminView which shows all requests, but the default filter only shows 'pending' (unpaid) requests.

  • We will add an AdminView for editing pending requests.

  • There will be an AdminForm for adding new requests, here you can fill in details, and select any number of PaymentUsers (must be at least 1!) to whom the request apply.

  • !TODO think of how we would like to handle late-cancellation fines to happen with the payment-request system

  • After creating a new request the user should be notified, we will do this by e-mail and by pushnotification. An admin can send-on-save or do it manually later

Tasklist

  • setup model
  • write tests for model
  • admin views
  • admin view tests
  • admin form
  • admin form tests
  • notification time
  • email tests
  • a connection between fines and service
  • connection tests
  • (optional) reminder date

Summary

How to test

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'

Notes

  • How can we ensure that Users with an expired Membership can still pay (both before the PaymentRequest is created and after if they didn't pay yet but their membership did expire)?

@mark-boute mark-boute marked this pull request as draft November 9, 2023 13:14
@mark-boute mark-boute added priority: medium A new feature or a bugfix that is non-critical. app:payments Issues regarding the payments-app labels Nov 9, 2023
@mark-boute mark-boute added the feature Issues regarding a complete new feature label Nov 9, 2023
@T8902 T8902 added the request-for-comments Author wants to have other people respond for their opinion label Nov 9, 2023
@DeD1rk
Copy link
Member

DeD1rk commented Nov 9, 2023

Upon creating a new request the user should be notified, we will do this by e-mail and by pushnotification

This should probably be configurable or a separate manual action. I think it's useful if you can also silenty create a PaymentRequest, send it later, or resend it.
Also, we might want to wait with adding pushnotifications until the app supports paying with a deeplink (very similar to how the app supports paying sales orders).

website/payments/models.py Outdated Show resolved Hide resolved
website/payments/models.py Outdated Show resolved Hide resolved
website/payments/models.py Outdated Show resolved Hide resolved
website/payments/models.py Outdated Show resolved Hide resolved
website/payments/models.py Outdated Show resolved Hide resolved
blank=True,
null=True,
)

Copy link
Member

Choose a reason for hiding this comment

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

There should probably be some field to decide (for bookkeeping purposes) whether the payable 'has to be paid sometime'? I don't know enough about that though.

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 confirm with roel/nick/someone else that knows about bookkeeping what requirements this field should uphold?

website/payments/models.py Outdated Show resolved Hide resolved
@mark-boute
Copy link
Contributor Author

Upon creating a new request the user should be notified, we will do this by e-mail and by pushnotification

This should probably be configurable or a separate manual action. I think it's useful if you can also silenty create a PaymentRequest, send it later, or resend it. Also, we might want to wait with adding pushnotifications until the app supports paying with a deeplink (very similar to how the app supports paying sales orders).

Changed to: After creating a new request the user should be notified, we will do this by e-mail and by pushnotification. An admin can send-on-save or do it manually later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app:payments Issues regarding the payments-app feature Issues regarding a complete new feature priority: medium A new feature or a bugfix that is non-critical. request-for-comments Author wants to have other people respond for their opinion
Projects
None yet
3 participants