Skip to content

Update list__select.html.twig#2859

Closed
sergiostrampelli wants to merge 1 commit intosonata-project:masterfrom
sergiostrampelli:patch-2
Closed

Update list__select.html.twig#2859
sergiostrampelli wants to merge 1 commit intosonata-project:masterfrom
sergiostrampelli:patch-2

Conversation

@sergiostrampelli
Copy link

when listing we are sure that only the list action is available, all other action could have been removed

You get an error if you remove the 'edit' route and try to use the admin for a sonata_type_model_list

when listing we are sure that only the list action is available, all other action could have been removed

You get an error if you remove the 'edit' route and try to use the admin for a sonata_type_model_list
@rande
Copy link
Member

rande commented Mar 27, 2015

If is better to check if the edit action is available isGranted and hasRoute method might help.

@sergiostrampelli
Copy link
Author

This is a list action, so if the list grant is already checked.
The edit action has been checked on the referring entity.
#2935

@OskarStark
Copy link
Member

that makes sense @sergiostrampelli

ping @soullivaneuh

@soullivaneuh
Copy link
Member

IMO it's BC break cause behavior is changed. What do you think @rande?

@OskarStark
Copy link
Member

not really, because before you would got an exception, right?

i think its a bug and the previous behaviour was because of the wrong implementation...

@OskarStark
Copy link
Member

ping @sonata-project/contributors


{% block field %}
<a class="btn btn-default" href="{{ admin.generateObjectUrl('edit', object) }}">
<a class="btn btn-default" href="{{ admin.generateObjectUrl('list', object) }}">
Copy link
Member

Choose a reason for hiding this comment

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

Why are you passing the object to the route? A list route doesn't need this @sergiostrampelli

Copy link
Member

Choose a reason for hiding this comment

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

this looks correct to me and this isn't changed by @sergiostrampelli , he only changed the route itselt

Copy link
Contributor

Choose a reason for hiding this comment

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

i think that was the point of the comment. when the route is changed to the list route, the list route does not need the object.

when is this block used? the problem i see with changing this from edit to list is that we now don't link to the edit view of a specific object, but instead to the list of objects which is less convenient. can we find a better solution for the problem? is it maybe more of a documentation issue that we need to explain what to overwrite when removing/renaming the edit route?

Copy link
Member

Choose a reason for hiding this comment

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

i think that was the point of the comment. when the route is changed to the list route, the list route does not need the object.

i think you are right

@core23 core23 added the patch label Apr 15, 2016
@soullivaneuh
Copy link
Member

According to the new Sonata version management and next major release plan, this project has been refactored regarding branching and versioning.

If you see this message, your PR concerns a patch or a minor release and is not targeting the right branch.

So I'm closing this one, but don't see it as a refusal. If you think your work is still relevant and want to continue, feel free to reopen it on the right branch (e.g. the default one).

Regards.

@dbu
Copy link
Contributor

dbu commented May 27, 2016

@soullivaneuh this looks like a fix that is still relevant.

@OskarStark
Copy link
Member

@dbu but this needs to be resubmitted to 3.x or we merge this into master and cherry pick it then to 3.x

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants