Thanks, this looks good to me!
Looks good to me as well!
Looks great, thank you!
Test failure does not seem related:
Drupal\Tests\comment\Functional\CommentPreviewTest 0 passes 94s 1 fails
alecsmrekar → created an issue. See original summary → .
The test fail does not seem related to this:
Drupal\Tests\image\Functional\ImageStylesPathAndUrlTest 0 passes 84s 1 fails
I opened a merge request that addresses this with a new approach. Simply, if a menu link is not translatable, the code should not be setting a language on the menu link entity.
Here are some test step that fail without the fix applied:
1. drush si
2. drush en -y workspaces content_translation
3. drush uli
4. Go to /admin/config/regional/language
and add the Spanish language
5. Go to /admin/config/regional/content-language
and enable translation for "Content", ensure to check "Article" under the "Content" fieldset
6. Switch to the Stage workspace
7. Create a new Article, check "Provide a menu link" and save
8. Go to /es/node/{}/translations
and add a Spanish translation
9. Note the revision ID: drush sqlq "select MAX(revision_id) from menu_link_content_field_revision"
10. Run drush php:eval 'echo \Drupal::entityTypeManager()->getStorage("menu_link_content")->loadRevision(REVISION_ID)->getMenuName()'
Notice that without the fix, the menu name comes blank, but with the fix, it prints the menu name.
alecsmrekar → made their first commit to this issue’s fork.
Thanks, I tested this and it works as expected, no errors :)
alecsmrekar → created an issue.
I think I addressed all the feedback, but there seems to be something wrong with composer in the test pipeline.
alecsmrekar → created an issue.
alecsmrekar → created an issue.
Opened a PR, and it seems the pipeline failure is not related to the changes made here.
Thanks Andrei, I pushed another commit enabling validation on the path.
alecsmrekar → created an issue. See original summary → .
alecsmrekar → created an issue. See original summary → .
Posted a MR, here are some test steps for this functionality:
1. ddev drush si standard -y
2. ddev drush en -y group_content_menu workspaces
3. Create a new group type (use default settings)
4. Create a new group content menu type. Add a field to it, call it field_test (a plain text field for example).
5. Go to /admin/group/types/manage/default/content and enable the plugin for the created group content menu type (use default settings).
6. Create a new group.
7. Add a new menu to it. Give it a label and populate field_test. Save it.
8. Switch to the stage workspace.
8. Edit the group content menu, update the label and field_test. Verify the form submits successfully and that the fiels are saved.
10. Switch back to the live workspace, try to edit the menu and ensure you see "The content is being edited in the Stage workspace. As a result, your changes cannot be saved.".
11. Publish the changes in the Stage workspace.
12. Go to the Live workspace, edit the group menu and ensure you see the updates values that you entered in the Stage workspace.
alecsmrekar → created an issue.
alecsmrekar → created an issue.
alecsmrekar → created an issue.
alecsmrekar → made their first commit to this issue’s fork.
The latest release uses the 3.x branch, see here: https://git.drupalcode.org/project/group_content_menu_bundles/-/blob/1.0...
I think we can close this.
alecsmrekar → created an issue.
alecsmrekar → created an issue.
alecsmrekar → created an issue.
but the real issue here is that Drupal\group\Plugin\Group\RelationHandlerDefault::getRelationshipLabel() doesn't account for the situation when NULL is actually returned
In my view getRelationshipLabel
is not wrong, it's respecting the return type of the GroupRelationship::getEntity
method. Why check if something is NULL if the called function says it'll never be NULL. The called function GroupRelationship::getEntity
is at fault here.
Adding this check would prevent the issue I reported in the issue description, but I think we should avoid tolerating wrong types, since types are very good guardrails.
alecsmrekar → created an issue.
alecsmrekar → made their first commit to this issue’s fork.
Maybe https://www.drupal.org/project/group_content_menu_bundles → can solve your problems
amateescu → credited alecsmrekar → .
alecsmrekar → created an issue.
Most entity types rely on an "update" operation instead of the "edit" operation. Paragraphs will check the "update" operation on it's parent entity. If the parent entity is a block, we'll need to convert "update" to "edit".
Here's a proposal for changing getBundlePermission:
public static function getBundlePermission(string $block_bundle, string $operation): string {
if ($operation === 'create') {
return 'create ' . $block_bundle . ' block content';
}
elseif ($operation === 'update') {
$operation = 'edit';
}
return $operation . ' any ' . $block_bundle . ' block content';
}
Perfect, thanks! I didn't apply your suggestion as-is, because $this->requestStack->getMainRequest();
can also return NULL, so I took that into account.
alecsmrekar → changed the visibility of the branch 3420768-baseurl-used-in to hidden.
I ran the failing tests locally on 10.2.x with this diff applied and they are passing just fine.
There is a test failure, but I can't reproduce it locally. And I am not sure if it's indeed related to the changes from the PR
Testing Drupal\Tests\language\Functional\LanguageUILanguageNegotiationTest
..E.. 5 / 5
(100%)
Time: 00:34.850, Memory: 4.00 MB
There was 1 error:
1)
Drupal\Tests\language\Functional\LanguageUILanguageNegotiationTest::testLanguageDomain
Behat\Mink\Exception\ElementNotFoundException: Button with
id|name|label|value "Save configuration" not found.
alecsmrekar → created an issue.
I opened this PR with some initial work done on the JS side, it basically just avoids splitting the string if the form element has the data-autocomplete-single
attribute.
alecsmrekar → made their first commit to this issue’s fork.
Rerolling the patch for D10.1 (without tests)
Updating the patch to read from the module/theme list service, instead of reading from cache directly. I also incorporated feedback from Stefanos.
alecsmrekar → created an issue.
alecsmrekar → created an issue.
Thanks, committed to 1.0.x branch
I'm not sure how that's going to be implemented, but I assume that we might need to do some kind of settings merge: from the library sections storage itself and from the the entity view display.
Tested, works great!
Attaching patch and interdiff
Here's a small adjustment to #60 to allows admins to see the broken blocks, instead of suppressing the error "This block is broken or missing..."
Fix for patch not applying
We could also add a uuid regex to the routing file, which would prevent accessing the route with unexpected arguments.
The attached patch prevent the error from being thrown.
alecsmrekar → created an issue.
alecsmrekar → created an issue.
alecsmrekar → created an issue.
alecsmrekar → created an issue.