Problem/Motivation

FieldItemList::preSave() calls the list's item's presave method, not the preSave method. Note the camelCase.

Proposed resolution

Simply switch the case of the method name to the correct form.

CommentFileSizeAuthor
#1 fielditemlist_presave-2284017-1.patch443 bytest0xicCode
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

t0xicCode’s picture

t0xicCode’s picture

Status: Active » Needs review
jhodgdon’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

I think this is correct. Thanks! But it is scary that the tests passed without this. Wonder if preSave was tested for lists of items that had their own preSave methods? Apparently not... probably it needs a test?

t0xicCode’s picture

Assigned: t0xicCode » Unassigned

I'm unassign myself since I'm not too sure where to start with test writting.

lokapujya’s picture

We have a test support module named "Field API Test". We could add a presave() to that and then somewhat copy the testDeterminingChanges() in NodeSaveTest. While looking into this, I also noticed that the "Node module tests" support module uses hooks. I wonder if it should be rerolled to override the FieldItemInterface methods (and if we extend the field support module in the same way, that it should use the interface too.

jhodgdon’s picture

The problem in this issue is that a method called "presave" is being called, not "preSave", on the field item class. So we need a plugin with a preSave() method, in order to test, and some way to verify that this method is called for each item in a list.

Berdir’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs tests

method calls in PHP are case insensitive. It is impossible to test this because there's no functional bug here.


class A {
  function test() {
    print "Yes, case insensitive!";
  }
}

$a = new A();
$a->tEsT();
alexpott’s picture

Title: FieldItemList preSave calls the wrong preSave method » camelCase FieldItemList preSave call
Status: Reviewed & tested by the community » Fixed

Committed eb2bfe7 and pushed to 8.x. Thanks!

  • alexpott committed eb2bfe7 on 8.x
    Issue #2284017 by t0xicCode: Fixed CamelCase FieldItemList preSave call.
    

Status: Fixed » Closed (fixed)

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