Ljubljana
Account created on 25 August 2017, about 8 years ago
  • tech journalist && occasionally helps w/ webdev at Radio Študent 
  • Backend Developer at drunomics 
  • Community organizer, Web developer at Kompot 
#

Merge Requests

More

Recent comments

🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana
🇸🇮Slovenia useernamee Ljubljana

I removed the ce display preview service for now but at some point we might want to implement it back as it had helpful methods which are now part of the ce display form.

I also added a test for the form, but it needs more improvements.

🇸🇮Slovenia useernamee Ljubljana

I checked MR and it looks good.

🇸🇮Slovenia useernamee Ljubljana

Added:
- another fallback using FieldItemInterface::generateSampleValue. I was lazy and added it at last place, but I'm considering moving it up the order
- per-entity preview (also includes fallbacks)
- added preview section to the manage custom element form in custom_elements_ui submodule. Currently it is not refreshed with ajax and it is always returning markup. Consider adding json preview as well (as a selection).

🇸🇮Slovenia useernamee Ljubljana

📌 Add API to preview custom elements in Drupal Active makes more sense when you see the nuxt preview provider service definition.

🇸🇮Slovenia useernamee Ljubljana

Passed technical review, but there are still phpcs and phpstan failures.

Internal discussion:

The comment in the CEPreviewInterface describes the situation but doesn't explain what are the limitations of plugins and the need to implement them as tagged services as well.
I wonder why resolver does not get injected the plugin manager and get preview plugins already collected by the manager? The only difference I see is that tagged services have the weight/priority and the plugins don't - but we could add them as a plugin config option and have them ordered that way.
I see that plugins are the way to have config option selectable in the UI and are thus needed. But why can't the plugin manager just implement getProvider method and use it instead of resolver?

well, not a limitation really, a different purpose. the plugins are available plugins to be applied to requests, but need config (base-url) first. they really are for provding configuration options in the UI. the services are for selecting the (configured) provider to actually work. (-> think domain module, select the right provider for each domain by request)
interesting idea with getProvider on the manager.
the plugins are not usuable yet, they lack the base-url when returned from the manager. there is no layer really where to add the base-url that would fit there I think.
it does not really fit for the domain module use-case also, e.g. when there are multiple frontends, with one backend, you migiht have multiple instances of the same plugin, with different base-urls. you can do this now by adding respective services on top of the plugins. but you cannot have multiple instance of e.g. "nuxt" with multiple base-urls in the manager?

🇸🇮Slovenia useernamee Ljubljana

The architecture with both tagged-services and plugins might seems a bit overwhelming, but in the end we need both. I tried documenting it nice in code/interface to clarify what is the purpose of each.

That was my thought as well. I did not find a place where the need for both is explained.

🇸🇮Slovenia useernamee Ljubljana

We have a custom elements preview service that generates preview custom elements per field. The main method gets arguments - custom ce display and field name, and optional arguments formatter, formatter options and entity. if formatter is not provided then it is deduced from the ce display. If entity is set, than that entity is used for preview instead of generating sample data. This is also covered by kernel test.

Further plan is to add preview in the custom elements ui form. But I'm not sure yet how to do it. Each field could have a preview shown in a row for their field formatter or another option is to have a whole entity previewed in a tab bellow Custom elements display options.

🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana
🇸🇮Slovenia useernamee Ljubljana

Validation was added by 📌 Upgrade OhDear's PHP-SDK to v4 Active but needs to be dependency injected.

🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana
🇸🇮Slovenia useernamee Ljubljana

I found some issues in the drush ohdear:maintenance command and they are fixed now.
I added a changelog.md to the module.
Please review.

🇸🇮Slovenia useernamee Ljubljana
🇸🇮Slovenia useernamee Ljubljana
🇸🇮Slovenia useernamee Ljubljana

useernamee created an issue.

🇸🇮Slovenia useernamee Ljubljana
🇸🇮Slovenia useernamee Ljubljana

One endpoint is broken:

Location 	<domain>/admin/reports/ohdear/uptime

Message 	OhDear\PhpSdk\Exceptions\NotFoundException: The resource you are looking for could not be found. in OhDear\PhpSdk\OhDear->handleRequestError() (line 67 of /app/vendor/ohdearapp/ohdear-php-sdk/src/MakesHttpRequests.php).

Will update the php-sdk and re-test.

🇸🇮Slovenia useernamee Ljubljana

useernamee made their first commit to this issue’s fork.

🇸🇮Slovenia useernamee Ljubljana

I've written a Kernel test. (Disclousure: I got the help from AI):

se Drupal\image\Entity\ImageStyle;
use Drupal\KernelTests\KernelTestBase;
use Drupal\stage_file_proxy\Controller\ImageStyleDownloadController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;

/**
 * Tests WebP support integration with stage_file_proxy module.
 *
 * @group ldp_media
 */
class StageFileProxyWebpTest extends KernelTestBase {

  /**
   * {@inheritdoc}
   */
  protected static $modules = [
    'system',
    'file',
    'image',
    'stage_file_proxy',
    'imageapi_optimize_webp',
    'system',
    'user',
    'field',
  ];

  /**
   * {@inheritdoc}
   */
  protected function setUp(): void {
    parent::setUp();

    $this->installEntitySchema('file');
    $this->installSchema('file', ['file_usage']);
    $this->installConfig(['system', 'image']);

    // Create a basic image style for testing.
    ImageStyle::create([
      'name' => 'test_style',
      'label' => 'Test Style',
    ])->save();
  }

  /**
   * Tests that stage_file_proxy uses WebP controller.
   */
  public function testWebpControllerIntegration() {
    // Get the stage_file_proxy controller from container.
    $controller = new ImageStyleDownloadController(
      $this->container->get('lock'),
      $this->container->get('image.factory'),
      $this->container->get('stream_wrapper_manager'),
      $this->container->get('file_system'),
      $this->container->get('module_handler')
    );

    // Use reflection to check the decorated controller type.
    $reflection = new \ReflectionClass($controller);
    $decorated_property = $reflection->getProperty('decorated');
    $decorated_property->setAccessible(TRUE);
    $decorated_controller = $decorated_property->getValue($controller);

    // Assert that WebP controller is being used.
    $this->assertInstanceOf(
      'Drupal\imageapi_optimize_webp\Controller\ImageStyleDownloadController',
      $decorated_controller,
      'Stage file proxy should use WebP controller when imageapi_optimize_webp is enabled.'
    );
  }

  /**
   * Tests WebP request processing through stage_file_proxy controller.
   */
  public function testWebpRequestProcessing() {
    // Create a request for WebP image style.
    $request = Request::create('/styles/test_style/public/test.webp', 'GET', [
      'file' => 'test.webp',
    ]);

    // Get image style entity.
    $image_style = ImageStyle::load('test_style');
    $this->assertNotNull($image_style, 'Test image style should exist.');

    // Get the stage_file_proxy controller.
    $controller = new ImageStyleDownloadController(
      $this->container->get('lock'),
      $this->container->get('image.factory'),
      $this->container->get('stream_wrapper_manager'),
      $this->container->get('file_system'),
      $this->container->get('module_handler')
    );

    // Test that the controller can handle WebP requests without fatal errors.
    try {
      $response = $controller->deliver($request, 'public', $image_style);
      // We expect this to fail gracefully (404 or similar) since we don't
      // have actual files, but it should not throw fatal errors related to
      // WebP processing.
      $this->assertNotEquals(500, $response->getStatusCode(), 'Request should not result in server error.');
      $this->assertTrue(TRUE, 'WebP request processing completed without fatal errors.');
    }
    catch (NotFoundHttpException $e) {
      // Expected when source file doesn't exist - this is fine.
      $this->assertTrue(TRUE, 'WebP request processing handled missing file correctly.');
    }
    catch (\Exception $e) {
      $this->fail('Unexpected exception during WebP processing: ' . $e->getMessage());
    }
  }

}
🇸🇮Slovenia useernamee Ljubljana

Test is very rudimentary. Just scraps the surface.

🇸🇮Slovenia useernamee Ljubljana

Added support for entity_reference_revisions and a very rudimentary test.

🇸🇮Slovenia useernamee Ljubljana

I came across this error:

AJAX-HTTP-Fehler ist aufgetreten.\nHTTP-Rückgabe-Code: 403\nIm Folgenden finden Sie Debugging-Informationen.\nPfad: /media-library?destination=/node/89/layout&_wrapper_format=drupal_ajax&ajax_form=1&media_library_opener_id=media_library.opener.field_widget&media_library_allowed_types%5Bimage%5D=image&media_library_selected_type=image&media_library_remaining=1&media_library_opener_parameters%5Bfield_widget_id%5D=field_image%3A-settings-block_form&media_library_opener_parameters%5Bentity_type_id%5D=block_content&media_library_opener_parameters%5Bbundle%5D=marketing_hero&media_library_opener_parameters%5Bfield_name%5D=field_image&media_library_opener_parameters%5Bentity_id%5D=672&media_library_opener_parameters%5Brevision_id%5D=3242&hash=jff4E8Emflz9Qzgoxd5_8uo0u43NeQ3_7KVn_dbfu7g&views_display_id=widget\nStatustext: error\nAntworttext: {"message":"Non-reusable blocks must set an access dependency for access control."}', name: "AjaxError", stack: "@<domain>/core/misc/ajax.js?v=10.4.8:196:32\n@<domain>/core/misc/ajax.js?v=10.4.8:1935:3\n" }

And patch created from MR11 fixed our issue together with the patch from 🐛 Layout builder fails to assign inline block access dependencies for the overrides section storage on entities with pending revisions Needs work .

🇸🇮Slovenia useernamee Ljubljana

Looks good. Please see my comment if it needs a fix, otherwise we can merge.

🇸🇮Slovenia useernamee Ljubljana

I reviewed the MR 127. Let's get it merged before 📌 Leverage renderless-container for simpler code Active .

The only remark that needs another look is from the .module file (template suggestions but the only template there currently exists is for renderless-container).

🇸🇮Slovenia useernamee Ljubljana

Code looks good and provides a relevant Kernel test. RTBC

🇸🇮Slovenia useernamee Ljubljana

Kernel test is very detailed and I think it thoroughly covers the XB to CE conversion.

🇸🇮Slovenia useernamee Ljubljana

I checked MR9 with upgrade status and the scan did not return any known issues. Moving to RTBC.

🇸🇮Slovenia useernamee Ljubljana

There seems to be little to no activity here. We (me and @fago) are still eager to co-maintain this module and add the improvements from the ticket description in the 3.x version.

🇸🇮Slovenia useernamee Ljubljana

passed-tr, I also tested it locally, works, pipeline is passing

🇸🇮Slovenia useernamee Ljubljana

upgrade status reports 2 issues, with only the second one being really important (add ^11 to the info.yml)
applying patch: https://www.drupal.org/files/issues/2024-03-18/media_entity_pinterest.8.... and creating a new release is all that needs to be done.

Media entity Pinterest,  8.x-2.7
Scanned on Fri, 07/11/2025 - 14:02

FILE: web/modules/contrib/media_entity_pinterest/media_entity_pinterest.install

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Ignore         29   Fetching deprecated class constant EXISTS_ERROR of interface
                    Drupal\Core\File\FileSystemInterface. Deprecated in         
                    drupal:10.3.0 and is removed from drupal:12.0.0. Use        
                    Drupal\Core\File\FileExists::Error instead.                 
--------------------------------------------------------------------------------

FILE: web/modules/contrib/media_entity_pinterest/media_entity_pinterest.info.yml

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Check manually 5    Value of core_version_requirement: ^9 || ^10 is not         
                    compatible with the next major version of Drupal core. See  
                    https://drupal.org/node/3070687.                            
--------------------------------------------------------------------------------
🇸🇮Slovenia useernamee Ljubljana

Upgrade status does not report any incompatibilities except for adding the 11 into the info.yml. Looks like it only needs merge and new release for d11 compatibility.

🇸🇮Slovenia useernamee Ljubljana

Looks like a just new revision revision is needed to roll out d11 compatibility.

🇸🇮Slovenia useernamee Ljubljana

@fago, I approved the PR, although the comments could be improved. I added some comments to the MR.

🇸🇮Slovenia useernamee Ljubljana

The code looks good. I added a cosmetic comment to display the actual ids of the ce displays that were changed instead of count.

🇸🇮Slovenia useernamee Ljubljana

I agree that the option B is better. There really is no need for additional config checkmark, it was the first thing that popped into my mind when I was thinking about the PR.

🇸🇮Slovenia useernamee Ljubljana

PR looks good to me.

For BC we could add an additional checkbox to add fields to CE display. A config option that is only visible if layout builder is checked and disabled by default.

🇸🇮Slovenia useernamee Ljubljana

I'm moving this into RTBC, but one small improvement would be moving all the in one trait.

All the renderPlain phpstan notifications are gone. There are some new ones in tests/src/Kernel/ThunderParagraphRenderMarkupTest.php

🇸🇮Slovenia useernamee Ljubljana

I was getting the error from this ticket during cron runs. after applying the #12 patch the error is gone. RTBC

🇸🇮Slovenia useernamee Ljubljana

@fago tests are failing now.

🇸🇮Slovenia useernamee Ljubljana

If I click on Codespaces button in github repo the code space fails.

If I click on the badge in the README.md it works.

I suspect the issue is that we removed the default devcontainer.json file which was the same as /base_with_nuxt_naked.

Helpful command:
Use Cmd/Ctrl + Shift + P -> View Creation Log to see full logs

🇸🇮Slovenia useernamee Ljubljana

Additional test case (added in last commit) looks good to me.

🇸🇮Slovenia useernamee Ljubljana

I think that MR is fine, I just added a minor comment on a method that could be generalized a bit and used in other contexts/tests as well.

🇸🇮Slovenia useernamee Ljubljana

Looks good. Most of assertions from functional tests are covered (and many new test cases are added, redirect and unsupported) in the new kernel test - I found one that's missing and I'm wondering if it is really needed (Overriding settings parameter is not allowed.).

🇸🇮Slovenia useernamee Ljubljana

I added tests coverage for multi value fields - which explain the result structure and a small fix to the formatter code.

The header formatter configuration is only there for the FE to know if it should style the first row/column as header. On field configuration there's additional setting called `ignore_table_header` which again still passes through the first row - the config option is only used to determine whether the field is empty if contains only the first row.

🇸🇮Slovenia useernamee Ljubljana

Tested locally and it fixes the issue described in the ticket.

**offtopic** I had some issues testing it locally with the layour_builder integration. Which I had to disable to test.

🇸🇮Slovenia useernamee Ljubljana

One things that needs a check is that ContentFormatCacheContext needs to implement an interface.

Kernel test is passing.

🇸🇮Slovenia useernamee Ljubljana

I removed the label and added the kernel test which was passing locally. I was checking the CI and I think that tests are not running there.

I was unsure about the header settings.

🇸🇮Slovenia useernamee Ljubljana

I reviewed the code and approved the PR.

I tested it locally and it works as it should.

Moving the ticket to RTBC.

🇸🇮Slovenia useernamee Ljubljana

@ajayyadav thank you for your response but when you render the field as custom elements there currently is no 'export as csv' option.

Anyways I had to adjust the processor to support multivalue fields as well, so there's another level of nesting added, but the output I think works out.

🇸🇮Slovenia useernamee Ljubljana

hrefLangcode is generally needed. If the the URL language detection is in place than hrefLangcode should be used as a prefix before path alias to make the path work.

Production build 0.71.5 2024