'Required contexts without a value: node' when editing menu

Re-roll of #12.

This is working for me.

#12 works with D9.3.22. It also solved my page_manager problem related to contexts and controllers matching.

When upgrading from 9.1.2 to 9.3.22 received this error caused by page_manager when attempting drush updb.

PHP Fatal error: Declaration of Drupal\page_manager\Entity\Page::getContexts() must be compatible with Drupal\page_manager\PageInterface::getContexts

Applying this patch at #17 solved the issue.

(I am in Drupal upgrade prison with a large site. Hopefully this will get indexed into Google and help any others in same boat).

We first used this patch as well and it seemed to fix our problem. But manipulating the request this way will lead to a loss of information on the route in some circumstances which will result in other, unfixable issues.

For us the solution was to use page managers PAGE_CONTEXT Event to add our missing context which looked kinda like this for our missing group context:

  public function onPageManagerPageContext(PageManagerContextEvent $event) {
    $page = $event->getPage();
    foreach ($page->getContexts() as $context) {
      if (!$context->hasContextValue()) {
        $context_definition = $context->getContextDefinition();
        if ($context_definition->getDataType() == 'entity:group') {
          // Tell page_manager about our context value.
          $page->addContext('group', new Context($context_definition, $this->getGroupFromRoute()));
        }
      }
    }
  }

@Hydra #20:
> But manipulating the request this way will lead to a loss of information on the route in some circumstances which will result in other, unfixable issues.

This is a bit foggy and not productive in getting this further. Can you elaborate?

Patch #17 applies cleanly on current core 10.2.2 and fixes the issue for me.

Code looks good and reasonable, and i can not see or guess what the #20 objection may mean.
(#17 manipulates a separate request and pushes / pops it onto the stack.)

This fix MAY be affected by the related long-standing core issue though.
So if people experience new problems with this patch, imho they should try applying the other too and report back.

Setting NW for an automated test, which might look like this:
- Create a page with a context, use the context to control page access
- Create a $url for that page and call $url->access()

@geek-merlin - it's a while ago I faced this issue. But as far as I can remember (and my git history tells me) we had a local task (tab) added to the group tasks which pointed to a page_manager page route. That page of course has the group context configured and used a custom condition plugin to determine the access. That condition used the group context to check for some information related to the group in order to grant or deny access. In combination with this patch, this resulted in an error on the local task access check level - kinda related to what berdir described what could happen to menu links in #5
Removing the patch and adding the event subscriber with the provided code in #20 solved that for us and solved the original issue we used the patch for in the first place.

#23: Thanks. Nothing that can be reproduced though.

Looked a bit deeper into this and found that the logic of #17 is not enough in many cases.

Rolled a MR that is way more robust and fixes the issue for me. Still needs tests though.

Made a simplification, then squashed.
Patch url: https://git.drupalcode.org/project/page_manager/-/commit/02e263051d80646...

Ah OK, this needs a rebase.
The related issue changed the code here a lot.

Maybe it solved this issue too? Let's see.

Finally i found some time to look into this thoroughly.
@berdir is right that the former fake-request approach is at least limited and possibly FUBAR.

So...
- created a SubRequestAccessChecker and fixed some core limitations.
- added tests, which are green.
- also test-only changes are red (nice we have that now!), which proves that the MR fixes the issue.

Nice.

Squashed commit for local application: https://git.drupalcode.org/project/page_manager/-/commit/d3764a67c50e69e...

Looks great by skimming!

btw why test-only fails? https://git.drupalcode.org/issue/page_manager-2837833/-/jobs/3186611

Meantime different approach started since #2613044-123: Requests are pushed onto the request stack twice, popped once →

Also some workaround via ✨ Add drupalGet() to KernelTestBase Active

@andypost #31
> Looks great by skimming!

Nice you looked into it and like it too! I think the approach has quite some potential.
Can you give a code review (make all corrections you think fit) and help move this forward?

#32: Looks enigmatic to me... wrong issue?

Let's handle 🐛 Requests are pushed onto the request stack twice, popped once Needs work then.

Squashed: https://git.drupalcode.org/project/page_manager/-/commit/5a62749ad8bf170...

Yep, that works for me on a complex site.

Ah, request context must be fixed too.