hestenet → credited jnicola → .
Marking as reviewed and tested by the community. Enough folks have weighed in on this one, it's good to go and improves the module.
Same issue for us at USDOJ:
Call to undefined method Drupal\Core\Field\BaseFieldDefinition::createDuplicate() in Drupal\content_type_clone\Form\CloneContentType::cloneContentTypeField() (line 67 of modules/contrib/content_type_clone/src/Form/CloneContentType.php).
This MR fixed the issue, though there was a drupal message stating:
"The field moderation_state cannot be cloned because the createDuplicate method does not exist."
Perhaps expected, and everything worked for the most part so.. good to go?
You know that the Google charts API this uses is defunct and this module is consequently abandoned?
Okay, figured out the double menu items was being caused by something else. I'd still love a code example of how to programatically set the values on a menu item though as I'm having some inconsistent results locally. Ah well.
Man would this be useful right about now... i'd even accept something at returns an array of plugins as I could reliably just grab the first!
Just ran into this in 10.3.x with views on the USDOJ website where the pager would sometimes not render without a discernable rhyme or reason. Sure enough, adjusting the pager ID to 99 ensured that there weren't issues.
I don't have steps to reproduce from scratch, nor the liberty of time to faff around with reproducing this, but the pager ID collision does appear to be something that can still happen.
Stands to reason you could perhaps write a test for this that has a view rendering content, some of which has a pager on it, and both using the same pager ID of 0, but the view in the content pager is empty, which in turn bubbles up to the overall view pager ID.
I wish there were an easy solution such as setting pager ID to the views limit + 1, but if the overal view is 25, and the view in the content displaying in the view is limited to 25, you'd have the same pager ID collision again.
Why does pager ID need to be numeric anyways? If it could be alphanumeric you could go view-name_dislay-name?
I'll try and find some time to look into this next week. It's a big Holiday here and I'm about to be out through next Monday!
Okay, NEVERMIND! Got it all working, not sure where that error came from, but it works good.
Definitely needs some documentation upgrades but overall this actually seems pretty robust and functional!
jnicola → created an issue.
No, 2.x should be good to go on this project. Could you elaborate on steps and what failed for you?
I'll try and find some time today or tomorrow to spin up a site from scratch and update the directions if necessary.
Huh well would you look at that, we don't have it in there!
https://git.drupalcode.org/project/simple_grouped_content/-/blob/1.2.x/c...
I'll have to look up how to suggest it. I think this should all be compatible with 9 and 10.
We've not tested this with 11.
Drupal 10.3
Are you using composer? Should be in the requirements there.
Double check that the save button is there and that it actually saves. Our problem is that we can get the Save button back using a related patch for Gin, but unfortunately the submission does not succeed with no ajax error nor drupal log error.
Oh and sorry composer errors. Annoying that this isn't seen as a drupal package for some reason. Perhaps it's because it started as a sandbox. I added back in the repositories code. Try it out again with the respositires code added in your composer.json.
It's not built for group 3.x yet, just 2.x and 1.x if you want to go that far back.
While group 3.x is recommended, a lot of downstream modules aren't fully sorted yet that this depends on, and the place I work at that gives me a very modest amount of time to work on this just barely got on 2.x, it'll be a while before i can retool this for 3.x.
That said, this has been working with us for 1.x and now 2.x for 4 years. It's a pretty stable foundation to build upon :)
So I've updated the landing page information and the versions, as we've had a few updates come out since. It's good to go, try it out and let me know if it works for you!
Hey there and thanks for trying this out!
You'll need to download / require Drupal with composer, and then require this as well. This will download all the other requirements. Then, as part of the Drupal Install you'll be prompted to pick an install profile, and this will be listed. Select this install profile, and it should work!
Let me know how that goes for you. I'll update the documentation this morning.
Ugh, go figure I spent all spring making the switch to 2.x for one of our two big products, and now 3.x is ready!!!
Folks are welcome to do so, if the problem even exists there.
We're on the life support 1.x stuff until we migrate to group 2.x
Okay got a merge request for it for anyone who may want this.
https://git.drupalcode.org/project/group_content_menu/-/merge_requests/43
Didn't investigate tests for this, but could reasonably write one I'm sure.
jnicola → changed the visibility of the branch 3487080-nodeformalter.php-issue-when to hidden.
jnicola → created an issue.
This is not fixed for us with latest in 3.x dev branch.
Linked tickets also seem to be closed for duplicate, or the one core issue just trails off with folks pointing to this project (gin).
What is the actual solution for folks using 3.x? Given how many people have come back here and mentioned it is still not working this does not seem to be marked appropriately as fixed.
We've got this issue. Been faffing with the code above, doesn't seem to work. Also doesn't really capture folks using gin as a base theme? Also just not sure about that first conditional of if the theme IS NOT gin? Without comments or a link to another issue I'm guessing at exactly what it's trying to resolve.
Here's our modified version that checks if it is GIN or a GIN sub theme, but for us isn't working. Even skipping the initial theme check (where you went != and I went == or in array) it does not seem to resolve the issue.
// ATTEMPT TO FIX GIN LB OVERRIDE ENTITY FORM MISSING SUBMIT BUTTON!
$theme = \Drupal::theme()->getActiveTheme()->getName();
$base_themes = array_keys(\Drupal::theme()->getActiveTheme()->getBaseThemeExtensions());
if (
$theme == 'gin'
|| in_array('gin', $base_themes)
) {
// Enable gin_lb theming for custom frontend! forms too.
$gin_lb_form_ids = [
'override_entity_form',
];
if (in_array($form_id, $gin_lb_form_ids)) {
$form['#after_build'][] = [
'Drupal\gin_lb\HookHandler\FormAlter',
'afterBuildAttachGinLbForm',
];
$form['#gin_lb_form'] = TRUE;
$form['#attributes']['class'][] = 'glb-form';
unset($form["revision_information"]);
unset($form["revision_log_message"]);
unset($form["revision"]);
unset($form["revision_log"]);
}
}If you're placing a block inside another block you'll want to pull all cache tags from the first block into the second.
Sounds like it worked for you though!
Ah, I am not seeing them for some reason. I'll go look again and see what I can address from them, thanks for your time as always!
Well, a few things to that end.
Yes, Project Browser is the way folks will likely find recipes. That said, as with most things in Drupal, the "next big thing" historically comes and goes, but it's the base concept that remains. That's the beauty of it all being so extensible, and has allowed Drupal to have reinvented many aspect of module management, config management, content creation, layout management. From panels, to beans, to features... can we say someone won't come along with something better than Project Browser in the future? I don't think staying true to Drupal's history we can.
Respecting the desire though to give Project Browser it's fair shake, while also recognizing extensibility is the ladder that got us all on this roof... I kept RecipeDiscovery built in such a way that while it heavily favors the two directories you suggest we try and focus RecipeDiscovery down to... it can be used to discover recipes in other locations.
So if install profiles ever make a comeback and ship with Recipes, or hey maybe the next big "starshot" will have it's own directory... whatever the next big thing is, this can accomadate for it, but favors core, just like ModuleDiscovery.
Ultimately, I think this threads the needle of being as true to the original ModuleDiscovery object as possible, while navigating the current desired direction of Drupal.
As for needing more work, could you please be specific so as this can meaningfully stay moving? Just saying "Do better, see this" leaves this perpetually in limbo. Plus, I did review your project browser and did borrow from some of it.
One simple idea how to test: You could write a custom controller that calls RecipeDiscovery and gets all of the recipes from core and outputs them in a nice list, or a table, or whatever makes you happy! If it finds all of the recipes as expected, that's good! If you want to see actual output, do a list output of each recipe returned to you!
You can also test this works by attempting to load the test recipes. where it will load all but one that shouldn't load because it throws an exception.
Would love it if some more folks got interested. I'm still flummoxed that a standardized way for folks to discover Recipes is an uphill battle with outright disinterest. It's open source, give people the tools and let it grow!
Eh, I suppose. I wonder just how much of Drupal would need to be redocumented or fixed once return types are specified and discover how much wasn't actually as expected...
Is there any loss to be had adding a sanity check here besides a milisecond or two of time?
@walkingdexter could you explain your resistance or what the issue would be in implementing this bit of code?
I don't think it's fair to say that getOperation() returning NULL is a third party problem, since getOperation does not specify return types.
We were able to fix this by adding ".text-formatted" selector to the accordion JS selectors, restricting this functionality in theory to just the WYSIWYG content. It worked for us, but we can't guarantee the selector will apply to your situation, and may not warrant a PR since the class can't for sure be relied on.
I fail to see why problems in 3.x and 2.x need to stop a solution from making it to 1.x.
Have you even determined the problem exists there?
This fixes the same error for me that I experience when deleting group_content_menu items.
I don't think it would be criminal to consider making the code here a bit more tolerant, especially since in it's tolerance everything is hunky dory.
Opened MR, and passes. Ready for review!
Got a working patch, attached.
jnicola → created an issue.
jnicola → changed the visibility of the branch 3446354-expand-discovery-class to hidden.
This has been updated and now has tests that check the RecipeDiscovery class works as expected.
Wouldn't mind some additional review from folks.
Ah okay, got it!
I've got time at work and am going to try and get some tests for the updated recipe discovery class in the MR and then see about getting this out.
We're getting the same error, and I can't see the difference for the fork you've opened here?
Escalating to critical since you can't use this at all nor edit anything else while it's enabled.
Alrighty, looks like this patch fixes things for us in 2.2
Specifically, the service changed, and then $group_relationship->getEntityTypeId() no longer returns the referenced entity type ('node' for example) it now just returns that it is a Group Relationship. Booo.
Chucking this in a note here for myself and others, the latest patch doesn't work with 2.x as path.alias_manager has changed to path_alias.manager (see the _ and . moved).
Getting this error as well as part of a Drupal 2.x upgrade.
No steps to reproduce, it would be unreasonable for us to do so as well given the size and complexity of our site.
Still, there's something amiss here in the upgrade.
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.
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?
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!