Add field:create command #4853

DieterHolvoet · 2021-10-07T09:50:44Z

Original PR here: #3528

This PR adds a command to create fields, similar to the field-create command that was present in Drush 8. It has already been used in multiple projects at the company I work at, and is in my opinion stable enough to be considered to be merged in Drush. The original PR was closed becaused generate field was supposedly similar, but it's not. This command adds a new field to an entity type, it doesn't create a new field type.

Tests

There is no test coverage yet.

Naming

I named the class FieldCreateCommands instead of FieldCommands because I think future (smaller) field-related commands should be kept seperate from the field-create command. The class is already almost 700 lines long and adding other commands would make it a bit too cluttered.

weitzman · 2021-10-07T13:58:34Z

Thanks for contributing this. The code looks very high quality. It occurs to me that some of the questions might better belong in an interact() method. I'm not sure how feasible that is. Lets also add a test of type 'integration' for this.

This does look like a different use case from gen field, which creates a field type and an instance. This command creates an instance of an existing type. Neither approach do what I like to do which is to add a base field (i.e. non-configurable field) to a bundle. Overall, I think we could improve the naming and descriptions of these two commands. I'm not sure how though. Input welcome

/cc @Chi-teck

Chi-teck · 2021-10-07T16:16:32Z

I agree, field-create and gen field are completely different things. If you run drush create:field command in Drush 10 you will get the following message.

field-create has been removed. Please try generate field command.

Defently, this message should not suggest generate field command.

Chi-teck · 2021-10-07T16:24:33Z

the field-create command that was present in Drush 8

Yes and no. Because that command only worked with Drupal 7. We failed to update it to support Drupal 8. The relevant issue is still open. #230

Note that field-create was just one of field related commands that existed in Drush 8.

field-clone Clone a field and all its instances.
field-create Create fields and instances. Returns urls for field editing.
field-delete Delete a field and its instances.
field-info View information about fields, field_types, and widgets.
field-update Return URL for field editing web page.

DieterHolvoet · 2021-10-11T12:20:24Z

It occurs to me that some of the questions might better belong in an interact() method. I'm not sure how feasible that is.

I used the @interact hook in a previous version, but I deliberately removed it. I can't remember the exact reason, it might have been because of certain issues I was having or it might have been the result of trying to simplify the code. This is the commit in which the change was done.

I do remember it being hard to completely finish the interactive part before the @command method, since some questions can only be asked after parts of the logic have been executed. No logic can be executed in the @interact hook since it is not being executed in non-interactive contexts.

If it's okay for you, I'd like to leave this as is. I will make sure that the command works perfectly when passing the --no-interaction option, which might not always be the case right now.

Lets also add a test of type 'integration' for this.

I don't have much experience with automatic testing but I'll try to get to this.

Neither approach do what I like to do which is to add a base field (i.e. non-configurable field) to a bundle.

I don't have any code lying around that does this. This sounds like a case for a generate subcommand since this will generate code.

Overall, I think we could improve the naming and descriptions of these two commands. I'm not sure how though. Input welcome

I thing renaming generate field to generate field-type is a start. Personally, I think field:create is the right name for this command.

Definitely, this message should not suggest generate field command.

I removed that placeholder command in this PR since we now have an actual replacement.

Note that field-create was just one of field related commands that existed in Drush 8.

I know, and we have replacements for pretty much every one of them in our wmscaffold repo:

field:create: Create a new field
field:delete: Delete a field
field:info: List all fields of an entity bundle
field:update: No command yet, but the required code is present in FieldCreateCommands::logResult.

Additionally, we have two commands to deal with base fields

base-field:info: List all base fields of an entity type
base-field-override:create: Create a new base field override

I'll create PR's for these commands later.

Chi-teck · 2021-10-11T12:40:43Z

I thing renaming generate field to generate field-type is a start.

drush generate field-type already exists.

DieterHolvoet · 2021-10-11T12:49:15Z

drush generate field-type already exists.

The output seems similar, are both necessary? In any case, the naming of those generators seems out of scope for this issue. Feel free to let me know if you don't agree with the naming of field:create.

weitzman · 2021-10-15T20:04:25Z

Those answers make sense to me. Thanks. Please fix code style issues that are failing the test suite now. You can see them locally with composer cs

weitzman

I've fixed test failures. Just one minor question remaining.

weitzman · 2021-10-25T16:50:04Z

src/Drupal/Commands/core/FieldCreateCommands.php

+     * @aliases field-create,fc
+     *
+     * @validate-entity-type-argument entityType
+     * @validate-optional-bundle-argument entityType bundle


Do these two annotations do anything? I dont see what checks them.

@DieterHolvoet ^

That's still a leftover from where I took the command, it's an abstraction for validating entity type/bundle arguments: https://github.com/wieni/wmscaffold/blob/release/v1/src/Commands/ValidatorsCommands.php

It's used in other commands I mentioned before: field:delete, field:info, base-field:info, base-field-override:create. If we're planning on incorporating those commands as well, it might be worth it to keep it abstract, if not we can copy/paste it into the class.

There are no changes to ValidatorsCommands in the PR so those annotations don't yet do anything. I'm OK with the abstraction, I just think they should have their corresponding code in Drush.

DieterHolvoet · 2021-11-04T13:50:07Z

@weitzman and I discussed this PR on Slack. Some things that need to be done before merging:

More manual testing
Write down a plan of how to call field:create and what to check for to be used for writing automated tests (by me)
Write automated tests (by @weitzman)
Add https://github.com/wieni/wmscaffold/blob/release/v1/src/Commands/LinkHooks.php#L27 to the PR
Once merged, try to get the Allowed Formats module to add https://github.com/wieni/wmscaffold/blob/release/v1/src/Commands/AllowedFormatsHooks.php#L26

DieterHolvoet · 2021-11-24T13:29:36Z

Some test scenarios given a node type article without fields. Still working on the other stuff listed above.

Entity types with bundles

drush fc node should ask for a bundle
drush fc node --no-interaction should complain about a missing bundle

Entity types without bundles

drush fc user should not let you choose a bundle and continue with the other questions instead
drush fc user user should not let you choose a bundle and continue with the other questions instead
drush fc user --no-interaction should not complain about a missing/invalid bundle argument
drush fc user user --no-interaction should not complain about a missing/invalid bundle argument

Existing field storages

drush fc node article --existing interactively lists existing node fields, eg. body
drush fc node article --no-interaction --existing-field-name="body" --field-label=Body --field-widget=text_textarea_with_summary successfully adds the body field
- When calling the same command a second time, it should complain about the field already existing.
drush fc node article --existing-field-name="body" --field-label=Body --field-widget=text_textarea_with_summary asks for optional fields before successfully adding the body field

New field storage

drush fc node article --field-name=body should complain about the field storage already existing
drush fc node article --field-type=entity_reference should at some point ask for Referenced entity type
drush fc node article --no-interaction --field-label=Test --field-name=field_test --field-type=entity_reference --field-widget=entity_reference_autocomplete --cardinality=-1 should complain about the missing target-type option
drush fc node article --field-label=Test --field-name=field_test --field-type=entity_reference --field-widget=entity_reference_autocomplete --cardinality=-1 --target-type=node should at some point ask for Referenced bundles, allowing me to choose multiple bundles. The target_bundles handler setting should change after.
drush fc node article --no-interaction --field-label=Test --field-name=field_test --field-type=entity_reference --field-widget=entity_reference_autocomplete --cardinality=-1 --target-type=node --target-bundle=article, the target_bundles handler setting should change

`LinkHooks`, `field-create-set-options` & `field-create-field-config` events

drush fc node article --field-type=link should at some point ask for Allowed link type and Allow link text, title and link_type should be in field settings config

Output

a summary containing field name, entity type and bundle should be printed
if the Field UI module is installed, a link to the field edit form should be printed

DieterHolvoet · 2021-11-24T15:04:18Z

@weitzman I did some final polishing of the code and I wrote out some test scenarios. This is ready to go from my side!

Something small I noticed: I don't really like the fact that the Cancel option is prepended to every choice list, why is this necessary if you can just Ctrl+C to exit like with every other command?

weitzman · 2021-11-24T16:35:03Z

Thanks for the tests scenarios. We cant easily test interactive behavior because the process hangs waiting for input. I will get as much coverage as I can.

That cancel choice has been there so long I don't recall its justification. I think you are the first person to actually call it out. I bet others have silently grumbled. Lets discuss in a different PR.

I will look into the unrelated test failure on drupal 9.3

src/Commands/ValidatorsCommands.php

- Fix issues when command is called non-interactively - Improve handling of entity types without bundles

weitzman · 2021-12-02T14:46:27Z

I finally added as many tests as I could.

We are not validating referenced bundles AFAICT. I've commented out the test for that. Not urgent.

Sorry about unrelated changes added to the PR.

weitzman · 2021-12-02T22:40:06Z

Its in! Thanks @DieterHolvoet.

DieterHolvoet · 2021-12-03T11:22:43Z

Its in! Thanks @DieterHolvoet.

Yay! Thanks for writing the tests 🥳

We are not validating referenced bundles AFAICT. I've commented out the test for that. Not urgent.

I created a follow up issue: #4912

DieterHolvoet · 2021-12-12T16:45:26Z

@weitzman did we forget to update this page of the docs, or does that happen automatically? https://www.drush.org/latest/commands/all/

DieterHolvoet · 2021-12-12T17:16:22Z

I added a merge request to the Allowed Formats module. I'm afraid it's not going to get merged anytime soon though, since the project has the Maintenance fixes only status.

DieterHolvoet added 4 commits October 7, 2021 11:25

Add field:create command

b54631a

Code style

70a3b9c

Refactor field:create command

8af446e

Remove empty legacy command

469ae36

DieterHolvoet changed the title ~~Add field:create command~~ WIP: Add field:create command Oct 7, 2021

DieterHolvoet marked this pull request as draft October 7, 2021 09:51

DieterHolvoet changed the title ~~WIP: Add field:create command~~ Add field:create command Oct 7, 2021

weitzman added 2 commits October 23, 2021 10:37

phpcs

d265359

Merge branch '11.x' into field-create

ec527f8

weitzman reviewed Oct 25, 2021

View reviewed changes

weitzman added this to the drush11 milestone Oct 30, 2021

Add validators so the new annotations are functional

017477c

Merge branch '11.x' into field-create

6b710a6

DieterHolvoet force-pushed the field-create branch from cdec746 to 1730859 Compare November 24, 2021 14:47

DieterHolvoet mentioned this pull request Nov 24, 2021

Mention PHP_CodeSniffer in CONTRIBUTING.md #4906

Merged

weitzman requested changes Nov 25, 2021

View reviewed changes

src/Commands/ValidatorsCommands.php Outdated Show resolved Hide resolved

DieterHolvoet mentioned this pull request Nov 25, 2021

Stop prepending the Cancel option to choice lists #4907

Closed

DieterHolvoet added 2 commits November 25, 2021 17:05

Fix some issues + small refactor

ccf1fe2

- Fix issues when command is called non-interactively - Improve handling of entity types without bundles

Improve various prompts

ad3c1ef

DieterHolvoet added 2 commits November 25, 2021 17:05

Add LinkHooks

a5a55e0

Apply phpcs fixes

707d7ef

DieterHolvoet force-pushed the field-create branch from aaced26 to 707d7ef Compare November 25, 2021 16:05

weitzman added 2 commits December 2, 2021 09:38

Add tests, and misc unrelated improvements

06c28b7

PHPCS

82d26c3

weitzman added 5 commits December 2, 2021 10:27

Try getSimplifiedErrorOutput()

47feef1

Fix UserCancelTest

3003fb4

Less string

fe2b8bc

Revert unneeded test case changes.

e543c3b

Merge branch '11.x' into field-create

9a879e6

weitzman marked this pull request as ready for review December 2, 2021 17:25

weitzman merged commit 0b5ab2b into drush-ops:11.x Dec 2, 2021

DieterHolvoet mentioned this pull request Dec 3, 2021

Validate the --target-bundle option of field:create #4912

Closed

DieterHolvoet deleted the field-create branch December 3, 2021 11:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add field:create command #4853

Add field:create command #4853

DieterHolvoet commented Oct 7, 2021 •

edited

weitzman commented Oct 7, 2021 •

edited

Chi-teck commented Oct 7, 2021 •

edited

Chi-teck commented Oct 7, 2021

DieterHolvoet commented Oct 11, 2021

Chi-teck commented Oct 11, 2021 •

edited

DieterHolvoet commented Oct 11, 2021

weitzman commented Oct 15, 2021

weitzman left a comment

weitzman Oct 25, 2021

weitzman Oct 30, 2021

DieterHolvoet Nov 2, 2021

weitzman Nov 2, 2021

DieterHolvoet commented Nov 4, 2021

DieterHolvoet commented Nov 24, 2021 •

edited

DieterHolvoet commented Nov 24, 2021

weitzman commented Nov 24, 2021

weitzman commented Dec 2, 2021

weitzman commented Dec 2, 2021

DieterHolvoet commented Dec 3, 2021

DieterHolvoet commented Dec 12, 2021

DieterHolvoet commented Dec 12, 2021

Add field:create command #4853

Add field:create command #4853

Conversation

DieterHolvoet commented Oct 7, 2021 • edited

Tests

Naming

weitzman commented Oct 7, 2021 • edited

Chi-teck commented Oct 7, 2021 • edited

Chi-teck commented Oct 7, 2021

DieterHolvoet commented Oct 11, 2021

Chi-teck commented Oct 11, 2021 • edited

DieterHolvoet commented Oct 11, 2021

weitzman commented Oct 15, 2021

weitzman left a comment

Choose a reason for hiding this comment

weitzman Oct 25, 2021

Choose a reason for hiding this comment

weitzman Oct 30, 2021

Choose a reason for hiding this comment

DieterHolvoet Nov 2, 2021

Choose a reason for hiding this comment

weitzman Nov 2, 2021

Choose a reason for hiding this comment

DieterHolvoet commented Nov 4, 2021

DieterHolvoet commented Nov 24, 2021 • edited

Entity types with bundles

Entity types without bundles

Existing field storages

New field storage

LinkHooks, field-create-set-options & field-create-field-config events

Output

DieterHolvoet commented Nov 24, 2021

weitzman commented Nov 24, 2021

weitzman commented Dec 2, 2021

weitzman commented Dec 2, 2021

DieterHolvoet commented Dec 3, 2021

DieterHolvoet commented Dec 12, 2021

DieterHolvoet commented Dec 12, 2021

DieterHolvoet commented Oct 7, 2021 •

edited

weitzman commented Oct 7, 2021 •

edited

Chi-teck commented Oct 7, 2021 •

edited

Chi-teck commented Oct 11, 2021 •

edited

DieterHolvoet commented Nov 24, 2021 •

edited

`LinkHooks`, `field-create-set-options` & `field-create-field-config` events