Steps to reproduce
Create 2 content type: CT1 and CT2,
Create a global flag "my_flag_test" and Check only CT1 for the "Flaggable types" field
=>"Display link as field" is UNchecked

To show the links, use a preprocess function like this one:

    $flag_service = \Drupal::service('flag.link_builder');
    $flag_link_var = $flag_service->build($node->getEntityTypeId(), $node->id(), 'my_flag_test')){
    $vars['flag_link_var'] = $flag_link_var;

Results
If you go to a CT1's page, everything is fine: the flag link is showed, but If you go to a CT2's page, you would expect that there is no tag, but unfortunately, the link tag is showed...

CommentFileSizeAuthor
#3 flag-Service_link_builder_wrongly_create_links_in_all_bundle-2788665-3-8.x.patch1.52 KBDuneBL
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DuneBL created an issue. See original summary.

joachim’s picture

joachim CreditAttribution: joachim commented

The flag.link_builder service is just a lazy builder. It doesn't handle access or applicability.

DuneBL’s picture

DuneBL
Primary language French
CreditAttribution: DuneBL commented
FileSize
flag-Service_link_builder_wrongly_create_links_in_all_bundle-2788665-3-8.x.patch1.52 KB

@joachim, thanks for the answer, it give me the idea to create a small patch to make this builder less lazy.
The attached patch check if the bundle of the entity is in the "Flaggable types" list of the flag.
Let me know if this patch is suitable.

...It doesn't handle acces...

As a side note, this builder is already checking the access through the getLink() function: getLink() is called from the builder and getLink() check if the user have access

  public function getLink(FlagInterface $flag, EntityInterface $entity) {
    ....
   // HERE IS THE ACCESS CHECK
    if ($flag->hasActionAccess($action)) {
      ...
     }
   }
joachim’s picture

joachim CreditAttribution: joachim commented
Status: Active » Postponed

Thanks for the patch, but I'm not sure this is the direction we want to take with this.

There's an issue about figuring out the access control -- take a look at that one.

DuneBL’s picture

DuneBL
Primary language French
CreditAttribution: DuneBL commented

I would like to focus on the patch:
If this is not the direction you want to take, could you let me know how you think a site builder should handle the visibility of the links (according to the bundle type) when he/she is using the link builder service?
Here are some ideas:
-Logic is inside the link builder (like my patch: the site builder doesn't have to worry about)
-Logic is handle by a custom function made by the site builder (not very funny)
-Logic is inside another Flag function like "$flag->isInBundleList($entity)"
-Another solution
-A site builder doesn't have to use the flag builder service to display the links, he must use another solution

Thanks you by advance for your help on this!

joachim’s picture

joachim CreditAttribution: joachim commented

Can I ask why you can't expose the flag link as a pseudofield and then output that in your template?

DuneBL’s picture

DuneBL
Primary language French
CreditAttribution: DuneBL commented

Of course you can ask ;-)
This is because I am displaying the links from within a page twig template, not from a node template...
I am doing this, to show the links ouside the node content.

To use the pseudo field inside a page template I assume I can :
1-dig into the content array until I discover the pseudo field
2-display this field
3-display the remaining content by using the "without" twig function (to not display again the pseudo field)

This process looks ugly to me (but I could be wrong; I mean another better process could exists) and, more important, I will have to create a page template for each node type in order to dig appropriately in the content array.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Agree with @joachim, I think it is your job to check that. for example in flag.module, this happens in flag_entity_view() for example (kind of implicitly, since the extra field doesn't exist on bundles where it is not added.

This makes sense, because this check is something that can be cached. It doesn't vary per-user, unlike the fact whether a user has access or should se the flag or unflag link. The point of a lazy builder is to run outside of caching, but that has the downside that has to run on every request, also within cached nodes.

joachim’s picture

joachim CreditAttribution: joachim commented

We should probably mark the lazy builder service as @internal, so that we're free to change the implementation as needed.

DuneBL’s picture

DuneBL
Primary language French
CreditAttribution: DuneBL commented

If I understand well, If a site builder wants to display the flag links without using the pseudo fields:
1-He must call the builder service
2-This service builder create links that shouldn't be created if the viewed bundle is not in the bundle list
3-The site builder, should remove the unwanted links

This is a builder story not so funny...
A-Why leaving this job to the site builder if this can be handled internally?
B-Why spend CPU time to create unwanted links?
C-Why the check added is my patch doesn't suit you?
D-Marking the lazy builder as internal, does it means that it will not be possible to create links without the pseudo fields?

At least, I would be happy to get an answer to question A

Thanks your for your help!

DuneBL’s picture

DuneBL
Primary language French
CreditAttribution: DuneBL commented

@Berdir
I take a look at flag_entity_view, and I have an idea to solve this problem.
(I say "problem" because on my side I feel it is a problem, but you are both much more experienced then I am, and you seem to think that there is no problem at all)

Anyway here is my idea:
We could create a function get_all_links(EntityInterface $entity, EntityViewDisplayInterface $display) which return a render array with the links.
In fact this function would do nearly exactly what flag_entity_view() is doing...
This leave us the opportunity to:
-Call this function from within flag_entity_view to add the returned render array to the build array
-Call this function from my preprocess_page function to get the links I want...

What do you think?