Support for plugins to register URL patterns #1237

Merged
merged 9 commits into from Aug 19, 2013

5 participants

@mitar

New pull request for #738. Hopefully this time not in the time of feature freeze.

@ojii

isn't this what apphooks are for?

@mitar

No. This is for plugins. For example, I have a markup plugin for Django CMS which renders previews when user is editing the plugin content in Django CMS. So it has to have some views accessible from users. By using API in this pull request, it can publish such views under /admin/ prefix. So this is just for plugins to be used when editing them.

@digi604
Divio AG member

tests please

@fivethreeo

also should be just a classmethod

@digi604
Divio AG member

i want this for 3.0

@digi604
Divio AG member

@mitar do you want to give this an other go?

@mitar

@digi604 What do you mean?

@digi604
Divio AG member

I still like the idea... and with some tests i might include it in 3.0

@mitar

Against which branch should I do it?

@digi604
Divio AG member

develop

@mitar

Rebased to current develop branch and added tests. But tests do not really work because plugin should be registered before Django urls are constructed. In a similar way Django admin solves this (with autodiscovery). So in my code I just made sure that plugins were loaded from admin autodiscovery. I am not sure what could be a solution to this.

@digi604 digi604 and 1 other commented on an outdated diff Jul 18, 2013
cms/plugin_pool.py
@@ -112,7 +116,25 @@ def get_plugin(self, name):
"""
self.discover_plugins()
return self.plugins[name]
+
+ def get_patterns(self):
+ # We want untranslated name of the plugin for its slug so we deactivate translation
+ lang = get_language()
@digi604
Divio AG member
digi604 added a line comment Jul 18, 2013

i think you should call: self.discover_plugins() here.

@mitar
mitar added a line comment Jul 18, 2013

And how will this find plugins in tests?

@digi604
Divio AG member
digi604 added a line comment Jul 18, 2013

you can do a overwrite settings and add something to installed apps... or add the plugin in cms/plugin_utils/cli.py to the INSTALLED_APPS

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

Fixed. :-) I used my own API wrong. :-) I had get_plugin_url instead of get_plugin_urls.

@digi604
Divio AG member

could you check python 3.3 support? The tests fail for that.

@mitar

Fixed. It fails now on some other test on Django 1.5, not affected with this pull request.

@yakky

Travis still flaky... after restart all the tests passed

@mitar

Great!

@yakky yakky and 1 other commented on an outdated diff Jul 21, 2013
cms/plugin_pool.py
@@ -112,7 +115,27 @@ def get_plugin(self, name):
"""
self.discover_plugins()
return self.plugins[name]
+
+ def get_patterns(self):
+ self.discover_plugins()
+
+ # We want untranslated name of the plugin for its slug so we deactivate translation
+ lang = get_language()
+ deactivate_all()
+
+ try:
+ url_patterns = []
+ for plugin in self.get_all_plugins():
+ p = plugin()
+ slug = slugify(force_unicode(p.name))
@yakky
yakky added a line comment Jul 21, 2013

Plugin name is not guaranteed to be unique, probably the plugin class name would be a better option here?

@mitar
mitar added a line comment Jul 21, 2013

And plugin class is unique?

@mitar
mitar added a line comment Jul 21, 2013

Is there some other "verbose name" available?

@yakky
yakky added a line comment Jul 21, 2013

Plugin class name is unique in the project (it's the registration key in the plugin pool)

@mitar
mitar added a line comment Jul 21, 2013

Is there already some function which makes nice verbose name from the class name?

@yakky
yakky added a line comment Jul 21, 2013

Nope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@yakky yakky and 1 other commented on an outdated diff Jul 21, 2013
docs/extending_cms/api_references.rst
@@ -208,6 +208,13 @@ cms.plugin_base
Custom form class to be used to edit this plugin.
+ .. method:: get_plugin_urls(instance)
+
+ Returns URL patterns for which the plugin wants to register views for.
+ They are included under Django CMS admin (probably ``/admin/cms/page/``)
@yakky
yakky added a line comment Jul 21, 2013

Could you clarify a bit more here? Do you mean: They are included under django CMS PageAdmin in the plugin path (e.g.: /admin/cms/page/plugin/<plugin-name/ in the default case) ?

@mitar
mitar added a line comment Jul 21, 2013

Yes.

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

Fixed.

@yakky yakky and 1 other commented on an outdated diff Jul 21, 2013
docs/extending_cms/api_references.rst
@@ -208,6 +208,13 @@ cms.plugin_base
Custom form class to be used to edit this plugin.
+ .. method:: get_plugin_urls(instance)
+
+ Returns URL patterns for which the plugin wants to register views for.
+ They are included under Django CMS PageAdmin in the plugin path
@yakky
yakky added a line comment Jul 21, 2013

nit-picking: django CMS not Django CMS :)

@mitar
mitar added a line comment Jul 21, 2013

What? I didn't know that. But Django is Django? And django CMS is django CMS? Strange.

@mitar
mitar added a line comment Jul 21, 2013

(Fixed.)

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

Fixed.

@yakky

LGTM

@stefanfoulis stefanfoulis referenced this pull request in aldryn/aldryn-people Jul 31, 2013
Closed

use plugin urls for vcard download #3

@yakky

Sorry @mitar, it remained unmerged and a further rebase is needed against develop.
Would you do that? I'll merge it as soon as it's rebased.

@yakky yakky was assigned Aug 17, 2013
@mitar

@yakky: Done.

@yakky yakky merged commit cf643b5 into divio:develop Aug 19, 2013

1 check passed

Details default The Travis CI build passed
@yakky

@mitar thanks! it took a while but it landed at last :-)

@digi604
Divio AG member

yep thanks @mitar

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