Problem/Motivation

Moderation state fields extend string and rely on string's sample generation during FieldItemList::generateSampleItems(). This leads to sample data which could be unsave-able since this has a tendency to produce strings like:
"fskldfjwlekrjasdknflkaweruasjkdnfaklsjdjrajsdnfkasdlfjasdklfjaweklrajsdffkl"

Unsurprisingly, that string is unlikely to match to any real moderation state names.

Proposed resolution

ModerationStateFieldItemList should implement generateSampleItems() itself and provide sane sample values.

The method could look thus:


  public function generateSampleItems($count = 1) {
    $state = $this->getModerationStateId();
    $values = [];
    for ($delta = 0; $delta < $count; $delta++) {
      $values[$delta] = $state;
    }
    $this->setValue($values);
  }

Big thanks to @abarrios for hunting this down.

Remaining tasks

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

Moderation state fields will now generate useful sample data.

CommentFileSizeAuthor
#13 3048962-13.patch2.67 KBpaulocs
#13 interdiff-10-12.txt747 bytespaulocs
#10 3048962-10.patch2.74 KBanmolgoyal74
#10 3048962-10-test_only.patch2.01 KBanmolgoyal74
#4 3048962-4.patch2.58 KBSam152
#4 3048962-4--test-only.patch1.86 KBSam152
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

EclipseGc created an issue. See original summary.

Sam152 credited abarrios.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext for Brisbane City Council commented

Good catch, crediting @abarrios.

I think we could actually just empty out the whole method, or just call off to computeValue:

  /**
   * {@inheritdoc}
   */
  public function generateSampleItems($count = 1) {
  }

The moderation state field is computed on demand, and would be populated depending on the sample value generated for the 'published' field. Any getters on the FieldItemList already call off to getModerationStateId.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext for Brisbane City Council commented
Status: Active » Needs review
FileSize
3048962-4--test-only.patch1.86 KB
3048962-4.patch2.58 KB

The last submitted patch, 4: 3048962-4--test-only.patch, failed testing. View results

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

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). 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.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now 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: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

scotwith1t’s picture

scotwith1t
Location Birmingham, AL
CreditAttribution: scotwith1t at Nationwide IT Services for US Geological Survey (USGS) commented

Tested against 8.9.8 and works like a charm. We ran into this with devel_generate creating nodes with entity reference field values and the related entities being created to populate those fields were subject to moderation states and being assigned weird devel_generate-y strings instead of valid values. Thanks for this!

anmolgoyal74’s picture

anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commented
FileSize
3048962-10-test_only.patch2.01 KB
3048962-10.patch2.74 KB

Re-rolled against 9.2.x.

The last submitted patch, 10: 3048962-10-test_only.patch, failed testing. View results

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at iO commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/content_moderation/tests/src/Kernel/ModerationStateFieldItemListTest.php
@@ -8,6 +8,7 @@
 use Drupal\Tests\content_moderation\Traits\ContentModerationTestTrait;
 use Drupal\workflows\Entity\Workflow;
+use Drupal\Tests\user\Traits\UserCreationTrait;

Do we still do these alphabetically? can't remember if we care about that

Other then that this looks great.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented
FileSize
interdiff-10-12.txt747 bytes
3048962-13.patch2.67 KB

Some issues I see that people say about it, but not sure if I already saw a maintainer saying about the alphabetic order.

I attach a patch that fixes it just to prevent.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 486bab8b49 to 9.2.x and e8a0c86a6a to 9.1.x. Thanks!

  • alexpott committed 486bab8 on 9.2.x 
    Issue #3048962 by Sam152, anmolgoyal74, paulocs, EclipseGc, abarrios,...

  • alexpott committed e8a0c86 on 9.1.x 
    Issue #3048962 by Sam152, anmolgoyal74, paulocs, EclipseGc, abarrios,...

Status: Fixed » Closed (fixed)

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