Payment review #67
Payment review #67
Conversation
ShiroUsagi-san
added
priority:high
Lymkwi
force-pushed
the
payment
branch
2 times, most recently
from
9f83f95
to
f686fee
Compare
|
Alright so in my opinion the one thing still absolutely needed is a callback system to implement product callbacks. That's it. |
|
For the refund, only admins can achieve this action. This action can only be performed in the djando admin panel, with an action on a transaction. In the front, maybe a form that sends an email to email address. This will reduce the risk to be misused |
|
So today I added:
|
|
From what I got from our last conversation, all that's left here is:
|
|
The only thing missing is a django action to allow admin to refund/cancel user participation in a tournament. But we getting very close to the final version 👌 |
|
Before merging this big piece of features, the only thing missing is to allow player to join a team. The idea is to use team password. The captain can generate it (generated randomly otherwise) and then an url with this password as a query param is given to the captain and they can invite their player with it. |
This is more linked with tournament code, as its pertains to the flow of joining teams, but not the flow of paying for a ticket or refunding it, provided that everything else works. |
Translate the strings used for validation errors introduced in the second to last commit, c357aaf.
Add a method that computes whether or not a team can be considered valid at any given point, based on quotas of people who paid. For now, the parameter `n` used is the number of people who joined the team. Very soon, it will be changed by the number of slots in a team (which is tournament dependent).
Add a Django admin action that triggers a refund of a transaction. Consequently, add a hook to the payment callback class that performs post-refund actions. For tournament, this post-refund hook finds any registration paid linked to the tournament and user in question, destroys it, and cancels associated tickets.
- Actually use the singleton object - Implement an expiration date - Check the expiration date when we get the token, refreshing it if need be - Add timeout parameters to the request calls, and propagate/log on errors if need be - Refactor some code
Transform all HelloAsso variables into constants in the settings of the whole project. Also adds organization slug, and normalizes variable names.
So far, it does nothing
- Fix tests (I hate timezones) - Fix a leak of data that resulted in all tournament information being visible in the `event/<id>/tournaments/` dereferencement route, which did not take into account the `is_announced` key
- Fix ticket creation missing a mandatory key (tournament) - Fix bug where despite buying a player ticket, you could end up validating a manager one - Change fetch_registration method to determine your type of registration from your product, not from the fact that a registration is available - Introduct the preparation hook to detect if a single registration of the correct type is ready to be paid before payment is initiated
Similar to Transactions, Payments cannot be deleted or manipulated, only viewed.
Register the order_id (which is not the same as the intent id) into a Transaction, also displaying it in the admin view
The grant_type used to retrieve an OAuth token is different whether or not you are using client credentials (first login), or refreshing a token (refreshes). Without this patch, initial obtention of tokens is impossible.
Properly redirect the user… by not redirecting them. Redirects in Django only really work within the same domain, so instead we return the URL in a JSON reply, and will let the frontend do its magic. At the same time, we move the call to the prepare_hook after creation of the transaction object (for validation purposes), but before the intent creation on HelloAsso's side. If the hooks return `False`, then something is wrong, and we cannot proceed with payment, needing instead to delete the Transaction.
Implement the notification channel that receives and handles two kinds of messages: - Order: those messages give us the layout of payments (typically here for us only one of them) and order number associated with our transaction, which we can add and update into the database - Payment: those messages let us known the status of a payment, which can trigger validation or invalidation of a transaction (invalidation has not been tested yet, as notifications only come when a payment succeeds) Properly implement refunding by iterating over defined payments for a transaction and refunding them individually. Note that that part has not been entirely tested yet, because our credentials do not allow us to refund yet.
Properly handle the fact that we could get notified about a transaction we do not know
- Remove the transaction refund admin action, mostly by commenting it out - Normalize state methods, so that if you want to change the state of an action, you do not have to run the hooks yourself or touch the transaction or verify states or anything (it also logs stuff) - Add Refunded as a supported Payment message type
|
Rebased. Abandon all hope, ye who reads this PR |
| default=ProductCategory.PIZZA, | ||
| choices=ProductCategory.choices, | ||
| ) | ||
| associated_tournament = models.ForeignKey( |
There was a problem hiding this comment.
do we keep this field or use a more generic foreignKey?
There was a problem hiding this comment.
I could try and work out how to do a more generic foreign key now to be honest
I'm just worried about breaking things and not being able to fix them before merge, but I can try and check if it's feasible at least
There was a problem hiding this comment.
as you want, you did a huge work here
insalan/tournament/serializers.py
Outdated
| @@ -54,8 +54,9 @@ class Meta: | |||
| """Meta options of the serializer""" | |||
|
|
|||
| model = Tournament | |||
| read_only_fields = ("id",) | |||
| fields = ["id", "event", "game", "name", "teams", "logo"] | |||
| read_only_fields = ("id","manager_price_onsite", "manager_price_onsite", | |||
There was a problem hiding this comment.
cashprize is readonly I think
There was a problem hiding this comment.
Unsure, it should be editable in case of new partnership increasing the cashprize I guess?
There was a problem hiding this comment.
isn't it for the fetching endpoint ?
There was a problem hiding this comment.
These are serializers, access rights should be managed in views (for example a ReadOnly | IsAdmin), however since we've put prices and all read-only, I guess we can do the same with the cashprize to be sure 🤷♀️
There was a problem hiding this comment.
I guess you're right
There was a problem hiding this comment.
Yeah I'll set them we read-only on serialization and editable in admin view
There was a problem hiding this comment.
Wait actually everything should be read-only?
We had `manager_price_onsite` in there twice.
This PR is about the last big stone of the back-end: payment API.
For now,
/payement/paymakes the request to helloasso but the post processing isn't implemented yet.There is also some CORS issues to solve
TODO
/payment/refundhandling callbacks
cleaning code
hardening
documentation
testing