Revert a2c58316b4047b6190b1e41adb90d052981d5780: back from 'field_body' to 'body'

[wimleers]

Why?

[chris-hamper]

Frontend apps that expect Articles to have a "body" instead of "field_body" will be incompatible with Reservoir. This is an issue for Ember apps, for instance, which have fixed models backing Drupal entities on the frontend.

A translation layer of some sort could be used, instead, but this will likely add confusion among developers who are unfamiliar with Drupal or decoupled architectures.

[wimleers]

That seems like a thin/shallow reason. Because it's not like it'll be impossible for sites to have a body field with another name. Or for the default article content model (node_type) to have a different set of fields. In fact, we encourage people to add more fields.

Isn't it then also a problem that there's an "Related articles" (field_related_articles) field?

We did https://github.com/acquia/reservoir/commit/a2c58316b4047b6190b1e41adb90d052981d5780 for a very clear reason, documented in the commit. If we revert that, then we need to justify why we're retracing our steps. And we need to also look at the complete picture (such as the field_related_articles example of a further modified Article content model).

[tedbow]

I think having Apps that are made for vanilla Drupal work automatically with Reservori would actually be a big win.

Because field_related_articles is not a required field then an app that was made for vanilla Drupal could still work for Reservoir. It just won't update/show this field.

[chris-hamper]

The reason for originally making this change was to decrease confusion around why the "body" field wasn't prefixed by "field_" like all the other fields are. However, I'm coming up with cases where this change will create more confusion, rather than decreasing it.

• App compatibility across distributions, resulting in increased complexity to work around it, or the app simply not being made to work with Reservoir without custom modification
• Drupal developers will expect Article to have "body"
• I could see components or modules being developed for decoupled functionality, either as PoCs or reusable components. If these were designed for use on vanilla Drupal, they would not be compatible with Reservoir's Articles

[wimleers]

But it's still possible for people to change content models, add new required fields, and it'll still break. Although that's of course always going to be a problem.

I think what I'm starting to hear is the need to "lock" content models, so that no more changes can be made to it. You can choose to unlock it, but then you'll need to confirm an explicit warning that this may break clients talking to this Reservoir.

The consequence would be that Reservoir would ship with a locked article content model, and as long as it's locked, it'd work with demo clients, but upon unlocking and making changes, that'd no longer be true.

[tedbow]

@wimleers I don't think it is matter of "locking" a content model. I more think it is a matter of working when you first install it whether it is Reservoir or vanilla Drupal. I think it would be a big win if someone could try either and apps work. Also it would be a big win if there is unknown demo app someone else makes for Drupal/jsonapi just worked with Reservoir.

Of course we could not guarantee some unknown app for Drupal/jsonapi would work but I think this change would greatly increase the odds.

If it was a big change that we needed to make to add this simplification across Reservoir and JSON/API I am not sure if it would be worth but this seems small.

[wimleers]

Oh yes, I agree with that. I'm just saying that the ability to lock it would help prevent confusion :)

[wimleers]

Just discussed this with @tedbow, @mattgrill, @chris-hamper and @prestonso and we concluded:

• we still think https://github.com/acquia/reservoir/commit/a2c58316b4047b6190b1e41adb90d052981d5780 was a good idea
• but compatibility with Drupal core & Contenta is more important
• we may want to revisit https://github.com/acquia/reservoir/commit/a2c58316b4047b6190b1e41adb90d052981d5780 in the future (albeit is very unlikely that this will happen, because the power of defaults means that it's likely never going to change)

So, 👍

[chris-hamper]

@wimleers I'm not clear on what that means

[wimleers]

Once that composer.json change is reverted (i.e. once this is simply reverting 100% of a2c58316b4047b6190b1e41adb90d052981d5780), this is ready.

[chris-hamper]

A straight revert of a2c5831 doesn't seem to work. I get an installation error

[wimleers]

@chris-hamper I think that might've been the brokenness that was introduced by #30 and fixed by #36. IOW: are you sure you were on HEAD? I just did the revert locally, composer install worked fine.

This is blocking the next release.

[chris-hamper]

@wimleers OK, this PR should be a straight-up revert now.

[wimleers]

Fixed by #39.