Support from Acquia helps fund testing for Drupal Acquia logo

Comments

toddtomlinson’s picture

Assigned: Unassigned » toddtomlinson
toddtomlinson’s picture

Status: Active » Needs review
FileSize
10.32 KB

Replaced get_file with getFile across several views.core files. Patch attached.

dbcollies’s picture

Status: Needs review » Needs work

The last submitted patch, Rename-Views-method-get_field-to-getField-2002910-1.patch, failed testing.

dbcollies’s picture

I believe there to be a bug in the test, not in the code submitted in comment #3. The test fails randomly, with no change in the code

toddtomlinson’s picture

Status: Needs work » Needs review
dbcollies’s picture

I think the patch in comment #2 (and I apologize that I didn't notice you had taken this task before I made my patch) is missing changes to the following two files:

  • core/modules/contextual/lib/Drupal/contextual/Plugin/views/field/ContextualLinks.php
  • core/modules/field/lib/Drupal/field/Tests/Views/HandlerFieldFieldTest.php

I suspect the second one is what's causing the failure in HandlerFieldFieldTest. I'm convinced that it's something outside of either of these patches that's causing the BlockRenderOrderTest random failure

dbcollies’s picture

Status: Needs review » Needs work

The last submitted patch, Rename-Views-method-get_field-to-getField-2002910-1.patch, failed testing.

oenie’s picture

Status: Needs work » Needs review
Issue tags: +Novice, +VDC
oenie’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/RssFields.phpundefined
@@ -187,15 +187,15 @@ function render($row) {
+  function getField($index, $field_id) {

Add public access modifier in front of the function to adher to the new OOP standards.

diarmy’s picture

Status: Needs work » Needs review
FileSize
683 bytes
9.54 KB

Added public access modifier to getField function in RssFields class as recommended in #11

Status: Needs review » Needs work

The last submitted patch, Rename-Views-method-get_field-to-getField-2002910-12.patch, failed testing.

diarmy’s picture

Updated instances of get_field() in HandlerFieldFieldTest.

diarmy’s picture

Status: Needs work » Needs review

Updating status to needs review.

oenie’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me now !

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Seems like we've added a new one in Drupal\contextual\Plugin\views\field\ContextualLinks

$rendered_field = $this->view->style_plugin->get_field($this->view->row_index, $field);
diarmy’s picture

Replaced more instances of get_field() with getField()

diarmy’s picture

Status: Needs work » Needs review
dawehner’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/argument/ArgumentPluginBase.phpundefined
@@ -740,7 +740,7 @@ function hasDefaultArgument() {
-  function get_default_argument() {
+  public function getDefaultArgument() {

@@ -1014,7 +1014,7 @@ function get_value() {
-      $arg = $argument->get_default_argument();
+      $arg = $argument->getDefaultArgument();

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/argument/Date.phpundefined
@@ -57,7 +57,7 @@ public function defaultArgumentForm(&$form, &$form_state) {
-  function get_default_argument($raw = FALSE) {
+  public function getDefaultArgument($raw = FALSE) {

Out of scope for this issue.

heddn’s picture

Let's see if this makes the testbot happy.

heddn’s picture

Status: Needs work » Needs review
dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Awesome!

oenie’s picture

Looks good to me !

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed b48343a and pushed to 8.x. Thanks!

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