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

Support class names whose name begins with lowercase letter #1896

Conversation

BlackbitDevs
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no

Steps to reproduce bug:

  1. Create class with lowercase first letter, e.g. product, extend \CoreShop\Component\Core\Model\Product
  2. Set up the custom class configuration like described in https://docs.coreshop.org/2.2.0/Development/Extending_Guide/Extend_CoreShop_DataObjects.html:
    core_shop_product:
        pimcore:
            product:
                classes:
                    model: 'Pimcore\Model\DataObject\Product'
  3. Go to "cart price rules", add new rule, add Products condition, click "Search" -> you see that the defined Product class is not preselected.

The other fixed thing is when you override a data object class and set core_shop_product.pimcore.product.classes.model to the overridden FQCN (e.g. \App\Model\DataObject\Product), then

$class = str_replace('Pimcore\\Model\\DataObject\\', '', $fullClassName);
does not have any effect.

@dpfaffenbauer
Copy link
Member

@BlackbitNeueMedien I fixed tests for 2.2, can you rebase to latest 2.2?

@BlackbitDevs
Copy link
Contributor Author

Have I missed something? I merged up-to-date 2.2 branch but it complains about missing database columns.

@dpfaffenbauer dpfaffenbauer modified the milestones: 2.2.12, 2.2.13 May 3, 2022

$classStackPimcoreClassName[$alias][] = $class;
try {
$reflectionClass = new ReflectionClass($definition['classes']['model']);
Copy link
Member

Choose a reason for hiding this comment

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

@BlackbitNeueMedien this can be tricky, the class could not exist yet, since they either weren't installed yet or not created yet.

@BlackbitDevs
Copy link
Contributor Author

The problem still exists, also in 3.0 - do you know why you closed it, @dpfaffenbauer ? Because #1896 (review) can be easily fixed.

@dpfaffenbauer
Copy link
Member

This is actually quite difficult to solve. Problem is that during Container build time the ClassDefinitions cannot be loaded cause some of them require the kernel to be booted. So to fix this, I load the tokens from the definition file and try to find the classname, feels a bit dirty to me honestly, you might wanna check: #2097

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

Successfully merging this pull request may close these issues.

2 participants