WIP: First sketch of the OG context module.

[RoySegall]

Goal

Port og-context module to OG8.

GroupResolver are plugins that can extract group for the pages or entities they are acting on. One can get the groups from the router, and another arbitrary one that will do it by the time of the day.

Those plugins pass their groups to the OgContextProvider that will select the "best matching" one, and return it. 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. User is member of the group

   Naming

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

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.

[RoySegall]

@amitaibu Before I'll start implementing the changes don't you want me to moe og context plugin into OG core?

[RoySegall]

I moved OG context into core. Now I can say I started the work on OG context.

[RoySegall]

Another update - Now we have the next two cool, IMO, API:

    $plugins = Og::contextHandler()->getPlugins();
    $plugin = Og::contextHandler()->getPlugin('entity');

[amitaibu]

I have updated the issue summary.

[amitaibu]

@RoySegall can you please rename the classes/ plugins to be the same as in the issue summary.

[RoySegall]

I changed the plugin to Group resolver from `OG context'

[RoySegall]

OK. I got the rename of the classes. Now I need to extend the OgContext form ContextProviderInterface

[amitaibu]

What's the status here?

[RoySegall]

I started to work on extending from ContextProviderInterface and I got interrupted. I'll try to get back to it tonight.

[RoySegall]

@amitaibu I extended OgContextInterface from ContextProviderInterface and I there are two methods I need to implements but I still can't see how they suppose to fit in.

If some one can throw some tips that would be awesome.

[RoySegall]

I dig a bit in the code and it seems that we need to create a service, in addition to the context group resolver, that will look like this:

  og.og_route_context:
    class: Drupal\og\ContextProvider\GroupRouteContext
    arguments: ['@current_route_match']
    tags:
      - { name: 'context_provider' }

and that service will use the logic I created so far. After that we can have in any plugins something like this:

/**
 * Provides a 'User Role' condition.
 *
 * @OgTask(
 *   id = "people",
 *   label = @Translation("People"),
 *   context = {
 *     "group" = @ContextDefinition("group", label = @Translation("Current group"))
 *   }
 * )
 */

But IMO this will be another PR. In the mean while I'll keep working in the tests.

[RoySegall]

I added a skeleton for the test and this time I did not got any weird errors from PHPUNIT.

[amitaibu]

Great. Can you please re-roll, and re-open this PR against Gizra/og

[RoySegall]

One the tests will work as expected I'll create new PR against gizra/OG. In the meanwhile don't close the PR.

10x.

[pfrenssen]

@RoySegall would you be OK if I continue on this? I have time to work on it this week.

I would like to start with adopting the naming scheme that @amitaibu suggested in https://github.com/amitaibu/og/pull/282#issuecomment-236657256:

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

And then secondly get the tests green.

[pfrenssen]

I had a look through the code but it seems to me that the chosen approach is more complex than needed. Maybe I am not seeing the use cases that require this complex approach, but the way I imagined this to work was like this:

• Some code needs to know the group context and calls OgContext::getRuntimeContexts().
• OgContext::getRuntimeContexts() loads the og.group_resolvers config to get the list of enabled group resolvers in the right order.
• OgContext::getRuntimeContexts() requests the enabled group resolver plugins from GroupResolverManager::getInstance().
• OgContext::getRuntimeContexts() calls each plugin in the right order, and if it finds one that returns a single result, it loads this group and returns it as context.

In my mind the whole procedure would be like 50 lines of code + a couple hundred lines for the default config, the individual plugins and tests.

This approach with the separate handler and saving plugin configuration in a dedicated GroupResolverNegotiation entity, and RETURN_ONLY_IN_STORAGE seems to be quite complicated for what we are trying to achieve.

[amitaibu]

I always enjoy seeing your code review!

I think your analysis is very good, with a single remark:

OgContext::getRuntimeContexts() calls each plugin in the right order, and if it finds one that returns a single result, it loads this group and returns it as context.

OgContextProvider::getRuntimeContexts() will call the OgGroupResolver plugins, and let them return all their values. That is, it will not break on the first group that is found.

Then, it will apply some simple (hardcoded) rule to get the "Best matching" group, by order:

1. User has that group in the session, because in the previous page requests they go it as context + user has access to it. This is for cases when yuu previously visited a group content that belonged to group A, and now you visit a group content that belongs to groups A and B -- so we should be nice and keep you in the A context.
2. User has view access to the group
3. The first rule

[pfrenssen]

Thanks for the feedback! With your permission I will continue this work and try to bring it to conclusion. I'll make a new PR with the simplified approach.

[amitaibu]

👍

[RoySegall]

@pfrenssen Any new on this one? I kind of lost context on this one in the past few weeks.