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

[4.3] Smart Search: Don't republish items upon save #39944

Merged
merged 2 commits into from Mar 4, 2023

Conversation

Hackwar
Copy link
Member

@Hackwar Hackwar commented Feb 25, 2023

Pull Request for Issue # .

Summary of Changes

In Smart Search you can disable entries in the index by unpublishing them. Unfortunately, the entries were re-published when an item was saved again. I earlier claimed that it wasn't possible to prevent that behavior in our current system because I thought the state of the content item and the state of the item in the index were the same field in the database. I was wrong and we actually have a field named "published" for the state of the item in the index and "state" for the state item of the content item. Sorry, @brianteeman 😉 (Maybe you can help me find the issue where this was reported? I can't find it right now...)

After some debugging it turned out that in our finder plugins we are using the reindex() method of the Adapter class in finder and that method falsely completely deletes the entry first from the index and then adds it back in again. This actually is far less performant than simply indexing again as the normal indexer does...

Testing Instructions

Index your content and unpublish an entry in the index. Edit the content and without changing anything, save it again.

Actual result BEFORE applying this Pull Request

The item in the index is published again.

Expected result AFTER applying this Pull Request

The item stays unpublished until you publish it again.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@ChristineWk
Copy link

I'm not sure, I don't know much about smart search, maybe this? #36980


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39944.

@Hackwar
Copy link
Member Author

Hackwar commented Feb 25, 2023

Unfortunately that issue is not the one I was looking for.

@Hackwar Hackwar added the Small A PR which only has a small change label Feb 25, 2023
@fabpacheco
Copy link

Or maybe #32458

@toivo
Copy link
Contributor

toivo commented Feb 26, 2023

I have tested this item ✅ successfully on 5a66c06

Tested successfully in Joomla 4.3.0-beta4 of 25 February using PHP 8.1.10


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39944.

@brianteeman
Copy link
Contributor

This one #38838 ?

@Hackwar
Copy link
Member Author

Hackwar commented Feb 27, 2023

Unfortunately none of these. I'm going to look further. But the issue of #38838 is very similar.

@brianteeman
Copy link
Contributor

OK - this time I am sure I found it #36980

@Hackwar
Copy link
Member Author

Hackwar commented Feb 27, 2023

That one is related to the taxonomies, which I'm still working on.

@Quy
Copy link
Contributor

Quy commented Mar 4, 2023

I have tested this item ✅ successfully on 5a66c06


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39944.

@Quy Quy removed the Small A PR which only has a small change label Mar 4, 2023
@Quy
Copy link
Contributor

Quy commented Mar 4, 2023

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39944.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 4, 2023
@obuisard obuisard added this to the Joomla! 4.3.0 milestone Mar 4, 2023
@obuisard obuisard merged commit 72a45ef into joomla:4.3-dev Mar 4, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 4, 2023
@obuisard
Copy link
Contributor

obuisard commented Mar 4, 2023

Thank you Hannes @Hackwar !

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.

None yet

8 participants