"Unflag not allowed text" does not work

If there was a solution, it would be here...

Review this patch please

+++ b/src/FlagService.php
@@ -257,7 +257,9 @@ class FlagService implements FlagServiceInterface {
+        drupal_set_message($flag->getMessage('flag'));

Calls to dsm() should not happen in the flag service, as this is an API, and could be called by things that are not user-controlled, such as cron or REST services.

Added a template for denied message rendering (flag-denied-message.html.twig), by default it will take unflag denied message.
In my opinion, Flag denied message should be added as well.
With this patch you could change message via hook_preprocess_flag_denied_message().

+++ b/templates/flag-denied-message.html.twig
@@ -0,0 +1,31 @@
+{# Set the remaining Flag CSS classes. #}
+{% set classes = [
+'flag',
+'flag-' ~ flag.id(),
+'flag-' ~ flag.id() ~ '-' ~ flaggable.id(),
+action_class
+] %}
+

Does it really make sense to use the same classes as the flag link here? It seems an additional class at least to identify it as a flag-message is appropriate.

@zenimagine, i added new patch that fixes it, thank you for testing it :)

@socketwench, You are right, so i changed it to flag-message

+++ b/templates/flag-denied-message.html.twig
@@ -0,0 +1,32 @@
+ * Default theme implementation for flag links.

That's not true.

Though do we really need a different template for this?

Though do we really need a different template for this?

@joachim, i think yes cause it more flex, but currently we have only Unflag Denied Message, so we should add something like Flag Denied Message, for example: if there is a limitation to flag something (maximum amount of participants on event)

Added new patch that allowed user to add FlagDeniedMessage

Though do we really need a different template for this?

IMHO this approach is welcome ...I like templates with a single responsibility I think the alternative design would lead to extra control logic in the template... whereas we can use the if statement already in ActionLinkTypeBase::getAsFlagLink()

As by way of review, I can see one minor flaw

flaggable is a unused variable in the template and so could be cut out of :-

The twig file and ActionLinkTypeBase::getAsFlagLink() and flag_theme() --- 'flag_denied_message'

Maybe there is as use case, under consideration that I cannot see?

I should have mentioned one thing I was thinking about ....

How to we deny access for AJAX action links ?

I think it can be done ... but I think we can leave those discussion to

#2764709: [Regression/Refactor] Restore Ajax link type "flag message" functionality

flaggable is a unused variable in the template and so could be cut out of...

That was intentional. Not for our use, but I can easily imagine cases where the themer would want the flaggable to grab some detail for display with the flag link.

Getting closer! I see two problems:

1. We don't set any helpful default text for the denied message, just an empty string. Instead of setting this in the config entity class, we could set this in FlagAddForm instead. That way it only applies to new flags.
2. Needs tests.

That was intentional.

Ok - I am happy with that....

Passing an object to a twig template ... that we are denying access to is acceptable.

The use case I can dream up is "a node as a book" ... and the customized message saying "you cannot access/flag the book [of title] xyz."

Added some tests.

1. +++ b/src/FlagInterface.php
   @@ -264,6 +264,33 @@ interface FlagInterface extends ConfigEntityInterface, EntityWithPluginCollectio
   +  /**
   +   * Get the flag's flag denied message text.
   +   *
   +   * @return string
   +   *   A string containing the flag denied message text.
   +   */
   +  public function getFlagDeniedText();
   ...
   +  /**
   +   * Gets the flag denied message.
   +   *
   +   * @param string $action
   +   *   The flag action, either 'flag' or 'unflag'.
   +   *
   +   * @return string
   +   *   The flag/unflag denied message text to use.
   +   */
   +  public function getDeniedText($action);
   

   I can't tell the difference between these descriptions.

2. +++ b/templates/flag-denied-message.html.twig
   @@ -0,0 +1,32 @@
   + * - message: The flag deny message.
   

   flag, or sometimes unflag too?

1. Should we change "Gets the flag denied message." to "Gets the flag denied message depending on action"
or remove getDeniedText($action) method and change this?:

$render = [
        '#theme' => 'flag_denied_message',
        '#flag' => $flag,
        '#flaggable' => $entity,
        '#action' => $action,
        '#message' => $flag->getDeniedText($action),
      ];

to

$render = [
        '#theme' => 'flag_denied_message',
        '#flag' => $flag,
        '#flaggable' => $entity,
        '#action' => $action,
        '#message' => ($action == 'flag')?$flag->getFlagDeniedText():$flag->getUnflagDeniedText(),
      ];

2. Lets change it to "- message: Access denied message."

Updated.

I think the test failures here are a sign of broken tests

as pointed out by bedir

https://www.drupal.org/project/flag/issues/2939121#comment-12449244

D8.6.x is going through growing pains .. services have become private and that is causing errors. D8.6.x will fix itself through a critical issue.

In short I am retesting on 8.5.x

Related issue that block this one was closed. And now we got errors with AJAX links test.

Does somebody know what is it? :)

Tests are passed. Please review it as it is a one of release blockers.

Hi, I'm using Flag (8.x-4.0-alpha3) and it's pretty amazing that "unflag_denied_text" property of flag configuration is not used anywhere in rendering process. I'm not sure if my comment is relevant, cause I see all those patches and the version of module in issue is different, but I myself have just adjusted rendering function getAsFlagLink in ActionLinkTypeBase class and that's all there is needed for it. It's basically taken from 7.x version of the module, so wouldn't that fix the problem?

  public function getAsFlagLink(FlagInterface $flag, EntityInterface $entity) {
    (...)
    if($access->isAllowed()) {
      (...)
    }
    else {
     // Check if action is unflag and make sure user does not have access, then render denied text
      if ($action == 'unflag' && !$access->isAllowed()) {
        $render = ['#markup' => $flag->getUnflagDeniedText()];
      }
    }
    (...)
  }

I tested the solution in Comment #41 and this worked for me.

I added patch for this there:
https://www.drupal.org/project/flag/issues/3059976#comment-13463196

I have simple solution based on #41

Module Flag 8.x-4.0-beta2. In file /src/ActionLink/ActionLinkTypeBase.php change line 126:

   $render = [];

to:

     // Check if action is unflag and make sure user does not have access, then render denied text
      if ($action == 'unflag' && !$access->isAllowed()) {
      $render = [
        '#theme' => 'flag',
        '#flag' => $flag,
        '#flaggable' => $entity,
        '#action' => 'flagged',
        // Use render array for title to allow limited markup in the link text.
        '#title' => ['#markup' => $flag->getUnflagDeniedText()],
      ];
      }

And in flag.html.twig change:

<div class="{{classes|join(' ')}}"><a{{ attributes }}>{{ title }}</a></div>

to:

{% if action == 'flagged' %}
	<div class="{{classes|join(' ')}}"><span>{{ title }}</span></div>
{% else %}
	<div class="{{classes|join(' ')}}"><a{{ attributes }}>{{ title }}</a></div>
{% endif %}

You can see how it works here https://tamadec.ru/pozdravleniya/s-dnem-rozhdeniya

i think we additionaly need a way to choose between public visible or only visible for users who can see the flag.

Confirmed #44 works. Converted to MR.

https://git.drupalcode.org/project/flag/-/merge_requests/25/diffs.patch

Patch updated for 8.x-4.0-beta4 support.

#49 works great. Thanks

#49 patch works nice! Would be great to have this on a stable release.