🇸🇮Slovenia @useernamee

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.

If it helps to anyone: I had to update config_split ^2 (from 1.9) and then error went away.

The most recent incident we had on our system was when there was an png formatted image that was wrongfully saved to have a jpg ending. It got resolved quickly but you might as well check if that's the issue on your system as well.

> However, the whole lupus_decoupled_search_api module... contains zero code. It's only dependencies on lupus_decoupled_views + search_api_db, and a README.

It contains tests. And I wouldn't want to move tests to lupus_decoupled_views (or any other place) since modules have different purpose.

> Isn't this better off being a recipe, instead of introducing a new 'empty' module?

It is a recipe (https://git.drupalcode.org/project/drupal_cms/-/blob/1.x/recipes/drupal_...) - we test that recipe works with lupus decoupled.

> Then we can move tests/modules and tests/src in the 'module root' tests/ folder. I see no issue with that.

There are other ways to add search functionality to lupus decoupled (with external service). This only tests that Drupal CMS Search recipe works.

Remove unnecessary exclusion.

Looks good to me, thank you @lavanytalwar & @chandansha for you contributions.

I added a fix for cspell errors, but there's an issue to discuss: There's a spelling error in the permission id (admininister). I just fixed it (instead of ignoring it), since usually it is only admin that is administrating the trusted redirect and they have the permission by default. A proper fix would be to add an update hook to check all roles permissions and check if this permission has to be re-added without a spelling error but since this is a small project I don't think it is necessary.

I'd just add BC break notice to the next release.

Thank you for you contribution.

I think we should also fix the pipelines warnings in this ticket: https://git.drupalcode.org/project/serve_plain_file/-/merge_requests/6/p...

@chandansha thank you for contribution. I think we should also take the opportunity to fix the cspell, phpcs and phpstan warnings in this ticket.

There are some spelling, lint and codesniffer issues. I'd propose these warnings get fixed before it is merged.

@chandansha - thank you for your contribution. I've added some cspell fixes to the code.

@deepali sardana thank you for your contribution but @mohd sahzad already opened a merge request which is a preferred issue workflow and his .gitlab-ci looks more in line with what we're aiming for. It just need some additional code sniffer fixes and fixes of spelling errors.

Looks good, thus merging.
Thank you kul.pratap for you contribution.

useernamee → created an issue.

fixed

Code looks good.

Thank you @dieterholvoet your PR looks good except for one tiny detail. Please check out.

- I did some restructuring and moved search api integration into its own module so views is independent of search api.
- I removed recipe data from README
- Added tests

> while they are solving the same thing.

Well, actually this issue focuses on a different aspect of the views integration into lupus_decoupled than the #3493780 (with the support for entity reference fields in view data row result (which is independent of search)). What this ticket does is that it allows rendering of fields that are entity references (and could be rendered as entities). But this is not strictly needed for Drupal CMS Search. The focus of this ticket is the support of fields format in view (on the other hand the focus of the #3493780 ticket is the support of Search API and the rest of configuration that comes with the Drupal CMS Search recipe). There is high overlap between the tickets, but in the commit history of the #3493780 first approach was a bit different, but then I noticed that doing things the way this solves 2 problems in one go.

> This needs tests.

Well, I'm a bit reserved about that. I gave another look to this ticket because I got deep understanding of the views support functionality while working on #3493780. Tests are welcome, but this is not strictly needed for DrupalCMS, so if anything I'd create a follow up for them.

I'll fix the phpcs issues, but let's postpone this ticket until #3493780 is merged.

Adding the antibot token to the form attributes should work. I'd just like to check again on conditions I don't think the first one is really necessary.

merged

Looks good.

useernamee → created an issue.

useernamee → created an issue.

useernamee → created an issue.

useernamee → created an issue.

Needs a cspell fix:

./README.md:58:22     - Unknown word (fago)       -- * Wolfgang Ziegler (fago) - https://www.drupal
	 Suggestions: [fado, fags, faro, fagot, fango]
./README.md:59:4      - Unknown word (Radoslav)   -- * Radoslav Terezka (hideaway)
	 Suggestions: []
./README.md:59:13     - Unknown word (Terezka)    -- * Radoslav Terezka (hideaway) - https:
	 Suggestions: [teresa, Teresa, Trek, terex, Terex]
./README.md:63:4      - Unknown word (drunomics)  -- * drunomics - https://www.drupal
	 Suggestions: [dunois, dynamics]
./trusted_redirect.permissions.yml:1:2       - Unknown word (admininister) -- 'admininister trusted redirect configuratio
	 Suggestions: [administer, administers]
./trusted_redirect.routing.yml:7:19      - Unknown word (admininister) -- _permission: 'admininister trusted redirect configuratio
	 Suggestions: [administer, administers]
CSpell: Files checked: 21, Issues found: 6 in 3 files.

and code style fix:

FILE: ...trusted_redirect-3479625/src/EventSubscriber/TrustedRedirectSubscriber.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The first
   |       |     wrong one is
   |       |     Drupal\Component\HttpFoundation\SecuredRedirectResponse.
   |       |     (SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses.IncorrectlyOrderedUses)
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------
FILE: ...modules/trusted_redirect_entity_edit/src/Service/EntityEditUrlResolver.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
--------------------------------------------------------------------------------
 156 | ERROR   | [ ] Description for the @return value is missing
     |         |     (Drupal.Commenting.FunctionComment.MissingReturnComment)
 178 | WARNING | [x] Inline @var declarations should use the /** */ delimiters
     |         |     (Drupal.Commenting.InlineVariableComment.VarInline)
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------
FILE: ...ct_entity_edit/src/EventSubscriber/TrustedRedirectEntityEditSubscriber.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The first
   |       |     wrong one is Drupal\Core\Routing\RouteMatchInterface.
   |       |     (SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses.IncorrectlyOrderedUses)
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------
Time: 163ms; Memory: 6MB
PHP CODE SNIFFER REPORT SUMMARY
--------------------------------------------------------------------------------
FILE                                                            ERRORS  WARNINGS
--------------------------------------------------------------------------------
...src/EventSubscriber/TrustedRedirectEntityEditSubscriber.php  1       0
..._redirect_entity_edit/src/Service/EntityEditUrlResolver.php  1       1
...t-3479625/src/EventSubscriber/TrustedRedirectSubscriber.php  1       0
--------------------------------------------------------------------------------
A TOTAL OF 3 ERRORS AND 1 WARNING WERE FOUND IN 7 FILES
--------------------------------------------------------------------------------
PHPCBF CAN FIX 3 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------
PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
--------------------------------------------------------------------------------
    SOURCE                                                                 COUNT
--------------------------------------------------------------------------------
[x] SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses.Incorrectl  2
[ ] Drupal.Commenting.FunctionComment.MissingReturnComment                 1
[x] Drupal.Commenting.InlineVariableComment.VarInline                      1
--------------------------------------------------------------------------------
A TOTAL OF 4 SNIFF VIOLATIONS WERE FOUND IN 3 SOURCES
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SOURCES AUTOMATICALLY (3 VIOLATIONS IN TOTAL)
--------------------------------------------------------------------------------

useernamee → created an issue.

useernamee → created an issue.

Moved the user form ce response test to user form module tests.

Since this branch was kind of stalled I took the liberty and add my proposal to support referenced entities in it.

Working on:

> this will slow down the setup for all the other test methods. Let's make a new Test class for form tests?

login test is covered already.

I gave this issue another look, because I came to the similar solution in 📌 Support for Search Index View and Rendered entity row Active . Here I focused on support for entity reference fields:

      if ($fields_plugin) {
        $custom_element = new CustomElement();
        foreach ($this->view->field as $name => $value) {
          // @todo Field row plugin is not yet supported and is broken for
          // multi value entity reference fields.
          // Actually, we should use this method for all entity reference fields not just multi value ones.
          if ($value->options['type'] === 'entity_reference_entity_view' && $value->multiple) {
            $entity = $value->getEntity($row);
            $targets = $entity->get($name)->referencedEntities();
            $referenced_custom_elements = [];
            $view_mode = $value->options['settings']['view_mode'] ?? 'default';
            foreach ($targets as $target) {
              $referenced_custom_elements[] = $this->getCustomElementGenerator()->generate($target, $view_mode);
            }
            $custom_element->addSlotFromNestedElements($name, $referenced_custom_elements);
          }
          else {
            $custom_element->setAttribute($name, $value->render($row));
          }
        }
      }

Merged.
Bug hunting season starts now.

Ugh, I came full circle and ended with implementing changes from 📌 Use CE Generator to build View rows Needs work .

useernamee → created an issue.

Looks good, thank you.

Ok, I figured it out and added a section to the README.md file.

Enabling confirmation was not problematic but there's a strange access check in webform that allowed me to access confirmation page only when logged in as admin:

/webform/src/WebformEntityAccessControlHandler::checkAccess

    // Check 'view' operation use 'submission_create' when viewing rendered
    // HTML webform or use access 'configuration' when requesting a
    // webform's configuration via REST or JSON API.
    // @see https://www.drupal.org/project/webform/issues/2956771
    if ($operation === 'view') {
      // Check is current request if for HTML.
      $is_html = ($this->requestStack->getCurrentRequest()->getRequestFormat() === 'html');
      // Make sure JSON API 1.x requests format which is 'html' is
      // detected properly.
      // @see https://www.drupal.org/project/jsonapi/issues/2877584
      $is_jsonapi = (strpos($this->requestStack->getCurrentRequest()->getPathInfo(), '/jsonapi/') === 0) ? TRUE : FALSE;
      if ($is_html && !$is_jsonapi) {
        $access_result = $this->accessRulesManager->checkWebformAccess('create', $account, $entity);
      }
      else {
        if ($account->hasPermission('access any webform configuration') || ($account->hasPermission('access own webform configuration') && $is_owner)) {
          $access_result = WebformAccessResult::allowed($entity, TRUE);
        }
        else {
          $access_result = $this->accessRulesManager->checkWebformAccess('configuration', $account, $entity); // <- this denies access to confirmation page !!!
        }
      }
      if ($access_result instanceof AccessResultReasonInterface) {
        $access_result->setReason('Access to webform configuration is required.');
      }
      return $access_result->addCacheContexts(['url.path', 'request_format']);
    }

Hello, thank you for contributing!
pipeline is now green, but I'd prefer it fixed in a cleaner way. see PR comment.