bavix / laravel-wallet

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

How to disable auto create wallet behavior ? #990

Closed ISNewton closed 2 months ago

ISNewton commented 3 months ago

Describe your task I have a delete user wallet method :

    public function delete(Request $request, User $supervisor)
    {
        $wallet = $supervisor->getWallet(User::getSupervisorWalletKey($request->institution->id));

        if ($wallet) {
            $wallet->delete();
        }

        return apiSuccess();
    }

It just deletes the user wallet by slug (not the default).

After the deletion succeed (I checked the db and the wallet is gone), when I query all users with their wallets , I get the same wallet I deleted before .

    public function index(Request $request)
    {
        $supervisors = User::query()
            ->select('id', 'name')
            ->with(['wallets' => function ($query) {
                $query->select('id', 'slug', 'uuid', 'holder_id', 'holder_type', 'balance', 'meta');
            }])
            ->whereHas('roles', function ($query) {
                $query->whereIn('system_roles.id', collect(SYSTEM_ROLES)->only('institution_manager')->toArray());
            })
            ->whereHas('wallets', function (Builder $query) use ($request) {
                $query->where('slug', User::getSupervisorWalletKey($request->institution->id));
            })
            ->get();

        return apiSuccess(ManagerPointsResource::collection($supervisors));
    }

Expected behavior I don't want the wallet to be auto created

Server: php version: 8.3 database: mysql 8.0 wallet version 7.3.6

Additional context Add any other context about the problem here.

rez1dent3 commented 3 months ago

Hello. In your code, most likely, automatic creation occurs when calling $user->balance inside apiSuccess. Remove HasWallet from the user model, you are working with multi-wallets.

P.S. Update the package to 10.x. Automatic wallet creation is disabled there.

ISNewton commented 3 months ago

Thanks @rez1dent3 .

usmanjdn7 commented 2 months ago

@rez1dent3 that same issue happend on balance when i call auth()->user()->balanceFloatNum its happend very rearly. i double check the request logs and request came once at a time.

laravel version 10.x php version 8.1 wallet version 10.1 mysql version 5.7.42

rez1dent3 commented 2 months ago

@usmanjdn7 Hello. This means you are saving or performing some actions on the wallet. Starting with 10.x, this problem cannot happen, since the wallet is not created.

usmanjdn7 commented 2 months ago

@rez1dent3 only call to get wallet balance, deposit and withdraw Api's are disabled.

image
rez1dent3 commented 2 months ago

@usmanjdn7 I double checked. Everything works correctly, the wallet is not saved. https://github.com/bavix/laravel-wallet/blob/2a7f315eb13a069228d24366a62e5605931c5524/tests/Units/Domain/BalanceTest.php#L174-L183

Please make a demo project with the problem. I will tell you what went wrong.

usmanjdn7 commented 2 months ago

i can't at the moment... below is the error report. it create the wallet on calling this method

image
rez1dent3 commented 2 months ago

@usmanjdn7 I have provided a test above that runs successfully on sqlite, mysql, postgres. The wallet is not created on laravel-wallet versions 10.0+.

On versions <10.0, your error will be exactly that. Perhaps you have redefined some of the package services and even after the update you may have this problem, but I can't help you here.

Unfortunately, I can't reproduce your problem. I need details.

usmanjdn7 commented 2 months ago

hi @rez1dent3 i just dig into the balanceFloatNum code, in getBalanceFloatAttribute accessor you call the getWallet without giving the $save bool which create the wallet because the default value is set to true. see the below screenshots.

Screenshot 2024-09-10 121124

Screenshot 2024-09-10 121212

rez1dent3 commented 2 months ago

@usmanjdn7 You're right. I'll check and fix it. It's strange that the tests pass. I need to fix the tests.

rez1dent3 commented 2 months ago

@usmanjdn7 The error was in the test I provided above. It's laravel magic... I was apparently tired and even null didn't bother me.

-     self::assertNull($buyer->balanceFloatNum); 
+     self::assertSame(0.0, $buyer->wallet->balanceFloatNum); 

Now the test covers all possible balances.

I probably rushed my answer and made a mistake and I apologize.


@ISNewton @usmanjdn7 Many thanks for the feedback guys. The edits were made in version 10.x and 11.x. New tags:

Thanks again)

usmanjdn7 commented 2 months ago

@rez1dent3 Many thanks to you for such good. :smiling_face_with_three_hearts: