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

EZP-31420: Deleted translation from Solr when deleted from Version #173

Merged
merged 6 commits into from
May 6, 2020

Conversation

mateuszdebinski
Copy link
Contributor

@mateuszdebinski mateuszdebinski commented Mar 5, 2020

Question Answer
JIRA issue EZP-31420
Bug/Improvement yes
New feature no
Target version 1,7
BC breaks no
Tests pass yes
Doc needed no

Description

Depends on: ezsystems/ezpublish-kernel#3013

Removing a content translation does not remove the associated SolR documents

TODO:

  • Implement feature / fix a bug.
  • Ask for Code Review.

Copy link
Member

@konradoboza konradoboza left a comment

Choose a reason for hiding this comment

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

…slation document by language code and content id
Copy link
Contributor

@andrerom andrerom left a comment

Choose a reason for hiding this comment

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

+1, but can we add tests for this somehow?

@andrerom
Copy link
Contributor

Oh, we need to handle DeleteTranslation as well right? As RemoveTranslation seems to be the deprecated one if I read @alongosz's master PR right.

@alongosz
Copy link
Member

Yes, RemoveTranslationSignal is emitted for BC. You could actually handle both RemoveTranslationSignal and DeleteTranslationSignal in one single DeleteTranslationSlot. Don't remember how it's handled on upper branch with Events though.

Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

+1, but as I said in ezsystems/ezpublish-kernel#3013 (review) I need to see a merge up plan, probably for 3.0 branch here (but including merge as-is to 2.0 due to gitflow).

@mateuszdebinski
Copy link
Contributor Author

+1, but as I said in ezsystems/ezpublish-kernel#3013 (review) I need to see a merge up plan, probably for 3.0 branch here (but including merge as-is to 2.0 due to gitflow).

Solr Slot moved to kernel event, so any more changes in 3.0

Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

Ok, so if Kernel handles everything in eZ Platform v3.0, then +1 :)

@alongosz alongosz changed the title EZP-31420: Added remove function before update solr document to prevent store unused document EZP-31420: Deleted translation from Solr when deleted from Version May 5, 2020
@lserwatka lserwatka merged commit bd81cff into 1.7 May 6, 2020
@lserwatka lserwatka deleted the EZP-31420 branch May 6, 2020 13:47
@lserwatka
Copy link
Member

You can merge it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

7 participants