This is a subissue of #2012776: [META] Improve validation constraint test coverage

This is a test for the Constraint Validation: EntityChangedConstraintValidator

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because automated test
Issue priority Normal because this is focused on one area.
Unfrozen changes Unfrozen because it only changes tests. Not a feature.
CommentFileSizeAuthor
#42 drupal8-entitychanged-2142993-42.patch2.48 KBstefan.r
#39 drupal8-entitychanged-2142993-39.patch13.07 KBstefan.r
#38 interdiff.txt1.73 KBstefan.r
#38 drupal8-entitychanged-2142993-38.patch10.51 KBstefan.r
#36 drupal8-entitychanged-2142993-34_0.patch10.72 KBYesCT
#35 interdiff.txt4.52 KBstefan.r
#34 drupal8-entitychanged-2142993-34.patch10.72 KBstefan.r
#34 drupal8-entitychanged-2142993-34.patch10.72 KBstefan.r
#32 drupal8-entitychanged-2142993-32.patch8.96 KBstefan.r
#26 drupal8-entitychanged-2142993-26.patch8.86 KBstefan.r
#23 drupal8-entitychanged-2142993-23.patch6.31 KBstefan.r
#18 interdiff.txt1.96 KBstefan.r
#18 drupal8-entitychanged-2142993-18.patch8.91 KBstefan.r
#14 drupal8-entitychanged-2142993-14.patch6.71 KBstefan.r
#14 interdiff.txt5.71 KBstefan.r
#10 drupal8-entitychanged-2142993-9.patch7.98 KBstefan.r
#7 drupal8-entitychanged-2142993-7.patch7.13 KBstefan.r
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mariancalinro’s picture

mariancalinro CreditAttribution: mariancalinro commented
Parent issue: » #2012776: [META] Improve validation constraint test coverage
stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Active » Postponed

Postponed upon discussion with fago as the EntityChanged constraint validator will be reworked (or might disappear).

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Why that? it already has been reworked, from NodeChanged to EntityChanged?

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented

See also #2145103: Provide non-configurable field types for entity created, changed and timestamp fields

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Issue tags: +Needs tests
mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

This still a concern?

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
FileSize
drupal8-entitychanged-2142993-7.patch7.13 KB
stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Postponed » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: drupal8-entitychanged-2142993-7.patch, failed testing.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review
FileSize
drupal8-entitychanged-2142993-9.patch7.98 KB

Fix for the failing node validation test as the property path is now changed.0 instead of changed.0.value due to the constraint in ChangedItem running on the field level.

Status: Needs review » Needs work

The last submitted patch, 10: drupal8-entitychanged-2142993-9.patch, failed testing.

Status: Needs work » Needs review

stefan.r queued 10: drupal8-entitychanged-2142993-9.patch for re-testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs review » Needs work

Nice work.

  1. +++ b/core/modules/system/src/Tests/Entity/EntityChangedConstraintValidatorTest.php
@@ -0,0 +1,67 @@
+ * Contains Drupal\system\Tests\Entity\EntityChangedConstraintValidatorTest.

    Should have a leading backlash: \Drupal\... 

  2. +++ b/core/modules/system/src/Tests/Entity/EntityChangedConstraintValidatorTest.php
@@ -0,0 +1,67 @@
+  public static function getInfo() {
+    return array(
+      'name' => 'Entity changed constraint',
+      'description' => 'Tests validation constraints for EntityChangedConstraintValidator.',
+      'group' => 'Validation',
+    );
+  }

    We no longer use getInfo(). Add @group Validation to the annotation instead. 

  3. +++ b/core/modules/system/tests/modules/entity_test/src/Entity/EntityTestChanged.php
@@ -0,0 +1,66 @@
+    $fields['changed'] = BaseFieldDefinition::create('changed')
+      ->setLabel(t('Changed'))
+      ->setDescription(t('The time that the entity was last edited.'))
+      ->setRevisionable(TRUE)
+      ->setTranslatable(TRUE);
+    return $fields;

    I think we should just add changed to the EntityTest entity type and use that. We have a large amount of entity types for specific tests already.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review
FileSize
interdiff.txt5.71 KB
drupal8-entitychanged-2142993-14.patch6.71 KB

Status: Needs review » Needs work

The last submitted patch, 14: drupal8-entitychanged-2142993-14.patch, failed testing.

Status: Needs work » Needs review

stefan.r queued 14: drupal8-entitychanged-2142993-14.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 14: drupal8-entitychanged-2142993-14.patch, failed testing.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
FileSize
drupal8-entitychanged-2142993-18.patch8.91 KB
interdiff.txt1.96 KB

Fix attached for the other tests that were failing upon entity_test having a "changed" field.

As I looked at the failing REST update test, where the EntityChanged validation constraint was making it fail as the PATCH request was passing along a "changed time" situated in the past, I was wondering if there is any case where we want the end user to be able to forcibly/manually update the "changed" field to a date in the past? I guess we don't want the EntityChanged validator to run there in such case?

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review

stefan.r queued 18: drupal8-entitychanged-2142993-18.patch for re-testing.

stefan.r queued 18: drupal8-entitychanged-2142993-18.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 18: drupal8-entitychanged-2142993-18.patch, failed testing.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review
FileSize
drupal8-entitychanged-2142993-23.patch6.31 KB

re-roll

stefan.r queued 23: drupal8-entitychanged-2142993-23.patch for re-testing.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Needs review » Needs work

EntityChangedConstraintValidatorTest is missing from the reroll, this is the main part of the patch.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
FileSize
drupal8-entitychanged-2142993-26.patch8.86 KB
stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review
fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

This will need to adapt once #2429037: Allow adding entity level constraints lands. It also introduces a separate test entity type which implements the EntityChangedInterface. Thus, we should be able to use it for testing here as well and so do away with the need for some other test changes as EntityTest remains unchanged?

anavarre queued 26: drupal8-entitychanged-2142993-26.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 26: drupal8-entitychanged-2142993-26.patch, failed testing.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented

Looks like #2429037: Allow adding entity level constraints is almost in, let me do a re-roll in the mean time...

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
drupal8-entitychanged-2142993-32.patch8.96 KB

reroll

Status: Needs review » Needs work

The last submitted patch, 32: drupal8-entitychanged-2142993-32.patch, failed testing.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Issue summary: View changes
FileSize
drupal8-entitychanged-2142993-34.patch10.72 KB
drupal8-entitychanged-2142993-34.patch10.72 KB

Seems we were sending an invalid changed time over REST without triggering a validation error before... So maybe the whole constraint currently isn't working?

This patch unsets the changed field in REST requests.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review
FileSize
interdiff.txt4.52 KB
YesCT’s picture

YesCT CreditAttribution: YesCT commented
Related issues: +#2420915: patches on issues not being sent to testbot for testing
FileSize
drupal8-entitychanged-2142993-34_0.patch10.72 KB

One of those is not hidden... so I think the test request should have gone.. but it did not.
#2420915-14: patches on issues not being sent to testbot for testing

re-uploading it so it goes to the bot.

Status: Needs review » Needs work

The last submitted patch, 36: drupal8-entitychanged-2142993-34_0.patch, failed testing.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
FileSize
drupal8-entitychanged-2142993-38.patch10.51 KB
interdiff.txt1.73 KB

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review
FileSize
drupal8-entitychanged-2142993-39.patch13.07 KB

Thanks. Here's a patch that should be green, for completeness. Just waiting to hear from @fago whether this issue ought to be closed as a duplicate of #2429037: Allow adding entity level constraints, as there's big overlap between both :)

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented
Status: Needs review » Postponed

As posted there, I think it still makes sense to add a dedicated test case. However, it will need a re-roll once #2429037 lands. As the other one is critical, let's postpone this on it.

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Issue summary: View changes
Status: Postponed » Needs work
Issue tags: +Need reroll, +Needs issue summary update

#2429037: Allow adding entity level constraints went in. unpostponing so the work can start on the reroll.

@stefan.r also, the beta evaluation needs some changes which I think you will see since we worked on the beta evaluation in the other issue.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review
Issue tags: -Need reroll
FileSize
drupal8-entitychanged-2142993-42.patch2.48 KB

Reroll, this runs the validator on the entity instead of on the field.

It also removes the field-level validation, the fix to the validator itself as well as the added changed field to entity_test entity type (as we now have 'entity_test_constraints'). So all that is left is the test anymore :)

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Issue summary: View changes
Issue tags: -Needs issue summary update
stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented

After this patch we only need #2142995: PrimitiveTypeConstraintValidator does not validate some values correctly anymore to have complete test coverage for #2012776: [META] Improve validation constraint test coverage.

In #2142995: PrimitiveTypeConstraintValidator does not validate some values correctly I could use some help on why this last test fails as I am not sure on why some of the primitive type constraints are validated the way they are.

mkalkbrenner queued 42: drupal8-entitychanged-2142993-42.patch for re-testing.

stefan.r queued 42: drupal8-entitychanged-2142993-42.patch for re-testing.

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

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should 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.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should 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.

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

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.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.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.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.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.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.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.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.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work

Needs to be updated for 10.1
See some deprecated calls in the patch.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs work » Postponed (maintainer needs more info)

This still a concern?

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Postponed (maintainer needs more info) » Closed (outdated)

Since there hasn't been movement or a follow up in 8 years going to close for now.

If still valid task please reopen.