Add payments relation on wallet #874
Conversation
This adds functionality to query the database for transfers in the other direction, payments. Lookup what transfers happened from another wallet to this one. Signed-off-by: George Klincarski <[email protected]>
Add payments relation on wallet
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #874 +/- ##
=============================================
- Coverage 100.00% 99.75% -0.25%
- Complexity 581 582 +1
=============================================
Files 88 88
Lines 2029 2034 +5
=============================================
Hits 2029 2029
- Misses 0 5 +5 ☔ View full report in Codecov by Sentry. |
|
Ill add tests if this is accepted |
src/Traits/HasWallet.php
Outdated
| * | ||
| * @return HasMany<Transfer> | ||
| */ | ||
| public function payments(): HasMany |
There was a problem hiding this comment.
- This should be in the Wallet model;
- The name is not good, because this is the receipt of a transfer (there are transactions: transfer, exchange, payment);
- No unit tests;
There was a problem hiding this comment.
Ok, what should this relationship be called?
There was a problem hiding this comment.
You can call it like this:
receivedTransfersI don't have a better idea.
According to the unit test, it is not necessary to write a new one. It is enough to pull a new method in an existing test and check the number of elements.
There was a problem hiding this comment.
And, most importantly. If you want a feature now, then PR must be done from the 10.x branch.
If from the master, then it is already 11.x, and this version will not support laravel 10.x
Signed-off-by: George Klincarski <[email protected]>
Signed-off-by: George Klincarski <[email protected]>
okay. Please specify for which version the change is needed? 10.x or 11.x |
Why am I asking this question? If for version 11.x, then I deceived you. This should be HasWallet + add to the contract \Bavix\Wallet\Interfaces\Wallet. And if for version 10.x, then it should be in the Wallet model. UPD: The bottom line is that you cannot make changes that break backward compatibility. And version 11.x has not yet been released. |
|
I will make this for version 10 and create new pull request for version 11Sent from my iPhoneOn 31 Jan 2024, at 10:49, Maxim Babichev ***@***.***> wrote:
@gkmk
Please specify for which version the change is needed? 10.x or 11.x
Why am I asking this question? If for version 11.x, then I deceived you. This should be HasWallet + add to the contract \Bavix\Wallet\Interfaces\Wallet.
And if for version 10.x, then it should be in the Wallet model.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Yeah, it's great. Then for 11.x from the master branch, and for 10.x from the 10.x branch, I have already written files. For 11.x -- HasWallet, as you did initially. But for 10.x -- Wallet model. This is necessary for backward compatibility |
|
I will close this PR in favor of 2 new ones based of their appropriate branches. |
Working on this makes test awkward. Have to update |
|
@gkmk Then you will have to change the contract, and for this you need to change the major version. It's slightly longer and remains 99.9% backwards compatible: $user->wallet->receivedTransfers |
This adds functionality to query the database for transfers in the other direction, payments. Lookup what transfers happened from another wallet to this one.