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 and test for #588 #591

Closed
wants to merge 1 commit into from
Closed

Conversation

powderflask
Copy link

Issue #588 identifies that when a model extends an existing plugin model rather than extending CMSPlugin directly, the plugin instance is not returned correctly from get_plugin_instance()

This patch includes a simple test case to demonstrate the problem, along with a fix that repairs the problem.

However, I don't think this is the best fix for this issue. The fix came from a comment left by the original developer, however the existing code was obviously intended to take advantage of the fact that the plugin model was already loaded, thus avoiding an extra DB fetch.
I am not familiar enough with this code to find where the plugin model is being attached to the CMSPlugin object - but I suspect that is where the actual fix should be made to permanently resolve this issue.

@powderflask
Copy link
Author

I just learned that it is django that adds the property with the sub-class name.... hmmm, perhaps the base bug is back in django... I'll look into it.

@powderflask
Copy link
Author

django works to spec. - it adds the sub-class property to the direct ancestor object. So, CMSPlugin has properties: picture, text, link, etc.
In the test case submitted here, class CustomLink extends link, so the property customlink is added to the link object, but is NOT available on the CMSPlugin object.
It's there if you follow the chain - in this case:
cmsplugin_object.link.customlink
I'm just not sure how to obtain it in general... will think on it further.

@ojii
Copy link
Contributor

ojii commented Nov 29, 2010

i will close this for now.

This pull request was closed.
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.

None yet

2 participants