Implementations of hook_node_update should check the "isDefaultRevision" property

I think the only bit here that's not addressed yet is search, which is in \Drupal\node\Entity\Node::postSave() now, there are also node_comment hooks that update the search index.

I attached a patch that checks for isDefaultRevision() being set to TRUE before running node_reindex_node_search and same situation for commentInsert or commentUpdate and or commentDelete at /core/modules/node/src/Hook/NodeHooks1.php

I'm starting to do some Core contribution so bear with me when reviewing.

All contributions must be merge requests.

Created MR to reflect the above comments.

Thanks @berdir , good to know, I'll do that next time.

Have not reviewed but issue summary appears incomplete

I improved the issue summary, hope it makes more sense.

Left some comments on the MR.

I documented the $comment parameter, added some more information and created a simple $this->assertTrue() in the testDeterminingChanges() test method (file core/modules/node/tests/src/Functional/NodeSaveTest.php) to test if the node that was updated is the default revision, and prints an informative message.

It seems like a pretty obvious validation but in the real scenario that is what is being check at ./src/Entity/Node::postSave() before updating the node access table and reindex the search table.

I'm open to any suggestions on this.

Leaving tests tag as the test-only pipeline is passing when I'd expect it to fail.

I refactored the test in a new function,testNodeDefaultRevision(), which checks that previous revisions of the node are not reindexed by looking at the index tables.

https://git.drupalcode.org/project/drupal/-/merge_requests/11353

The test-only pipeline is not passing this time

https://git.drupalcode.org/issue/drupal-1522154/-/jobs/4911642

Rebased and slightly tidied up the test.

Ran testonly feature

1) Drupal\Tests\node\Functional\NodeSaveTest::testNodeReindexDefaultRevision
Failed asserting that actual size 1 matches expected size 0.
/builds/issue/drupal-1522154/core/modules/node/tests/src/Functional/NodeSaveTest.php:245
FAILURES!
Tests: 5, Assertions: 52, Failures: 1.

Shows the coverage

Summary appears to be complete and matches the solution.

Believe all feedback has also been addressed.

Couple of comments on the MR.

Feedback appears to be addressed.

@danrod, thanks for the issue summary update.

Everything appears to be in order here.

Rebased and made some slight tweaks to the test so we don't need to install search module for every other test case in NodeSaveTest

Credits

Couple of questions on the MR, keeping at RTBC for now

One is an obvious typo that I'll self apply

@berdir flagged in #22

there are also node_comment hooks that update the search index

They are as follows:

1. \Drupal\node\Hook\NodeHooks1::commentInsert
2. \Drupal\node\Hook\NodeHooks1::commentUpdate
3. \Drupal\node\Hook\NodeHooks1::commentDelete

But comments only reference a node by entity-id, not entity and revision ID, so in that scenario I think we're always going to have the default revision

Setting back to needs review for someone to confirm my thinking on that