Skip to content

Apphooks should be loaded on unpublished pages #2605

Open
yakky opened this Issue Feb 9, 2014 · 7 comments

4 participants

@yakky
yakky commented Feb 9, 2014

This allows editing an application hooked to a page behind the curtains, otherwise editor can't interact with application data unless page is published.
@digi604 any comment on this?

@digi604
Divio AG member
digi604 commented Feb 10, 2014

The apphook would be accessible ... but the page is not published...

@digi604 digi604 added this to the 3.0 milestone Feb 10, 2014
@yakky
yakky commented Feb 10, 2014

Doesn't look like it's working this way ATM. More detailed tests tomorrow. Assigning to me for review

@yakky yakky self-assigned this Feb 10, 2014
@digi604
Divio AG member
digi604 commented Feb 10, 2014

i know that we resolve the apphooks in the views.py... could we check if edit is true there and staff = true? Is it resolved there as well after the restart? If yes we could check permissions there as well

@yakky
yakky commented Feb 11, 2014

Showing the main view it's quite straightfoward: code in https://github.com/divio/django-cms/blob/develop/cms/views.py#L96-L120 will take care of this, provided that we change https://github.com/divio/django-cms/blob/develop/cms/views.py#L108 from:

not page.is_published(current_language) and request.toolbar.edit_mode:

to

if not page.is_published(current_language) and request.toolbar.edit_mode and not request.user.is_staff:

"Deeper" views will remain unaccessible because urls are loaded statically from the public version of the page (see #2480 (comment)).

A different approach might be feasible but postponing to 3.1 it's a better option.
I'll open a different issue for this

@yakky yakky modified the milestone: 3.X, 3.0 Mar 2, 2014
@marcinn
marcinn commented Jan 27, 2016

These changes will improve faulty apphooks design:

  1. apphooks should be always loaded, when server starts, even if no apphook is bound to any page (published or not)
  2. there may be an table with enabled apphooks to dynamically enable/disable selected (see how Trac plugins works, just get this idea and implement); after enabling/disabling server should be restarted, but not after adding or removing a page (!); this is not same as removing app from INSTALLED_APPS
  3. when page_url can't resolve page by slug, a "empty" link should be returned, because page_url is related to dynamic content, but may be used in a static area (i.e. layout of the page); this should work same as no variable defined in template until you enable strict template debug in your settings.

Thanks.

@yakky
yakky commented Jan 27, 2016

The idea of apphooks is to plug directly in django urlconf to be a zero-overhead solution to dynamic application hookin
Thus you can't load an apphook, if you don't know in which part of the tree it should attached to.
That's the reason for the reload when publishing the page (this is anyhow solved in 3.2 thanks to the reload middleware)
I can't understand what you mean at point 3

@marcinn
marcinn commented Jan 27, 2016

you can't load an apphook, if you don't know in which part of the tree it should attached to

Maybe this a casue of faulty design (in the context of url resolvers).

For apphooks to work they have to be attached to a page: they are designed to be attached to a page. Class is loaded at runtime, but they are loaded in the urlconf only after they are attached to a page (how could you include the apphook urls in the global urlconf otherwise?)

this is anyhow solved in 3.2 thanks to the reload middleware

No, this is not a solution. System without apphook loaded still throws an exception. Apphook can't be loaded, because there is no published page with that app. The design should be changed instead of implementing workarounds.

Reloading is not a workaround: it's how Django urlconf works :)

can't understand what you mean at point 3

Base template may contain {% page_url %} tags with slugs set to undefined/unpublished/non-existent pages. Whole app goes down by raising an exception. Have you tried to add a page using in-page editor when exception was raised and traceback is shown?

{% page_url %} is designed to fail, but if you use the {% page_url .. as .. %} syntax, that will fail silently with no traceback (but this is not related to apphooks)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.