Bug #11855

Search index is not updated when edits affect descendant records

Added by Dan Gillean over 1 year ago. Updated 11 months ago.

Status:VerifiedStart date:01/10/2018
Priority:MediumDue date:
Assignee:-% Done:

0%

Category:Search / BrowseEstimated time:20.00 hours
Target version:Release 2.5.0
Google Code Legacy ID: Tested version:2.4
Sponsored:Yes Requires documentation:

Description

To reproduce:

  • Create a record with several children, and add a creator "FOOBAR"
  • Run a search:populate to ensure that child records with inherited creators can be searched (see notes on #11788)
  • Search for FOOBAR and confirm that record and children are all returned.
  • Now change the creator name to "BARFOO" (aka new creator)
  • Search for BARFOO
Resulting error
  • only the parent record is returned
  • search index is not automatically updated for descendant records
Expected result
  • No manual re-index is required - relevant descendant records are updated in the index
  • Search returns all records where creator is BARFOO, whether directly added or inherited

Notes
As of #11788, the ES field in question here is inheritedCreators. There are other fields affected by this as well, such as the repository and partOf fields.

Note from Radda on 11788:

Like the 'repository' and 'partOf' fields, the 'inheritedCreators' field is not being updated in ES when an ancestor changes its creator(s) or when the actor changes. Considering that all descendants should be updated each time an IO changes in ES, this should be done in an asynchronous job launched from the method that updates the IOs in ES.


Related issues

Related to Access to Memory (AtoM) - Bug #11788: Inherited creator names will not return search results fo... Verified 12/08/2017
Related to Access to Memory (AtoM) - Task #11906: Avoid unnecessary ES update of related description when a... Verified 01/25/2018
Related to Access to Memory (AtoM) - Bug #10087: When updating repository info, nested repository document... Duplicate 06/29/2016

History

#1 Updated by Dan Gillean over 1 year ago

  • Related to Bug #11788: Inherited creator names will not return search results for matching lower-level records added

#2 Updated by José Raddaoui Marín over 1 year ago

We should create this job to accept other parts of the application where multiple records are being updated in ES, for example:

https://github.com/artefactual/atom/blob/stable/2.4.x/apps/qubit/modules/term/actions/editAction.class.php#L578-L589
https://github.com/artefactual/atom/blob/qa/2.5.x/lib/model/QubitActor.php#L146-L153

Edited: Added QubitActor link

#3 Updated by Dan Gillean over 1 year ago

  • Target version set to Release 2.4.1
  • Sponsored changed from No to Yes

#6 Updated by José Raddaoui Marín over 1 year ago

  • Related to Task #11906: Avoid unnecessary ES update of related description when a term name is changed through the GUI added

#7 Updated by Nick Wilkinson over 1 year ago

  • Assignee set to Mike Gale

Hi Mike, can you please look into this? It's related to #11884, which you're also working on.

#8 Updated by Mike Gale over 1 year ago

  • Status changed from New to QA/Review
  • Assignee changed from Mike Gale to Mike Cantelon

Hi Mike, can you CR this please? https://github.com/artefactual/atom/pull/667

thank you!

#9 Updated by José Raddaoui Marín over 1 year ago

I think note 2 has not been taken in consideration in the end :(

#10 Updated by Mike Cantelon over 1 year ago

  • Status changed from QA/Review to Feedback
  • Assignee changed from Mike Cantelon to José Raddaoui Marín

Hi Radda. I guess that means more work is needed? The PR, as it stands, seems fine.

#12 Updated by José Raddaoui Marín over 1 year ago

  • Assignee changed from José Raddaoui Marín to Mike Gale

Hi Mike G. and C.,

First of all, I think this is not working as we're calling a new function without a required parameter. But, more important, it looks like it's replacing the entire documents with only the inherited partial data. I have added a few comments in the PR.

About addressing update 2 from this ticket, it will be a great enhancement and it shouldn't take too much time. I've detailed how I'd do it in the PR.

#14 Updated by Mike Gale about 1 year ago

  • Assignee changed from Mike Gale to José Raddaoui Marín

Checking back with Radda how close I am to the required CR changes.

#15 Updated by José Raddaoui Marín about 1 year ago

  • Assignee changed from José Raddaoui Marín to Mike Gale

Looking good so far. I'll take a deeper look to the partial updates changes, but the improvements in the job to make it work with multiple IOs are still to be done ;)

#16 Updated by José Raddaoui Marín about 1 year ago

I have been able to look at it already. There is a few more comments in the PR.

#17 Updated by Nick Wilkinson about 1 year ago

  • Assignee changed from Mike Gale to José Raddaoui Marín

Hi Radda, reassigning this to you.

#18 Updated by José Raddaoui Marín about 1 year ago

  • Status changed from Feedback to In progress

#19 Updated by José Raddaoui Marín about 1 year ago

  • Status changed from In progress to Code Review
  • Assignee changed from José Raddaoui Marín to Nick Wilkinson

Ready for code review.

#20 Updated by Nick Wilkinson about 1 year ago

  • Assignee changed from Nick Wilkinson to Steve Breker

Hi Steve, passing to you for CR.

#21 Updated by Steve Breker about 1 year ago

  • Assignee changed from Steve Breker to José Raddaoui Marín

CR complete - Looks great!

#22 Updated by Steve Breker about 1 year ago

  • Status changed from Code Review to Feedback

#23 Updated by José Raddaoui Marín about 1 year ago

  • Status changed from Feedback to QA/Review
  • Assignee changed from José Raddaoui Marín to Dan Gillean

Hi Dan,

This has been merged in qa/2.5.x but I'm not sure if we want this in stable/2.4.x as it brings a lot of changes. If we really want it in stable I'd like if you could give it a thorough test in qa before doing so.

Notes for testing:

Descendants should be updated always when an ancestor description changes. If the update is done through the GUI a new job will be triggered. Changing a term (place, subject or genre only) will update all it's related descriptions. Changing an actor will do the same for its related descriptions via events and if the event is a creation event it will also update their descendants. When a repository changes the job is triggered too to update the related descriptions and their descendants. In all this cases a notification is shown to inform the user.

Please, test normal updates, but also test tasks and other jobs, like importing updating matches through the CLI and the GUI.

#24 Updated by Dan Gillean about 1 year ago

  • Status changed from QA/Review to Feedback
  • Assignee changed from Dan Gillean to José Raddaoui Marín

Radda this is looking really good! But I found one case that seems to regularly produce an error. If you remove an existing creator and add a new creator name at the same time then the job produces an error - actually, there are 3 different jobs listed on the jobs page, all with the same error:

[info] [2018-03-29 14:02:02] Job 2003046 "arUpdateEsIoDocumentsJob": Job started.
[info] [2018-03-29 14:02:02] Job 2003046 "arUpdateEsIoDocumentsJob": Updating descendants of 1 description(s).
[info] [2018-03-29 14:02:03] Job 2003046 "arUpdateEsIoDocumentsJob": Exception: Couldn't find actor (id: )

My guess is that there's something wrong with the order of operations. It's trying to remove the parent actor, and then looking for that actor in the children - but those relations no longer exist because they were using inheritance, so an error is thrown, maybe?

I did this using the demo data in my local 2.5 Vagrant box, and working with the Kantokoski (Koski), Koivula & Korpela Family fonds. To reproduce:

  • Navigate to the fonds description and enter edit mode
  • In the Context area, click the "X" to remove the current creator
  • Add a new creator, "FOOBAR"
  • Scroll down and click save
  • go to the jobs page

#26 Updated by Dan Gillean about 1 year ago

See my comments here as well: https://projects.artefactual.com/issues/11788#note-12

I'm beginning to suspect that the error reported there in fact has more to do with this issue. But still not sure.

#28 Updated by José Raddaoui Marín about 1 year ago

  • Status changed from Feedback to Code Review
  • Assignee changed from José Raddaoui Marín to Nick Wilkinson
  • Target version changed from Release 2.4.1 to Release 2.5.0

Great catches Dan!

I have fixed the multiple job call in this PR.

The actor id is caused by the inherited creators changes, which are already added to stable/2.4.x, so they are in a different PR, linked in the related ticket.

#29 Updated by Nick Wilkinson about 1 year ago

  • Assignee changed from Nick Wilkinson to Mike Cantelon

Hi Mike, passing to you for CR.

#30 Updated by Mike Cantelon about 1 year ago

  • Status changed from Code Review to Feedback
  • Assignee changed from Mike Cantelon to José Raddaoui Marín

Looks good to me!

#31 Updated by José Raddaoui Marín about 1 year ago

  • Status changed from Feedback to QA/Review
  • Assignee changed from José Raddaoui Marín to Dan Gillean

Merged in qa/2.5.x

#32 Updated by Dan Gillean about 1 year ago

  • Status changed from QA/Review to Verified

Looks good!

#33 Updated by Dan Gillean 11 months ago

  • Status changed from Verified to Feedback
  • Assignee changed from Dan Gillean to José Raddaoui Marín

Hi Radda!

Either there's been a regression, or I missed something during testing. The related descriptions sidebar on the actor view page is not being updated after an event relationship is removed via the actor page.

To reproduce

  • Find a description linked to an actor
  • Navigate to the authority record view page
  • Confirm that the related description is displayed on the authority record page in the left-hand context menu (e.g. "Creator of" menu)
  • Enter edit mode on the authority record
  • In the Relationships area, remove the link to the description (click on the X icon). Save the authority record
Resulting error
  • Sidebar still shows link to description in "Creator of" left-hand context menu
  • Clicking through to the description shows no related creator
  • Clearing browser cache / using a new incognito browser has no effect
Expected result
  • Related description is no longer shown in the left context menu after being removed via the relationships area in the actor edit page

#34 Updated by José Raddaoui Marín 11 months ago

  • Status changed from Feedback to Code Review
  • Assignee changed from José Raddaoui Marín to Nick Wilkinson

Ready for code review in https://github.com/artefactual/atom/pull/733

Please notice that the update is done asynchronously in some cases and the description may still be there after the form save and redirect to the index page. They should be out when the job ends.

#35 Updated by Nick Wilkinson 11 months ago

  • Assignee changed from Nick Wilkinson to Mike Cantelon

#36 Updated by Mike Cantelon 11 months ago

  • Assignee changed from Mike Cantelon to José Raddaoui Marín

Looks good to me!

#37 Updated by Mike Cantelon 11 months ago

  • Status changed from Code Review to Feedback

#38 Updated by José Raddaoui Marín 11 months ago

  • Status changed from Feedback to QA/Review
  • Assignee changed from José Raddaoui Marín to Dan Gillean

Merged in qa/2.5.x

#39 Updated by Dan Gillean 11 months ago

  • Status changed from QA/Review to Verified
  • Assignee deleted (Dan Gillean)

Looks good - thanks Radda!

#40 Updated by Mike Cantelon 10 months ago

  • Related to Bug #10087: When updating repository info, nested repository documents not updating for holdings in ES added

Also available in: Atom PDF