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

Fix for re-ordering plugins when JS provides language as empty string #3194

Merged
merged 3 commits into from
May 26, 2014

Conversation

damianmoore
Copy link
Contributor

I was having a problem where I couldn't re-order plugins within a placeholder, similar to this issue: #3058

This caused an error to popup on move "order parameter did not have all plugins of the same level in it" and then the move plugin would be removed.

I found that the move_plugin() method of admin/placeholderadmin.py was not finding the other plugins correctly as it was searching for a language of '' (empty string) rather than the language that the plugins had ('en-gb'). This is because the JavaScript POST included a plugin_language of empty string.

As far as I can see, this JS variable comes from a GET parameter of the request so is not reliably there. Falling back to the language of the plugin seems a sensible fix to me.

Maybe there is some extra configuration would otherwise solve this, but I can't find it.

@johnraz
Copy link
Contributor

johnraz commented May 24, 2014

Just being curious what did you set CMS_LANGUAGE settings to?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.17%) when pulling 3889b11 on damianmoore:fix_for_plugin_ordering into 2d3e10a on divio:develop.

@damianmoore
Copy link
Contributor Author

@johnraz I don't have CMS_LANGUAGES set in my settings as it's not a multi-lingual site. The variable is not mentioned here so I assume it's not required: http://docs.django-cms.org/en/latest/getting_started/installation/integrate.html#configuration-and-setup . I do have LANGUAGE_CODE = 'en-gb', though and this seems to be what the plugin instances languages are being set to.

@johnraz
Copy link
Contributor

johnraz commented May 24, 2014

Ok same thing happen to me without CMS_LANGUAGES... I guess there may be a relation :-)

@johnraz
Copy link
Contributor

johnraz commented May 24, 2014

@yakky : side note regarding the travis build, same issue as on my other PR ... seems something is broken ❌

@yakky
Copy link
Member

yakky commented May 24, 2014

Please rebase against develop

@damianmoore
Copy link
Contributor Author

@yakky rebased

@coveralls
Copy link

coveralls commented May 24, 2014

Coverage Status

Coverage decreased (-0.09%) to 88.022% when pulling 7b924de on damianmoore:fix_for_plugin_ordering into 2d3e10a on divio:develop.

@yakky
Copy link
Member

yakky commented May 26, 2014

Thanks

yakky added a commit that referenced this pull request May 26, 2014
Fix for re-ordering plugins when JS provides language as empty string
@yakky yakky merged commit 39a9110 into django-cms:develop May 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants