Problem/Motivation
Going to copy the discussion that happened in the MR of the previous issue where this code was removed:
->addOption('exclude-field', 'e', InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, "The name of a field that should not be exported. Does not apply to referenced entities. This option can be used multiple times.", [])
Sascha Grossenbacher
@berdir 6 days ago
Developernot sure if this exclude fields feature is really needed and in scope for this? as I commented, I don't really see a case where we could possible have a recursion and I never had a use case where this would have been useful. with references, this could be quite unpredictable as it is applied to all dependencies as well.
Jim Birch
Jim Birch
@thejimbirch 6 days ago
DeveloperI have no opinion on the references, but when creating recipes, I have had to edit the content entities to remove field-like things (Metatag, Scheduler).
Adam G-H
Adam G-H
@phenaproxima 3 hours ago
Author DeveloperIt's in scope, yeah; see @murz's comment in https://www.drupal.org/project/drupal/issues/3532951#comment-16170082 and subsequent replies. Although accidental infinite loops are prevented, being able to control which dependencies are exported (by excluding fields) lies in scope.
with references, this could be quite unpredictable as it is applied to all dependencies as well.
Agreed, that's why it's not applied to dependencies.
Sascha Grossenbacher
Sascha Grossenbacher
@berdir 2 hours ago
DeveloperAgreed, that's why it's not applied to dependencies.
I missed the reset at the end of the loop. FWIW, I feel like it's just as likely needed on dependencies. The very comment you linked to shows that IMHO. You might want to include the articles but not their related pages. you can't do that now.
There are ways to handle that, you could support a notation that makes this explicit, such as --exclude-fields=related_articles.related_pages. Of course that requires a more complex queue handling as you need to keep track of the relation tree for each reference.
Also, the current logic get more complicated when adding support for all/multiple entities, sooner or later you'd need to refactor this.
( I still think things adds quite a bit of complexity and isn't a strict requirement for this feature, but don't have a strong opinion on this. the comment from @thejimbirch is interesting in that it shows a more practical use case for this but at the same time, is actually not related to references)
Adam G-H
Adam G-H
@phenaproxima 1 hour ago
Author DeveloperI think you've convinced me. Let's punt this to another issue so we can consider the edge cases more thoroughly. The main thing we need in this issue is defense against circular dependencies, but we already have that.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3537670
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
phenaproximaAn idea -- how about if the
--exclude-fieldoption accepts not a field name, but a full field ID, likenode.article.field_related_pages. That would make it entirely clear which fields should be excluded from which entities, regardless of where they are in the dependency tree.It would also be very easy to implement; we would just check fields as we loop over the content entity's field definitions in
Exporter::export(), and mark the field non-exportable if its ID (FieldConfigInterface::id()) is in the list.