Skip to content

Mypayment extended#58

Open
Rotheem wants to merge 8 commits intodevfrom
mypayment-extended
Open

Mypayment extended#58
Rotheem wants to merge 8 commits intodevfrom
mypayment-extended

Conversation

@Rotheem
Copy link
Copy Markdown

@Rotheem Rotheem commented Mar 19, 2026

Description

Please explain the changes you made here.

Checklist

  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the documentation, if necessary

@armanddidierjean armanddidierjean changed the base branch from dev to merge-eclair March 19, 2026 08:17
@Rotheem Rotheem force-pushed the mypayment-extended branch from 9cc4c12 to 1e78c9d Compare March 19, 2026 14:54
@Rotheem Rotheem changed the base branch from merge-eclair to dev-merged March 19, 2026 14:57
Base automatically changed from dev-merged to dev March 19, 2026 14:59
@Rotheem Rotheem force-pushed the mypayment-extended branch from b774157 to 159040a Compare March 19, 2026 15:02
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 77.63713% with 53 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
app/core/mypayment/utils_mypayment.py 60.81% 29 Missing ⚠️
app/core/mypayment/endpoints_mypayment.py 78.57% 15 Missing ⚠️
app/core/mypayment/cruds_mypayment.py 80.00% 5 Missing ⚠️
app/core/mypayment/exceptions_mypayment.py 71.42% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Rotheem Rotheem force-pushed the mypayment-extended branch from 3067516 to 50fa93e Compare March 24, 2026 12:08
@Rotheem Rotheem force-pushed the mypayment-extended branch from 5e02895 to 7fe4cd1 Compare April 6, 2026 16:50
Copy link
Copy Markdown

@armanddidierjean armanddidierjean left a comment

Choose a reason for hiding this comment

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

We could create a method that would allow to call either request_transaction or request_store_transfer. Maybe it could be a good idea to separate mypayment internal and public utils

message = Message(
title=f"💸 Nouvelle demande de paiement - {request_info.name}",
content=f"Une nouvelle demande de paiement de {request_info.total / 100} € attend votre validation",
action_module=settings.school.payment_name,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This should be the module root

message = Message(
title=f"💳 Paiement - {store.name}",
content=f"Une transaction de {transaction.total / 100} € a été effectuée",
action_module=settings.school.payment_name,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This should be the root

tot: Total amount of the transaction, in cents
iat: Generation datetime of the payment order
key: Id of the WalletDevice that generated the payment order, will be used to get the public key to verify the signature
store: If the payment is destined to a store
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
store: If the payment is destined to a store
store: If the payment is intended to be banked by a store or by an other user

Comment on lines +356 to +357
name: str
note: str | None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could we clarify a bit this part? Maybe store_note like in RequestEdit?



async def request_store_transfer(
user: schemas_users.CoreUser,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You ask for both the user and the user_id in transfer_info

Comment on lines +167 to +169
"""
Create a transaction request for a user from a store.
"""
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you make the docstring more explicit, stating what will happen:

  • create a mypayment payment request between the user wallet and the store wallet
  • the request need to be accepted be the user using ... endpoint

user: schemas_users.CoreUser,
transfer_info: schemas_mypayment.StoreTransferInfo,
db: AsyncSession,
payment_tool: PaymentTool,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We need a specific PaymentTool of type mypayment. Letting the user pass its own payment_tool may lead to — hard to discover — issues.

I would suggest wrapping this method in a dependency that would ask for its own payment tool

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