Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add field:delete command #4926

Merged
merged 7 commits into from Dec 16, 2021
Merged

Conversation

DieterHolvoet
Copy link
Contributor

@DieterHolvoet DieterHolvoet commented Dec 12, 2021

This PR adds a command to delete fields, similar to the field-delete command that was present in Drush 8. It has already been used in multiple projects as part of the wmscaffold module and is in my opinion stable enough to be considered to be merged in Drush.

Remarks

Test scenarios

  • ./vendor/bin/drush fd => Not enough arguments (missing: "entityType").
  • ./vendor/bin/drush fd node => Asks for bundle
  • ./vendor/bin/drush fd node --no-interaction => The bundle argument is required.
  • ./vendor/bin/drush fd node article => Asks for field name
  • ./vendor/bin/drush fd node article --no-interaction => The field-name option is required.
  • ./vendor/bin/drush fd node article --no-interaction --field-name=field_test => Deletes field_test. If that field does not exist: Field with name 'field_test' does not exist on bundle 'article'.

TODO

  • Add tests

@DieterHolvoet
Copy link
Contributor Author

I'm not sure whether we should place the traits in the same namespace as the command, or whether we just shouldn't use any traits. Opinions are welcome.

Comment on lines 133 to 143
$fieldConfig->delete();

// If there is only one bundle left for this field storage, it will be
// deleted too, notify the user about dependencies.
if (count($fieldStorage->getBundles()) <= 1) {
$fieldStorage->delete();
}

$message = 'The field :field has been deleted from the :type content type.';
} else {
$message = 'There was a problem removing the :field from the :type content type.';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drupal will not let you delete a field with data in it. Can we show that exception message, instead of "there was a problem"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the exact same error message as shown in the UI: https://git.drupalcode.org/project/drupal/-/blob/9.3.x/core/modules/field_ui/src/Form/FieldConfigDeleteForm.php#L104

Isn't it possible that the field storage being locked can have other causes? Because if that's the case, we should keep the error message generic, just like field_ui does. Btw, I just tested it and I can delete fields with data in them using field:delete.

@@ -26,7 +26,9 @@

class FieldCreateCommands extends DrushCommands implements CustomEventAwareInterface
{
use AskBundleTrait;
use CustomEventAwareTrait;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we don't use a CustomEvent directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do call the getCustomEventHandlers method in FieldCreateCommands, right?

@weitzman weitzman merged commit cc23dbe into drush-ops:11.x Dec 16, 2021
@DieterHolvoet DieterHolvoet deleted the field-delete branch December 16, 2021 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants