Refactor the Cleanup query

[eerison]

Refactor the cleanup query

the clean up code uses this query: https://github.com/sonata-project/SonataPageBundle/blob/3.x/src/Entity/SnapshotManager.php#L228-L288, and we should refactor this to DQL query, maybe this can help: https://stackoverflow.com/questions/6637506/doing-a-where-in-subquery-in-doctrine-2

should be really good if you can test this using mysql and oracle database. but if you have problems to test oracle post the comment here.

[eerison]

This ticket is not a block for #1415 , but should be really good if we could refactor this

[eerison]

@VincentLanglet

make sense to add this into roadmap 4.0 ?

[VincentLanglet]

make sense to add this into roadmap 4.0 ?

It does not seem needed to me, since this change will be BC. So it can be done before or after the 4.0 release, can't it ?

[eerison]

it is not a BC, But I thought that we're using the roadmap to track the tickets that should be in release 4.0!

I know it's not a must, it's just a wish :P

[VincentLanglet]

it is not a BC, But I thought that we're using the roadmap to track the tickets that should be in release 4.0!

I try to track the tickets that we need for the 4.0.

I know it's not a must, it's just a wish :P

It's still a good suggestion. But not a blocker for 4.0. There is already so much to do, the question is "What are the minimal changes needed in order to release a bundle compatible with SonataAdmin4 and Symfony5". ;)

[eerison]

What are the minimal changes needed in order to release a bundle compatible with SonataAdmin4 and Symfony5

Good question :P, I'll try to check the deprecations that I got in test and see if I can create some ticket to fix them.

[Hanmac]

if it is okay i can look at the queries too

[Hanmac]

i didn't had time to debug yet, but does this look like a good replacement for the enable snapshots query?

        $qb = $this->getRepository()->createQueryBuilder('s');
        $expr = $qb->expr();
        $qb->update()
            ->set('publication_date_end', $date->format('Y-m-d H:i:s'))
            ->where($expr->notIn('id', $snapshotIds))
            ->andWhere($expr->in('page_id', $pageIds))
            ->andWhere($expr->isNull('publication_date_end'))
            ->getQuery()->execute();

for this query?

        // @todo: strange sql and low-level pdo usage: use dql or qb
        $sql = sprintf(
            "UPDATE %s SET publication_date_end = '%s' WHERE id NOT IN(%s) AND page_id IN (%s) and publication_date_end IS NULL",
            $this->getTableName(),
            $date->format('Y-m-d H:i:s'),
            implode(',', $snapshotIds),
            implode(',', $pageIds)
        );

[VincentLanglet]

Seems ok to me. Pr is welcome :)

[Hanmac]

are such Query changes 3.x or 4.x stuff?

[VincentLanglet]

It's supposed to be BC so 3.x I would say

[Hanmac]

ugh the current test cases are making problems because i seem to can't to make the query builder execute with big chunk of meta data.

i probably need to make this into a KernelTestCase and let it run against test database

[Hanmac]

first try: https://github.com/sonata-project/SonataPageBundle/pull/1446

i need more testcases for this

[VincentLanglet]

Now https://github.com/sonata-project/SonataPageBundle/pull/1446 is merged, can we close this issue @eerison ?

[eerison]

yes we can :) 🥳