🇸🇮Slovenia @useernamee

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.

Code looks good, thank you @kul.pratap

I've implemented most of the PR comments.

I think the one regarding the documentation was already covered by ld_form README file.

I added additional type attribute to custom element because that data was lost after I utilized more ld_form code and simplified the output.

I was surprised by isApiResponse method code quadruplication. This should probably be caught before.

useernamee → created an issue.

useernamee → created an issue.

With lupus_stark being D11 compatible → I added it to the lupus_decoupled. I tested it quickly locally but I did not see any comparable difference.

useernamee → changed the visibility of the branch 3479270-add-lupus-stark-for to active.

@roderik here's my take on this:

I guess we could also slightly change the config_rewrite to replace only the output_values.

config_rewrite:
  replace: ["output_values"]

But since output_values are the same in the rest_menu_items/config/install and in the lupus_decoupled_menu/config/rewrite I'm not sure why it is even needed. All it does is that it removes

add_fragment: 1
base_url: ''
allowed_menus: {  }

from the rest_menu_items.config which doesn't look desirable.

That being said. If there's no need to change the values of above config options we don't even need the config_rewrite module in lupus_decoupled at all.

useernamee → changed the visibility of the branch 3479270-add-lupus-stark-for to hidden.

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

Merged already.

useernamee → created an issue.

I've tested the MR and it fixes the issue. Once MR comments are addressed it can go into RTBC.

In our case the whole content rendered as item in the feed is off not just the title.

useernamee → created an issue.

useernamee → created an issue.

@fago we are already testing the logging in in https://git.drupalcode.org/project/lupus_decoupled/-/blob/1.x/modules/lupus_decoupled_user_form/tests/src/Functional/LupusSessionDomainFunctionalTest.php.

But since the main test prepares environment for testing the API output I did put the login form API output in the main module test. I'll move the endpoint testing to the form module.

All 3 subtickets need review:
- 📌 Add tests for Lupus Decoupled views support Active
- 📌 Add tests for lupus_decoupled_form Active
- 📌 Add tests for Lupus Decoupled Block Active

useernamee → changed the visibility of the branch 3485928-add-tests-for to active.

useernamee → changed the visibility of the branch 3485928-add-tests-for to hidden.

useernamee → created an issue.

yes, please resolve the conflict.

greggles → credited useernamee → .

Please review: https://github.com/drunomics/lupus-decoupled-website/pull/76/files