- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add field:delete command #4926
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
Conversation
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. |
fc8ecec
to
09f0ffc
Compare
$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.'; |
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
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
AskBundleTrait
andValidateEntityTypeTrait
are also included in Add field:info command #4928, Add field:base-override-create command #4929 and Add field:base-info command #4930.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