Account created on 1 December 2010, almost 14 years ago
#

Merge Requests

More

Recent comments

🇸🇮Slovenia alecsmrekar

Test failure does not seem related:
Drupal\Tests\comment\Functional\CommentPreviewTest 0 passes 94s 1 fails

🇸🇮Slovenia alecsmrekar

The test fail does not seem related to this:
Drupal\Tests\image\Functional\ImageStylesPathAndUrlTest 0 passes 84s 1 fails

🇸🇮Slovenia alecsmrekar

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.

🇸🇮Slovenia alecsmrekar

Thanks, I tested this and it works as expected, no errors :)

🇸🇮Slovenia alecsmrekar

I think I addressed all the feedback, but there seems to be something wrong with composer in the test pipeline.

🇸🇮Slovenia alecsmrekar

Opened a PR, and it seems the pipeline failure is not related to the changes made here.

🇸🇮Slovenia alecsmrekar

Thanks Andrei, I pushed another commit enabling validation on the path.

🇸🇮Slovenia alecsmrekar

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.

🇸🇮Slovenia alecsmrekar

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.

🇸🇮Slovenia alecsmrekar

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';
  }
🇸🇮Slovenia alecsmrekar

Perfect, thanks! I didn't apply your suggestion as-is, because $this->requestStack->getMainRequest(); can also return NULL, so I took that into account.

🇸🇮Slovenia alecsmrekar

alecsmrekar changed the visibility of the branch 3420768-baseurl-used-in to hidden.

🇸🇮Slovenia alecsmrekar

I ran the failing tests locally on 10.2.x with this diff applied and they are passing just fine.

🇸🇮Slovenia alecsmrekar

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.
🇸🇮Slovenia alecsmrekar

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.

🇸🇮Slovenia alecsmrekar

Updating the patch to read from the module/theme list service, instead of reading from cache directly. I also incorporated feedback from Stefanos.

🇸🇮Slovenia alecsmrekar

Thanks, committed to 1.0.x branch

🇸🇮Slovenia alecsmrekar

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.

🇸🇮Slovenia alecsmrekar

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..."

🇸🇮Slovenia alecsmrekar

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.

🇸🇮Slovenia alecsmrekar

I tried to run tests on the main branch, and I get the same failures locally, so I assume that the errors are not a consequence of the changes in the PR.

🇸🇮Slovenia alecsmrekar

This is caused by the redirect module. The patch in here should fix it: https://www.drupal.org/project/redirect/issues/3314032

Production build 0.71.5 2024