Problem/Motivation

In order to do better profiling, we need good placeholder data in core. We're seeing that at #2308683: [meta] Create a 'Profiling' install profile, for testbot-powered simple profiling and easier local profiling. We also need placeholder data for for stubbing out sites as they are built.

Proposed resolution

Put parts of devel_generate module in core. This issue is about letting Field types generate their own sample values.

Remaining tasks

None.

User interface changes

None.

API changes

Adds a generateSampleItems() method to FieldItemListInterface, and generateSampleValues() static methods to *Item classes.

CommentFileSizeAuthor
#60 interdiff.txt2.32 KBmoshe weitzman
#60 generate_placeholder-2320157-60.patch40.88 KBmoshe weitzman
#55 generate_placeholder-2320157-55.patch39.37 KBmoshe weitzman
#53 interdiff.txt548 bytesmoshe weitzman
#53 generate_placeholder-2320157-53.patch39.37 KBmoshe weitzman
#51 interdiff.txt1.77 KBmoshe weitzman
#51 generate_placeholder-2320157-51.patch39.14 KBmoshe weitzman
#47 interdiff.txt16.17 KBmoshe weitzman
#47 generate_placeholder-2320157-47.patch39.38 KBmoshe weitzman
#43 interdiff.txt20.29 KBmoshe weitzman
#43 generate_placeholder-2320157-43.patch40.95 KBmoshe weitzman
#40 interdiff.txt31.06 KBtim.plunkett
#39 generate_placeholder-2320157-39.patch38.22 KBmoshe weitzman
#34 interdiff.txt19.54 KBmoshe weitzman
#34 generate_placeholder-2320157-34.patch34.15 KBmoshe weitzman
#28 interdiff.txt4.59 KBmoshe weitzman
#28 generate_placeholder-2320157-28.patch32.92 KBmoshe weitzman
#26 generate_placeholder-2320157-26.patch32.85 KBmoshe weitzman
#24 2320157-generate.interdiff.23-24.txt4.56 KBpenyaskito
#24 2320157-generate-24.patch32.74 KBpenyaskito
#23 generate_placeholder-2320157-23.patch32.82 KBmoshe weitzman
#21 interdiff.txt6.3 KBmoshe weitzman
#21 generate_placeholder-2320157-21.patch33.52 KBmoshe weitzman
#19 interdiff.txt10.04 KBWim Leers
#19 generate_placeholder-2320157-19.patch33.24 KBWim Leers
#17 interdiff.patch2.21 KBmoshe weitzman
#17 generate_placeholder-2320157-17.patch32.43 KBmoshe weitzman
#13 generate_placeholder-2320157-13.patch32.57 KBmoshe weitzman
#9 generate_placeholder-2320157-9.patch12.72 KBmoshe weitzman
#6 generate_dummy_content-2320157-6.patch7.62 KBmoshe weitzman
#2 generate_dummy_content-2320157-2.patch7.61 KBmoshe weitzman
#1 generate_dummy_content-2320157-1.patch10.77 KBmoshe weitzman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

Issue tags: +Performance
FileSize
10.77 KB

LinkItem class now has generate(), along with minimalist testing. I think validity testing is the responsibility of Field Validation system which can be a followup. Field validation applies to user submitted data just as much as generated data.

moshe weitzman’s picture

Now with the right patch

penyaskito’s picture

Status: Active » Needs review

I like the idea.

The last submitted patch, 1: generate_dummy_content-2320157-1.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 2: generate_dummy_content-2320157-2.patch, failed testing.

moshe weitzman’s picture

Status: Needs work » Needs review
FileSize
7.62 KB

Fix method name to fix test fatal.

moshe weitzman’s picture

Title: Generate dummy content for Field types » Generate placeholder content for Field types
swentel’s picture

I like the idea. Can't really think of a reason why we wouldn't do this.

2 small things in docs found.

  1. +++ b/core/lib/Drupal/Component/Utility/Random.php
    @@ -155,4 +155,91 @@ public function object($size = 4) {
    +   * Build a string containing Latin words, often used as placeholder text.
    +   *
    +   * @param int $word_count
    +   * @param bool $title
    +   *   Uppercase all the words in the string.
    +   * @return string
    

    Documentation needs some cleanup

  2. +++ b/core/lib/Drupal/Core/Field/FieldItemInterface.php
    @@ -208,6 +208,13 @@ public function update();
    +   * Useful when populating a site with dummy content during site building or profiling.
    

    80 chars

moshe weitzman’s picture

Fixed those nits - thanks.

Now with date, taxonomyRef, entity_ref fields implementing generate() along with tests for each. Feedback welcome.

dawehner’s picture

+++ b/core/lib/Drupal/Component/Utility/Random.php
@@ -155,4 +155,93 @@ public function object($size = 4) {
+   * Build a string containing Latin words, often used as placeholder text.
...
+  public function createGreeking($word_count, $title = FALSE) {

Greek != Latin.
As far as I remember back from school these words are latin.

penyaskito’s picture

Some nitpicks.

  1. +++ b/core/lib/Drupal/Component/Utility/Random.php
    @@ -155,4 +155,93 @@ public function object($size = 4) {
    +   * @param int $word_count
    +   * @param bool $title
    +   *   Uppercase all the words in the string.
    

    Even if it is obvious, I guess we should add docs for $word_count.
    Could we rename $title to $uppercase?

  2. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
    @@ -210,6 +210,17 @@ public function preSave() {
    +    if ($referenceble = \Drupal::service('plugin.manager.entity_reference.selection')->getSelectionHandler($this->getFieldDefinition(), $this->getentity())->getReferenceableEntities()) {
    

    $referenceable?

  3. +++ b/core/modules/datetime/src/Plugin/Field/FieldType/DateTimeItem.php
    @@ -104,6 +104,23 @@ public function settingsForm(array &$form, FormStateInterface $form_state, $has_
    +    $timestamp = time()-mt_rand(0, 86400*365);
    

    Spaces before and after -.

penyaskito’s picture

moshe weitzman’s picture

New patch with all field types implemented. I an now both validating and saving the test entity and have verified that Field constraints are being applied.

  1. #11.1 - I refuse to add more words which say the exact same thing as the variable name. Reist with repetitive docs
  2. #11.2 - This comes from the method name getReferenceableEntities
  3. #11.3 - Fixed

Status: Needs review » Needs work

The last submitted patch, 13: generate_placeholder-2320157-13.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 13: generate_placeholder-2320157-13.patch, failed testing.

moshe weitzman’s picture

Status: Needs work » Needs review
FileSize
32.43 KB
2.21 KB

Hopefully fix fails.

Status: Needs review » Needs work

The last submitted patch, 17: interdiff.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Needs review
FileSize
33.24 KB
10.04 KB

Looks simple & sane. Comes with test coverage. Good to go AFAICT.

That leaves only nitpicks. I already fixed most, which leaves only a few:

  1. +++ b/core/lib/Drupal/Component/Utility/Random.php
    @@ -155,4 +155,149 @@ public function object($size = 4) {
    +   * @return string
    +   *   Nonsense latin words.
    +   */
    +  public function createGreeking($word_count, $title = FALSE) {
    

    This confused me very much, just like dawehner in #10. #12 contained the answer. Perhaps link to http://en.wikipedia.org/wiki/Greeking here, to avoid future issues being opened about this? :)

    (Or, alternatively, something like createRandomPhrase()?)

  2. +++ b/core/lib/Drupal/Component/Utility/Random.php
    @@ -155,4 +155,149 @@ public function object($size = 4) {
    +   *    The expected word length.
    

    Doesn't "expected" imply the possibility for a deviation, i.e. "could be more, could be less"?

    Wouldn't "desired" or some other adjective be more accurate?

  3. +++ b/core/lib/Drupal/Component/Utility/Random.php
    @@ -155,4 +155,149 @@ public function object($size = 4) {
    +  public function generateImage($extension = 'png', $min_resolution, $max_resolution) {
    

    Wow, cool!

Wim Leers’s picture

Status: Needs review » Needs work
moshe weitzman’s picture

Status: Needs work » Needs review
FileSize
33.52 KB
6.3 KB

Fixed Wim's items from #19 and added PathItem::generate()

moshe weitzman’s picture

Issue summary: View changes
moshe weitzman’s picture

Woops, I had missed Wim's patch. Thats incorporated here. See his interdiff in #19.

penyaskito’s picture

Renamed $title parameter to $capitalize. Some coding standards nitpicks.

This patch is adding calls in Drupal/Component/Utility/Random to drupal_* functions, which makes me think that we should move it to Drupal/Core/Utility/Random

effulgentsia’s picture

This patch is adding calls in Drupal/Component/Utility/Random to drupal_* functions, which makes me think that we should move it to Drupal/Core/Utility/Random

Or, generateImage() could return the image handle, and the directory/file creation could be moved to FileItem::generate().

moshe weitzman’s picture

Moved the drupally bits of of Random::generateImage as per effulgentsia in #25. I moved too quickly and did a --amend do I can't provide an interdiff. The only changes were the top and bottom of Random::generateImage() its call in ImageItem::generate().

penyaskito’s picture

#26 misses changes at #24.

moshe weitzman’s picture

Woops, now incorporated #24. Sorry about that.

yched’s picture

Will try to look at the patch soonish.

Meanwhile, nitpicky, but I'd like if we could find a more specific method name for"generate()" - somthing that conveys the idea that it's about dummy sample content. Was clearer while those lived in *devel*_generate.module, much less if it's a generate() method among the other ones in FieldItemInterface.

Would generateSample() work ?

Wim Leers’s picture

#29: I felt the same way, but couldn't think of something better. I kind of like generateSample(). Or maybe setRandomValue(), or setSampleValue()?

(But let's be careful to not over-bikeshed this. :))

yched’s picture

Well, nothing forces the value to be "random", right ?
Also, if anything, it would be more a getter than a setter ?

I think generateSample() still has my vote. Or (question to native english speakers) does "sample" sound weird if used as a standalone noun ? generateSampleValue() then ?

effulgentsia’s picture

Also, if anything, it would be more a getter than a setter ?

Well, as currently implemented, generate() performs a $this-setValue() on what it generates, so it's a setter. However, what about renaming it to generateSampleValue() and making it return the property values array only, requiring the caller to then also call ->setValue() with that result if that's what it wants to do? Pro: makes each step more explicit. Con: for the common case, it requires the caller to call two methods instead of one.

Related, I think we should remove the "generate" and "create" prefixes within the methods being added to the Random class, since for that class, it's already implied, and it would better match the existing methods in that class.

yched’s picture

Well, as currently implemented, generate() performs a $this-setValue() on what it generates, so it's a setter

Oh - I should have actually looked at the patch :-p

I'd tend to agree with one dedicated method to generate the raw values, and them being set separately by the caller. Stricly speaking, the "sample generation" is the logical unit each field type needs to override, the "set" part is common.

moshe weitzman’s picture

Renamed method to generateSampleValue() and leaving setValue() to the caller.

yched’s picture

Status: Needs review » Needs work
  1. +++ b/core/lib/Drupal/Component/Utility/Random.php
    @@ -155,4 +155,150 @@ public function object($size = 4) {
    +  public function createSentences($min_word_count, $capitalize = FALSE) {
    ...
    +  public function generateWord($length) {
    ...
    +  public function generateParagraphs($paragraph_count = 12) {
    ...
    +  public function generateImage($destination, $min_resolution, $max_resolution) {
    

    - generate / create ?
    The existing methods in the Random class use no verb at all (->name(), ->string()...), we should be consistent with what's already in there ?

    - The relation between the methods is not what you'd expect:
    generateParagraphs() uses createSentence(), but createSentence() does not use generateWord().

    --> Maybe add "latin" in the method names for paragraphs and sentences, and leave word as a separate, unrelated thing ?

    - about "word": the class already has string() and name(), do we really need a 3rd variant ?
    If so, it should be added next to the other ones, rather than in the middle of the "latin" stuff it has no connection with.

  2. +++ b/core/lib/Drupal/Core/Field/FieldItemInterface.php
    @@ -208,6 +208,17 @@ public function update();
       /**
    +   * Generates placeholder, valid field values.
    +   *
    +   * Useful when populating site with placeholder content during site building
    +   * or profiling.
    +   *
    +   * @return array
    +   *   An associative array of values.
    +   */
    +  public function generateSampleValue();
    

    IMO we could add that it's the responsibility of the implementation to respect the field settings and produce values that will pass validation.

    Also, could we add it somewhere else than between delete() and deleteRevision() ? :-p

  3. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/BooleanItem.php
    @@ -113,5 +113,13 @@ public function getSettableOptions(AccountInterface $account = NULL) {
    +  public function generateSampleValue() {
    ...
    +  }
     }
    

    Missing empty line after last method

  4. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/BooleanItem.php
    @@ -113,5 +113,13 @@ public function getSettableOptions(AccountInterface $account = NULL) {
    +      $values['value'] = array_rand(array_flip($allowed));
    +      return $values;
    

    that kind of code would look slightly simpler if the returned array was inlined directly in the return statement.

    Matter of taste, feel free to ignore.

  5. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/DecimalItem.php
    @@ -97,4 +97,51 @@ public function preSave() {
    +
    +
    ...
    +  function getDecimalDigits($decimal) {
    

    - Extra empty line
    - Method misses visibility - protected ?

  6. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/IntegerItem.php
    @@ -102,4 +102,12 @@ public static function schema(FieldStorageDefinitionInterface $field_definition)
    +    $values['value'] = mt_rand($this->getSetting('min'), $this->getSetting('max') ?: 10000);
    

    Will that work if 'min' is not set ?
    if 'max' is 0 ?

  7. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/NumericItemBase.php
    @@ -113,4 +113,19 @@ public function getConstraints() {
    +  function truncateDecimal($decimal, $num) {
    

    lacks visibility

  8. +++ b/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
    @@ -312,6 +314,56 @@ public function preSave() {
    +      if ($path = $random->generateImage(drupal_realpath($destination), $min_resolution, $max_resolution)) {
    ...
    +      else {
    +        return FALSE;
    +      }
    

    Not sure what will happen then ?

  9. +++ b/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
    @@ -312,6 +314,56 @@ public function preSave() {
    +    $this->setValue($values);
    

    leftover - thus lacks test coverage ?

  10. +++ b/core/modules/path/src/Plugin/Field/FieldType/PathItem.php
    @@ -88,4 +89,13 @@ public function delete() {
    +    $values['alias'] = str_replace(' ', '-', $random->createSentences(3));
    +    return $values;
    

    We generate a sample alias using capitalized sentences ? Surprising ?

    Also, is that enough to provide a sample PathItem ? No pid ?

  11. The patch leaves out a bunch Core field types ?
    ChangedItem
    CreatedItem
    LanguageItem
    MapItem
    StringItem
    StringLongItem
    TimestampItem
    UriItem
    UuidItem

[edit: removed a couple "plz move generateSampleValue() implementation to the end of the class" items, pointless]

yched’s picture

+++ b/core/modules/entity_reference/src/Tests/EntityReferenceItemTest.php
@@ -112,6 +112,14 @@ public function testContentEntityReferenceItem() {
+    $values = $entity->field_test_taxonomy_term->first()->generateSampleValue();
+    $entity->field_test_taxonomy_term->first()->setValue($values);
+    $values = $entity->field_test_taxonomy_vocabulary->first()->generateSampleValue();
+    $entity->field_test_taxonomy_vocabulary->first()->setValue($values);

Yeah, that's tedious :-/

We could add FieldItemList::generateSampleItems($count), that would take care of creating the items and setting their sample values ?
--> $entity->field_foo->generateSampleItems(3);

yched’s picture

Which also makes me think that FieldItemInterface::generateSampleValues() should IMO be static, with a FieldDefinitionInterface param : you shouldn't need to have an instanciated Item object in order to create a sample one.

FieldItemList::generateSampleItems() would then figure out the item class, call the static method to get the sample values, and call $this->createItem($delta, $sample_values).

yched’s picture

BTW, having the ability to generate sample field values in core means we could provide "entity previews" from the "Manage display" UI screens (could be in contrib, or even in core 8.1.x ?)

moshe weitzman’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
38.22 KB

Attached patch implements everything mentioned by yched in the last 3 comments. I only omitted 3 items (see below). Hopefully I got the static method stuff right. Sorry about the missing interdiff. I am trying to improve my habits.

I have one problem which is causing fails in EntityReferenceItemTest, TaxonomyTermReferenceItemTest, and BooleanItemTest. Now that generateSampleValue() is called statically, I can't use $this in a line like $referenceable = $manager->getSelectionHandler($field_definition, $this->getentity())->getReferenceableEntities(). is there some way I can get an $entity or avoid the call to getSelectionHandler()?

1. Maybe add "latin" in the method names for paragraphs and sentences, and leave word as a separate, unrelated thing ?

I thought about this and I don’t see that adding Latin to the name as a net win. The two method names just get longer. Also, the behavior of word() is different enough from string() and name() to justify its existence. I was a bit surprised as well.

10. Also, is that enough to provide a sample PathItem ? No pid ?

The pid gets created during \Drupal\path\Plugin\Field\FieldType\PathItem::insert

11. The patch leaves out a bunch Core field types

I just focused on the field types that are likely to be added by users to ContentEntities. What would be the value to adding generation for ChangedItem, uuid, etc. I don’t mind doing it, but lets identify which ones and why.

tim.plunkett’s picture

FileSize
31.06 KB

You can always create an interdiff after the fact.

git checkout -b before 8.0.x;
curl https://www.drupal.org/files/issues/generate_placeholder-2320157-34.patch | git apply --index;
git commit -m 'before';
git checkout -b after 8.0.x;
curl https://www.drupal.org/files/issues/generate_placeholder-2320157-39.patch | git apply --index;
git commit -m 'after';
git diff before..after > interdiff.txt

Status: Needs review » Needs work

The last submitted patch, 39: generate_placeholder-2320157-39.patch, failed testing.

yched’s picture

is there some way I can get an $entity or avoid the call to getSelectionHandler()?

The $entity would need to be passed as a param to the generateSampleValue() method ? It seems right that the method is "generate sample values for this $field_definition in this $entity" ?
FieldItemList::generateSampleItems() can encapsulate this by taking care of passing $this->getEntity().

I just focused on the field types that are likely to be added by users to ContentEntities. What would be the value to adding generation for ChangedItem, uuid

Well, node->created uses CreatedItem, node->title uses StringItem... - how can we generate sample nodes if we can't generate default values for those ?

Not saying that this issue here has to cover *all* core field types before it gets in, we open followups for the less used ones, but maybe at least covering the field types used by the "visible" base fields of core entity types (or maybe just nodes or users ?) would be worthwhile ?

I don’t see that adding Latin to the name as a net win. The two method names just get longer. Also, the behavior of word() is different enough from string() and name() to justify its existence

Okay - can we then just move the word() method out of the sentence() / paragraphs() group, and next to string() / name() ?
And maybe add a comment clarifying how it differs from those two ?

moshe weitzman’s picture

Status: Needs work » Needs review
FileSize
40.95 KB
20.29 KB

OK, now passing $entity as suggested. Thanks.

Moved word() and added some Doxygen: * Generate a string that looks like a word (letters only, alternating consonants and vowels).

I added StringItem::generateSampleValue. I'm skipping Changed field type since that gets set for all entitites in ChangedItem::preSave

@TimPlunkett - nice tip!

Status: Needs review » Needs work

The last submitted patch, 43: generate_placeholder-2320157-43.patch, failed testing.

effulgentsia’s picture

+++ b/core/lib/Drupal/Core/Field/FieldItemInterface.php
@@ -217,6 +218,23 @@ public function delete();
+  public static function generateSampleValue(FieldDefinitionInterface $field_definition, EntityInterface $entity);

Passing $entity here doesn't make sense to me. At the very least, we should make it optional. But do we want it at all? For ER, getSelectionHandler() has it as optional, which means we can choose to make generateSampleValue() not pass it in. For ListItemBase, we have #2238085: [regression] options_allowed_values() signature doesn't allow for Views filter configuration open and critical, so we can punt its implementation until then. For AllowedValuesInterface field types in contrib, we have #2305397: Field type (item) classes implementing AllowedValuesInterface::getPossible(Values|Options)() is incompatible with Views and possibly other use cases, but even without that, each one can implement its own workaround as discussed in that issue.

For use cases where we're generating sample values to add to a new entity, there's no information in $entity that would be useful to the implementation. So the only logical reason for passing it in would be if we wanted to support adding sample values to an entity that already exists, and therefore, might already have meaningful (non-sample) information on it that needs to affect the sample value we want to generate: is that a real use case we want to support?

yched’s picture

For use cases where we're generating sample values to add to a new entity, there's no information in $entity that would be useful to the implementation. So the only logical reason for passing it in would be if we wanted to support adding sample values to an entity that already exists, and therefore, might already have meaningful (non-sample) information on it that needs to affect the sample value we want to generate

The reasoning makes sense - still, the entity type / bundle might need to affect the sample value ? Can't really think of an actual example though.

Yeah, maybe not passing $entity, and punting on #2238085: [regression] options_allowed_values() signature doesn't allow for Views filter configuration for ListItemBase::generateSampleValue() is simpler.

moshe weitzman’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
39.38 KB
16.17 KB

No longer passing $entity and commented out ListItemBase::generateSampleValue() with a link to #2238085: [regression] options_allowed_values() signature doesn't allow for Views filter configuration. Does that sound like a good way to proceed for now?

effulgentsia’s picture

Works for me.

  1. +++ b/core/lib/Drupal/Core/Field/FieldItemList.php
    @@ -281,6 +281,18 @@ public function view($display_options = array()) {
    +      $values[$delta] = $field_type_class::generateSampleValue($field_definition, $this->getEntity());
    

    Remove 2nd argument.

  2. +++ b/core/modules/field/src/Tests/FieldUnitTestBase.php
    @@ -126,6 +126,24 @@ protected function entitySaveReload(EntityInterface $entity) {
    +    else {
    +      $entity->save();
    +    }
    

    Is there a reason to save the entity in the tests? I think validating should be sufficient.

  3. +++ b/core/modules/options/src/Plugin/Field/FieldType/ListItemBase.php
    @@ -66,6 +66,18 @@ public function getSettableOptions(AccountInterface $account = NULL) {
    +//    $field_name = $field_definition->getName();
    +//    if ($allowed = $entity->$field_name->first()->getPossibleValues()) {
    +//      $values['value'] = array_rand(array_flip($allowed));
    +//      return $values;
    +//    }
    

    Since we don't know what the implementation will need to be until that issue lands (but we know it won't involve having a $entity), maybe let's remove this. We can leave the function wrapper and @todo though.

effulgentsia’s picture

still, the entity type / bundle might need to affect the sample value ?

We can get the entity type from the field definition. IMO, bundle should not affect any implementation directly, only implicitly via the field definition. However, one use case I thought of for passing $entity is if we ever want sample values to depend on the entity's language. But I suggest leaving that out for now and waiting for a use case that actually requires that.

effulgentsia’s picture

+++ b/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
@@ -312,6 +316,56 @@ public function preSave() {
+        // $image->setOwner($account);

Oh, interesting. Which account should we set this to? Not every entity type has an author/owner, and we're not passing in $entity anyway.

moshe weitzman’s picture

Implemented 1,3 from #48. I keps $entity->save() there just because it has caught errors a couple times which $entity->validate did not catch. In theory, we don't need to save but in practice it is helpful. I don't feel strongly either way.

#50: The file entity seems to save without any owner and I think thats fine for now. If we were to populate it, I imagine we would use $entity->user_id (or whatever its called). However, these generated entities don't yet have a user_id. We can add that but its going to be quite slow just like all entity reference population. Those do an entity_load_multiple() of every possible target entity AFAICT. The irony is that we only need the IDs in our case. Maybe we should bypass getReferenceableEntities and call buildEntityQuery() ourselves?

Status: Needs review » Needs work

The last submitted patch, 51: generate_placeholder-2320157-51.patch, failed testing.

moshe weitzman’s picture

Status: Needs work » Needs review
FileSize
39.37 KB
548 bytes

Forgot a use statement.

Status: Needs review » Needs work

The last submitted patch, 53: generate_placeholder-2320157-53.patch, failed testing.

moshe weitzman’s picture

Status: Needs work » Needs review
FileSize
39.37 KB

Rerolled. No other changes.

moshe weitzman’s picture

We are green here. Reviews welcome.

yched’s picture

No more remarks on my side. @effulgentsia, feel free to RTBC :-)

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community

Looks great.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/file/src/Plugin/Field/FieldType/FileItem.php
@@ -304,6 +306,25 @@ public function getUploadValidators() {
+    $destination = $settings['uri_scheme'] . '://' . $settings['file_directory'] . $random->word(10) . '.txt';

This will generate possibly duplicate file names - I think this can be avoided by using $random->name(10, TRUE) instead. Creating a duplicate file name is problematic because this will generate an error.

moshe weitzman’s picture

Status: Needs work » Needs review
FileSize
40.88 KB
2.32 KB

Fixed #59 and added telephone field support. Any other field types will happen in follow ups.

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed fe85f9b and pushed to 8.0.x. Thanks!

diff --git a/core/lib/Drupal/Core/Field/FieldItemInterface.php b/core/lib/Drupal/Core/Field/FieldItemInterface.php
index d64d2f3..14993cb 100644
--- a/core/lib/Drupal/Core/Field/FieldItemInterface.php
+++ b/core/lib/Drupal/Core/Field/FieldItemInterface.php
@@ -7,7 +7,6 @@
 
 namespace Drupal\Core\Field;
 
-use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\TypedData\ComplexDataInterface;
 
diff --git a/core/lib/Drupal/Core/Field/FieldItemList.php b/core/lib/Drupal/Core/Field/FieldItemList.php
index ccd1cb6..d10ecb4 100644
--- a/core/lib/Drupal/Core/Field/FieldItemList.php
+++ b/core/lib/Drupal/Core/Field/FieldItemList.php
@@ -284,7 +284,7 @@ public function view($display_options = array()) {
    public function generateSampleItems($count = 1) {
     $field_definition = $this->getFieldDefinition();
     $field_type_class = \Drupal::service('plugin.manager.field.field_type')->getPluginClass($field_definition->getType());
-    for ($delta=0; $delta < $count; $delta++) {
+    for ($delta = 0; $delta < $count; $delta++) {
       $values[$delta] = $field_type_class::generateSampleValue($field_definition);
     }
     $this->setValue($values);
diff --git a/core/modules/datetime/src/Plugin/Field/FieldType/DateTimeItem.php b/core/modules/datetime/src/Plugin/Field/FieldType/DateTimeItem.php
index 88a3320..4b37a3c 100644
--- a/core/modules/datetime/src/Plugin/Field/FieldType/DateTimeItem.php
+++ b/core/modules/datetime/src/Plugin/Field/FieldType/DateTimeItem.php
@@ -7,7 +7,6 @@
 
 namespace Drupal\datetime\Plugin\Field\FieldType;
 
-use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Field\FieldDefinitionInterface;
 use Drupal\Core\Field\FieldStorageDefinitionInterface;
 use Drupal\Core\Form\FormStateInterface;
diff --git a/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php b/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
index 32eafdd..1a14cc2 100644
--- a/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
+++ b/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
@@ -8,7 +8,6 @@
 namespace Drupal\image\Plugin\Field\FieldType;
 
 use Drupal\Component\Utility\Random;
-use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Field\FieldDefinitionInterface;
 use Drupal\Core\Field\FieldStorageDefinitionInterface;
 use Drupal\Core\Form\FormStateInterface;
diff --git a/core/modules/path/src/Plugin/Field/FieldType/PathItem.php b/core/modules/path/src/Plugin/Field/FieldType/PathItem.php
index f7fec6a..cff3fa6 100644
--- a/core/modules/path/src/Plugin/Field/FieldType/PathItem.php
+++ b/core/modules/path/src/Plugin/Field/FieldType/PathItem.php
@@ -8,7 +8,6 @@
 namespace Drupal\path\Plugin\Field\FieldType;
 
 use Drupal\Component\Utility\Random;
-use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Field\FieldDefinitionInterface;
 use Drupal\Core\Field\FieldStorageDefinitionInterface;
 use Drupal\Core\Field\FieldItemBase;
diff --git a/core/modules/text/src/Plugin/Field/FieldType/TextItemBase.php b/core/modules/text/src/Plugin/Field/FieldType/TextItemBase.php
index e936ce4..8b70203 100644
--- a/core/modules/text/src/Plugin/Field/FieldType/TextItemBase.php
+++ b/core/modules/text/src/Plugin/Field/FieldType/TextItemBase.php
@@ -8,7 +8,6 @@
 namespace Drupal\text\Plugin\Field\FieldType;
 
 use Drupal\Component\Utility\Random;
-use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Field\FieldDefinitionInterface;
 use Drupal\Core\Field\FieldItemBase;
 use Drupal\Core\Field\FieldStorageDefinitionInterface;

Fixed a few unneeded use statements which were being added and a minor code style nit.

  • alexpott committed fe85f9b on 8.0.x
    Issue #2320157 by moshe weitzman, Wim Leers, penyaskito, tim.plunkett:...

Status: Fixed » Closed (fixed)

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