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

Hardcoded id in getFilterForm #807

Open
ndoulgeridis opened this issue Nov 7, 2014 · 2 comments
Open

Hardcoded id in getFilterForm #807

ndoulgeridis opened this issue Nov 7, 2014 · 2 comments

Comments

@ndoulgeridis
Copy link
Contributor

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
Copy link
Member

ioleo commented Nov 7, 2014

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

can you make a PR?

@ndoulgeridis
Copy link
Contributor Author

Ok did it

Fix hardcoded id #808

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

No branches or pull requests

2 participants