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

CMSAttachMenu failing to register after 3.0.12 #3936

Closed
mrhanlon opened this issue Mar 10, 2015 · 32 comments
Closed

CMSAttachMenu failing to register after 3.0.12 #3936

mrhanlon opened this issue Mar 10, 2015 · 32 comments

Comments

@mrhanlon
Copy link

I noticed that after upgrading from 3.0.10 to 3.0.12 that a CMSAttachMenu was not registering on a page. It would periodically appear and disappear. In the advanced settings sometimes I would see zero, one, or two options for the same menu. I only have one CMSAttachMenu menu defined in a single app in my project. I attempted to delete and recreate the page to no avail. I noticed in the database, if there was only one option available in advanced settings, the page record showed UserMenu for navigation_extenders. If there were two options, it would be UserMenu:190 or UserMenu:193 depending which option I selected.

Ultimately I reverted to 3.0.10 and the issue went away.

My menu class:

from menus.base import NavigationNode
from cms.menu_bases import CMSAttachMenu
from menus.menu_pool import menu_pool
from django.utils.translation import ugettext_lazy as _

class UserMenu(CMSAttachMenu):

    name = _('User Menu')

    def get_nodes(self, request):
        nodes = []

        menu_id = 1
        root_id = menu_id

        # root node
        greeting = 'Hello, {0}'.format(request.user.get_full_name() or request.user) if request.user.is_authenticated() else 'Log in'
        title = '<i class="fa fa-user hidden-xs hidden-md hidden-lg"></i><span class="hidden-sm">{0}</span>'
        n = NavigationNode(title.format(greeting), "/user/", menu_id)
        nodes.append(n)

        # anonymous menus
        menu_id += 1
        n = NavigationNode(_('Log in'), "/login/", menu_id, root_id, attr={'visible_for_authenticated':False})
        nodes.append(n)

        menu_id += 1
        n = NavigationNode(_('Register'), "/register/", menu_id, root_id, attr={'visible_for_authenticated':False})
        nodes.append(n)

        menu_id += 1
        n = NavigationNode(_('Help'), "/help/ticket/new/guest/", menu_id, root_id, attr={'visible_for_authenticated':False})
        nodes.append(n)

        # authenticated menus
        menu_id += 1
        dashboard_id = menu_id
        n = NavigationNode(_('Dashboard'), "/user/dashboard/", menu_id, root_id, attr={'visible_for_anonymous':False})
        nodes.append(n)

        menu_id += 1
        n = NavigationNode(_('Log out'), "/logout/", menu_id, root_id, attr={'visible_for_anonymous':False})
        nodes.append(n)

        # user section dashboard sub-menu
        menu_id += 1
        n = NavigationNode(_('Dashboard'), "/user/dashboard/", menu_id, dashboard_id, attr={'visible_for_anonymous':False})
        nodes.append(n)

        menu_id += 1
        n = NavigationNode(_('Projects'), "/user/projects/", menu_id, dashboard_id, attr={'visible_for_anonymous':False})
        nodes.append(n)

        menu_id += 1
        n = NavigationNode(_('Help Desk'), "/user/help/", menu_id, dashboard_id, attr={'visible_for_anonymous':False})
        nodes.append(n)

        menu_id += 1
        n = NavigationNode(_('Profile'), "/user/profile/", menu_id, dashboard_id, attr={'visible_for_anonymous':False})
        nodes.append(n)

        return nodes

menu_pool.register_menu(UserMenu)
@yakky
Copy link
Member

yakky commented Mar 15, 2015

@mrhanlon could you give #3950 a try?

@mrhanlon
Copy link
Author

@yakky It looks like this fixed it for me. I had to edit the page to attach the menu again, but it is working as expected. Thanks!

@yakky
Copy link
Member

yakky commented Mar 18, 2015

@mrhanlon thanks for testing. Yes menu might need to be attached again as a spurious menu might have been attached before

@mbi
Copy link
Contributor

mbi commented Mar 24, 2015

FWIW I also stumbled unto this issue and can confirm #3950 fixes it.

@mkoistinen
Copy link
Contributor

Thanks, @yakky for the fix and all those reporting!

@mbi
Copy link
Contributor

mbi commented Apr 16, 2015

I'm still seeing this in 3.0.13

@mbi
Copy link
Contributor

mbi commented Apr 22, 2015

@yakky ping?

I'm still seeing all CMSAttachMenu instances being suggested twice in the admin (and neither is working when attached to the page). This is happening both in 3.0.13 and 3.1.0. The latest version that doesn't seem to have this problem is 3.0.10

Am I doing something wrong?

screen shot 2015-04-22 at 11 00 55

@yakky
Copy link
Member

yakky commented Apr 22, 2015

Have you wiped out local cache and updated aldryn-apphooks-config too?

@mbi
Copy link
Contributor

mbi commented Apr 22, 2015

I had (and just have again) cleared the local cache.

I'm not using Apphook namespaces (my apps are attached only once), nor aldryn-apphooks-config or any other namespace configuration mechanism. Am I supposed to? The documentation doesn't seem to imply this is required.

@mbi
Copy link
Contributor

mbi commented Apr 23, 2015

@yakky ping? Should this be reopened?

@vstoykov
Copy link
Contributor

I think yes, because we also experience this problem with 3.0.13.

First I need some clarification about how CMSAttachMenu should actually work if it is registered alone or in AppHook or in AppHook attached multiple times.

  • If it is simple CMSAttachMenu it will show in a dropdown for every page (Which is normal)
  • If it is registered and attached to page by AppHook it will show n-times, where n is number of pages to which AppHook is attached (and because every page has draft and public version it shows actually n*2 after page publication).

I'm not sure if it needs to show at all in the dropdown when the menu is attached to page by AppHook, and to allow user to attach it twice by using one of it's instances.

Also when AppHook with menu is attached to a page then menu elements doesn't work as expected when using show_sub_menu. Sometimes elements are shown, some times not (switching between live and draft version of a page). With show_menu and showing the whole tree everything is ok.

UPDATE:
Now I saw that when show_submenu do not show correctly attached elements then in the "main menu" element is not marked as selected. (Current URL is virtual page handled by the apphook)

@vstoykov
Copy link
Contributor

What I've discovered is that when MenuPool._mark_selected is called this is before NavExtender modify the tree (before setting correct parent for every attached node and before removing all useless nodes).
The result is that selected node is removed from the tree (in most cases when visiting draft version of the page) and show_submenu can't display any nodes because there is no selected page and there are no ancestors.

@mbi Is your problem related only to show_submenu or to show_menu also?

@mbi
Copy link
Contributor

mbi commented Apr 27, 2015

@vstoykov I'm only using show_menu on this particular project.

@mkoistinen
Copy link
Contributor

mkoistinen commented Apr 27, 2015 via email

@mbi
Copy link
Contributor

mbi commented Apr 27, 2015

@mkoistinen With that branch the menus seem to get rendered correctly (yay!) but I'm still getting duplicate entries in the "attached menu" dropdown in the page's advanced settings.

@vstoykov
Copy link
Contributor

@mkoistinen for me it looks also ok.

About double rendered menus I think that the option for attaching menu when AppHook is attached need to be disabled in order people to not be confused.
Also page.navigation_extenders to be cleared because CMSAttachedMenu.get_instances will get correct pages through apphook and to prevent accidentally attaching the same menu again (which can double the menu items).

@dzonder
Copy link

dzonder commented May 6, 2015

I still see issues with app menus in 3.1.

Lets assume app is available as page at /shop/ and app menus contains available products.
When accessing urls which are not in app menus (i.e. /shop/addresses/, /shop/addresses/add/) everything works fine and menu is visible.
But when accessing a product (i.e. /shop/product/pslug1/) app menus disappear.

3.0.10 worked fine but I needed Django>=1.7 support.

Is anyone still working on this?

@vstoykov
Copy link
Contributor

vstoykov commented May 9, 2015

@dzonder changes made by @mrhanlon are note merged yet and issue persist in current released versions. You can try the branch mentioned by him and tell if the problem is fixed there for you.

Until issue is resolved you can try a workaround by putting this in menu.py in some of your applications:

from menus.menu_pool import menu_pool
from menus.modifiers import Marker

class FixNodesTree(Marker):
    """
    The purpose of this modifier is to workaround Django CMS bug
    """
    post_cut = False

    def modify(self, request, nodes, namespace, root_id, post_cut, breadcrumb):
        if not post_cut:
            menu_pool._mark_selected(request, nodes)
            return super(FixNodesTree, self).modify(request, nodes, namespace, root_id, post_cut, breadcrumb)
        return nodes

menu_pool.register_modifier(FixNodesTree)

@bramstoeller
Copy link

Yep, same here, in version 3.1.0. I see the double entries in the advances settings, and the menu's don't show up if the actual content is below a different page as the menu.

Strangely enough it does seem to work quite fine, unpublished, published and even when I log out. But as soon as I reload apache (or restart the debug server) the menu's disappear.

@vstoykov Thanks for your suggestion, but it doesn't seem to have any effect on the behavior of my menu's...

@mkoistinen
Copy link
Contributor

Awesome feedback guys. To be clear, most of the issue has been solved in the above-mentioned PR for the 3.0.x branch. What remains is more of an ugly cosmetic issue with the Advanced Page Settings. I'll try to fix this asap.

Once these are committed to the Support 3.0.x branch, we'll port it up to 3.1 too.

@bramstoeller
Copy link

Hi all, what is the status of this bug?

I'm developing a website that's supposed to go live on the 1st of June. It's almost finished except for the menu's.

They organize workshops, courses and trainings, so I made an application to manage these. They all share the same models, but they are divided in three categories. The urls of the courses are: /some-fixed-name/[category_slug]/[slug]. I created three menus that filter on the category: WorkshopMenu, CourseMenu, TrainingMenu. These should show up under the pages /workshops/, /courses/, /trainings/.

When I assign a menu to a page, it shows up properly and all works as expected, until I restart the webserver. Than they just disappear and the page admin shows that no menu is selected. It shows the same behaviour in development mode (manage.py runserver) and in live mode (service apache2 reload).

I don't really know how to proceed, all URLs are correct and unique and I don't seem to get any errors.

Any thoughts on this?

@jsma
Copy link
Contributor

jsma commented May 26, 2015

@mkoistinen Anything we can do to help get this to the finish line? Is the only thing left to fix the fact that the menus are shown multiple times in the admin?

@mkoistinen
Copy link
Contributor

Sorry all, I've been particularly consumed with personal things lately (like moving to another country in less than 1 week). However, I am going to try to focus on this again starting.... now! =)

@jsma
Copy link
Contributor

jsma commented May 26, 2015

Thanks @mkoistinen ! About to move 3,000 miles myself, I feel your pain. I'll watch the PR #4042 and let us know there if you need any help testing etc.

@mkoistinen
Copy link
Contributor

Anyone care to test the updated version of this branch: https://github.com/divio/django-cms/tree/issues/fix_breadcrumbs

@jsma
Copy link
Contributor

jsma commented May 26, 2015

Will be able to in a couple hours, thanks!

@yakky yakky reopened this Jun 5, 2015
@yakky
Copy link
Member

yakky commented Jun 5, 2015

reopening this as it's not fixed yet

@yakky
Copy link
Member

yakky commented Jun 7, 2015

This should have been fixed in #4042
@jsma @AStone @vstoykov @mbi @mrhanlon @dzonder could you please give latest support/3.0.x branch a try?

@vstoykov
Copy link
Contributor

vstoykov commented Jun 7, 2015

I will test these changes tomorrow at work and will told you if my problems are gone.

@vstoykov
Copy link
Contributor

vstoykov commented Jun 8, 2015

With current support/3.0.x branch a can't see the problem. For me it looks ok.

@yakky
Copy link
Member

yakky commented Jun 8, 2015

Thanks @vstoykov !

@yakky
Copy link
Member

yakky commented Jul 3, 2015

Fixed in #4042

@yakky yakky closed this as completed Jul 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants