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

Ecommerce Framework | Pass tenant config to isActive method in product #92

Open
andreas-gruenwald opened this issue Sep 3, 2019 · 4 comments

Comments

@andreas-gruenwald
Copy link
Contributor

andreas-gruenwald commented Sep 3, 2019

Feature Request

Sometimes it is necessary to access the current tenant name or the tenant configuration in the isActive() method of a product on index building.

Currently this is not possible, as the tenant context is not passed to the product.

Compare
https://github.com/pimcore/pimcore/blob/c797fed93781df7dad21873b5d56834d8e5753a0/bundles/EcommerceFrameworkBundle/IndexService/Worker/AbstractBatchProcessingWorker.php#L135

https://github.com/pimcore/pimcore/blob/877413a3d6a454f4827139460fc9bf29e4384794/bundles/EcommerceFrameworkBundle/Model/IndexableInterface.php#L52

I used a hack to access the tenant configuration:

$tenantName = Factory::getInstance()->getEnvironment()->getCurrentAssortmentTenant(); //is null in CLIs, such as the indexservice bootstrap command.
if (empty($tenantName)) {
   $traceLog = debug_backtrace();
   foreach ($traceLog as $trace) {
   $object = $trace['object'];
   if ($object instanceof IWorker) {
      $tenantName = $object->getTenantConfig()->getTenantName();
      break;
   }
}

However, it would be better to have the possibility to access the tenant, e.g. via

Factory::getInstance()->getEnvironment()->getCurrentAssortmentTenant();
```, or an additional method argument.
@fashxp
Copy link
Member

fashxp commented Feb 23, 2021

What would be the use case for that?
Why not use ìsActive` in ConfigInterface for defining, if a product is active for a certain tenant?

Challenge is, that isActive from products might also be called when there is no tenant information available, like for example here. So this was more meant for a global active flag.

@andreas-gruenwald
Copy link
Contributor Author

What would be the use case for that?
Why not use ìsActive` in ConfigInterface for defining, if a product is active for a certain tenant?

Challenge is, that isActive from products might also be called when there is no tenant information available, like for example here. So this was more meant for a global active flag.

In our case the active state depends on the workflow state associated to the related tenant.
More precisely, a product is only active for a specific tenant (and depending whether productList true/false), if it is translated. We need the tenant context to determine the translation state
in the isActive() method.

Here is a full example:

    public function isActive($inProductList = false, $tenantName = "")
    {
        if (!$this->getOSDoIndexProduct()) {
            return false;
        }

        //tenant name from environment is not always reliable -> better use worker info if available.
        //accept overhead.
        $tenantName = $tenantName ?: Factory::getInstance()->getEnvironment()->getCurrentAssortmentTenant();
        //if (empty($tenantName)) {
            $traceLog = debug_backtrace();
            foreach ($traceLog as $trace) {
                $object = $trace['object'];
                if ($object instanceof IWorker) {
                    $tenantName = $object->getTenantConfig()->getTenantName();
                    break;
                }
            }
        //}

        $localState = $this->getLocalTranslationState($tenantName, $this);

        if ($inProductList) {
            $isPublished = in_array($localState, [
                ProductWorkflowTranslationState::PUBLISHED_CHANGED,
                ProductWorkflowTranslationState::PUBLISHED
            ]);
        } else {
            $isPublished = in_array($localState, [
                ProductWorkflowTranslationState::PUBLISHED_CHANGED,
                ProductWorkflowTranslationState::PUBLISHED,
                ProductWorkflowTranslationState::TEMP_LOCKED,
                ProductWorkflowTranslationState::TEMP_LOCKED_CHANGED,
            ]);
        }

        return $isPublished;
    }

This might be a conceptual issue, but during implementation it felt that this was the right place for the code.

@fashxp
Copy link
Member

fashxp commented Feb 23, 2021

hmm, not sure if we can provide that information at this place reliable.

@andreas-gruenwald
Copy link
Contributor Author

Factory::getInstance()->getEnvironment()->getCurrentAssortmentTenant(); is also returning an empty tenant name...

@brusch brusch transferred this issue from pimcore/pimcore May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants