🇧🇪Belgium @msnassar

To be rebased after merging 📌 Automated Drupal 11 compatibility fixes for feeds_dependency Needs review

Move to RTBC

msnassar → created an issue.

msnassar → created an issue.

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

RTBC++
I have tested it with media_avportal https://www.drupal.org/project/media_avportal/issues/3431855 📌 Automated Drupal 11 compatibility fixes for media_avportal Needs review . All works as expected.

Here is a patch from the MR

Now the test is passing. See https://git.drupalcode.org/issue/message_digest-3510956/-/pipelines/439923 (Previous major)..
There are some tests failing due to other issues e.g. errors in views settings (Not related to this change).

OK now we have a failing test. lets make it passing.

msnassar → created an issue.

I have tested it with https://github.com/openeuropa/oe_bootstrap_theme
It works as expected. However the pipeline shows some issues. I have fixed cspell and ESLint issues. For the phpstan, if we want to fix them, we should either drop the support for drupal older than 10.3 or do backward compatibility. Let me know your thoughts?

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

msnassar → created an issue.

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

Upload patch..

I didn't fix the following to demonstrate the issue 🐛 Case mismatch between loaded and declared class names Active

 ------ ----------------------------------------------------------------------------- 
  Line   src/Plugin/UiPatterns/SettingDataProvider/MenuDataProvider.php               
 ------ ----------------------------------------------------------------------------- 
  9      Class                                                                        
         Drupal\ui_patterns_settings\Plugin\UiPatterns\SettingType\LinksSettingType   
         referenced with incorrect case:                                              
         Drupal\ui_patterns_settings\Plugin\UIPatterns\SettingType\LinksSettingType.  
  165    Class                                                                        
         Drupal\ui_patterns_settings\Plugin\UiPatterns\SettingType\LinksSettingType   
         referenced with incorrect case:                                              
         Drupal\ui_patterns_settings\Plugin\UIPatterns\SettingType\LinksSettingType.  
 ------ ----------------------------------------------------------------------------- 

msnassar → created an issue.

msnassar → created an issue.

The pipeline is green. It is recommended to review and merge 📌 Add gitlab ci Active first...

msnassar → changed the visibility of the branch project-update-bot-only to hidden.

The MR in Draft because of 📌 Add gitlab ci Active . CI MR should be merged first then rebase this one... Anyway, I will leave this to the maintainers... So mark as Ready for review

All looks good... Moved to RTBC..

msnassar → changed the visibility of the branch project-update-bot-only to hidden.

I have encountered the issue while testing 📌 Automated Drupal 11 compatibility fixes for facets_form Needs review . I confirm that the MR is fixing the issue... The removed code has been introduced in #3223764: Create facets_form_date_range sub-module → this commit... We could have a failing test by removing this change.

msnassar → changed the visibility of the branch project-update-bot-only to hidden.

msnassar → changed the visibility of the branch project-update-bot-only to hidden.

Seems that this issue has already been solved in https://github.com/openeuropa/composite_reference/pull/20/files

I have cherry picked the fix from MR21. Now the tests are green again...

I think the exiting code in this test can be improved... Anyway, the next step is to demonstrate that we might not need this views plugin anymore!

Now we have a failing tests... Lets have them passing by fixing the reported issue.

For now we should ignore the failing tests in D11 builds. This is going to be fixed in another issue.

The tests using D10 are passing, this demonstrate that our tests are not good. So in order to demonstrate this issue, lets write a test that fails .
BTW, from group 2, I think we might not need this views plugin anymore! But lets demonstrate this later or in another ticket.

Better to review and merge 📌 Add gitlab ci Active

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

@mykola dolynskyi see this module
https://www.drupal.org/project/datetime_range_timezone →

Looks good to me...

Would be nice to review and merge before 📌 Automated Drupal 11 compatibility fixes for datetime_timezone Needs review

msnassar → created an issue.

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

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

1. Added gitlab ci for D10 only
2. Fix typos

msnassar → created an issue.

• Added gitlab ci for D10
• Fixed reported phpcs
• Added cspell: The majority of the words are coming from docker-compose.yml.dist. I think we could remove? An alternative is to use ddev.

msnassar → created an issue.

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

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

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

The MR is ready for review. However, there is an issue with field_permissions 1.4 that should be merged first. See 🐛 Wrong Plugin ID when looping throug Plugins Active

I confirm the issue. This is reproducible with version 1.4 due to 📌 Modify form structure and use core 'states' JS for each access plugin Fixed .

The issue can be reproduced when user_fields_visibility module is enabled. When I select 'user_fields_visibility' plugin, I see the config form for 'custom' plugin...

I also confirm the MR fixes the issue.

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

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

Since we use patches only. I upload a patch also.

msnassar → created an issue.

Hello,
I found overlap issue in https://new.drupal.org/about/overview/technical

This is a duplicate of ✨ Group content tokens for label and referenced entity Needs review

The module doesn't send notifications... However, you can do this by using hooks or contrib modules that provide a notification functionality...

msnassar → created an issue.

When indexing media with unavailable resource using search api, I see the following error in the log:
Warning: Undefined variable $resource_url in Drupal\media\Plugin\media\Source\OEmbed->getMetadata() (line 253 of core/modules/media/src/Plugin/media/Source/OEmbed.php)

Steps to reproduce:
- Create media entity using live stream video using e.g. youtube.
- From youtube, finish the steaming and make it unavailable.
- Use search api to index media.
- run search index.
- Check the log.

I think we should use media source field value to display the resource url as follows:

$this->logger->error(
  $e->getMessage() . ' Resource URL: %resource_url | Media ID: %media_id',
  ['%resource_url' => $media->getSource()->getSourceFieldValue($media), '%media_id' => $media->id()]
);

Patch for Resolution #2

Patch for Resolution #1

msnassar → created an issue.

msnassar → created an issue.

As we are using patches only, I upload also patch...
If everyone agrees with the new concept, then I will work on fixing the tests...

msnassar → created an issue.

I did quick debug.... I confirm the issue... It seems this is due to the fact that we check for entity create access but not update. See here

I believe, in case the entity is exist, we instead have to check the access to update. You can get the entity and its type from MediaLibraryState e.g. MediaLibraryState::fromRequest($this->requestStack->getCurrentRequest())
Then to check for the access to update, we use entityAccessfrom Drupal\group\Plugin\Group\RelationHandler.

As soon as I have the time, I will look deeply into it. However, patch is very welcome :)

Sorry for late answer...

This is not a typo... It is intended to check the access to create/edit the root host entity (in your case the page content type). I recommend to look into this code snippet. Usually, if you have the access to edit the root host entity, you should be able to add/upload new media to it. Please, let me know your finding...

I am hiding your patch as it has a security vulnerability...

I already have the plan to do this feature. But unfortunately, I don't have the time. Most probably it will be a submodule in Group Media Library → module. Also it would be nice to have due to ✨ Allow 3rd party to easily add new params to media_library_opener_parameters Active fix before.
Anyway, if you want this feature now, you could do it in a custom module by swapping the plugin class (as suggested by the issue)...

Just linked similar issues

The log message field might have a different name e.g. in media it is revision_log_message. Meaning either we get the log message field name using $this->entity->getEntityType()->getRevisionMetadataKey('revision_log_message'); or in entity-moderation-form.html.twig we just output the form elements using original entity form keys. However the later doesn't ensure existing form alter will continue to work.

Here are new patches for D10.2 and D10.3 that remove the piece of code that shuffles the form elements around into their respective original keys and instead use the keys from the entity form itself in the template entity-moderation-form.html.twig... However, I am not sure how I could handle the BC here.

Screenshot with the new patch:

msnassar → created an issue.

Test only patch to demonstrate the issue.

msnassar → created an issue.

@akshay_d
I couldn't reproduce the issue in #16...
I followed your steps and besides I used both block and embed display types...
Also tested it with/without having indexed items...
For me: $this->total_items is "0" in all cases!