add a context provider

[chx]

Our site is pretty much split into subsites by group. Displaying the relevant blocks on a group node or a node belonging to a group is possible with this context provider. If it's not generic enough just won't fix this and I will dump it into a separate sandbox module. Here's how placing a block with * "node" = @ContextDefinition("entity:node", label = @Translation("Node"), required = FALSE), looks:

[chx]

Everything before $node instanceof NodeInterface is core code, copy-paste from NodeRouteContext, and so are the parts including and after $cacheability. Reviewing those are pointless. Speaking of pointless, propably checking whether $node instanceof NodeInterface is pointless but it's cheap enough...

[pfrenssen]

Why would you limit this to nodes? To me the idea of using a node as a group type is odd. I know it was common to do it like this in D7, but to me it makes more sense to use a custom Group entity type.

[chx]

We are a D7 port and this PR needs to reuse #252 anyways which unhardwires node. I will probably do it during Drupalaton (are you coming?)

[amitaibu]

I believe there's a similar effort by @RoySegall on #252 . The idea is indeed to provide a Context provider that is being fed by OgContext plugins.

[amitaibu]

Maybe worth joining efforts on that PR?

[RoySegall]

My PR was a shallow one - just plugin that return the first group without all the special logic. I added a test that send the user to a group but it fails for some reason which I'm not aware of yet.

If @chx want to join in and help with the tests - he is more than welcome.

[chx]

I do not quite know what that PR is about but it's not about this. I tried to understand but failed -- and when I searched for ContextProvider then I found none. Please help me understand, I do not quite understand the core context system but I read #252 and I haven't found any core interfaces implemented or tagged services so I don't get it.

[amitaibu]

Sure, I'm not too familiar with #252, so I will explain the idea in generel, which is porting Og-Context module to OG8.

Here's the logic:

• We would have OgContext plugins. Each plugin knows how to return group content according to different rules. We could have one that finds it in the router, and another that finds it in a query string.
• Those plugins return their matched groups to some OgContextProvider, which will decide on the "best matching group" and set the context. The best matching logic is currently (by order):

1. Show a group a user has access to
2. If user visited on previous page content under group B, and they are now visiting a content under group A & Group B - we will show them group B as context - for continuity.
3. A user is member of

Here's the relevant code from OG7:

• The "Plugins"
• The "context provider"

@chx , Hope it clarifies a bit the intention :)

[chx]

So #252 is WIP and not yet integrating with core but eventually it wants to...?

[amitaibu]

Indeed it's WIP.

But as @RoySegall stated, he's open to collaboration, so if you'd like to tackle this task differently in terms of steps to reach the goal, that's of course possible to discuss.

[chx]

I still do not know whether we have the same goals :) I am providing a context for core, mostly for blocks. It uses a different logic to what you described:

1. Get a node from URL; if it's a group return it
2. If it belongs to a group, return the first one.

That's it. But, it's a core context provider. I simply do not see what is the end goal of yours, I am guessing it might also want to be a ContextProvider but I do not know.

[amitaibu]

I think we're close, however your use case is just hardcoded to getting groups out of nodes (although agreed it will be a common case).

So my goal is that instead of hardcoding the specific check for nodes in the core's ContextProvider, we delegate the job of getting that groups to OgContext plugins. So you could have one plugin that gets the context from a node, and another arbitrary one that will do it by the time of the day.

Those plugins pass their groups to the ContextProvider that will indeed select the "best matching" one, and return it.

In short: The current logic you put in place seems like a great start, and the next step is baking the OgContext plugins in.

[chx]

So you want to add a ContextProvider to #252 to actually use the logic created there. And then I guess add the logic from here to there as well?

[chx]

And to clarify further: I have no idea what hook_og_context_negotiation_info is, what module it integrated with and ... anything at all. I do not see the connection points to the rest of Drupal in the D7 version either.

[RoySegall]

#252 need to handle a smaller scope - return the first group that the context handler will find. After the merge then we can move to higher scope. This is the case of the PR as I can see it.

[chx]

So let's clarify: there's no such thing as a "context handler". You are making a horribly confusing mess by calling it "OgContext" when core has a context subsystem.

[amitaibu]

I haven't gone over #252 properly yet, so I just know there's the effort but can't say much about the code there.

If you are ok and have time to work on the above goals, we can take your PR as a starting point as-well. I'm sure @RoySegall won't be insulted :)

And to clarify further: I have no idea what hook_og_context_negotiation_info is, what module it integrated with and ... anything at all.

No problem, I just wanted to link to the OG7 code that does the same. The gist of it is, that we are porting the same concepts, only with D8 code.

[amitaibu]

You are making a horribly confusing mess by calling it "OgContext"

Yes, we can of course change it. It's mostly since it was called in D7 og-context module. Context is only second to the word Content in Drupal when it comes to over using and abusing words 😉

[chx]

OK, so let's get back here. #252 has nothing to do with this PR then :) except perhaps I could reuse the "entity" OgContext plugin.

[amitaibu]

#252 has nothing to do with this PR then :)

I guess that's debatable 😉 I think they are both related to the end goals -Having a pluggable context provider. They just tackle it from different ends. I wouldn't mind tackling this task in small PRs as long as the end goal is clear (unless of course you think there are better ways).

In terms of names, to prevent confusion with the core context how about:

• Context provider : class OgContext implements ContextProviderInterface {
• Plugin manager: class GroupResolverManager extends DefaultPluginManager {
• Single Plugin: class GroupResolver extends GroupResolverInterface {

[chx]

The names sound good. There are some challenges left if you want to indeed create class OgContext implements ContextProviderInterface. But I agree we eventually need to.

[amitaibu]

The names sound good.

Thanks for pointing out the ones I thought off sucked 😄

There are some challenges left if you want to indeed create class OgContext implements ContextProviderInterface

What difficulties do you foresee?

[chx]

I just discussed some of this with @timplunkett and in particular

public function getAvailableContexts() {
  $context = new Context(new ContextDefinition('entity:node', $this->t('Node from URL')));
  return ['node' => $context];

}

this makes me wonder as core does not have a term from URL, a user from URL etc context but og will have to have one as groups are any entity type so while of course it's very doable we are sort of on uncharted territory.

[amitaibu]

we are sort of on uncharted territory.

My favorite place 😉

[timplunkett]

The ones that exist in core were the result of straight ports of parts of the block visibility system. User/Term from URL could easily be added to core in 8.3 or any minor.

[chx]

Thanks for shoving up! What about an EntityContextProvider which does the same as NodeContextProvider but for every entity? And yes, it's relevant to this discussion because that's essentially what we would be doing sprinkled with some og logic

[amitaibu]

What about an EntityContextProvider which does the same as NodeContextProvider but for every entity?

Yes, that's exactly what OG will need. It would be wonderful if it could extended a core EntityContextProvider

[pfrenssen]

In our project we are using the OgRouteContext context provider that is bundled with OG Menu. It loops over all parameters on the route and checks if they are a group, regardless of their entity type.

I think that version would be a good starting point. It needs some tweaks though, it cannot deal with routes that have multiple group parameters.

https://github.com/ec-europa/og_menu/blob/8.x-1.x/src/ContextProvider/OgRouteContext.php

[pfrenssen]

Ping @sandervd who wrote the ContextProvider in OG Menu.

[chx]

That code is alright but compare to #252 and see the discussion above and the code in here. We do not just want a group entity to appear as the context value but the group the per route entity belongs to.

[amitaibu]

We do not just want a group entity to appear as the context value but the group the per route entity belongs to.

In the end the OgContextProvider will return a single group (what I call "the best matching"), but indeed it should get all the context candidates from the GroupResolver plugins

[chx]

Did you do the rename already to GroupResolver :) ?

[amitaibu]

Only in my brain, not in code yet :)

[sandervd]

I really like plugin based approach of @RoySegall.
I would have probably invoked an event from inside ContextProvider, but the plugin approach is much more considerate towards end users (configurable weight etc.)...

If I'm not mistaking, the next step would be to wrap all of this inside a ContextProvider, right?

However, when we do so, will there be only one Og context generated, or should it be possible to create multiple 'contexts' (with individual settings)?
(Let's say when using multiple group types on one site?)

[amitaibu]

If I'm not mistaking, the next step would be to wrap all of this inside a ContextProvider, right?

Yes. I'll add some overview to #252, so it's easier to pickup the work there.

However, when we do so, will there be only one Og context generated, or should it be possible to create multiple 'contexts' (with individual settings)?

The GroupResolver plugins will return multiple groups, but eventually OgContextProvider will select only a single one. The logic is that when your theme/language/whatever needs to change according to context it should act only on a single context.

@chx I'll close this one, and lets move collaboration and discussion to #252

[amitaibu]

I'd like to circle back to this one. OG8 is taking the approach oh simplifying stuff from OG7, and after some thought I think that we can un-port OG-Context module and just have a non-node specific context provider in.

The reason is that the context from the route is the most common one, and if a dev would like to extend it, they could have their own context provider.

@RoySegall @pfrenssen will that work for your use cases?

[pfrenssen]

I'm not familiar with what OGContext was doing in D7, but what I understood from previous discussions is that it was able to figure out the group context of things like group content that belongs to multiple groups, depending on the user session.

For me it makes sense to have a plugin based system, I see this similar to how for example language negotiation works. My context could depend on:

• The route object, if it is a group.
• The route object, if it is group content.
• The previous group context from the user session, in case the user moves from a page that belongs to one group to a page that belongs to multiple groups.

For complex sites a developer will need custom rules, but this would be another plugin that can complement the ones that are already there.

[RoySegall]

For me it makes sense to have a plugin based system

My PR provided the infrastructure for that. I'll get back to this one this week.