Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] globalscopes are not applied on export #2730

Closed
5 tasks done
ctf0 opened this issue Jun 25, 2020 · 19 comments
Closed
5 tasks done

[BUG] globalscopes are not applied on export #2730

ctf0 opened this issue Jun 25, 2020 · 19 comments
Labels

Comments

@ctf0
Copy link

ctf0 commented Jun 25, 2020

Prerequisites

Versions

  • PHP version: PHP 7.4.7 (cli) (built: Jun 12 2020 00:00:24) ( NTS )
  • Laravel version: Laravel Framework 7.17.2
  • Package version: 3.1.19

Description

package doesn't respect the global-scopes on export ex.

protected static function booted()
    {
        static::addGlobalScope('latest_items', function (Builder $builder) {
            $builder->latest(
                $builder->qualifyColumn('created_at')
            );
        });
    }

query is correct ex.

select * from `requests` order by `requests`.`created_at` desc  

but the exported sheet is not sorted.

Steps to Reproduce

  • add data with different dates
  • add globalscope to ordeby
  • export the sheet via query

Expected behavior:

  • globalscopes should be respected.

Actual behavior:

  • sheet is not sorted according to globalscope.
@ctf0 ctf0 added the bug label Jun 25, 2020
@patrickbrouwers
Copy link
Member

I would need to see a bit more code, I'm not sure what "static" points to and I can't see what kind of query you use.

@ctf0
Copy link
Author

ctf0 commented Jun 25, 2020

the query is simple as Request::query() and the static is a global scope under the model, example updated

@patrickbrouwers
Copy link
Member

Can you try putting that global scope in a service provider, I'm not sure the booted callbacks gets called due to performance reasons.

@ctf0
Copy link
Author

ctf0 commented Jun 25, 2020

actually booted is the correct place https://laravel.com/docs/master/eloquent#global-scopes

@patrickbrouwers
Copy link
Member

I'm not saying it isn't. I'm trying to find out why it's not being called.

@ctf0
Copy link
Author

ctf0 commented Jun 26, 2020

yeah i understand, np i will give it a try.

note that using through a service provider will make it take longer for the fw to resolve the class and its binding b4 using it.

@ctf0
Copy link
Author

ctf0 commented Jun 30, 2020

sorry for the late reply, tried the service provider but sadly gave the same result.

@patrickbrouwers
Copy link
Member

Are you using from query and should queue?

@ctf0
Copy link
Author

ctf0 commented Jun 30, 2020

using FromQuery yes & no queuing

@ctf0
Copy link
Author

ctf0 commented Jun 30, 2020

i believe the issue is with the export function not the query, because i can get a correct sql.

@patrickbrouwers
Copy link
Member

It seems Laravel's chunk/chunkById methods don't respect the global scopes. They work when I specify the orderBy on the query itself.

You could try to reproduce without this package and see:

$first = Request::query()->first();
Request::query()->chunk(100, function($chunk) use($first) {

// Compare first results
dd($first, $chunk->first());
});

Not sure if bug in Laravel or if it's intended to work like this.

@ctf0
Copy link
Author

ctf0 commented Jun 30, 2020

i've tried to search if others had the same issue, only one ticket was made but then closed at the same day without any further details.

maybe we can try to use cursors instead ? which is better than chunking for large data sets.

@patrickbrouwers
Copy link
Member

patrickbrouwers commented Jun 30, 2020

That will be the default in 3.2 yes.

In 3.1 you can return the cursor in the collection method

@ctf0
Copy link
Author

ctf0 commented Jun 30, 2020

so this bug should be fixed in next version u mean ?

@patrickbrouwers
Copy link
Member

Yes it should work out of the box in the next version

@ctf0
Copy link
Author

ctf0 commented Jul 1, 2020

any estimation when will that be ?

@patrickbrouwers
Copy link
Member

Sorry, I don't have any ETA for it.

public function collection()
{
    return Request::query()->cursor()
}

^ this should already be enough for you I think

@ctf0
Copy link
Author

ctf0 commented Jul 1, 2020

i dont think this would work with the query setup, still thanx again, if you want u can close this ticket.

@patrickbrouwers
Copy link
Member

It would replace the query() method your have right now. Give it a go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants