🇸🇮Slovenia @useernamee

Code looks good and provides a relevant Kernel test. RTBC

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

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

Thank you!

useernamee → created an issue.

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.

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

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

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.

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

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

Looks good

Looks good

useernamee → created an issue.

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

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.

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.

looks good

moving to rtbc

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

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

@fago tests are failing now.

@fago: https://git.drupalcode.org/project/lupus_ce_renderer/-/merge_requests/47...

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

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

Looks good to me. RTBC

useernamee → created an issue.

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.

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

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.

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.

All PR comments are resolved. RTBC

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

Kernel test is passing.

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.

I reviewed the code and approved the PR.

I tested it locally and it works as it should.

Moving the ticket to RTBC.

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

useernamee → created an issue.

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.

It would be nice to have operation to open backend api similar as we have for nodes and webform fox DX.

I approved the PR. Should we document ce name drupal-view-{VIEW}-{DISPLAY_TYPE} somewhere?

I checked the code and I'm wondering whether there is no nicer way to get the plugin type than substracting the prefix from the plugin id

    // Remove "custom_elements_" prefix from display type.
    $display_type = strpos($view->getDisplay()
       ->getPluginId(), 'custom_elements_') === 0 ?
          substr($view->getDisplay()->getPluginId(), 16) :
          $view->getDisplay()->getPluginId();

I checked the code and approved the PR.

Code looks good, needs testing.

Generally I'd argue that tests for specific functionality that a module provides should live in that module. We have however broken that pattern at the beginning with the general test and later on with the views integration.

For general tests:
- https://git.drupalcode.org/project/lupus_decoupled/-/blob/1.x/tests/src/...
- https://git.drupalcode.org/project/lupus_decoupled/-/blob/1.x/tests/src/...
I think it is fine if they stay on the top level module, but the views tests should go into the lupus_decoupled_views submodule.

quick fix: https://github.com/drunomics/lupus-decoupled-project/pull/25/files

Thank you for review @arthur. I've created a new lupus-decoupled-project PR.

Follow up tickets:

• 📌 Review and improve processes described in Play online. Active
• 📌 Configure nginx to properly handle $_SERVER['REMOTE_ADDR'] in codespaces Active
• 📌 Add possibility to see ddev commands output in GitHub Codespaces Active

I'd like to open the discussion on removing the gitpod URLs from README and documentation. gitpod.io links are still working and devcontainer standard is supported by the gitpod flex so I guess we don't have to remove it completely.

useernamee → created an issue.

useernamee → created an issue.

useernamee → created an issue.

A move towards GitHub Codespaces has been made in:
- https://github.com/drunomics/lupus-decoupled-project/pull/20
- https://github.com/drunomics/lupus-decoupled-project/pull/22

Documentation has to be improved in:
- https://github.com/drunomics/lupus-decoupled-website/pull/109

Thank you @arthur. I wasn't paying attention to caching.

- Added cacheable dependencies to the response.
- Cache is tested.

I removed the missing config option and added a watchdog error for that case instead:

{"system.site":{"name":"Drush Site-Install","slogan":"lupus.digital Publishing","mail":"admin@example.com","missing":null},"missing":{"missing":null}}

One issue left to review: 📌 Add tests for lupus_decoupled_form Active

Confirmed that the MR fixes the issue.

> I am not sure about the /api/lupus prefix also, what about just /api/site-info or /lupus-site-info ?

I went with the /api/site-info but this is also a limitation of the Rest resource which I picked for the task, since it would seem nicer to have a resource at the endpoint of /site-info, for uniformity.

Looks good, will merge.

I created the Lupus Decoupled Site Info module and added some initial configuration.

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

I think that provided MR works fine, I'd just like to highlight the other way of adding data to ce-api responses - adding the lupus_ce_renderer_response_data request attribute. It could also go into custom code:

  public function addBasicUserInfo() {
    $request = $this->getCurrentRequest();
    $lupus_ce_renderer_response_data = $request->attributes->get('lupus_ce_renderer_response_data');
    $lupus_ce_renderer_response_data['current_user'] = $this->getCurrentUserInfo();
    $request->attributes->set('lupus_ce_renderer_response_data', $lupus_ce_renderer_response_data);
  }

Additionally, there is hook_lupus_ce_renderer_response_alter as well.

Tested locally and PR works fine, thus moving into RTBC.

After further investigation I found out that Gitpod Classic is going to be shut down in April 📌 Gitpod Classic will shut down, making Drupalpod obsolete Active .

I did try to override the .gitpod.yml file but wasn't successful (It probably needs to get merged into the develop branch and I'm a bit afraid of testing it there).

maybe we could write instructions how to enable the modules in gitpod terminal:

      ddev composer require "drupal/responsive_preview":"^2.0" "drupal/rest_log": "^2.3" "drupal/schema_metatag":"^3.0" "drupal/webform":"^6.0" "drupal/search_api": "^1.0" "drupal/search_api_exclude": "^2.0"
      ddev drush en lupus_decoupled_responsive_preview

testing gitpod with standard installation profile.

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

- reRolled the PR https://git.drupalcode.org/project/lupus_decoupled/-/merge_requests/112#...
- added PR remarks https://github.com/drunomics/lupus-decoupled-website/pull/94#pullrequest...

I checked into this issue and did not find a nice way to hook into the toUrl method https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

Maybe I wasn't thorough enough but $options parameter on UrlGeneratorInterface seem to be defined in advance: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Routing%2...

I had issue testing post request response body until I found rewind method:

$post->getBody()->rewind();

I added a sentence to the docs: https://github.com/drunomics/lupus-decoupled-website/pull/94

merged

Code looks good & tests are passing.
RTBC

Will merge and create a release.

- removed the module and moved the tests to the base module.
- updated tests

I added a MR, since I was already working on ld.

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