Part of meta-issue #2052421: [META] Rename Views properties to core standards

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Reroll the patch if it no longer applies. Yes Instructions
Files: 
CommentFileSizeAuthor
#40 drupal8.views-module.FieldPluginBase.2052591-40.patch36.68 KBl0ke
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,023 pass(es). View
#36 interdiff-2052591-33-36.txt14.98 KBl0ke
#36 drupal8.views-module.FieldPluginBase.2052591-36.patch36.83 KBl0ke
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal8.views-module.FieldPluginBase.2052591-36.patch. Unable to apply patch. See the log in the details link for more information. View
#33 drupal8.views-module.FieldPluginBase.2052591-33.patch33.87 KBmartin107
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,189 pass(es), 13 fail(s), and 3 exception(s). View
#28 drupal8.views-module.FieldPluginBase.2052591-28.patch33.87 KBSumeet.Pareek
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,553 pass(es), 13 fail(s), and 3 exception(s). View
#17 interdiff-11-12.txt970 bytesmartin107
#17 drupal8.views-module.FieldPluginBase.2052591-12.patch31.71 KBmartin107
FAILED: [[SimpleTest]]: [MySQL] 59,481 pass(es), 12 fail(s), and 3 exception(s). View
#13 interdiff-10-11.patch908 bytesmartin107
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View
#13 drupal8.views-module.FieldPluginBase.2052591-11.patch30.81 KBmartin107
FAILED: [[SimpleTest]]: [MySQL] 59,477 pass(es), 12 fail(s), and 3 exception(s). View
#11 interdiff.txt14.61 KBmartin107
#11 drupal8.views-module.FieldPluginBase.2052591-10.patch29.96 KBmartin107
FAILED: [[SimpleTest]]: [MySQL] 59,508 pass(es), 13 fail(s), and 153 exception(s). View
#9 find-usages-twig.png537.36 KBYesCT
#9 drupal8.views-module.FieldPluginBase.2052591-9.patch32.75 KBYesCT
FAILED: [[SimpleTest]]: [MySQL] 57,177 pass(es), 6 fail(s), and 150 exception(s). View
#9 interdiff-6-9.txt2.74 KBYesCT
#6 drupal8.views-module.FieldPluginBase.2052591-6.patch34.23 KBYesCT
FAILED: [[SimpleTest]]: [MySQL] 57,680 pass(es), 6 fail(s), and 150 exception(s). View
#6 interdiff-2-6.txt29.84 KBYesCT
#2 drupal8.views-module.FieldPluginBase.2052591-2.patch5.23 KBYesCT
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php. View

Comments

YesCT’s picture

Assigned: Unassigned » YesCT
YesCT’s picture

Title: Copy of [META] Rename Views properties to core standards » In FieldPluginBase Rename Views properties to core standards
Status: Active » Needs review
FileSize
5.23 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php. View

Should check other classes that extend FieldPluginBase to see if they were using any of its properties?
If they are, we should rename them in those classes too?

There are a bunch of them
https://api.drupal.org/api/drupal/core%21modules%21views%21lib%21Drupal%...

Some of the name replacements were for "vars"
do they need any different treatment?

I gave the properties doc blocks too. Should we do that in general?
It was difficult to figure out what exactly the descriptions should be.

YesCT’s picture

Ah, #1856630-17: [Change notice] [META] Rename Views methods to core standards mentions vars

Note: The PHP 4 method of declaring a variable with the var keyword is still supported for compatibility reasons (as a synonym for the public keyword).

Status: Needs review » Needs work

The last submitted patch, drupal8.views-module.FieldPluginBase.2052591-2.patch, failed testing.

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.phpundefined
@@ -51,22 +51,35 @@
+  protected $fieldAlias = 'unknown';
...
+  protected $aliases = array();

All this variables are used in potentially every subclass. In phpstorm you can rename variables with shift+f6 and rename all usages at the same time. MAGIC

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.phpundefined
@@ -51,22 +51,35 @@
+   $additionalFields = array();

This is the php syntax problem. You either need protected or public.

YesCT’s picture

Status: Needs work » Needs review
FileSize
29.84 KB
34.23 KB
FAILED: [[SimpleTest]]: [MySQL] 57,680 pass(es), 6 fail(s), and 150 exception(s). View

cool.

this is "everything" including comments.
need to check things like the renders.

posting this first, then we will see what it changed too much.

YesCT’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/RowPluginBase.phpundefined
@@ -153,7 +153,7 @@ public function render($row) {
-      '#field_alias' => isset($this->field_alias) ? $this->field_alias : '',
+      '#fieldAlias' => isset($this->field_alias) ? $this->field_alias : '',

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/RssFields.phpundefined
@@ -179,7 +179,7 @@ public function render($row) {
-        'field_alias' => isset($this->field_alias) ? $this->field_alias : '',
+        'fieldAlias' => isset($this->field_alias) ? $this->field_alias : '',

+++ b/core/modules/views/views.moduleundefined
@@ -100,7 +100,7 @@ function views_theme($existing, $type, $theme, $path) {
-    'row' => array('view' => NULL, 'options' => NULL, 'row' => NULL, 'field_alias' => NULL),
+    'row' => array('view' => NULL, 'options' => NULL, 'row' => NULL, 'fieldAlias' => NULL),

+++ b/core/modules/views/views.theme.incundefined
@@ -343,8 +343,8 @@ function template_preprocess_views_view_grouping(&$variables) {
+ * $field->fieldAlias says what the raw value in $row will be. Reach it like
+ * this: @code { $row->{$field->fieldAlias} @endcode

these I thought could use a closer look.

Status: Needs review » Needs work

The last submitted patch, drupal8.views-module.FieldPluginBase.2052591-6.patch, failed testing.

YesCT’s picture

Assigned: YesCT » Unassigned
Status: Needs work » Needs review
FileSize
2.74 KB
32.75 KB
FAILED: [[SimpleTest]]: [MySQL] 57,177 pass(es), 6 fail(s), and 150 exception(s). View
537.36 KB

I'll be away from the computer for a while, and discussion is going on regarding some of this.
I'll take some notes and unassign in case someone wants to try something.

find-usages-twig.png

based on discussion between @dawehner @Cottser @berdir

"Implementation" http://twig.sensiolabs.org/doc/templates.html#variables
it just lowercases everything and compares, I don't see any conversion between under_score and camelCase https://github.com/fabpot/Twig/blob/master/lib/Twig/Template.php#L403

field_alias to fieldAlias would not match since it would convert fieldAlias to fieldalias

how can I know that the field_alias that's referenced in that twig template is referring to the one from FieldPluginBase?

{{ dump() }} in the template file might give you something useful but making that better is a work in progress over a couple of issues
#1783134: [META] Make it possible to inspect twig variables and get information about objects and render arrays and #1922304: Remove TwigReference objects in favor of a high speed implementation by using NodeVisitors more cleverly

In general though I would be looking at the hook_theme() definition, preprocess function, etc
This is a weird case though :)

views_theme:8?
views_theme: Implement hook_theme(). Register views theming functions. => views_theme($existing, $type, $theme, $path) => http://api.drupal.org/api/function/views_theme/8

the field_alias is part of a $variables array that is used in the foreach to register a theme hook for row style plugins
the @see there says to see template_preprocess_views_view_field()

for this we should leave #field_alias as underscores to be consistent with render API and just do:
isset($this->fieldAlias) ? $this->fieldAlias : '',
that way we would leave views_theme() as is...

If we had a row Twig template then it would get the 'field_alias' variable defined in views_theme()
we still need to update views-view-field though because that is accessing an object property

views_view_field() is just getting the row object, not going through row theming... FieldPluginBase::theme() which is called from StylePluginBase::renderFields()

row plugins do have a field alias
but it is not documented on the RowPluginBase class

it being set in the query method
in RowPluginBase
'#fieldAlias' => isset($this->field_alias) ? $this->field_alias : '',

I think this should document (use?) FieldPluginBase

---
So...
leaving views-view-field.html.twig template with: * data = row[field.field_alias]
And changing to
'#field_alias' => isset($this->fieldAlias) ? $this->fieldAlias : '',
in RowPluginBase

--
is + * this: @code { $row->{$field->fieldAlias} @endcode
missing a bracket?

The last submitted patch, drupal8.views-module.FieldPluginBase.2052591-9.patch, failed testing.

martin107’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
29.96 KB
FAILED: [[SimpleTest]]: [MySQL] 59,508 pass(es), 13 fail(s), and 153 exception(s). View
14.61 KB

Simple Reroll

The last submitted patch, 11: drupal8.views-module.FieldPluginBase.2052591-10.patch, failed testing.

martin107’s picture

FileSize
30.81 KB
FAILED: [[SimpleTest]]: [MySQL] 59,477 pass(es), 12 fail(s), and 3 exception(s). View
908 bytes
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

One bug of many cleared.

in DataFieldRow.php there was an outstanding conversion of field_alias to fieldAlias.

Status: Needs review » Needs work

The last submitted patch, 13: interdiff-10-11.patch, failed testing.

The last submitted patch, 13: drupal8.views-module.FieldPluginBase.2052591-11.patch, failed testing.

The last submitted patch, 13: drupal8.views-module.FieldPluginBase.2052591-11.patch, failed testing.

martin107’s picture

Status: Needs work » Needs review
FileSize
31.71 KB
FAILED: [[SimpleTest]]: [MySQL] 59,481 pass(es), 12 fail(s), and 3 exception(s). View
970 bytes

Last patch removed 1 error and 150 exceptions from test.
This patch is expected to remove 1 error!

Hoping someone else might have better luck.

The last submitted patch, 17: drupal8.views-module.FieldPluginBase.2052591-12.patch, failed testing.

headly’s picture

Status: Needs review » Needs work

I've changed the status of the ticket back to "needs work".

martin107’s picture

Issue tags: +Needs reroll
JacobSanford’s picture

Issue tags: -Needs reroll

Removing 'Needs reroll' as patch applies cleanly for me.

martin107’s picture

Hmm this is what I get when I apply to the 8.x branch

It looks to me like the patch no longer applies

Jacob could you post what commit you tested against ?

mine was

commit 1167235b96d773457f26f952368f4fa643d36e00
Author: Jennifer Hodgdon
Date: Mon Jun 2 10:24:20 2014 -0700

and the result of

git apply drupal8.views-module.FieldPluginBase.2052591-12.patch

was

drupal8.views-module.FieldPluginBase.2052591-12.patch:444: trailing whitespace.
 error: core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/field/TitleLink.php: No such file or directory
error: core/modules/comment/lib/Drupal/comment/Plugin/views/field/Comment.php: No such file or directory
error: core/modules/comment/lib/Drupal/comment/Plugin/views/field/LastTimestamp.php: No such file or directory
error: core/modules/comment/lib/Drupal/comment/Plugin/views/field/NodeNewComments.php: No such file or directory
error: core/modules/comment/lib/Drupal/comment/Plugin/views/field/Username.php: No such file or directory
error: core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php: No such file or directory
error: core/modules/file/lib/Drupal/file/Plugin/views/field/File.php: No such file or directory
error: core/modules/file/lib/Drupal/file/Plugin/views/field/FileMime.php: No such file or directory
error: core/modules/file/lib/Drupal/file/Plugin/views/field/Uri.php: No such file or directory
error: core/modules/history/lib/Drupal/history/Plugin/views/field/HistoryUserTimestamp.php: No such file or directory
error: core/modules/node/lib/Drupal/node/Plugin/views/field/Node.php: No such file or directory
error: core/modules/node/lib/Drupal/node/Plugin/views/field/Path.php: No such file or directory
error: core/modules/node/lib/Drupal/node/Plugin/views/field/Revision.php: No such file or directory
error: core/modules/node/lib/Drupal/node/Plugin/views/field/RevisionLink.php: No such file or directory
error: core/modules/rest/lib/Drupal/rest/Plugin/views/row/DataFieldRow.php: No such file or directory
error: core/modules/search/lib/Drupal/search/Plugin/views/field/Score.php: No such file or directory
error: core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/views/field/LinkEdit.php: No such file or directory
error: core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/views/field/Taxonomy.php: No such file or directory
error: core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/views/field/TaxonomyIndexTid.php: No such file or directory
error: core/modules/user/lib/Drupal/user/Plugin/views/field/Link.php: No such file or directory
error: core/modules/user/lib/Drupal/user/Plugin/views/field/Name.php: No such file or directory
error: core/modules/user/lib/Drupal/user/Plugin/views/field/Permissions.php: No such file or directory
error: core/modules/user/lib/Drupal/user/Plugin/views/field/Roles.php: No such file or directory
error: core/modules/user/lib/Drupal/user/Plugin/views/field/User.php: No such file or directory
error: core/modules/views/lib/Drupal/views/Plugin/views/field/EntityLabel.php: No such file or directory
error: core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php: No such file or directory
error: core/modules/views/lib/Drupal/views/Plugin/views/field/MachineName.php: No such file or directory
error: core/modules/views/lib/Drupal/views/Plugin/views/field/Markup.php: No such file or directory
error: core/modules/views/lib/Drupal/views/Plugin/views/field/Serialized.php: No such file or directory
error: core/modules/views/lib/Drupal/views/Plugin/views/field/TimeInterval.php: No such file or directory
error: core/modules/views/lib/Drupal/views/Plugin/views/row/RowPluginBase.php: No such file or directory
error: core/modules/views/lib/Drupal/views/Plugin/views/row/RssFields.php: No such file or directory
error: core/modules/views/lib/Drupal/views/Tests/Handler/FieldUnitTest.php: No such file or directory
JacobSanford’s picture

Issue tags: +Needs reroll

Completely my error, I was squelching file-not-found errors. Resetting tag.

dcam’s picture

Issue summary: View changes

Updating in preparation for DrupalCon Austin sprints. See http://www.hook42.com/blog/prepping-drupalcon-austin-sprints-sprint-lead....

dbcollies’s picture

I'm going to start working on this

dbcollies’s picture

Status: Needs work » Closed (fixed)
Related issues: +#2052421: [META] Rename Views properties to core standards

I've reviewed this, and it appears the work has already been completed. The only issue I found was with the attribute tableAlias, which is inherited from Drupal\views\Plugin\views\HandlerBase.

dbcollies’s picture

Status: Closed (fixed) » Needs work

I misunderstood the standards. This issue still needs to be worked.

Sumeet.Pareek’s picture

Issue tags: -Needs reroll
FileSize
33.87 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,553 pass(es), 13 fail(s), and 3 exception(s). View

Re-rolled the patch so that it cleanly applies now after the PSR4 changes. This helped - https://groups.drupal.org/node/424758

martin107’s picture

Status: Needs work » Needs review

triggering testbot

Status: Needs review » Needs work

The last submitted patch, 28: drupal8.views-module.FieldPluginBase.2052591-28.patch, failed testing.

aczietlow’s picture

Assigned: Unassigned » aczietlow

I'm going to start working on this.

aczietlow’s picture

Assigned: aczietlow » Unassigned
martin107’s picture

Status: Needs work » Needs review
FileSize
33.87 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,189 pass(es), 13 fail(s), and 3 exception(s). View

Straight reroll before I can work on any errors.

no conflicts just merging and auto-merging.

PS oh and cleared white space issue.

Status: Needs review » Needs work

The last submitted patch, 33: drupal8.views-module.FieldPluginBase.2052591-33.patch, failed testing.

l0ke’s picture

Assigned: Unassigned » l0ke
l0ke’s picture

Status: Needs work » Needs review
FileSize
36.83 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal8.views-module.FieldPluginBase.2052591-36.patch. Unable to apply patch. See the log in the details link for more information. View
14.98 KB

I've made the patch from scratch, a lot of stuff were changed.
I think there is no reason to make fieldAlias and aliases protected, otherwise we should provide getters and setters that might should be a separate issue.
Also tests should now pass.

l0ke’s picture

Issue tags: +#ams2014contest

Status: Needs review » Needs work

The last submitted patch, 36: drupal8.views-module.FieldPluginBase.2052591-36.patch, failed testing.

l0ke’s picture

Status: Needs work » Needs review
Issue tags: - +Amsterdam2014
FileSize
36.68 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,023 pass(es). View

Simple re-roll.

YesCT’s picture

Status: Needs review » Reviewed & tested by the community

I read through the patch, also applied it and looked at it in my IDE. looks good.

if things need to be protected, let's do that in a separate issue.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
      // Build the condition using the selected search fields.
      foreach ($style_options['options']['search_fields'] as $field_alias) {
        if (!empty($field_alias)) {
          // Get the table and field names for the checked field.
          $field = $this->view->query->fields[$this->view->field[$field_alias]->field_alias];
          // Add an OR condition for the field.
          $conditions->condition($field['table'] . '.' . $field['field'], $value, 'LIKE');
        }
      }

How come this ->field_alias was not converted? It is \Drupal\entity_reference\Plugin\views\display\EntityReference. I just this is because we are not converting RowPluginBase::$field_alias - but this seems super fragile considering the patch does things like to row plugins.

+++ b/core/modules/views/src/Plugin/views/row/OpmlFields.php
@@ -200,7 +200,7 @@ public function render($row) {
-      '#field_alias' => isset($this->field_alias) ? $this->field_alias : '',
+      '#fieldAlias' => isset($this->field_alias) ? $this->field_alias : '',

+++ b/core/modules/views/src/Plugin/views/row/RowPluginBase.php
@@ -175,7 +175,7 @@ public function render($row) {
-      '#field_alias' => isset($this->field_alias) ? $this->field_alias : '',
+      '#fieldAlias' => isset($this->field_alias) ? $this->field_alias : '',

+++ b/core/modules/views/src/Plugin/views/row/RssFields.php
@@ -178,7 +178,7 @@ public function render($row) {
-      '#field_alias' => isset($this->field_alias) ? $this->field_alias : '',
+      '#fieldAlias' => isset($this->field_alias) ? $this->field_alias : '',
YesCT’s picture

thinking through this

alexpott points out that in OpmlFields there is a $this->field_alias and that it might should be converted in this issue also.
I looked in OpmlFields
there are some $this->words where words are not defined properties on the OpmlFields class, but are known...
like $this->options which comes from the PluginBase
class OpmlFields extends RowPluginBase
abstract class RowPluginBase extends PluginBase
but where is $this->field_alias coming from?
seems like it might be a magic property being set on OpmlFields

should it also be extending something else that has field_alias property? or should itself have a field_alias property?

YesCT’s picture

I'm not sure if the last bit of #43 is a bug that needs a separate issue or not.

I was going to mark this postponed according to https://www.drupal.org/contribute/core/beta-changes
but it might get in under the amsterdam exception.. except it has been more than a month now.

YesCT’s picture

Issue tags: -Novice

No novice task on this anymore, and we dont even know if it should be worked on right now, so removing the novice tag per https://www.drupal.org/core-mentoring/novice-tasks

alexpott’s picture

Status: Needs work » Postponed

Postponing as per #43

alexpott’s picture

Version: 8.0.x-dev » 8.1.x-dev

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.