Sane add plugin

[ojii]

Historically, when a user wanted to add a plugin to a placeholder, upon selecting the plugin, the CMS would create a plugin instance in the database, then show the user the add form. In various cases where the user would not complete the process, "ghost plugins" would be created. These have always been somewhere between a nuisance and an actual problem, sometimes going as far as crashing an site.

This pull request attempts to solve this by not creating a plugin until after the user hits save on the add form.

This changes the contract for CMSPluginBase subclasses and brings them more in line with it's parent, ModelAdmin. Specifically has_add_permission now needs to be properly implemented, instead of just using has_change_permission, get_form actually has to respect the obj parameter (which can be None) and add_view will actually be called. Further, a add_view_check_request helper function was added to CMSPluginBase which verifies a HttpRequest to add_view has the required GET parameters. This may be used by subclasses wishing to modify add_view.

This pull request breaks at least djangocms-text-ckeditor and djangocms-link, both of which have pull requests to make them compatible (see django-cms/djangocms-text-ckeditor#179 and django-cms/djangocms-link#29).

This will break any plugin that assumes an object to be around in get_form or add_view either by assuming self.cms_plugin_instance to be available or in the case of get_form that the obj argument is not None.

Todo

• Write code
• Frontend integration
• Make tests pass
• Testing frontend integration

[ojii]

@FinalAngel could you have a look at this, especially ojii@256b691? (note: I fixed the spaces/tabs in the next commit)

[FinalAngel]

@ojii looks sane coding wise, would also speed up loading time, are you sure that plugin_parent is not required anymore?

[ojii]

Yep plugin_parent is optional: https://github.com/divio/django-cms/pull/3516/files#diff-51463da400f2283be98f002910bacac4R221

[ojii]

Test on here will fail until django-cms/djangocms-text-ckeditor#179 and django-cms/djangocms-link#29 get merged...

[ojii]

@yakky @mkoistinen looks like the failures are unrelated to code (within selenium failing to connect to the remote webdriver), any clue how to fix this? Just keep hitting "restart" until it works?

[yakky]

All failures are related to custom user models. What happens locally when using it?

[ojii]

The problem is SELENIUM=1 not the user model, see for example https://travis-ci.org/divio/django-cms/jobs/47177191 it's failing to authenticate with saucelabs. @digi604 it uses your account, any idea why this is failing?

[yakky]

@ojii can't we just switch to --xvfb?

[yakky]

I cannot get the selenium tests to run locally
On Django 1.7 it cannot load the static files (which works when using server argument to develop.py``)
On Django 1.6 it fails with javascript errors:

======================================================================
ERROR: test_add_plugin_in_text_plugin (cms.tests.frontend.AddPluginTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/srv/repo/django-cms-3.0/cms/tests/frontend.py", line 622, in test_add_plugin_in_text_plugin
    self.wait_page_loaded()
File "/srv/repo/django-cms-3.0/cms/tests/frontend.py", line 150, in wait_page_loaded
    self.wait_loaded_tag('body')
File "/srv/repo/django-cms-3.0/cms/tests/frontend.py", line 141, in wait_loaded_tag
    timeout
File "/srv/repo/django-cms-3.0/cms/tests/frontend.py", line 132, in wait_until
    WebDriverWait(self.driver, timeout).until(callback)
File "/var/local/venvs/cms-3.1/local/lib/python2.7/site-packages/selenium/webdriver/support/wait.py", line 63, in until
    value = method(self._driver)
File "/srv/repo/django-cms-3.0/cms/tests/frontend.py", line 140, in <lambda>
    lambda driver: driver.find_element_by_tag_name(tag_name),
File "/var/local/venvs/cms-3.1/local/lib/python2.7/site-packages/selenium/webdriver/remote/webdriver.py", line 326, in find_element_by_tag_name
    return self.find_element(by=By.TAG_NAME, value=name)
File "/var/local/venvs/cms-3.1/local/lib/python2.7/site-packages/selenium/webdriver/remote/webdriver.py", line 662, in find_element
    {'using': by, 'value': value})['value']
File "/var/local/venvs/cms-3.1/local/lib/python2.7/site-packages/selenium/webdriver/remote/webdriver.py", line 173, in execute
    self.error_handler.check_response(response)
File "/var/local/venvs/cms-3.1/local/lib/python2.7/site-packages/selenium/webdriver/remote/errorhandler.py", line 166, in check_response
    raise exception_class(message, screen, stacktrace)
WebDriverException: Message: [JavaScript Error: "e is null" {file: "file:///tmp/tmpoAVcV9/extensions/fxdriver@googlecode.com/components/command-processor.js" line: 7854}]'[JavaScript Error: "e is null" {file: "file:///tmp/tmpoAVcV9/extensions/fxdriver@googlecode.com/components/command-processor.js" line: 7854}]' when calling method: [nsICommandProcessor::execute]

[yakky]

In other pull requests tests run fine by using the same travis configuration.
I can't get what's wrong here...

[ojii]

Okay I give up. I'm happy to fix any issues with the code but simply give up getting this to work on travis. If anyone wants to take over this, that would be awesome.

[coveralls]

Coverage decreased (-0.3%) to 90.318% when pulling 0a82783 on ojii:sane-add-plugin into 5378787 on divio:develop.

[ojii]

closing in favor of #3804