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
Fix deaccession deletion issue, refs #12460 #871
Conversation
@@ -45,8 +45,6 @@ public function save($connection = null) | |||
|
|||
public function delete($connection = null) | |||
{ | |||
QubitSearch::getInstance()->delete($this->accession); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are updating the index when we save/add an accession, then will we need to also '->update' on a delete? Or is the '->update' in save() not necessary either?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did some testing and looked at the code and it seems like the parent accession's document exists independently of whether or not there's a deaccession so I've removed both of the ElasticSearch alterations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I've gotten rid of the update
in the save because of this.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the PR.
@@ -45,8 +45,6 @@ public function save($connection = null) | |||
|
|||
public function delete($connection = null) | |||
{ | |||
QubitSearch::getInstance()->delete($this->accession); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did some testing and looked at the code and it seems like the parent accession's document exists independently of whether or not there's a deaccession so I've removed both of the ElasticSearch alterations.
@@ -45,8 +45,6 @@ public function save($connection = null) | |||
|
|||
public function delete($connection = null) | |||
{ | |||
QubitSearch::getInstance()->delete($this->accession); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I've gotten rid of the update
in the save because of this.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @mcantelon! Thanks for looking at that ->update() method!
Thanks @sbreker! |
16444f3
to
3beb188
Compare
Fixed issue where deleting a deaccession would remove the accession/accural related to it from the search index.
3beb188
to
830814b
Compare
Fixed issue where deleting a deaccession would remove the
accession/accural related to it from the search index.