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

Fixed reversion in PublisherFix #595

Merged
0 commits merged into from Nov 23, 2010
Merged

Fixed reversion in PublisherFix #595

0 commits merged into from Nov 23, 2010

Conversation

fivethreeo
Copy link
Contributor

All tests now pass.

Just a few issues with the patch.

I use the page attribute of CMSPlugin in the selective_post_save_receiver. https://github.com/fivethreeo/django-cms/pull/new/publisherfix#L11R51

Passing the version pk of the revisioned Page along with obj to save_model https://github.com/fivethreeo/django-cms/pull/new/publisherfix#L0R281

@ojii
Copy link
Contributor

ojii commented Nov 22, 2010

why is there a css and html changes in a publisher fix pull request?

@ojii
Copy link
Contributor

ojii commented Nov 22, 2010

cms/utils/reversion_hacks.py lines 50-53 need refactorying. Just a plain try-except won't do here. Use the proper lookup using the ORM to see if such a query exists. Generally try...except... should be avoided.

In cms/models/pagemodel.py lines 418++, what is that try...except... there for? At least the Exception type should be limited. Same goes for 438++ and 454++, and aggain in cms/models/pluginmodel.py 254++

@fivethreeo
Copy link
Contributor Author

Fixed the mentioned issues.

All tests pass.

But I noticed a new issue with reverting pages with deleted translations , but that is for another bug/ test.

@fivethreeo
Copy link
Contributor Author

All tests pass, but subclasses of CMSPlugin are not added to Revision, needs more work.

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