🇮🇱Israel @amitaibu

@loze Thanks for the screenshot. I believe each element warrants its own issue or discussion, allowing us to keep this MR concise.

Are there any other changes that need to be made in the MR, or is it ready for review?

> Is the intent of this setting to grant all permissions for the administrator role?

Yes, that was the intent on 7.x. The idea was that sometimes you'd like the group owner to act like UID 1 on a site. But on some sites, the group owner doesn't need to have all those privileges.

Revisiting this concept, I think a better solution would be not special-casing the group owner. Instead, if you'd like to give them more access, you can auto-assign them an OG Role with more permissions.

Merged, thanks!

Thanks, I've removed the reference to GitHub.

Looks good, thanks. Can you change the MR to be against 2.x branch please.

@gogowitsch I see the changes haven't been applied to the MR. Also it's still showing ':link' (should be `@link`)
And it still has the `:edit` which isn't needed anymore.

It would be great if you could address those.

Thank you for the explanation, however, I still think we need to change:

> The word Edit is a separately translated tab title.

It works in English, but it might not work in other languages (Hebrew is one of them, where the word changes depending on the sentence).

But yeah, I agree it would have been nicer for something to "Just work" without all the extra baggage.

> There must be a more efficient way to achieve this, rather than copy-pasting the same configuration everywhere?

@aaronbauman I don't think there is one currently. Additionally, ddev requires maintaining the `.ddev` directory with several files, so I'm unsure if it's possible to create one without those extra files. It's also worth noting that ddev is the recommended → solution by Drupal for local development.

If it helps, we're using it for the Organic Groups module, and it's really convenient. 😊

Great 💪

Will you be using a queue for processing the items, or is it entirely custom?

🎉

> I was halfway through writing this when you posted your comment, so I'm going to post it anyway, then consider your points above/

haha, no worries :)

> This phase can store JSONAPI data in the metadata if it has it available, so it doesn't need to be fetched from the remote again.

👍

That's an interesting challenge!

Short recap

• We should move away from one big batch and break the work into small tasks.
• Tasks can fail. We need safe retries for the same action.

Possible flow

Assuming we start with one “parent” entity:

1. Put entity ID 1 into a queue named items_to_fetch_info.
2. Get entity ID 1 from queu and call JSON:API to fetch the entity and all related references. At this stage we only care about entity types and IDs. No processing yet.
3. Build the tree of related entities and flatten it into a list ordered bottom-up. Grandchildren first, then children, then the parent.
4. Queue that ordered list into a process_items queue.
5. Store a parent reference on each queue item, so items know which item they depend on. This can live in the payload or in a dedicated field if you extend the queue item entity.
6. Process one entity at a time. If it fails, let the queue retry later with backoff.
7. Once the parent entity is processed, the whole unit is done.

Advanced Queue advantage

We can query queue items with EntityQuery and apply your own logic. Example: do not fetch a child for processing until its parent is completed.

> we can maybe just reach into it with the queue API.

I think that's the way to go. The advantage is shifting the concept from "I process it right now", to:

1. Gather the dependencies by order, and queue them by order.
2. Now they can be processed one by one, however long it takes
3. If something is changed in the entities while they are still processed, we can re-queue, and re-process

This means that debugging and logging are becoming slightly easier, as you'd be dealing with one entity at a time, rather than a big tree of entities.

I would also recommend relying on https://www.drupal.org/project/advancedqueue → . Having completed queue items stay forever/ long period is great for auditing. Also, the queue items are entities and thus fieldable. Allowing adding metadata on top of them.

> Have you had any issues with #3539265: pulled dependencies don't participate in the batch, risk of PHP execution timeouts?

Not so far :)

Setting back to Needs review, as MR is now against the 4.x version

amitaibu → created an issue.

Merged, thanks!

After cloning the module locally, you can now:

ddev poser
ddev symlink-project
ddev install

And also run tests, for example ddev phpunit --filter NodeOverrideTest

amitaibu → created an issue.

Using https://www.drupal.org/project/salesforce/issues/3543367 ✨ Add DDEV Drupal Contrib integration Active I've confirmed tests are indeed passing locally, and they are

ddev phpunit --filter SalesforcePullEventTest 
ddev phpunit --filter SalesforcePushAllowedEventTest

I've updated the README (copied from OG) and it's working as expected.

After cloning the module locally, you can now:

ddev poser
ddev symlink-project
ddev install

Ready for review 🙂

I should have read the README. This works fine

ddev poser
ddev symlink-project
ddev install

With the MR ddev composer install works

ddev composer install                           
Installing dependencies from lock file (including require-dev)
Verifying lock file contents can be installed on current platform.
Nothing to install, update or remove
Generating autoload files
46 packages you are using are looking for funding.
Use the `composer fund` command to find out more!

However I still get an error on ddev install

ddev install         
PHP Fatal error:  Uncaught AssertionError: assert($this->bootstrap instanceof DrupalBoot8) in /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php:119
Stack trace:
#0 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(119): assert()
#1 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(108): Drush\Boot\BootstrapManager->bootstrap()
#2 /var/www/html/vendor/drush/drush/src/Application.php(165): Drush\Boot\BootstrapManager->setUri()
#3 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(97): Drush\Application->refineUriSelection()
#4 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(40): Drush\Runtime\Runtime->doRun()
#5 /var/www/html/vendor/drush/drush/drush.php(140): Drush\Runtime\Runtime->run()
#6 /var/www/html/vendor/bin/drush.php(119): include('...')
#7 {main}
  thrown in /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php on line 119

amitaibu → created an issue.

drush can now be installed with ddev composer require drush/drush

But next issue is that ddev drush cr gives an error

PHP Fatal error:  Uncaught AssertionError: assert($this->bootstrap instanceof DrupalBoot8) in /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php:119
Stack trace:
#0 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(119): assert()
#1 /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php(108): Drush\Boot\BootstrapManager->bootstrap()
#2 /var/www/html/vendor/drush/drush/src/Application.php(165): Drush\Boot\BootstrapManager->setUri()
#3 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(97): Drush\Application->refineUriSelection()
#4 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(40): Drush\Runtime\Runtime->doRun()
#5 /var/www/html/vendor/drush/drush/drush.php(140): Drush\Runtime\Runtime->run()
#6 /var/www/html/vendor/bin/drush.php(119): include('...')
#7 {main}
  thrown in /var/www/html/vendor/drush/drush/src/Boot/BootstrapManager.php on line 119

I'm able to access the site, but I'm getting errors on `ddev composer require drush/drush`

With it I can now execute, for example:

ddev phpunit --filter RestClientTest                      
PHPUnit 11.5.34 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.23

....................                                              20 / 20 (100%)

Time: 00:00.087, Memory: 8.00 MB

OK, but there were issues!
Tests: 20, Assertions: 41, PHPUnit Deprecations: 61.

Maybe worth adding something like what we have on Organic groups? (provided by https://github.com/ddev/ddev-drupal-contrib)

Added via https://www.drupal.org/project/salesforce/issues/3543367 ✨ Add DDEV Drupal Contrib integration Active

amitaibu → created an issue.

Tests seem to pass on the MR, but I want to ensure it's indeed working. How do you execute tests locally?
Maybe worth adding something like what we have on Organic groups?

Working on an MR. Let's see if Claude AI gets the tests close to working :)

amitaibu → created an issue.

Merged, thanks!

amitaibu → changed the visibility of the branch 3538579-1.x to hidden.

I've created a new MR - https://git.drupalcode.org/project/entity_share/-/merge_requests/126

> Could you rebase onto 4.0.x please?

Sure.

Context: We have a Central site and a few Satellite sites. On the Central site we're using the Translation system. So 1 Language == 1 Satellite site.

On the central site, upon hook_node_update(), I call this logic, which will auto-subscribe/ sync my updated node on the satellite sites.

namespace Drupal\general_entity_sync_server;

use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Database\Connection;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Logger\LoggerChannelFactoryInterface;
use Drupal\Core\Messenger\MessengerInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Drupal\entity_share_websub_hub\Subscription;
use Drupal\node\NodeInterface;
use GuzzleHttp\Client;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\Response;

/**
 * Defines a class for implementing hooks.
 *
 * @internal
 */
final class GeneralEntitySyncHooks implements ContainerInjectionInterface {

  use StringTranslationTrait;

  /**
   * GeneralSsoServerHooks constructor.
   */
  public function __construct(
    private readonly EntityTypeManagerInterface $entityTypeManager,
    private readonly Client $httpClient,
    private readonly LoggerChannelFactoryInterface $loggerChannelFactory,
    private readonly MessengerInterface $messenger,
    private readonly ConfigFactoryInterface $configFactory,
    private readonly Connection $database,
    private readonly Subscription $webHubSubscriptionManager,
  ) {}

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container) {
    return new static(
      $container->get('entity_type.manager'),
      $container->get('http_client'),
      $container->get('logger.factory'),
      $container->get('messenger'),
      $container->get('config.factory'),
      $container->get('database'),
      $container->get('entity_share_websub_hub.subscription'),
    );
  }

  /**
   * Notify Satellite sites for subscription of new translations.
   *
   * We call this on node update hook and not insert hook because insert hook
   * is not invoked when a new translation is created.
   *
   * @param \Drupal\node\NodeInterface $node
   *   Node.
   */
  public function notifySatelliteSites(NodeInterface $node) {
    // Get node details.
    $entity_type_id = $node->getEntityTypeId();
    $node_type = $node->getType();
    $langcode = $node->language()->getId();
    $uuid = $node->uuid();
    $title = $node->getTitle();

    // Get existing subscriptions.
    $subscriptions = $this->webHubSubscriptionManager->checkSubscriptions($node);
    $subscribed_channels = [];
    if (!empty($subscriptions)) {
      // Batch query to get all subscribed channel IDs at once.
      $results = $this->database->select('entity_share_websub_hub_subscription', 't')
        ->fields('t', ['channel_id'])
        ->condition('t.sid', $subscriptions, 'IN')
        ->execute()
        ->fetchCol();

      // Use array_flip for O(1) lookups.
      $subscribed_channels = array_flip($results);
    }

    /** @var \Drupal\entity_share_server\Entity\ChannelInterface[] $channels */
    $channels = $this->entityTypeManager->getStorage('channel')->loadMultiple();

    foreach ($channels as $channel) {
      // Skip if the entity doesn't match entity type and bundle configured to
      // the channel.
      if ($entity_type_id !== $channel->get('channel_entity_type') || $node_type !== $channel->get('channel_bundle')) {
        continue;
      }

      // Skip if the channel language doesn't match the current node's language.
      $channel_langcode = $channel->get('channel_langcode');
      if ($langcode !== $channel_langcode) {
        continue;
      }

      $channel_id = $channel->id();

      // Skip if already subscribed.
      if (isset($subscribed_channels[$channel_id])) {
        continue;
      }

      // Prep data for POST request to satellite sites.
      $satellite_site_url = $this->configFactory->get('general_entity_sync_server.settings')->get("channel_to_window_sites_mapping.$channel_langcode");
      $data = [
        'remote_id' => 'tgeneral_central',
        'channel_id' => $channel_id,
        'uuid' => $uuid,
      ];

      try {
        $response = $this->httpClient->post($satellite_site_url . "/entity-websub/auto-subscribe", [
          'json' => $data,
        ]);
        if ($response->getStatusCode() == Response::HTTP_OK) {
          // Display success message to users.
          $this->messenger->addMessage($this->t('@bundle content @title also synced to @site_id', [
            '@bundle' => $node_type,
            '@title' => $title,
            '@site_id' => $channel_id,
          ]));
        }
      }
      catch (\Exception $e) {
        // Log error for other status codes which will throw exception.
        $this->loggerChannelFactory->get('general_entity_sync_server')->error($this->t('Cannot sync @bundle content @title to @site_id. Sync failed with error: @error', [
          '@bundle' => $node->bundle(),
          '@title' => $node->getTitle(),
          '@site_id' => $channel_id,
          '@error' => $e->getMessage(),
        ]));
      }
    }
  }

}

Happy to follow a different path if you think there is one better

amitaibu → created an issue.

Let's keep the API as is. We can introduce new methods, `buildReferencedEntitiesWithViewModes` and `buildEntitiesWithViewModes` .

So we don't end up with one argument acting in different ways.

Merged, thanks

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

haha, no worries.

Yes, let's go ahead with such a helper

> I'm not really sure what to do for adding a test.

I encourage you to give it a try, I'm around to support you if needed :)

Here's an existing example - https://git.drupalcode.org/project/og/-/blob/1a8edc6084a2a7449e00075f0eb...

Maybe I'm missing something -- doesn't `$ogRole->label` hold the regular name? Can't we query that?

> Would it make sense to introduce a role name (label-based) lookup helper so that the behavior mirrors D7?

Do you have a new table in mind for that?

The issue is that you can have one or multiple OG audience fields, but there's no hardcoded 'og_audience' name. It's just a default.

In that regard, it's similar to any entity reference fields. If you accidentally delete those, you'll also lose functionality. Or @joachim, do you have anything else in mind?

@dipakmdhrm, thanks - the pipeline is failing. If you can look at it, please.

> regardless of field name, without assuming a specific implementation or locking down unused fields.

👍

Just throwing an idea: `OgRole` is a fieldable entity. So you could have a `field_simple_label` that would hold the name of the role as you expect to.

Then Views core Contextual filters could be setup to filter by that field.

Does that make sense for your use case?

Thanks, @loze. Can you please attach a screenshot showing what has changed?

Also, could you add tests to the MR?

Just to make sure I understand. I don't remember how it works on D7, but based on what you wrote, you want each role label to appear only once (Admin, Member, Non-admin).
And it should work regardless of the group type, because you want it to filter by the role label?

Can you give your use case, that might help finding the right solution

Merging, thanks

The code looks good. I see there's a failed job, likely not related to this MR

@joelpittet can you please attach a screenshot showing this Views plugin in action

Here's the link to the issue with the patch
https://www.drupal.org/project/footnotes/issues/3464154#comment-16117378 ✨ Enable individual footnotes while footer is disabled Active

For some reason, I don't see it was merged on the issue - but it was https://git.drupalcode.org/project/og/-/commit/585cd45aff56012ac033de471...

Thanks 🙏

Thanks folks. Can we add a test to confirm this behavior?

Here it is. Not a direct comparison, but should give an idea of how different it is (very).

https://www.gizra.com/blog/pevb-video/

> Should we move this to 2.x?

Yes

Yeah, it sounds like a bug.

We should only show OG-specific handlers that are valid for OG reference fields.

A bit of context, and then I'll try to answer what I know :)

The reason we have the OG field, is that we could check for an existing of such a field instance, to determine if the node (or any other entity) is a group or group content.

Next, by having the OG field, we can create Selection handlers that would target only this field. That is, by knowing it's an OG reference field, then the selection handler already knows it should show only references to entities that are OG Groups.

> Investigate why og:default is being replaced by default:node.

Sounds like a bug.

> Decide whether OG should override the selection handler at runtime rather than during field creation.

It's better on field creation. We should follow as much as we can what Drupal core is doing, and avoid magic. The sin of the OG complex field is that it's trying to do that. Show both the "My groups" and the "Other groups". But if it was a "regular" ER (entity reference) field, we would achieve it by having some custom code that is as simple as creating an outgroup select list, grouped by "member" or "non-member".

> This also forces the user to select a bundle because:

Like a regular ER field, we should also let the user select a bundle. As one field could reference Group1 bundles, and another could reference Group2 bundles.

That's a great suggestion. I'm working on my BoF for DrupalCon, so I would have that info soon.

The short answer, is that we're skipping the rendering of the fields (along with the fields formatters), or any special layout module.
Instead we have full control over what's printed on the page. The node acts as a content repository, and we craft the output with re-usable render arrays.

More info to come :)

Heya!

Thanks, maybe you can instead continue the work on https://git.drupalcode.org/project/pluggable_entity_view_builder/-/merge... ?

> are we relying on Drupal’s permissions, or are we using our own?

On the Site's level there's only the `administer groups`, that should give a site admin all access to all groups.
Then, the rest of the permissions are on the OG level.

Thank you @herved, the idea of leveraging the access API has indeed came up in the past, and it would be a nice addition. So happy to see your PoC!

Specifically, for your use case, I would approach it by adding a new "CoC member" OG Role. A "regular" Member wouldn't have those permissions.

Remember that `Create Content` is a bit more complex, since at the time the user is presented the form to create the content, that non-saved content doesn't belong to any group yet. So you'd have to rely on some `OG Context`/ prepopulating the reference to the group.

@claudiu.cristea. Thanks. You already have `Administer releases`. However, when I try to set 2.x as recommended, it appears disabled.

Ready for review?

Thanks 🙏

@quicksketch! It's good to have you around. Are you using OG 8.x with Backdrop?

> It would also be an opportunity to drop the "8.x-" prefix

👍

Thanks. Could you also include tests?

@joelpittet Thanks, I see phpstan is failing.

@claudiu.cristea, would you like to review before merging?

We are modernzing OG, so we are dropping support for 9 - https://www.drupal.org/project/og/issues/3503767 📌 Support Drupal >=10.3. Modernize the code Active

So I think we can have a similar MR for Message

> Fixing credits

Sorry 🙏

Thanks @claudiucristea, merged. DDEV is our goto solution as well.

@joelpittet, feel free to merge; no need to wait for me. 😊

Good move 🙏

New release -- https://www.drupal.org/project/pluggable_entity_view_builder/releases/1.... →

Merged, thanks

> I am just looking at PEVB at the moment. I am not using it in a production sense.

Cool. You can see it in action also here

I believe a generic Entity reference solution might have been helpful, such as a module that normalizes the ER references to a single table. However, I couldn't find one available, and "Drupal core to handle this" will likely not happen.

So, let's continue with your MR; thank you 🙏

Heya @bluegeek9, good to see you on this project as well :) Are you using PEVB?

> Should I resolve the issues reported by phpcs and phpstan in this MR?

It would be wonderful if you could, but if you're not up for it, that's fine. We haven't actually checked phpcs and phpstan yet, have we?

Sorry, the {og_membership} table does exist, but doesn't help you in your use case. Since Og membership is now only for users (I now understand what you meant by "previous" 😊)

I have one more question before proceeding with a og_group_index table. Let's say these three fields were standard entity reference fields, not linked to OG. You would still require a helper table to help with the Views/ query. I'm curious if a solution for this might already exist.

Thanks, the use case is clear now.

> The previous og_membership table was effectively a implicit taxonomy_index table

We're still using `og_membership` - couldn't that be used?

https://git.drupalcode.org/project/og/-/blob/dc7f4e138e90d4fa234894b3ec6...

@joelpittet thanks. What is the real-world use case for this? Do you have multiple OG reference fields because each one is attached to a different subscription type (e.g., "normal" subscription vs. "premium" one)?

joelpittet → credited amitaibu → .

@miksha  Thanks 😊

Thanks,

> Do we still need to support Drupal 9 (from info.yml)?

Yes, we should. Like we have it still on OG

Thanks 😊

Thanks, guys. Moving to another module is a great idea.

> Note that Group module is doing something similar by relying on flexible_permissions module (which is now added in core as Access Policy API)

It may not fit into OG access, which has a field to determine if it's private or public (and write a record into the node grants system). But it can likely be used in OG core, where we do some heavy lifting to decide if a user has access. In fact, some years ago, as we prepared for this blog post @kristiaanvandeneynde has mentioned he had plans to get this functionality out of Group. We also said it would be nice for both Group and OG to rely on it. Ideally, reducing both of the module's own code.

@miksha, you can use https://drupal-mrn.dev/ to create release notes

I've also granted your maintainer rights, so you could create releases

Thanks, can you please create an MR? It's easier to review :) And you can always use the `.diff` from the MR in your composer.json

@claudiu.cristea thanks!

Thanks

Thanks folks. Would anyone be willing to add a test to the MR?

Thank you 🙏

Done, I've added a note on the README on GH, and archived the repo (see image)