search

bavix / laravel-wallet

It's easy to work with a virtual wallet
https://bavix.github.io/laravel-wallet/
MIT License
1.12k stars 224 forks source link

Add payments relation on wallet #874

Closed gkmk closed 7 months ago

gkmk commented 7 months ago

This adds functionality to query the database for transfers in the other direction, payments. Lookup what transfers happened from another wallet to this one.

codecov[bot] commented 7 months ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (387a348) 100.00% compared to head (cedac1c) 99.75%.

Files Patch % Lines
src/Models/Wallet.php 0.00% 5 Missing :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gkmk commented 7 months ago

Ill add tests if this is accepted

rez1dent3 commented 7 months ago

Ill add tests if this is accepted

okay.

Please specify for which version the change is needed? 10.x or 11.x

rez1dent3 commented 7 months ago

@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.

UPD: The bottom line is that you cannot make changes that break backward compatibility. And version 11.x has not yet been released.

gkmk commented 7 months ago

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: @.***>

rez1dent3 commented 7 months ago

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

gkmk commented 7 months ago

I will close this PR in favor of 2 new ones based of their appropriate branches.

gkmk commented 7 months ago

But for 10.x -- Wallet model. This is necessary for backward compatibility

Working on this makes test awkward. Have to update tests/Infra/Models/Buyer.php with the same function from the Wallet model now. But since the Wallet model uses src/Traits/HasWallet.php why are we not updating this trait and the model instead in v10?

rez1dent3 commented 7 months ago

@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