Problem/Motivation

The condition in flag_entity_build_defaults_alter looks wrong. Flag fields are always added to the entity display ...

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
d8-flag-entity-display.patch544 byteswebflo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Good catch but I think your patch is wrong too :)

What you want is "Skip if not shown as field *or* not enabled for this display", no?

Berdir’s picture

(08:40:44 PM) berdir: webflo: https://www.drupal.org/node/2415245#comment-9558695
(08:45:26 PM) webflo: berdir: i guess you are right, but on the other hand. if flag does not expose an extra field there will be no return value from getComponent
(08:46:05 PM) webflo: berdir: maybe we could remove the setting and solve this problem only in field ui?
(08:51:46 PM) berdir: webflo: not sure I understand. you are correct that it works like this as well, but that just means it would work without the showAsField() call. I think it's "more correct" to not call getComponent() if there is no field :)
(08:52:01 PM) berdir: not sure i understand was about the field ui part
(08:55:54 PM) berdir: webflo: also, I don't remember 100% (I implement that logic there, together with an intern, last summer) but it is possible that my line of thinking was that not having a field means that we do not know if the flag will be there or not, so we added the cache key to be sure
(08:58:31 PM) webflo: berdir: ok, but flag cloud expose the field to field ui always and it is disabled by default
(08:59:09 PM) berdir: webflo: fine with me, I think we just made the existing behavior work and didn't think about changing it :)
(09:00:08 PM) berdir: webflo: maybe it should be controlled by the link plugin, because many of them are actually not compatible with it, e.g. adding the links throug hook_node_links() can not be controlled through entity view display components
(09:03:21 PM) webflo: berdir: yeah makes sense. lets fix the tests before we try to refactor
penyaskito’s picture

See flag_entity_view() where we have the same pattern. If that's the fix, we may need to change there too.

joachim’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Sounds like this needs work. Also, needs tests! :)

joachim’s picture

I think this issue might be obsolete.

joachim’s picture

Status: Needs work » Closed (duplicate)

Yup, obsolete. The patch no longer applies, and we now have tests covering this.