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

Call processInputs when available, fixing WrongType #184

Merged

Conversation

mauritsvanrees
Copy link
Member

This fixes issue #183.
I added a test that saves the advanced control panel. And I tested it manually in the browser. Tested in Plone 5.2 on Py 2 and 3, and on Plone 6. All seems well.

This should help surface some `WrongType`  exceptions.
The field is SourceText, which is unicode on Python 2, and str on Python 3.
So on Python 2, creating a string of it results in a WrongType error.
See #183
This reverts commit d6db78d.

No longer needed after calling processInputs.
Copy link
Member

@ale-rt ale-rt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, anyway removing the hostname_blacklist might be a breaking change.
What about keeping it as deprecated?

@mauritsvanrees
Copy link
Member Author

Not needed: hostname_blacklist was never in a release.

For clarity, the field with this name is still there. But the method that I remove here was only a helper method added in PR #178, one of the WrongType issues.

Copy link
Member

@ale-rt ale-rt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Did not notice it :)

Otherwise when you upgrade from Plone 5.2.1, and try to save the theming control panel, you get an error:

```
Traceback (innermost last):
  Module ZPublisher.WSGIPublisher, line 162, in transaction_pubevents
  Module ZPublisher.WSGIPublisher, line 359, in publish_module
  Module ZPublisher.WSGIPublisher, line 262, in publish
  Module ZPublisher.mapply, line 85, in mapply
  Module ZPublisher.WSGIPublisher, line 63, in call_object
  Module plone.app.theming.browser.controlpanel, line 65, in __call__
  Module plone.app.theming.browser.controlpanel, line 215, in update
  Module plone.registry.recordsproxy, line 59, in __setattr__
AttributeError: custom_css_timestamp
```
@mauritsvanrees
Copy link
Member Author

@jenkins-plone-org please run jobs

@mauritsvanrees
Copy link
Member Author

Hurray, Jenkins is running again, and the tests are passing on all versions.

@ale-rt
Copy link
Member

ale-rt commented Jun 11, 2020

Good to be merged for me.

@mauritsvanrees mauritsvanrees merged commit fdbb576 into master Jun 11, 2020
@mauritsvanrees mauritsvanrees deleted the maurits/issue-183-save-advanced-controlpanel-wrongtype branch June 11, 2020 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants