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

Various fixes #35

Merged
merged 10 commits into from
Sep 12, 2024
Merged

Various fixes #35

merged 10 commits into from
Sep 12, 2024

Conversation

mauritsvanrees
Copy link
Member

I was testing plone/plone.classicui#13 and this led to some improvements. See individual commit messages for details, but largely it is this:

  • Do not export or import translations when plone.app.multilingual is not available. I got an error initially.
  • Do not export or import discussions/comments when plone.app.discussion is not available.
  • Add a fixer for the allow_discussion key: this should only contain True or False when this is explicitly set on the object.
  • Blacklisted portlets were not imported when there was no accompanying change in the actual portlet list. Practically speaking: the distribution blacklists some portlets in the Members folder, but this was ignored on import.
  • Export adds a newline at the end of all files. This matches the .editorconfig settings that we have in most Plone packages.

I have used this code to import and export a Classic UI site with the mentioned plone.classicui pull request, and have updated it.

BTW, is it too late in the portlets.json to rename blacklist_status to something like disallowlist_status? I don't feel strongly about this, but using "black" in this context is frowned upon. For backwards compatibility we could still read the old name as well.

…l is not installed.

The distribution may have a `translations.json` with only an empty string.  That should not break like this:

```
Traceback (innermost last):
  Module ZPublisher.WSGIPublisher, line 181, in transaction_pubevents
  Module ZPublisher.WSGIPublisher, line 391, in publish_module
  Module ZPublisher.WSGIPublisher, line 285, in publish
  Module ZPublisher.mapply, line 98, in mapply
  Module ZPublisher.WSGIPublisher, line 68, in call_object
  Module plone.rest.service, line 21, in __call__
  Module plone.restapi.services, line 19, in render
  Module plone.distribution.services.sites.add, line 49, in reply
  Module plone.distribution.api.site, line 165, in create
  Module plone.distribution.api.site, line 104, in _create_site
  Module plone.distribution.handler, line 44, in default_handler
  Module plone.exportimport.importers, line 57, in import_site
  Module plone.exportimport.importers.base, line 70, in import_data
  Module plone.exportimport.importers.translations, line 17, in do_import
TypeError: object of type 'NoneType' has no len()
```
…onally.

Only register them when `plone.app.multilingual` and `plone.app.discussion` are installed.
Prevent error when an exporter does not actually export any paths, but returns None, like before I made these two conditional:

```
$ bin/export-distribution parts/zeoclient-classic/etc/zope.conf Plone
Exporting Plone site at /Plone
 Target path: /Users/maurits/community/plone-coredev/6.1/src/plone.classicui/src/plone/classicui/distributions/classic/content
Traceback (most recent call last):
  File "/Users/maurits/community/plone-coredev/6.1/bin/export-distribution", line 250, in <module>
    sys.exit(plone.distribution.cli.export())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/maurits/community/plone-coredev/6.1/src/plone.distribution/src/plone/distribution/cli/__init__.py", line 44, in export
    results = get_exporter(site).export_site(path)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/maurits/community/plone-coredev/6.1/src/plone.exportimport/src/plone/exportimport/exporters/__init__.py", line 72, in export_site
    paths.extend(exporter.export_data(path))
TypeError: 'NoneType' object is not iterable
```
Set it to neutral None, unless it is explicitly set.
`json.dump` does not add this, so we explicitly do it.
Otherwise when you manually edit a file and you use an editor that respects the standard `.editorconfig` that we have in most Plone packages, you always get a diff because there is a newline at the end.
We now have some data with a blacklist_status, and this gave an error.
@ericof
Copy link
Member

ericof commented Sep 12, 2024

I do not think it is too late to fix the naming in portlets

Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

Thanks, helpful improvements.

BTW, is it too late in the portlets.json to rename blacklist_status to something like disallowlist_status? I don't feel strongly about this, but using "black" in this context is frowned upon. For backwards compatibility we could still read the old name as well.

I support this.

src/plone/exportimport/exporters/__init__.py Outdated Show resolved Hide resolved
src/plone/exportimport/exporters/base.py Show resolved Hide resolved
@mauritsvanrees
Copy link
Member Author

In portlets I have deprecated blacklist_status in favor of disallowlist_status. We still read the old key for backwards compatibility.

And I added six news snippets. :-)

news/35.bugfix.1 Outdated Show resolved Hide resolved
news/35.bugfix.5 Outdated Show resolved Hide resolved
@mauritsvanrees mauritsvanrees merged commit 84fd598 into main Sep 12, 2024
7 checks passed
@mauritsvanrees mauritsvanrees deleted the maurits-fixes branch September 12, 2024 18:31
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.

3 participants