Allow UnMasquerade link in the "Masquerade" block

Well, the patch in #33 didnt work for us at all. The masquerade block shows up for the admin user, but a normal user who has no access to that block to begin with cannot see the unmasquerade link and we're not about to give them access to the admin toolbar. Would it not make sense to make a 2nd block and a new permission called "unmasquerade" that can then be given to the "authorized user" role?

claudiu.cristea → made their first commit to this issue’s fork.

Added test coverage bu I have no idea why tests are not triggered or are no showing here. Any clue?

Adding a patch with teh same changes as in MR to see if we can run tests

Looks ready! just few questions and nits

1. +++ b/src/MasqueradeCallbacks.php
   @@ -70,4 +71,23 @@ class MasqueradeCallbacks implements TrustedCallbackInterface {
   +        'unmasquerade' => [
   +          '#type' => 'link',
   +          '#title' => new TranslatableMarkup('Switch back'),
   +          '#url' => Url::fromRoute('masquerade.unmasquerade'),
   

   Would be great to pass here a destination for redirect

   Wondering why you need 'unmasquerade' array key?! It looks useless

2. +++ b/src/MasqueradeCallbacks.php
   @@ -70,4 +71,23 @@ class MasqueradeCallbacks implements TrustedCallbackInterface {
   +    return ['#markup' => ''];
   

   as lazy builder it should return empty string instead of render array

3. +++ b/src/Plugin/Block/MasqueradeBlock.php
   @@ -99,6 +111,14 @@ class MasqueradeBlock extends BlockBase implements ContainerFactoryPluginInterfa
      public function build() {
   +    if ($this->configuration['show_unmasquerade_link'] && $this->masquerade->isMasquerading()) {
   +      return [
   +        'unmasquerade' => [
   +          '#lazy_builder' => ['masquerade.callbacks:renderSwitchBackLink', []],
   +          '#create_placeholder' => TRUE,
   +        ],
   +      ];
   +    }
        return $this->formBuilder->getForm(MasqueradeForm::class);
   

   the same here - 'unmasquerade' looks useless

An array containing an array isnt "useless". It sounds to me like you're viewing it as an unused element and that's just not the case. Explain what makes it "useless".

Re @crystaldawn because render will needs useless iteration in both cases to render children but all we need is just a link or empty string from lazy builder. Moreover both render arrays are not alterable so no key needed

I find it ready, gonna commit the MR tomorrow

And that's why extra wrapper array still required to use "#type" 🐛 Lazy builder broken (#type defaults not loaded) Needs work

Commited to 8.x-2.x, thank you

Created new release https://www.drupal.org/project/masquerade/releases/8.x-2.0-rc2 →

Note that the release note for 8.x-2.0-rc2 indicates that it includes this issue, which it does not. The commit for this issue was made after 8.x-2.0-rc1 was tagged and released. Ideally the release note should be edited to remove it.

@DanChadwick thank you! fixed rc2 release and created https://www.drupal.org/project/masquerade/releases/8.x-2.0-rc3 →

Untested, but from reviewing the code, just thought to confirm if this code snippet was correct?

+      if (isset($configuration['show_unmasquerade_link'])) {
+        $block->getPlugin()->setConfigurationValue('show_unmasquerade_link', FALSE);
+        return TRUE;
+      }

Shouldn't it be if (!isset($configuration['show_unmasquerade_link'])) instead?

@codebymikey looks like, it needs another patch

Like so?

Do we need another post-update script to fix this mistake?

I'm not sure if there's an easy way to address this with another post update hook, the best bet is to just put in the hotfix as quickly as possible to mitigate the damage.

Do we want a new critical issue for the patch or is #58 sufficient for now?

#58 seems sufficient to me.

Thank you! created new release https://www.drupal.org/project/masquerade/releases/8.x-2.0-rc4 →

Automatically closed - issue fixed for 2 weeks with no activity.