Updated: Comment 0

Problem/Motivation

Much like back in the days with node_delete() you don't want to make the API be kinda different depending on the current user.
One example is book_node_presave which checks for the current user.
An alternative approach would be to set a new revision on book_node_builder.

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#3 book-2218691-3.patch1.21 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue summary: View changes
chx’s picture

chx CreditAttribution: chx commented

Note that migrate_drupal runs migrate_drupal_module_implements_alter to nuke book_node_presave then copies it to migrate_drupal_node_presave with a killswitch:

function migrate_drupal_node_presave(EntityInterface $node) {
  // Always save a revision for non-administrators.
  if (empty($node->beingMigrated) && !empty($node->book['bid']) && !\Drupal::currentUser()->hasPermission('administer nodes')) {
    $node->setNewRevision();
// rest of book_node_presave follows.

this shouldn't be necessary.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Priority: Major » Normal
Status: Active » Needs review
FileSize
book-2218691-3.patch1.21 KB

Something like this.

chx’s picture

chx CreditAttribution: chx commented
Assigned: Unassigned » pwolanin
chx’s picture

chx CreditAttribution: chx commented

I can confirm this solves the problem migrate faces; assigning to pwolanin for review.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs review » Reviewed & tested by the community

Looks ok, though all the hook_node_X implementations should be using NodeInterface. I guess we can fix that in a follow-up.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented

3: book-2218691-3.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +quickfix, +blocker

Adding all the tags.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: book-2218691-3.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review

3: book-2218691-3.patch queued for re-testing.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC after bot fluke.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Issue tags: -quickfix +Quick fix

Fixing tag so I can find this again once it's back from testbot.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Assigned: pwolanin » Unassigned

un-assigning since the review is done

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

  • Commit ddff1b3 on 8.x by catch: 
    Issue #2218691 by dawehner: Move the current user out of the node API...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.