symfony2admingenerator / AdmingeneratorGeneratorBundle

(old-legacy) Admingenerator for Symfony2, parse generator.yml files to build classes
http://symfony2admingenerator.org/
MIT License
360 stars 125 forks source link

Hardcoded id in getFilterForm #807

Open ndoulgeridis opened 10 years ago

ndoulgeridis commented 10 years ago

Hello,

I am migrating from Propel to Doctrine and found a potential bug in the

Admingenerator\GeneratorBundle\Resources\templates\Doctrine\ListBuilderAction.php.twig line 92.

There we have:

%- for filter in builder.filters.display -%}
            {%- if 'entity' == builder.getFieldGuesser().getDbType(model, filter) or 'collection' == builder.getFieldGuesser().getDbType(model, filter) -%}
            {% set filterModel = builder.getFieldGuesser().getModelType(model, filter) %}
              if (isset($filters['{{ filter }}'])) {
                  $this->getDoctrine()
                  ->getManagerForClass(get_class($filters['{{ filter }}']))
                  ->getUnitOfWork()
                  ->registerManaged($filters['{{ filter }}'], array('id' => $filters['{{ filter }}']->get{{ builder.getFieldGuesser().getModelPrimaryKeyName(filterModel)|capitalize }}()), array());
              }

            {%- endif %}

        {% endfor -%}

The code:

 ->registerManaged($filters['{{ filter }}'], array('id' => $filters['{{ filter }}']->get{{ builder.getFieldGuesser().getModelPrimaryKeyName(filterModel)|capitalize }}()), array());

is wrong I think as it uses hardcode array('id' => $filters What if the filters primarykey is not id but idfoo?

I suggest to change it to:

->registerManaged($filters['{{ filter }}'], array('{{ builder.getFieldGuesser().getModelPrimaryKeyName(filterModel) }}' => $filters['{{ filter }}']->get{{ builder.getFieldGuesser().getModelPrimaryKeyName(filterModel)|capitalize }}()), array());

Please let me know if this acceptable in order to fix it.

Thanks a lot.

ioleo commented 10 years ago

@crash21 hi, thanks for this report, ill look into it shortly

can you make a PR?

ndoulgeridis commented 10 years ago

Ok did it

Fix hardcoded id #808