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] Query to retrieve order state crash #145

Closed
kpitn opened this issue Aug 20, 2024 · 4 comments
Closed

[BUG] Query to retrieve order state crash #145

kpitn opened this issue Aug 20, 2024 · 4 comments
Assignees
Labels
bug Something isn't working fixed Issue has been fixed

Comments

@kpitn
Copy link

kpitn commented Aug 20, 2024

Description

I can't run entity scheduler, when using order state.

Query executed is :

SELECT main_table.entity_id FROM sales_order AS main_table
LEFT JOIN opengento_gdpr_erase_entity AS ogee ON main_table.entity_id=ogee.entity_id AND ogee.entity_type="order" WHERE (main_table.customer_id IS NULL) AND (customer_is_guest = 1) AND (state IN('')) AND (updated_at <= '2014-08-23 07:30:14') AND (ogee.erase_id IS NULL)

Result in error : Integrity constraint violation: 1052 Column 'state' in where clause is ambiguous

I quickly made a patch for that :

--- Model/Customer/CustomerChecker.php
+++ Model/Customer/CustomerChecker.php
@@ -40,8 +40,8 @@
     public function canErase(int $customerId): bool
     {
         if (!isset($this->cache[$customerId])) {
-            $this->criteriaBuilder->addFilter(OrderInterface::STATE, $this->config->getAllowedStatesToErase(), 'nin');
-            $this->criteriaBuilder->addFilter(OrderInterface::CUSTOMER_ID, $customerId);
+            $this->criteriaBuilder->addFilter('main_table.' . OrderInterface::STATE, $this->config->getAllowedStatesToErase(), 'nin');
+            $this->criteriaBuilder->addFilter('main_table.' . OrderInterface::CUSTOMER_ID, $customerId);
             $orderList = $this->orderRepository->getList($this->criteriaBuilder->create());

             $this->cache[$customerId] = !$orderList->getTotalCount();
--- Model/Order/SourceProvider/GuestFilterModifier.php
+++ Model/Order/SourceProvider/GuestFilterModifier.php
@@ -29,8 +29,8 @@
      */
     public function apply(Collection $collection, Filter $filter): void
     {
-        $collection->addFieldToFilter(OrderInterface::CUSTOMER_ID, ['null' => true]);
-        $collection->addFieldToFilter(OrderInterface::CUSTOMER_IS_GUEST, ['eq' => 1]);
-        $collection->addFieldToFilter(OrderInterface::STATE, ['in' => $this->config->getAllowedStatesToErase()]);
+        $collection->addFieldToFilter('main_table.'.OrderInterface::CUSTOMER_ID, ['null' => true]);
+        $collection->addFieldToFilter('main_table.'.OrderInterface::CUSTOMER_IS_GUEST, ['eq' => 1]);
+        $collection->addFieldToFilter('main_table.'.OrderInterface::STATE, ['in' => $this->config->getAllowedStatesToErase()]);
     }
 }

I don't know how you want to resolv this.

@kpitn kpitn added the bug Something isn't working label Aug 20, 2024
@thomas-kl1
Copy link
Member

Hello @kpitn

Thank you for reporting this issue.
I'll take a look on this one later this week!

Regards,

@kpitn
Copy link
Author

kpitn commented Aug 20, 2024

Two others things in you recent refactoring

You forget to refactor config.xml :

                <erasure>
                    <pending>
                        <customer>
                            <enabled>1</enabled>
                            <template>gdpr_email_erase_pending_template</template>
                            <identity>general</identity>
                            <copy_method>bcc</copy_method>
                        </customer>

This result in crash when sending and email, cause identity is empty and template too

And another one, the delay variable is not send to the email template
I made a quick patch, because you declare private const CONFIG_PATH_ERASURE_DELAY = 'gdpr/erasure/delay';
And no public function exist to get delay

--- Model/Customer/Notifier/MailSender.php
+++ Model/Customer/Notifier/MailSender.php
@@ -26,7 +26,7 @@
     public function __construct(
         View $customerViewHelper,
         TransportBuilder $transportBuilder,
-        ScopeConfigInterface $scopeConfig,
+        protected ScopeConfigInterface $scopeConfig,
         StoreManagerInterface $storeManager,
         array $configPaths
     ) {
@@ -42,8 +42,11 @@
      */
     public function send(CustomerInterface $customer): void
     {
+        $delay = (int)$this->scopeConfig->getValue('gdpr/erasure/delay', \Magento\Store\Model\ScopeInterface::SCOPE_STORE, $storeId);
+
         $storeId = $customer->getStoreId() === null ? null : (int)$customer->getStoreId();
         $vars = [
+            'delay' => $delay > 0 ? $delay/60 : 0,
             'customer' => $customer,
             'store' => $this->storeManager->getStore($customer->getStoreId()),
             'customer_data' => [

Thanks for your time to maintain an open source module

@thomas-kl1
Copy link
Member

Thanks for sharing these issues!
The refactoring on develop is still ongoing, I'll make sure to push these ones next time!

@thomas-kl1
Copy link
Member

Is fixed in 5.0.0-beta1

@thomas-kl1 thomas-kl1 added fixed Issue has been fixed and removed in progress Work is in progress labels Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Issue has been fixed
Projects
None yet
Development

No branches or pull requests

2 participants