amitaibu / og

A fork to work on OG8
https://github.com/Gizra/og
29 stars 16 forks source link

Port Og-context module #242

Open amitaibu opened 8 years ago

amitaibu commented 8 years ago

@RoySegall @pfrenssen maybe you can get some poor dev(s) to work on this one in Dev days 😉 . It's a fun module, and with D8's new plugins it can be really cleaned up.

RoySegall commented 8 years ago

Do i smell another plugin type in OG?

amitaibu commented 8 years ago

I think the more plugins we have, the better the module is, no? Same goes for services and dependency injection. we need more and more!! 😈

pfrenssen commented 8 years ago

This would be really nice to have at this point. My colleague @sandervd already wrote a ContextProvider for OG in the D8 port of OG Menu. We are already leveraging this in our project, not only for showing menus from OG Menu, but to generically determine if the page shown belongs to a group.

Here's the code: https://github.com/ec-europa/og_menu/blob/8.x-1.x/src/ContextProvider/OgRouteContext.php

This currently iterates over all route parameters and if a group entity is found then it provides the group as context. This is a very broad catch-all approach, but it is a good starting point for this issue.

I am going to be at Drupal Dev Days from Thursday, but I have a big 4 hour workshop which I still need to prepare, so there will be little time to code unfortunately.

amitaibu commented 8 years ago

wrote a ContextProvider

Which module is providing this ContextProvider?

amitaibu commented 8 years ago

Oh, sorry I see it in core, didn't know about this one.

amitaibu commented 8 years ago

@pfrenssen I wonder if using ContextProvider is the right approach given it's a service? I would image some kind of an OgContextResolver plugins

amitaibu commented 8 years ago

(cc @sandervd)

RoySegall commented 8 years ago

In D7 branch we had a context per entity(and not all) such as: user, node, term and comment.

do you think we need to do the same separation in D8? Feel a bit redundant because the entity API has been improved.

pfrenssen commented 8 years ago

Probably not, like the example implementation in OG Menu, that only has one context that is compatible with any entity type.

pfrenssen commented 8 years ago

I'm not familiar with the D7 version of OG Context, what would be an example of 2 different context plugins? I have trouble imagining use cases for these plugins.

amitaibu commented 8 years ago

have trouble imagining use cases for these plugins.

for example:

pfrenssen commented 8 years ago

Ohh!! I see. Yes, very useful!

pfrenssen commented 8 years ago

We will need this in the next sprint, I can help on this probably somewhere starting next week. We are using the generic context from OG Menu at the moment, but we now need a cache context to use on our blocks that vary by OgRole.

If I'm going to work on that cache context I'd rather do it using the context from OG Context than the one from OG Menu.

amitaibu commented 8 years ago

, I can help on this probably somewhere starting next week.

That would of course be perfect.

but we now need a cache context to use on our blocks that vary by OgRole.

Indeed, the way I see it there will be an OgContext service that will invoke OgContetxPlugin plugins which fetch context for the service. And then you can cache it.

RoySegall commented 8 years ago

And I can assume this need to be a service since it would, probably, will be injected to plugins and other services.

pfrenssen commented 8 years ago

Will we be able to order the plugins, and select which one(s) will run?

I haven't thought a lot about it yet, but we need some semi-reliable way to determine the correct group on group content pages that belong to multiple groups. It would be great if you could have two plugins working together to provide the correct group in that case.

For example, we could have a UrlOgContext that runs first. According to the URL the group content belongs to two groups, so it can return an array of two groups. Then we can have a second plugin like a SessionOgContext that will keep track of the previous page that the user has visited, and will remember the previous group context.

The plugin manager could gather all the data from the active plugins and decide on the group which best matches the current page.

Maybe with a confidence interval? For example the plugins could return:

[
  // According to the url "node/123" we are on a group content page that belongs
  // to both "group1" and "group2". We are not certain about which is more applicable.
  'url' => [
    'group1' => OgContextInterface::NEUTRAL,
    'group2' => OgContextInterface::NEUTRAL,
  ],
  // According to the session info, the user is coming from a page that belonged
  // to group 1, but we are not sure if this new page is also from group 1.
  'session' => [
    'group1' => OgContextInterface::NEUTRAL,
  ],
  // The page has a query argument "?group=group1" that indicates with certainty
  // that the current page is to be seen in context of group 1.
  'query_argument' => [
    'group1' => OgContextInterface::CONFIDENT,
  ],
]

We could loop through the different plugins in the order set up by the admin, and return the first CONFIDENT result. If no confident results are returned, we can add up the NEUTRAL groups, and return the one with the highest count, since that would be the best match. If this doesn't yield anything, we simply return the first group.

This exact scenario won't work so well though if we are on a page that doesn't belong to a group or group content, if the UrlContext and QueryArgumentContext return nothing, and the SessionContext carries over the group from the previous page, then the group context will still be present:

[
  // The url "user/logout" does not belong to any group.
  'url' => [
  ],
  // According to the session info, the user is coming from a page that belonged
  // to group 1, but we are not sure if this new page is also if group 1.
  'session' => [
    'group1' => OgContextInterface::NEUTRAL,
  ],
  // There is no query argument.
  'query_argument' => [
  ],
]

This would still yield 'group1' as a result. We could solve it probably by passing the result sets of previous plugins to the SessionContext plugin, and only let it return something when any group has been found by the other plugins.

How was this dealt with in D7? If it was not dealt with we have a great opportunity here to make it work very nicely and powerful for D8 :)

amitaibu commented 8 years ago

Yeah, D7 deals with running several plugins, and finding the best match (e.g. the one you visited before that we hold in your $session )

On Jul 19, 2016 5:17 PM, "Pieter Frenssen" notifications@github.com wrote:

Will we be able to order the plugins, and select which one(s) will run?

I haven't thought a lot about it yet, but we need some semi-reliable way to determine the correct group on group content pages that belong to multiple groups. It would be great if you could have two plugins working together to provide the correct group in that case.

For example, we could have a UrlOgContext that runs first. According to the URL the group content belongs to two groups, so it can return an array of two groups. Then we can have a second plugin like a SessionOgContext that will keep track of the previous page that the user has visited, and will remember the previous group context.

The plugin manager could gather all the data from the active plugins and decide on the group which best matches the current page.

Maybe with a confidence interval? For example the plugins could return:

[ // According to the url "node/123" we are on a group content page that belongs // to both "group1" and "group2". We are not certain about which is more applicable. 'url' => [ 'group1' => OgContextInterface::NEUTRAL, 'group2' => OgContextInterface::NEUTRAL, ], // According to the session info, the user is coming from a page that belonged // to group 1, but we are not sure if this new page is also if group 1. 'session' => [ 'group1' => OgContextInterface::NEUTRAL, ], // The previous page has set a query argument "?group=group1" that indicates // with certainty that the current page is to be seen in context of group 1. 'query_argument' => [ 'group1' => OgContextInterface::CONFIDENT, ], ]

We could loop through the different plugins in the order set up by the admin, and return the first CONFIDENT result. If no confident results are returned, we can add up the NEUTRAL groups, and return the one with the highest count, since that would be the best match. If this doesn't yield anything, we simply return the first group.

This exact scenario won't work so well though if we are on a page that doesn't belong to a group or group content, if the UrlContext and QueryArgumentContext return nothing, and the SessionContext carries over the group from the previous page, then the group context will still be present:

[ // The url "user/logout" does not belong to any group. 'url' => [ ], // According to the session info, the user is coming from a page that belonged // to group 1, but we are not sure if this new page is also if group 1. 'session' => [ 'group1' => OgContextInterface::NEUTRAL, ], // There is no query argument. 'query_argument' => [ ], ]

This would still yield 'group1' as a result. We could solve it probably by passing the result sets of previous plugins to the SessionContext plugin, and only let it return something when any group has been found by the other plugins.

How was this dealt with in D7? If it was not dealt with we have a great opportunity here to make it work very nicely and powerful for D8 :)

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/amitaibu/og/issues/242#issuecomment-233646941, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHrC3fcm0m708fwwaZ_-ELQ-ZUsc2Byks5qXNyCgaJpZM4I56lC .

amitaibu commented 8 years ago

Roy, you probably won't need to inject the service. The service is in fact OgContextManager - it invokes the plugins to get their results.

On Jul 19, 2016 5:30 PM, "Amitai Burstein" amitai@gizra.com wrote:

Yeah, D7 deals with running several plugins, and finding the best match (e.g. the one you visited before that we hold in your $session )

On Jul 19, 2016 5:17 PM, "Pieter Frenssen" notifications@github.com wrote:

Will we be able to order the plugins, and select which one(s) will run?

I haven't thought a lot about it yet, but we need some semi-reliable way to determine the correct group on group content pages that belong to multiple groups. It would be great if you could have two plugins working together to provide the correct group in that case.

For example, we could have a UrlOgContext that runs first. According to the URL the group content belongs to two groups, so it can return an array of two groups. Then we can have a second plugin like a SessionOgContext that will keep track of the previous page that the user has visited, and will remember the previous group context.

The plugin manager could gather all the data from the active plugins and decide on the group which best matches the current page.

Maybe with a confidence interval? For example the plugins could return:

[ // According to the url "node/123" we are on a group content page that belongs // to both "group1" and "group2". We are not certain about which is more applicable. 'url' => [ 'group1' => OgContextInterface::NEUTRAL, 'group2' => OgContextInterface::NEUTRAL, ], // According to the session info, the user is coming from a page that belonged // to group 1, but we are not sure if this new page is also if group 1. 'session' => [ 'group1' => OgContextInterface::NEUTRAL, ], // The previous page has set a query argument "?group=group1" that indicates // with certainty that the current page is to be seen in context of group 1. 'query_argument' => [ 'group1' => OgContextInterface::CONFIDENT, ], ]

We could loop through the different plugins in the order set up by the admin, and return the first CONFIDENT result. If no confident results are returned, we can add up the NEUTRAL groups, and return the one with the highest count, since that would be the best match. If this doesn't yield anything, we simply return the first group.

This exact scenario won't work so well though if we are on a page that doesn't belong to a group or group content, if the UrlContext and QueryArgumentContext return nothing, and the SessionContext carries over the group from the previous page, then the group context will still be present:

[ // The url "user/logout" does not belong to any group. 'url' => [ ], // According to the session info, the user is coming from a page that belonged // to group 1, but we are not sure if this new page is also if group 1. 'session' => [ 'group1' => OgContextInterface::NEUTRAL, ], // There is no query argument. 'query_argument' => [ ], ]

This would still yield 'group1' as a result. We could solve it probably by passing the result sets of previous plugins to the SessionContext plugin, and only let it return something when any group has been found by the other plugins.

How was this dealt with in D7? If it was not dealt with we have a great opportunity here to make it work very nicely and powerful for D8 :)

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/amitaibu/og/issues/242#issuecomment-233646941, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHrC3fcm0m708fwwaZ_-ELQ-ZUsc2Byks5qXNyCgaJpZM4I56lC .

RoySegall commented 8 years ago

@amitaibu When looking at the D7 version the URL negotiation need entity reference pre populate. There is no D8 version for this module and my next question - which negotiation strategies we need? For now we pulling the group from the current viewed entity. Do you want to pull the membership from the current logged in user for example?

@pfrenssen Do you know any other ways to pull the group? Maybe something you faced with in one of your projects?