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

Added support for images in liveSearch results. #3489

Merged
merged 2 commits into from
Apr 7, 2022

Conversation

agitator
Copy link
Member

@agitator agitator commented Apr 7, 2022

No description provided.

@mister-roboto
Copy link

@agitator thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

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!

Copy link
Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

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

Tested and it works.

@mauritsvanrees
Copy link
Member

All green.

@mauritsvanrees mauritsvanrees merged commit afd49b1 into master Apr 7, 2022
@mauritsvanrees mauritsvanrees deleted the images-in-search branch April 7, 2022 21:21
@mauritsvanrees
Copy link
Member

I have merged all related branches.
Thanks!

@yurj
Copy link
Contributor

yurj commented Apr 8, 2022

KeyError: 'Interface plone.base.interfaces.controlpanel.ISearchSchema defines a field search_show_images, for which there is no record.'
My custom theme/package, upgrading a site from alpha3 to alpha4 (buildout-coredev). I saw this:
plone/plone.app.upgrade@9d1a564
and then re run the upgrades by hand for 6004 (even if I upgraded the Plone site with the upgrade button in Zope). It is because it is alpha?

@mauritsvanrees
Copy link
Member

I copied a Plone 5.2 Python 3 database to 6.0.0a4, ran @plone-upgrade, and this was fine.

But this can happen when you had previously already applied the upgrade to 6004. For example like this:

  • Start with a 6.0.0a3 site.
  • You used the coredev buildout yesterday. Or you updated to https://dist.plone.org/release/6.0-dev/ yesterday. Or you can reproduce this now by adding that link to the find-links and pinning plone.app.upgrade = 3.0.0a4.dev0.
  • You ran @@plone-upgrade. This runs the upgrades to 6004, but it is lacking my commit for the ISearchSchema.
  • You upgrade to 6.0.0a4. @@plone-upgrade has nothing to do, because you are already to 6004.
  • Or you upgrade to latest coredev. @@plone-upgrade upgrades you from 6004 to 6005, which is a dummy upgrade.

With such a scenario, you would indeed be missing this key from the search schema. Rerunning the upgrade to 6004 fixes it. Or explicitly run import step plone.app.registry from profile plone.app.upgrade.v60.to6004.

Does this explain it?
Or is something else going on? I do hope that the move of ISearchSchema from Products.CMFPlone to plone.base does not negatively influence this. But so far it seems good for me.

@mauritsvanrees
Copy link
Member

I had to check:

  • In Plone 5.2 Py 3 I changed all fields in the search control panel.
  • Copied database to Plone 6 and ran upgrade.
  • The new field is there.
  • The existing fields have the same values as in 5.2.

So that is good.

@yurj
Copy link
Contributor

yurj commented Apr 8, 2022

I've updated the code to latest coredev this morning, the plone site was 6004 with an older coredev. Then I did @@plone-upgrade. Checked the site, missing search_show_images then portal_setup -> upgrades -> cmfplone:default -> old upgrades -> run 6004.

Now I get it. I cannot update the coredev without re running the same level (6004) I will miss the changes in upgrade steps because I'm already 6004.

Thanks for the hints, I got it!

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.

4 participants