We have a lot of Drupal\user\User, Drupal\node\Node, etc.. references lying around after #1763974: Convert entity type info into plugins, let's fix them.

Some usages are actually bugs at this point (use Drupal\user\User;). This should also help a lot when/if we decide to shorten the naming convention.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, updated_entity_classes.patch, failed testing.

andypost’s picture

Status: Needs work » Needs review

updated_entity_classes.patch queued for re-testing.

sun’s picture

Status: Needs review » Reviewed & tested by the community
tim.plunkett’s picture

+++ b/core/lib/Drupal/Core/Entity/Entity.phpundefined
@@ -251,7 +251,7 @@ public function getIterator() {
-  public function access(\Drupal\user\User $account = NULL) {
+  public function access(\Drupal\user\Plugin\Core\Entity\User $account = NULL) {

+++ b/core/lib/Drupal/Core/Entity/Field/Type/EntityTranslation.phpundefined
@@ -233,7 +233,7 @@ public function isEmpty() {
-  public function access(\Drupal\user\User $account = NULL) {
+  public function access(\Drupal\user\Plugin\Core\Entity\User $account = NULL) {

+++ b/core/lib/Drupal/Core/TypedData/AccessibleInterface.phpundefined
@@ -15,12 +15,12 @@
-  public function access(\Drupal\user\User $account = NULL);
+  public function access(\Drupal\user\Plugin\Core\Entity\User $account = NULL);

None of these should be inline like that, they should all have use statements.

The rest is all docs, but those 3 above are worrying. A sign of no test coverage?

amateescu’s picture

tim.plunkett’s picture

Checked with git diff --color-words, looks good.

catch’s picture

Status: Reviewed & tested by the community » Postponed

I've re-opened the other patch for more discussion, postponing this one.

tstoeckler’s picture

Status: Postponed » Reviewed & tested by the community

Actually this actually just makes the status quo consistent. I don't see a reason not to commit this, even if we end up changing the implementation. I fear that when we decide to roll that one back, that it won't be able to revert cleanly anyway, and this will make it easier to grep for "Plugin/Core/Entity". Moving back to RTBC. I don't feel strongly here, though, so in case someone marks it back I won't mind.

catch’s picture

Status: Reviewed & tested by the community » Postponed

OK no I actually think this is weird. We're not using real plugins for entities, just annotation discovery, so having the class name in the Plugin namespace is confusing.

tim.plunkett’s picture

Berdir’s picture

Status: Postponed » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 1828852-update_entity_classes-5.patch, failed testing.

hussainweb’s picture

Issue summary: View changes
Status: Needs work » Closed (cannot reproduce)

Is this still relevant? I am hard-pressed to find even one instance of the issue described above in the latest HEAD. I guess it got sorted out with everything else? I am marking this as Closed (cannot reproduce) for now. Please reopen if you find this is incorrect.