Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

toolbar name dependent on order of get_or_create_menu() calls #2697

Closed
timgraham opened this Issue Feb 21, 2014 · 7 comments

Comments

Projects
None yet
3 participants
Contributor

timgraham commented Feb 21, 2014

As documented, self.toolbar.get_or_create_menu('page') can result in the menu's verbose_name being None if that call is invoked before a call that includes the menu's name, like self.toolbar.get_or_create_menu('page', _('Page').

Contributor

timgraham commented Feb 21, 2014

(affects side and position arguments as well)

@digi604 digi604 added this to the 3.0 milestone Feb 24, 2014

@digi604 digi604 added Easy Picking blocker and removed blocker labels Feb 24, 2014

Contributor

yakky commented Mar 3, 2014

Could you check in latest develop?
#2751 should have fixed it

Contributor

timgraham commented Mar 3, 2014

I can't reproduce (although I'm not sure the failure was deterministic in the first place), but we can close this for now, thanks.

@timgraham timgraham closed this Mar 3, 2014

Contributor

timgraham commented Mar 10, 2014

It happened again on latest develop. You can see in this code that if the menu key is already in self.menus, it'll be returned as-is, ignoring the arguments of subsequent calls. So if self.toolbar.get_or_create_menu('page') runs before a call that includes the name: self.toolbar.get_or_create_menu('page', _('Page') the menu will be named None.

@timgraham timgraham reopened this Mar 10, 2014

Member

digi604 commented Mar 10, 2014

according to

https://github.com/divio/django-cms/blob/develop/cms/toolbar/items.py#L236

is verbose name required. So self.toolbar.get_or_create_menu('page') should fail anyway.... no?

Contributor

timgraham commented Mar 10, 2014

Sorry, I've linked to the wrong instance of get_or_create_menu(). I've corrected it in the comment above.

Member

digi604 commented Mar 10, 2014

Ok an easy fix would be that if the verbose_name is something we set it again on the returned menu

@digi604 digi604 added the blocker label Mar 12, 2014

@digi604 digi604 added a commit to digi604/django-cms that referenced this issue Mar 12, 2014

@digi604 digi604 fixes #2697 443c5ef

@digi604 digi604 closed this in #2884 Mar 12, 2014

@digi604 digi604 added a commit that referenced this issue Mar 12, 2014

@digi604 digi604 Merge pull request #2884 from digi604/fix-2697
fixes #2697 (out of order menu manipulations)
0163f39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment