Call decorated storage methods in ViewsUI explicitly [#1945348]

Follow up for #1446382-27: Need a reliable way to determine if a specific bundle for an entity type is translatable through #33

Problem/Motivation

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewUI.phpundefined
@@ -985,6 +985,13 @@ public function getOriginalEntity() {
+    return $this->__call(__FUNCTION__, func_get_args());

looks like confusing, possibly infinite looping magic.

But it's a common pattern, in many existing methods in that class.

It pass the call through to __call() which forwards any method call to $this->storage.

Proposed resolution

Instead of just documenting though, change it to call $this->storage directly instead of doing that indirection which just slows things down.

The same way that EntityBCDecorator does it, which means simply $this->storage->isTranslatable() in this case or $this->storage->setNewRevision($value) for an example with arguments. The additional human-overhead of doing this once is not big but it helps both with performance and it's better to understand.

Remaining tasks

make this issue summary accurate

User interface changes

No UI changes.

API changes

No API changes.

Original report by @xjm, @Berdir, @tim.plunkett

in #1446382-27: Need a reliable way to determine if a specific bundle for an entity type is translatable through #33

Comment	File	Size	Author
#10	1945348-10.patch	10.25 KB	damiankloip
#10
#8	1945348-8.patch	10.45 KB	damiankloip
#8
#7	1945348-7.patch	0 bytes	damiankloip
#7
#3	1945348.patch	10.46 KB	damiankloip
#3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

xjm

she/her

English

CreditAttribution: xjm commented 17 March 2013 at 21:06

Priority:	Minor	» Normal
Issue tags:		+Needs issue summary update, +VDC

Comment #2

xjm

she/her

English

CreditAttribution: xjm commented 17 March 2013 at 21:07

Component:

entity system

» views_ui.module

Comment #3

damiankloip CreditAttribution: damiankloip commented 27 March 2013 at 13:00

Status:

Active

» Needs review

File	Size
1945348.patch	10.46 KB

Comment #4

Berdir

German

Switzerland

CreditAttribution: Berdir commented 27 March 2013 at 13:10

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewUI.phpundefined
@@ -733,296 +734,289 @@ public function getFormProgress() {
   /**
-   * Passes through all unknown calls onto the storage object.
-   */
-  public function __call($method, $args) {
-    return call_user_func_array(array($this->storage, $method), $args);

If we also want to support calls to the storage that is not defined in the interface, then we could keep it, just not use it for the explicitly defined methods. I don't know if there are any such calls, I guess the testbot will tell us?

Comment #5

damiankloip CreditAttribution: damiankloip commented 27 March 2013 at 13:28

Yeah, testbot can tell us. Sure thought, if you want I can just add that back in. Or do you want to wait to see if we actually need it? Maybe it does make sense to have it anyway, not sure.

Comment #6

27 March 2013 at 14:28

Status:

Needs review

» Needs work

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

Comment #7

damiankloip CreditAttribution: damiankloip commented 27 March 2013 at 14:55

Status:

Needs work

» Needs review

File	Size
1945348-7.patch	0 bytes

epic fail there.

Comment #8

damiankloip CreditAttribution: damiankloip commented 27 March 2013 at 15:17

File	Size
1945348-8.patch	10.45 KB

Bah..

Comment #9

27 March 2013 at 17:09

Status:

Needs review

» Needs work

The last submitted patch, 1945348-8.patch, failed testing.

Comment #10

damiankloip CreditAttribution: damiankloip commented 27 March 2013 at 17:42

Status:

Needs work

» Needs review

File	Size
1945348-10.patch	10.25 KB

ok, maybe __call needs to stay.

Comment #11

27 March 2013 at 18:32

Status:	Needs review	» Needs work
Issue tags:	-Needs issue summary update, -API clean-up, -VDC

The last submitted patch, 1945348-10.patch, failed testing.

Comment #12

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 27 March 2013 at 18:46

Status:	Needs work	» Needs review
Issue tags:		+Needs issue summary update, +API clean-up, +VDC

#10: 1945348-10.patch queued for re-testing.

Comment #13

dawehner

German

CreditAttribution: dawehner commented 27 March 2013 at 20:31

Status:

Needs review

» Reviewed & tested by the community

Nice!

Comment #14

xjm

she/her

English

CreditAttribution: xjm commented 27 March 2013 at 21:36

Title:

change __call(__FUNCTION__ strangeness to direct calls

» Call decorated storage methods in ViewsUI explicitly

Is this title accurate? I am parroting @dawehner. :D

Comment #15

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 31 March 2013 at 05:25

Status:

Reviewed & tested by the community

» Fixed

Gosh, that's much nicer!

Committed and pushed to 8.x. Thanks!

Comment #16

olli CreditAttribution: olli commented 5 April 2013 at 16:16

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewUI.php
@@ -743,286 +744,286 @@ public function __call($method, $args) {
   public function isTranslatable() {
-    return $this->__call(__FUNCTION__, func_get_args());
+    return $this->isTranslatable();
   }

#1962130: Fatal Error: ViewUI::isTranslatable endless loop