With the #1818556: Convert nodes to the new Entity Field API the initial conversion has been done, but most access to $node properties/fields still goes via the bc-decorator entity.

To complete the conversion we need to change node_load(_multiple())() to return the original (NG-)entity and fix all usages of node properties to use the new entity field syntax: $node->title becomes $node->title->value.

Proposed resolution steps

#1 -

most frequently accessed things are those that we already have methods for, methods that are the same for NG and BC: id(), bundle(), getRevisionId(), label() (already used in most places)... suggestion would be to do a first iteration and convert everything we can to those methods.

#3 -

We might have multiple passes in this issue and a commit for each pass. Another step, could be to have a single patch for each module integrating with Node to convert the reltated additions to actual entity fields.

#9 - make the issue as META and each conversion file as separate issue to make easy all big patches in queue to easy merge changes

Part of #1346214: [meta] Unified Entity Field API

Steps

  • change node->nid to node->id()
  • change node->title to node->label()
  • change node->type to node->bundle()
CommentFileSizeAuthor
#96 1939994-96.patch116.63 KBBerdir
#96 1939994-96-interdiff.txt2.23 KBBerdir
#94 1939994-94.patch116.75 KBBerdir
#94 1939994-94-interdiff.txt591 bytesBerdir
#92 1939994-92.patch116.82 KBBerdir
#92 1939994-92-interdiff.txt6.06 KBBerdir
#90 1939994-90.patch145.36 KBBerdir
#88 1939994-88.patch288.85 KBrlmumford
#88 interdiff.txt575 bytesrlmumford
#86 node-bc-removal-1939994-86.patch288.46 KBBerdir
#86 node-bc-removal-1939994-86-interdiff.txt2 KBBerdir
#84 node-bc-removal-1939994-84.patch287.97 KBBerdir
#78 node-bc-removal-1939994-78.patch286.16 KBBerdir
#78 node-bc-removal-1939994-78-interdiff.txt51.3 KBBerdir
#76 node-methods-1939994-76.patch248.84 KBBerdir
#76 node-methods-1939994-76-interdiff.txt5.51 KBBerdir
#74 node-methods-1939994-74.patch243.76 KBBerdir
#74 node-methods-1939994-74-interdiff.txt5.09 KBBerdir
#72 node-methods-1939994-72.patch237.43 KBBerdir
#72 node-methods-1939994-72-interdiff.txt7.1 KBBerdir
#70 node-methods-1939994-70.patch227.37 KBBerdir
#65 node-methods-1939994-65.patch161.59 KBBerdir
#62 node-methods-1939994-62.patch163.95 KBBerdir
#60 node-methods-1939994-61.patch440.87 KBBerdir
#60 node-methods-1939994-61-bc-removal.patch493.09 KBBerdir
#60 node-methods-1939994-61-interdiff.txt9.37 KBBerdir
#60 node-methods-1939994-61-bc-removal-interdiff.txt88.5 KBBerdir
#59 node-methods-1939994-59-nid-only.patch316.1 KBBerdir
#59 node-methods-1939994-59-nid-only-interdiff.txt2.59 KBBerdir
#57 node-methods-1939994-57-nid-only.patch316.11 KBBerdir
#57 node-methods-1939994-57.patch427.13 KBBerdir
#57 node-methods-1939994-57-bc-removal.patch478.61 KBBerdir
#55 node-methods-1939994-55.patch431.33 KBBerdir
#55 node-methods-1939994-55-nid-only.patch318.46 KBBerdir
#55 node-methods-1939994-55-bc-removal.patch482.66 KBBerdir
#53 node-methods-1939994-53.patch410.89 KBBerdir
#53 node-methods-1939994-53-bc-removal.patch454.69 KBBerdir
#53 node-methods-1939994-53-bc-removal-interdiff.txt66.32 KBBerdir
#49 node-methods-1939994-49.patch409.89 KBBerdir
#49 node-methods-49-bc-removal.patch423.41 KBBerdir
#49 node-methods-1939994-49-bc-removal-diff.txt24.77 KBBerdir
#47 node-methods-1939994-47.patch409.69 KBBerdir
#45 node-methods-1939994-45.patch409.96 KBBerdir
#45 node-methods-1939994-45-interdiff.txt2.21 KBBerdir
#43 node-methods-1939994-43.patch409.69 KBBerdir
#41 node-methods-1939994-41.patch409.69 KBBerdir
#38 node-methods-1939994-38.patch414.41 KBBerdir
#36 node-methods-1939994-36.patch412.25 KBBerdir
#34 node-methods-1939994-34.patch412.5 KBBerdir
#32 node-methods-1939994-32.patch412.55 KBBerdir
#30 node-methods-1939994-30.patch410.59 KBBerdir
#30 node-methods-bc-removal-1939994-30.patch418.46 KBBerdir
#28 node-methods-1939994-28.patch410.65 KBBerdir
#28 node-methods-bc-removal-1939994-28.patch418.53 KBBerdir
#26 node-methods-1939994-26.patch411.94 KBBerdir
#26 node-methods-bc-removal-1939994-26.patch419.37 KBBerdir
#24 node-methods-1939994-24.patch411.96 KBBerdir
#24 node-methods-bc-removal-1939994-24.patch419.38 KBBerdir
#22 node-methods-1939994-22.patch412.68 KBBerdir
#22 node-methods-bc-removed-1939994-22.patch420 KBBerdir
#20 node-bc-removal-1939994-20.patch421.04 KBBerdir
#18 node-bc-removal-1939994-18.patch421.26 KBBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

I've been thinking quite a bit about the best approach to tackle this.

What I've seen in the taxonomy term issue is that the most frequently accessed things are those that we already have methods for, methods that are the same for NG and BC: id(), bundle(), getRevisionId(), label() (already used in most places).

My suggestion would be to do a first iteration and convert everything we can to those methods. That's going to be a huge but for the most part trivial patch (global search and replace, fix the broken ones like empty($node->id()) stuff). That's a lot of code that we don't have to touch again and doing it in one go should be the least disruptive for other issues I think.

podarok’s picture

agree with @Berdir about first iteration

plach’s picture

Yes, this soulds like a plan. We might have multiple passes in this issue and a commit for each pass. Another step, could be to have a single patch for each module integrating with Node to convert the reltated additions to actual entity fields.

podarok’s picture

#3 another step looks like a META follow-up issue

EDIT
Updated summary

podarok’s picture

Issue summary: View changes

Updated issue summary.

plach’s picture

Sorry, I did not explain myself: #3 is not an alternative to #1. I just meant to have a patch for each module integrating with Node. But remaining in this issue, just to keep the patch size manageable.

plach’s picture

Issue summary: View changes

Updated issue summary.

podarok’s picture

#5 didn`t post it like alternative
following up )

fago’s picture

Agreed, sounds like a good plan.

Berdir’s picture

Assigned: Unassigned » Berdir

Alright, started working on a patch but this is going to conflict a lot with some other active issues, e.g. the entityreference/taxonomy reference one, taxonomy term NG conversion and comment as field, which are going to be harder to re-roll than this. So it probably makes sense to wait with this for a while.

Berdir’s picture

Issue summary: View changes

Updated issue summary.

andypost’s picture

Updated summary.
Suppose better make the issue as META and each conversion file as separate issue to make easy all big patches in queue to easy merge changes

andypost’s picture

Issue summary: View changes

Added steps and link to meta

podarok’s picture

Title: Complete conversion of nodes to the new Entity Field API » [META] Complete conversion of nodes to the new Entity Field API

better title + updated summary due to #9

podarok’s picture

#8 @Berdir good to see in summary as blockers those issues

Berdir’s picture

Assigned: Berdir » Unassigned
Issue tags: +Entity Field API

Additionally to the standard methods, we want to add methods on NodeInterface for things like status, changing status, created/changed and so on. That will make it easier to replace those as well.

Still not sure what to do about the comment as field issue. I will probably start by defining an interface and a NodeBCDecorator similar to what I did for users to come up with a good interface and then start to do search & replace, possibly in separte patches or split up by module or so.

andypost’s picture

@Berdir, comment-field still lacks of dynamic entity reference to the attached entity

Berdir’s picture

That's not really relevant for this :)

What I mean is that this issue and the comment as field one will conflict *a lot*. And given that you already moved a lot code to generic entity code, you probably took care of a lot of ->nid => ->id() and similar changes already, so it would be easier to start after that is done. That said, we need to get rid of the BC decorator asap and need to start moving here and you have to re-roll almost daily anyway...

Berdir’s picture

Issue tags: +Field API NG blocker

@timplunkett started doing exactly that in #2015123: Expand NodeInterface to provide getters and setters

fago’s picture

This does not really block re-newing field API though, does it? It blocks being able to remove the BC decorator.

effulgentsia’s picture

Priority: Major » Critical

Raising to critical per #1818580-7: [Meta] Convert all entity types to the new Entity Field API. Nodes are Drupal's most important entity type, so it's not like we can release without finishing this.

Berdir’s picture

Status: Active » Needs review
FileSize
421.26 KB

First patch. This removes the BC layer from nodes and starts converting everything we can to methods.

I have a branch that has the removal and each method change in separated commits, I'm just trying to see where we are with the tests. We can look how we want to split it up.

Status: Needs review » Needs work

The last submitted patch, node-bc-removal-1939994-18.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
421.04 KB

Fixed syntax error, was introduced because @alexpott committed a patch that added that interface too this morning, and it didn't conflict when I rebased :) Let's see if there are more.

Status: Needs review » Needs work

The last submitted patch, node-bc-removal-1939994-20.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
420 KB
412.68 KB

Ok, this should be better.

The first patch just does the method conversions. This shouldn't have too many fails left.

The second patch removes the BC layer, this shows how far we're away from removing it.

Status: Needs review » Needs work

The last submitted patch, node-methods-bc-removed-1939994-22.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
419.38 KB
411.96 KB

This should fix most of the fails for real ;)

Status: Needs review » Needs work

The last submitted patch, node-methods-bc-removal-1939994-24.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
419.37 KB
411.94 KB

Rebased.

Status: Needs review » Needs work

The last submitted patch, node-methods-bc-removal-1939994-26.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
418.53 KB
410.65 KB

Ok, I had the node form controller already switched to NG, that caused it to ignore a number of stuff that was set there. This should be better. Also fixed a bunch of other tests. Something with access still isn't quite right, don't know what yet.

Status: Needs review » Needs work

The last submitted patch, node-methods-bc-removal-1939994-28.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
418.46 KB
410.59 KB

Re-roll :)

Status: Needs review » Needs work

The last submitted patch, node-methods-bc-removal-1939994-30.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
412.55 KB

Just the non-removal patch, This should fix the remaining test fails.

Lost time tracking down a bug that I didn't introduce and couldn't understand how I could be introducing it: #2031183: Improve test coverage for node authored on widget

Biggest change is the introduction of a setLanguage() method on EntityInterface(), we need this because language() caches the language it has. Alternatively, we could implement onChange( to handle this.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-32.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
412.5 KB

This is going to conflict a lot ;)

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-34.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
412.25 KB

Merge with the configurable languages issue didn't go well.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-36.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
414.41 KB

This will hopefully fix the last translation tests.

fago’s picture

Status: Needs review » Needs work

Awesome!

+++ b/core/lib/Drupal/Core/Template/Attribute.php
@@ -102,6 +103,9 @@ public function offsetExists($name) {
+      if ($value instanceof Field) {
+        var_dump($value);

Left over?

+++ b/core/modules/field/field.attach.inc
@@ -82,7 +82,7 @@
- * found in $node->type. This property can be omitted if the entity type only
+ * found in $node->bundle(). This property can be omitted if the entity type only

In that context changing it does not make sense.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/StylePluginBase.php
@@ -478,6 +478,10 @@ public function renderGroupingSets($sets, $level = 0) {
+            //var_dump($render);
+            //hide($render['links']);
+            //hide($render['body']);

Needs to be removed.

fago’s picture

oh, and this should the language fix from #1868004-68: Improve the TypedData API usage of EntityNG as well I guess

Berdir’s picture

Status: Needs work » Needs review
FileSize
409.69 KB

Thanks for the review, killed setLanguage() and instead added the fix from there, removed the debug stuff, reverted the change in field.attach.inc.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-41.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
409.69 KB

Fixed syntax error.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-43.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
2.21 KB
409.96 KB

Fixed some fails, haven't figured out the Node translation UI test yet.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-45.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
409.69 KB

Re-roll.

Will soon start to branch this off to get it in per method (group of related methods if patch size isn't too bug) but want to see what the tests have to say.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-47.patch, failed testing.

Berdir’s picture

Lost the interdiff from #45.

Also updated the removal patch, merged node_submit() into the form controller, no idea why this wasn't already done and merged in the patch from #1980822: Support any entity with path.module to make path aliases more or less work. They are correctly saved, but something with the language isn't correct yet, in my manual test it was saved as language en, which did not work, when I manually changed it to und, then it worked.

Berdir’s picture

Status: Needs work » Needs review
Berdir’s picture

Issue tags: +sprint

Tagging

Status: Needs review » Needs work

The last submitted patch, node-methods-49-bc-removal.patch, failed testing.

Berdir’s picture

Re-roll, the node translation UI test will still fail, but the bc removal patch should do much better.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-53-bc-removal.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
482.66 KB
318.46 KB
431.33 KB

Ok, some new patches with more conversions and test fixes.

- Full conversion of methods, still the same failures.
- Only the nid changes (including a few more that I missed so far), this should be green and if so, will create a separate issue to get that in.
- Patch with BC removal.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-55-bc-removal.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
478.61 KB
427.13 KB
316.11 KB

Pretty heavy re-roll after the uid patch and some controller conversions got it. Fixed the translation fails of the nid patch, let's see if something else broke.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-57-bc-removal.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
2.59 KB
316.1 KB

I thought I fixed those fails already, must have lost that.

Just the patch then, which should now finally pass.

Berdir’s picture

Oh, didn't reference #2041287: Convert $node->nid to $node->id() and $node->isNew() yet, that's where the nid patch is now reviewed and hopefully committed soon, with your help :)

These are re-rolls on top of that, went through the NG test fails and fixed a lot of them. Just uploading to see how the testbot is doing. A few tricky test fails are remaining, but this should take care of a lot of them.

There's some crazy stuff in forum.module (surprise!), with $topic->last_post, which sometimes was a node object and sometimes a stdClass. changed to to entity_create() for now, maybe we should go the other way and always make it a stdClass, for performance reasons (or a different object but that I think would be out of scope)

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-61-bc-removal.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
163.95 KB

Updated patch without the nid part.

xmacinfo’s picture

node->label() still does not make sense.

We need to keep node->title(), or developers will think Drupal is gone insane. :-)

Is there an issue with a patch to put back node->title()?

That Change record, [#1697182], does not specify why or how…

This will allow for improvements to the translatability of a node's title

.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-62.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
161.59 KB

Ok, reverted those method calls in that preprocess for now, I think what we should do there is always use a stdClass object, the documentation on it is lousy anyway and core only uses it to populate the other variables.

chx’s picture

Could we remove the getBCentity call from load and see whether it passes?

Berdir’s picture

It doesn't. And it's not that easy to remove the BC decorator ;) See the -bc-removal patches (+ interdiffs, which shows the difference), which I'm also uploading from time to time, last in #60.

There are still a lot of fails left, some possibly due to missing method usage but most due to different behavior, configurable fields (for which we have no methods, obviously).

Some of those fails are easy to fix, e.g. "Object of class Drupal\Core\Entity\Field\Field could not be converted to string" stuff is usually straight-forward but other things are... tricky. That's why I'm getting this in piece by piece, to have the simple search & replace stuff out of the way so we can then in the end focus on the non-trivial things.

chx’s picture

Status: Needs review » Reviewed & tested by the community

Then let's get this in.

Berdir’s picture

Status: Reviewed & tested by the community » Needs review

Thanks, let's keep this open to track the overall progress, opened #2049039: Convert node properties to methods. to get the current green patch in, please review/RTBC there.

Berdir’s picture

FileSize
227.37 KB

Re-roll of the full patch, can't reproduce some of these test fails, let's see where we stand.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-70.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
7.1 KB
237.43 KB

Fixed a number of tests,

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-72.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
5.09 KB
243.76 KB

Reroll based on the patch in the methods issue and removing the NodeBCDecorator class.

The last patch resulted in a lot of additional fails due to the removal of various getBCEntity() calls, let's see if this is better now that the bc decorator for users got removed.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-74.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
5.51 KB
248.84 KB

Another re-roll based on the other issue and some fixes for forum and other tests.

Status: Needs review » Needs work

The last submitted patch, node-methods-1939994-76.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
51.3 KB
286.16 KB

Re-roll, some tests fixed. Book is causing me serious troubles, crazy crazy stuff. This is just some experimenting there.. it's doing all kinds of unholy things with $node->book, sometimes joined menu_link data, sometimes it's an actual menu_link entity, which by now is quite a different thing and only works due to the array bc stuff there (not related to EntityBCDecorator).

Currently thinking about makin mlid an entity reference there or something like that. We'll see.

Status: Needs review » Needs work

The last submitted patch, node-bc-removal-1939994-78.patch, failed testing.

xjm’s picture

This is an approved API change per #1983554-8: Remove BC-mode from EntityNG (tagging on behalf of @webchick).

klausi’s picture

rlmumford’s picture

The patch doesn't apply anymore, if someone rerolls this I'm happy to have a crack at fixing the tests.

Berdir’s picture

Assigned: Unassigned » Berdir

@rlmumford: Hi! ;) I'm doing a lot of rebasing here to keep the simple replacements separated, the idea is to get them in separately from this in #2049039: Convert node properties to methods.. That issue unfortunately got a bit stuck on #2056721: Remove TypedDataInterface::getType(). So it's better when I do the re-roll to keep my git history.

I'll try to do that asap, but there's not much point in that until the typed data issue is committed as I will have to do some heavy search & replace afterwards. But we can probably continue with fixing tests once I re-rolled, let's sync in IRC when that happened.

Be warned, you might get to see some dark places while trying to fix these tests :)

Berdir’s picture

Status: Needs work » Needs review
FileSize
287.97 KB

Full re-roll based on the referenced issue to see where we are...

Status: Needs review » Needs work

The last submitted patch, node-bc-removal-1939994-84.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
2 KB
288.46 KB

Messed up the forum_submitted merge conflict, this should fix those noticed. Also found the reason for the node access language fails.

Status: Needs review » Needs work

The last submitted patch, node-bc-removal-1939994-86.patch, failed testing.

rlmumford’s picture

Status: Needs work » Needs review
FileSize
575 bytes
288.85 KB

Just going to see if this fixes one of the failed tests.

Status: Needs review » Needs work

The last submitted patch, 1939994-88.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
145.36 KB

Yay, the method issue got in, updated patch.

I'm going to try a different approach with the book module.

Status: Needs review » Needs work

The last submitted patch, 1939994-90.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
6.06 KB
116.82 KB

Re-roll and fixed those tests.

- Node revision turned out to be trivial, I just compared the field object instead of the value
- For book, I reverted the previous changes and used a custom entity builder callback that assigns the book stuff to the node without having to convert it to a field. I hope we can live with that because making it a real field would be very complicated. We could look into it in a follow-up.
- Same for re-translate checkbox of the translation module. That could also be a normal submit callback as it's not really data, more like an action, so no need to go through the node entity hooks.

Let's see if there was new code that relies on the BC decorator since my last re-roll. If not, we might have a green patch.

Status: Needs review » Needs work

The last submitted patch, 1939994-92.patch, failed testing.

Berdir’s picture

Title: [META] Complete conversion of nodes to the new Entity Field API » Complete conversion of nodes to the new Entity Field API
Status: Needs work » Needs review
FileSize
591 bytes
116.75 KB

Close. This should really be green then.

tim.plunkett’s picture

Very little to complain about! Much easier to review with git diff --color-words

  1. +++ b/core/modules/book/book.module
    @@ -327,10 +327,21 @@ function book_form_node_form_alter(&$form, &$form_state, $form_id) {
    +  debug($form_state['values']['book']);
    

    Ahem.

  2. +++ b/core/modules/book/lib/Drupal/book/Tests/BookTest.php
    @@ -281,7 +281,7 @@ function testBookExport() {
    -      $this->assertRaw(check_markup($node->body[Language::LANGCODE_NOT_SPECIFIED][0]['value'], $node->body[Language::LANGCODE_NOT_SPECIFIED][0]['format']), 'Node body found in printer friendly version.');
    +      $this->assertRaw(check_markup($node->body->value, $node->body->format), 'Node body found in printer friendly version.');
    

    It's a beautiful thing!

  3. +++ b/core/modules/forum/forum.module
    @@ -278,17 +278,18 @@ function forum_node_validate(EntityInterface $node, $form) {
    +          // @todo: Does this work?
    

    Well, does it?

  4. +++ b/core/modules/translation/translation.module
    @@ -560,13 +570,13 @@ function translation_language_switch_links_alter(array &$links, $type, $path) {
    +        $nid = $translations[$langcode] instanceof EntityInterface ? $translations[$langcode]->id() : $translations[$langcode]->nid;
    
    +++ b/core/modules/translation/translation.pages.inc
    @@ -42,7 +42,8 @@ function translation_node_overview(EntityInterface $node) {
    +      $nid = $translations[$langcode] instanceof EntityInterface ? $translations[$langcode]->id() : $translations[$langcode]->nid;
    

    These look a bit odd. When is it not an entity?

Berdir’s picture

FileSize
2.23 KB
116.63 KB

- Re-rolled
- Removed debug()
- I can make it even more beatiful ;)
- I think that @todo is ok, but some manual testing of forum.module certainly wouldn't hurt, our test coverage there is certainly not perfect ;) Thinking about it, especially the shadow copy stuff needs to be checked.
- Yeah, the $translations stuff is ugly, sometimes translation.module creates stdClass objects there, sometimes it's a node object. Not too worried about this as there's a critical issue to remove translation.module.

chx’s picture

Status: Needs review » Reviewed & tested by the community

I would say it's good to go. I am a bit ambivalent on $node->getAuthor()->getUsername() vs $node->uid->entity->name as it's nice to have IDE support, doxygen, whatnot but then again a uniform way wouldn't hurt either -- but getAuthor is already in HEAD so let's get this in so that we can finally get rid of the BC decorator.

tim.plunkett’s picture

+++ b/core/modules/book/lib/Drupal/book/Tests/BookTest.php
@@ -281,7 +281,7 @@ function testBookExport() {
-      $this->assertRaw(check_markup($node->body[Language::LANGCODE_NOT_SPECIFIED][0]['value'], $node->body[Language::LANGCODE_NOT_SPECIFIED][0]['format']), 'Node body found in printer friendly version.');
+      $this->assertRaw($node->body->processed, 'Node body found in printer friendly version.');

Wow, you weren't kidding.

+1 RTBC

Berdir’s picture

#96: 1939994-96.patch queued for re-testing.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Nice.

Committed/pushed to 8.x!

Berdir’s picture

Issue tags: -sprint

Removing sprint tag.

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.