🇺🇸United States @jnicola

jnicola → created an issue.

I see this was reopened ... are we doing this or not?

Ah yep, this was due to actually happen at some point as the API was supposed to go away YEARS ago!

Unless there's something new for Google Charts or equveilant, probably time to consider this as done.

@thejimbirch it's sort of ready!

Since I resolved everything, RecipeDiscovery was moved to RecipeConfigurator (see https://www.drupal.org/project/drupal/issues/3447210 📌 Move RecipeDiscovery into RecipeConfigurator Needs review ) so I had to redo the work on RecipeDiscovery. So I redid the work, added some functionality based on some suggestions from @phenaproxima and had pinged them for a code review. I'd love for you to take a gander at it as well!

I also need to add tests. Writing those would be the goal early this week, but I am having trouble getting DDev to run phpunit tests. I couldn't even get ddev to spin up Drupal 11 without some notable deviation from the main Ddev directions, so I'm sure the problem lies somewhere in thee.

If anyone reading this has availability to pair through writing Phpunit tests with me, I'd be forever grateful!

thejimbirch → credited jnicola → .

Looks good to me! Could you elaborate on some ways to test?

@jimburch I think this is actually on hold and may not even happen at all unfortunately.

The main push apparently is for this all to exist in project browser. No idea how far out that getting brought into core is.

I also opened a ticket for the new DiscoveryClass, which I included in here but has changed some, etc etc.
https://www.drupal.org/project/drupal/issues/3446354 ✨ Expand Drupal\Core\Recipe\RecipeDiscovery to allow discovering available recipes, likely for use in Project Browser Active

Recipe discover also needs a bit of time since we need to move validation from Recipe creation to it's own stand alone method.
https://www.drupal.org/project/drupal/issues/3446329 📌 Recipes' ConfigConfigurator should not immediately check for conflicts with active config Active

Testing with Drupal 11 by just adding Drupal 11 to the info yml file. This works!

I don't believe removing the requiremnt for devel/devel is right though?

Otherwise though this works in Drupal 11!

Moving to the new recipe component!

@alexpott I believe the linked Project Browser work will need it as it needs a way to display all the available local recipes.

We also developed a recipe_ui module at Drupalcon that will also need to discover recipes. The intention was to put it in core, but since the Project Browser push is the main focus, it would be a contrib module... if we even push out what we built at all.

In general though being able to discover recipes seems like a useful chunk of functionality for folks so as everyone isn't reinventing the wheel.

Tested locally with the recipe UI stuff I've got built, works!

So I don't think the $path to constructor idea is wise, but left it since it's not my work.

If anything, that should be removed.

Otherwise, people can specify any path they want to search... which is exactly what you specified we don't want to be doing.

Also, ExtensionDiscovery is explicit about paths, it's not unreasonable to be explicit here.

I can alter the constructor and chase down all code using this as is, but I chose to alter as little other code as possible.

I think a follow up issue to tackle that would be reasonable to keep this issue scope to just the ability to get a scan of all discoveries in the directories Recipes currently intends to restrict users to.

Ready for review by others.

Tested with early version of Recipe UI, is detectable now without throwing an exception and applies cleanly without error!

I also reviewed the build error, looks like it failed in Cspell for a spelling error but this only adds:
name: 'Example'

So I feel confident the build failure is invalid and possibly from some upstream unrelated change. Couldn't see specific error in Cspell output however!

Good work @edutrul and nice to meet you at Drupalcon, I hope you enjoy your time in Portland!

So per our conversations all over the place, this issue gets merged for the RecipeDiscovery class ✨ Expand Drupal\Core\Recipe\RecipeDiscovery to allow discovering available recipes, likely for use in Project Browser Active , we can refactor the fetching of Recipes.

I do think we would also want to then remove/alter the methods getRecipeDescription in favor of just $recipe->description.

As for the project image, we may want to add a method to the Recipe Object itself to fetch the image associated with the recipe instead of putting the logic in here.

Keep this all single purpose to just exposing Discovered Recipes to Project Browser?

Some of us have been talking about it at length over in slack. We're also chatting about it as a group at the Drupalcon2024 general contribution Recipes table.

The decision appears to be that a discovery class would be useful. There are restrictions around paths. This also does not exactly copy ExtensionDiscovery. It just follows the method naming conventions from there.

This currently affects nothing as far as Recipes per our local testing and none of the code already in discovery (a single method) is influenced by this addition.

The discovery class can now return an array of Recipe objects as available in core/recipes and /recipes.

This can be used by layout builder or any other future effort that should desire to acquire a list of all recipes that can be loaded as Recipe Objects, and should help prevent redundant code from coming to exist such as in a plugin in Project Browser and then onwards in other places wishing to display a list of recipes.

jnicola → created an issue.

Oh and another bit... the Example recipe fails to load and throws an exception. We had to remove it to make this work.

It should be noted, this would be an early alpha, it can be expanded, but the form is there, you can submit and things apply, and an array of all Recipes that don't throw exceptions is there so if the UI is to be improved, it surely can be!

Okay so working with @maskedjellybean on this today, we were able to create a working UI. The patch is attached, and yes a fork is better but this isn't actually a core issue yet and (insert drupal procedure stuff her)

We expanded RecipeDiscovery to have the same methods as core Extensiondiscovery, and then got them working to find available recipes in core, provide a list of all recipes in core that don't throw exceptions when loading the Recipe object for that Recipe, and then allow users to select recipes, which on form submit are applied.

It does work! ... we just navigate the whole Drupal core rigamarole of moving this to core, but can it even be core, should we even do this because project browser is someday doing this, even if we can have this now instead of later... etc etc.

I just chatted with those folks since they're a table over here at DrupalconPDX.

Per the table:
-With modules if you have the module locally it will display that you have it, and display whether it is installed or not, and if not installed provide you the option to install.
-"The more forward thinking way would be to build this in project browser".

I think as a potential interim though, it wouldn't be the worst idea to build this.

I say we do a UI module to get us by for now, while another issue tackles project browser integration.

As I understand it, project browser will allow seeing ALL recipes out there.

I envision recipes needs an equivalent to "extend" (/admin/modules) so you can see what revisions you have locally already and apply them if you wish to.

Or will project browser allow you to filter based on what you have locally?

jnicola → created an issue.

jnicola → created an issue.

Marking fixed until I hear otherwise :) Probably will add to a second alpha release here shortly!

Okay, I reviewed the issue further and I believe the issue was actually the missing module requirement in the info.yml file for drupal:core_event_dispatcher, a submodule of hook_event_dispatcher

While I believe your code would fix the error, the event would simply never be triggered.

I've updated the info.yml file AND just to make sure everything is working I put all the other requirements in composer.json as well.

Please try again using the latest development branch commit.

Link to bitcache is for sale, should probabl deprecate this all?

Hey there, could you let me know what version of hook event dispatcher you are using? We're utilizing 4.0 release candidate. If you're using 3.x this won't work probably as I think they changed all of their hooks classes.

I think that may be your problem. I should probably mark this as D10 exclusive and setup a bunch of requirements in here for group version, Drupal version, and hook_event_dispatcher version.

Oh cool, thanks for sharing! I just put this out this AM and I haven't tested standing this up from scratch. I'll take a gander tomorrow!

Looking for something possibly semi-related but... this ticket is clearly lacking enough information to be actionable!

Just ran into this issue, thankfully we have really good test coverage that caught the issue.

Specifically our issue was with hook_field_widget_WIDGET_TYPE_form_alter

Google for "Drupal 10 deprecated hooks" does not yield any useful information on this topic either.

Running into this as well.

Okay, I believe this patch does the trick. It combines the post_update hook and the fake plugins.

What it does not do, and I have not tested yet, is the bit phma mentions when there is content in the field already. I'll write another patch for that if necessary.

Works as expected. Good to go!

Man I wish we could get you steps to reproduce! We've been trying to consistently get this and it just happens one time out of a thousand or so. One theory I have is it is happening when "poormans cron" is triggered on a Drupal request resulting from an AJAX request, and Drupal isn't fully bootstrapped. So NULL is passed in, and then it attempts to load the Drupal service and that in turn returns NULL but FEEDS isn't looking for that.

Another coworker says they can trigger this all from CLI drush cron calls though so... maybe I'm not right about that?

Nice, thanks for the patch! A very easy fix indeed. We may utilize it. I guess I just wonder if this is a symptom of a larger issue or if this should have some logic in the constructor and throw an exception there about not having a file system instantiated yet, etc etc.

Relevant contrib code:

/**
   * {@inheritdoc}
   */
  public function cleanUp() {
    if ($this->filePath) {
      $this->fileSystem->unlink($this->filePath);
    }
  }

Offhand the error appears to be the sanity check is looking for if there is a filepath, and if so, call that method on filesystem. Welp, it's possible apparently to get to that point with a filesystem being established.

File system is set in the constructor:

  /**
   * Constructs an HttpFetcherResult object.
   *
   * @param string $file_path
   *   The path to the result file.
   * @param array $headers
   *   An array of HTTP headers.
   * @param \Drupal\Core\File\FileSystemInterface $file_system
   *   (optional) The file system service.
   */
  public function __construct($file_path, array $headers, FileSystemInterface $file_system = NULL) {
    parent::__construct($file_path);
    $this->headers = array_change_key_case($headers);
    if (is_null($file_system)) {
      $file_system = \Drupal::service('file_system');
    }
    $this->fileSystem = $file_system;
  }

In theory... we either get NULL for file system or an object that meets the FileSystemInterface requirement, which in turn has the method UnLink, so that's a dead end. So we have to be passing in NULL, which in turn calls \Drupal::service('file_system');... which may be returning NULL in some instances?

It's late and that's as far as I've puzzled.

jnicola → created an issue.

Attaching a patch version of my initial work from that fork incase folks want to grab this in such a fashion.

All I did was add a field widget. No display handling. I left a TODO in there to eventually display useful debug information for admins, if that's what even is contained in that field?

This no matter what alleviates fatal errors relating to trying to drag that field into form display though!

jnicola → created an issue.

We're running into a similar issue. No updates on how we manage to handle it.

Hey there, so we're using feeds 8.x-3.0-beta4, which should have this code in it... but I get the following error still when trying to drag the widget out of hidden into display so I can try and troubleshoot it.

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "hidden" plugin does not exist.

It appears to me that this issue may not be resolved? This project is also far newer than the 4 years ago that this was resolved (2 years old now) so... not sure what is missing?

NOTE: Just tested with 8.x-3.x-dev. Same issue?

Looks good to me, and yeah come to think of it there really isn't a reason it can't be entity agnostic!

Merging.

Glad someone else ran into this same issue and found it helpful. I not only found it maddening to deal with, but when I have explained it to others they just assumed I didn't do something right or didn't know what I was talking about!

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

Ah okay, gotcha. Unfortunately that change happened but our local config had kint_extended in there still, so it was just defaulting to a standard dumper. Easy enough fix in our case though, just change back to kint, export local config etc etc.

So I get all of that, but the memory issues do still exist for me, and the radio button choice to choose "kint extended" goes away.

We are also missing this option in Drupal 9.5.9 with PHP 8.1

Okay I believe you've been added! Close the ticket if so!

Strange, I didn't get an email from drupal.org if you used my contact form. I'll add you now!

Still seeing this issue with beta4

jnicola → created an issue.

$connection wasn't nescessarily established and could confuse some folks.

Alright, another update.

After experimenting with this, I was able to ascertain that $revision_ids is an array of a single NULL value when programmatically creating content with content moderation on it. In our case it is a group content type. We're experiencing this in 9.5.x, creating a simple group entity with very simple values.

I tried using the foreach patch above, which worked... but when standing the site up for automated testing it would error out on config imports due to memory limitation issues. Not sure how the two relate?

I was able to resolve the issue by instead of using a foreach, adding a few sanity checks prior to the array_flip causing the warning:

if ($revision_ids && !is_null(reset($revision_ids))) {

Attached is the patch, which does not follow the correct issue queue naming conventions, but eh whatever take it out of my bonus that I don't get ;)

Hope this helps somebody, and perhaps with some simpler steps to reproduce and a test we could get this onwards to core.

Also, just tested the patch locally. Resolved my issues.

Issue was programmatically creating a pretty simple group from array values. No idea where the array flip complaint came from.

This is likely an issue worth getting some review on. A simple start would be to create a bare bones drupal install with just content moderation and programatically create an entity to see if it can be reproduced, and increase complexity until the warning shows up.

I'm having the same issue creating a group entity in Behat from simple array values and a content moderation state.

We had this same issue. I was able to reproduce the issue with the below behat test, and with the patch in place the test is able to pass.

I'd recommend separating out layout builder concerns. They are possibly unrelated and shouldn't bog down what is a simple cache context fix.

Scenario: Active trail works on group menu and is cached correctly.
    Given group:
      | label            | type          | path              | status |
      | Behat test group | default_group | /behat-test-group | 1      |
    And I am logged in as a user with the "ohsu_contributor" role
    And I am a group member with the "senior_editor" role
    # Create a custom piece of content in the group with group menu.
    # @todo Replace with singular step.
    When I visit the form to create a "basic_page" in my group
    And I fill in "Title" with "Test menu 1"
    And I press the "Save" button
    # Below is equivelant to: And I check the box "Provide a menu link"
    And I check the box "field_group_content_menu_link[0][enabled]"
    And I fill in "Menu link title" with "Behat Test Gcontent Menu Link 1"
    And I fill in the meta description field as necessary
    And I press the "Add new content item" button
    Then I should see a created successfully message
    # Create ANOTHER custom piece of content in the group with group menu.
    # @todo Replace with singular step.
    When I visit the form to create a "basic_page" in my group
    And I fill in "Title" with "Test menu 2"
    And I press the "Save" button
    # Below is equivelant to: And I check the box "Provide a menu link"
    And I check the box "field_group_content_menu_link[0][enabled]"
    And I fill in "Menu link title" with "Behat Test Gcontent Menu Link 2"
    And I fill in the meta description field as necessary
    And I press the "Add new content item" button
    Then I should see a created successfully message
    # Confirm all links are in the group menu for the group.
    When I view the group with label "Behat test group"
    Then I should see the text "Behat Test Gcontent Menu Link 1" in the "group sidebar menu" region
    Then I should see the text "Behat Test Gcontent Menu Link 2" in the "group sidebar menu" region
    When I click "Behat Test Gcontent Menu Link 1" in the "group sidebar menu" region
    Then the ".nav-link.nav-link--active" element should contain "Behat Test Gcontent Menu Link 1"
    When I click "Behat Test Gcontent Menu Link 2" in the "group sidebar menu" region
    Then the ".nav-link.nav-link--active" element should contain "Behat Test Gcontent Menu Link 2"

Merged, thanks for updating!

Incase anyone finds this question and wants a spelled out answer:

You can disable the default paragraph by going to "form display" on your content type, and edit the field widget settings (gear icon to the right of your field)

EX PATH: admin/structure/types/manage/CONTENT-TYPE/form-display

You will then adjust "Default paragraph type" to NONE and you will not have a default paragraph displayed.

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

Looking through the code now, it fires ContentExportForm::snapshot()

A method with no commenting or documentation at all. Gross.

Looks like on install it attempts to do a full site export right away! Snapshot gets every entity type, calls ::generateExportBatch() for each entity type.

The batches called for each type are generateSiteUUIDFile() and processContentExportFiles()

Generate Site UUID file doesn't seem to do more than just a singular site UUID.

processContentExportFiles() with the snapshot param writes every single entity of every single type to a table?!?

The more I look at this, the more it seems this module is designed for sinking entire sites between each other, not portions of entire sites.

That appears to be the case with generating a full snapshot in a database table... which god forbid you have a massive site with millions of pieces of content because you just doubled that amount of content to yet another database table...

I'd really like to piggy back this and expand on it since the INSTALL section is missing the CRITICAL AND VITAL STEP of configuring $content_directories['sync'] in settings.php.

They really need this in the readme...

You need to add the following to your sites settings file:

global $content_directories;
$content_directories['sync'] = 'whatever-base-path-you-have/content_sync';

@jbaum_13 --> Not sure what combination of modules you are using, but we've got a lot of experience with crazy menus here at OHSU if you ever want to talk architecture that helps handle all of that!

We've had plenty of mistakes to learn from and a decent amount of success that might work for you as well :) Feel free to reach out if you want.

For a patch that someone asked for a while ago and I missed, here's what we wrote and are utilizing at OHSU as part of what we're up to. It's definitely not ready to be utilized into care as is and hasn't been tested beyond our very specific monolithic menu approach (which we're moving away from) so no guarantees this has any value at all, infact it may make things worse so implement at your own risk, haha.

I havne't given it any thought. The google API this uses was supposed to be deprecated 4-5 years ago. I don't have any projects utilizing this.

Submit a patch and I'll accept it, or I'm available to fix this up on a contract basis. Otherwise, I got other more pressing tasks at hand.