Problem/Motivation

Yet another bug unveiled by #2590993: Create stub entities with proper default values! We use generateSampleValue() to fill in stubs for referenced entities during migrate, and an entity with a uri field fails validation because the value is something like swuuobrouutidis... The problem is that UriItem does not implement generateSampleValue(), so it falls through to StringItem::generateSampleValue().

Proposed resolution

Implement UriItem::generatSampleValue() to produce a valid uri.

Remaining tasks

Do it, with a test.

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#4 2605150-4.patch1.59 KBmikeryan
#4 2605150-4-FAIL.patch556 bytesmikeryan
#2 2605150-2.patch1.05 KBmikeryan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikeryan created an issue. See original summary.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
Status: Active » Needs review
Issue tags: +Needs tests
FileSize
2605150-2.patch1.05 KB

Tried to start with a unit test to demonstrate the failure, but got hung up trying to figure out how to mock the pieces to get me a field definition to pass generateSampleValue(). So, for now, here's just a fix without a test...

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
Issue tags: +rc target triage

Aiming for RC because without it we'll need a work-around in #2590993: Create stub entities with proper default values.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
Issue tags: -Needs tests
FileSize
2605150-4-FAIL.patch556 bytes
2605150-4.patch1.59 KB

Most of the other primitive types had tests for generateSampleValue(), added one to UriItemTest. That test seems rather thin generally compared to the other field types, but out-of-scope for this immediate issue...

The last submitted patch, 4: 2605150-4-FAIL.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 4: 2605150-4.patch, failed testing.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
Status: Needs work » Needs review

That's better.

The last submitted patch, 4: 2605150-4-FAIL.patch, failed testing.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

Looks great. Uses \Drupal\Component\Utility\Random::word(), therefore it will always generate valid hostnames.

The last submitted patch, 4: 2605150-4-FAIL.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -rc target triage +rc target

D'oh. Nice catch. Speaking of catch, showed this to him and he felt it was fine to get in during RC, so making an RC target.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed ea543f1 on 8.0.x 
    Issue #2605150 by mikeryan: UrlItem::generateSampleValue() does not...

  • webchick committed ea543f1 on 8.1.x 
    Issue #2605150 by mikeryan: UrlItem::generateSampleValue() does not...

Status: Fixed » Closed (fixed)

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