Skip to content

Improve customer selection with website scope #147

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

Merged

Conversation

kpitn
Copy link

@kpitn kpitn commented Aug 20, 2024

With new version, module fetch all websites in gdpr_erase_entity_scheduler cron

  foreach ($this->storeManager->getWebsites() as $website) {
            if ($this->config->isErasureEnabled($website->getId())) {
                try {
                    $this->eraseEntityScheduler->schedule($this->entityTypeList->getEntityTypes(), $website);
                } catch (Exception $e) {
                    $this->logger->error($e->getMessage(), ['exception' => $e]);
                }
            }
        }

we can improve customer query by scoping with website_id

@@ -38,6 +38,7 @@ public function apply(AbstractDb $collection, WebsiteInterface $website): void
'e.entity_id=cv.customer_id',
null
);
$collection->getSelect()->where('website_id = ?', $website->getId());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use the addFieldToFilter method available on collection here:
$collection->addFieldToFilter('website_id', $website->getId());

We use 'where' at line .46 because it's a complex condition, and we use 'joinLeft' because the default 'join' method available on collection force inner joins.

@thomas-kl1
Copy link
Member

that's true @kpitn , good improvement!

@kpitn kpitn force-pushed the feat-improve-customer-selection branch from 888fb8a to e04a55b Compare August 20, 2024 15:40
@kpitn
Copy link
Author

kpitn commented Aug 20, 2024

I update the PR with your feedback

@kpitn kpitn requested a review from thomas-kl1 August 20, 2024 15:44
@thomas-kl1 thomas-kl1 merged commit ac18010 into opengento:develop Aug 20, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants