Adds portal.add_catalog_indexes and portal.add_catalog_metadata methods
#578
Conversation
|
@rohnsha0 thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment: To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
There was a problem hiding this comment.
Let's double-check that the docs render properly. We've been having some issues with pull request previews on RTD due to our misconfiguration.
You will need to add the new methods in the file docs/api/index.md so they show up in https://ploneapi--578.org.readthedocs.build/api/index.html#api-portal.
docs/portal.md
Outdated
| % self.assertIn('modified_by', catalog.indexes()) | ||
| % self.assertIn('creation_user', catalog.indexes()) | ||
|
|
||
| This function returns a list of the names of the indexes that were added. |
There was a problem hiding this comment.
Move above.
| This function returns a list of the names of the indexes that were added. |
src/plone/api/portal.py
Outdated
| @required_parameters("wanted_indexes") | ||
| def add_catalog_indexes(wanted_indexes, reindex=True, logger=None): | ||
| """ | ||
| Add the specified indexes to portal_catalog if they don't already exist. |
There was a problem hiding this comment.
| Add the specified indexes to portal_catalog if they don't already exist. | |
| Add the specified indexes to portal_catalog. |
src/plone/api/portal.py
Outdated
| - logger: Optional logger instance | ||
| Returns: | ||
| - List of newly added index names |
There was a problem hiding this comment.
| - List of newly added index names | |
| - List of newly added index names | |
| Example: | |
| - :ref:`portal-add-catalog-indexes-example` |
|
@rohnsha0 RTD previews are not rendering correctly. We have a fix in #506 that you can cherry-pick here, or wait until that PR is merged and merge it into your branch. Specifically, the changes in these files need to be ported:
Thank you! |
|
#506 was just merged to allow docs to build. Updating branch. Let's see if that fixes this PR. |
|
Docs are rendering correctly now. |
Co-authored-by: Steve Piercy <[email protected]>
ah, there was another error which got fixed somehow. something again with |
github-project-automation
bot
moved this from In Progress
to Approved
in Plone Documentation
|
@plone/classicui-team could I get a review from one of y'all? |
There was a problem hiding this comment.
I have some concerns about this implementation:
- the issue mentioned is outdated and creating indexes/metadata with genericsetup is simple and works like it should. (No more dataloss on recreation)
- the fact, that reindex here is per default
Truewhen adding an index would cause troubles with long running tasks on large sites - adding a metadata means, that you have to either reindex your whole catalog or write your own script, which reindexes your needed objects.
- If it comes to special indexes (like
DateRecurringIndex) there is no way to properly configure theRRULEwith this method, so you have to write your own script again.
that said, I dont really see a benefit from this ... I'd put more effort porting this doc here to plone 6+ https://5.docs.plone.org/external/plone.app.dexterity/docs/advanced/catalog-indexing-strategies.html
github-project-automation
bot
moved this from Approved
to In Progress
in Plone Documentation
There are open issues in https://github.com/plone/documentation/issues?q=is%3Aissue%20state%3Aopen%20catalog The files are already converted to MyST on the https://github.com/plone/documentation/tree/5.2-myst/5.2-myst |
| def add_catalog_indexes(indexes_to_add, reindex=True, logger=None): | ||
| """Add the specified indexes to portal_catalog if they don't already exist. | ||
| :param indexes_to_add: [required] List of tuples in format (index_name, index_type) | ||
| :type indexes_to_add: list |
There was a problem hiding this comment.
How do I pass index properties here? This is essential.
|
The current implementation is incomplete. Overall I do not think we need this feature. We introduce just another way to do the same we can do better using GenericSetup. |
closes #404
📚 Documentation preview 📚: