🇮🇳India @bhanu951

Review comments addressed.

I tried to test the script on macOs and the script doesn't install the Drupal CMS as expected. Below are the steps I followed as per IS.

1. git clone git@git.drupal.org:project/drupal_cms.git drupal-cms
2. cd drupal-cms
3. cp -r project_template cms-test
4. cd cms-test
5. `vi launch-drupal-cms.sh` and comment out `COMPOSER_CREATE=${COMPOSER_CREATE:-drupal/cms --stability="RC"}` and uncommnet
`export COMPOSER_CREATE='drupal/cms --stability=dev --repository={"type":"vcs","url":"https://github.com/phenaproxima/test-ddev-cms.git"}'`
6. Run ./launch-drupal-cms.sh from cms-test directory.

Then I got below error.

cms-test $./launch-drupal-cms.sh
it usually does not make sense to `ddev config` in a subdirectory of an existing project. Is it possible you wanted to `ddev config` in parent directory /Users/bhanu951/Projects/Contrib/drupal-cms?

@nicxvan : Thanks for the review.

> My question is did we consider adding a helper ?

AFAIK, there wasn't any discussion in this regards.

> It seems like a lot to use the whole registry just to get the templates in the engine.

Agreed.

> I really think this piece should be moved to a themetemplatefinder.

Seems reasonable. Where do you suggest its location ?

How does core/lib/Drupal/Core/Theme/Themetemplatefinder.php sound ?

Ad do we need interface for this ?

@smustgrave

Phpcs failing without those nit changes hence keeping them.

@smustgrave issue is rescoped check #40, patch rerolls are not correct. MR 3248 is relevant.

bhanu951 → changed the visibility of the branch 11.x to hidden.

bhanu951 → created an issue.

Rebased against latest head and moved changes oops hook files

Rebased against head and moved changes to OOPS hooks.

Rebased to latest head, seems it ready for review.

Made changes to MR and address review comments.

Now sure how to handle adding new constructor argument.

Created new issue to add handle adding new constructor argument. 📌 Inject the file_system service in the theme registry service Active .

Should we handle adding new constructor argument in this issue or in other issue ?

bhanu951 → created an issue.

Adding MR changes as patch for backup before rebase.

Bot false positive.

bhanu951 → created an issue. See original summary → .

bhanu951 → created an issue.

Re-rolled latest patch against 11.x branch.

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

Seems issue summary is good. Update the code against latest head.

Changed Target Branch to 11.x and updated latest changes as patch for backup before target branch change.

Thanks @ longwave, @kreynen, @greggles for clarification will try to use 3.x and make the port.

I am planning to work on this port , currently The JS library https://github.com/Valve/fingerprintjs2 redirects to https://github.com/fingerprintjs/fingerprintjs which is licensed under Business Source License 1.1 https://mariadb.com/bsl11/ is it okay to use in this module or do we need to look for alternative library.

The BSL prohibits the licensed code from being used in production — without explicit approval from the licensor.

https://fossa.com/blog/business-source-license-requirements-provisions-h...

https://www.tldrlegal.com/license/business-source-license-bsl-1-1

Does anyone has any suggestions ?

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

Fixed Tests and updated depreciations.

Seems its ready for review.

Uploading MR changes as backup patch before changing target branch.

Uploaded MR Changes as patch before changing target.

Updated the MR to use message

"You have used a one-time login link. You can set your new password now."

Re-rolled patch from #108 to 11.x branch.

It still needs update hook to update new config.

Rebased the MR #909 against 11.x branch and pushed changes.

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

If a site is using 2.0.0-alpha2 is it possible to switch to the 8.x-1.7 branch which is the recommended release?

I tried this earlier and this broke functionality to skip TFA for administrators.

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

Updated IS. Setting NR.

Reroll of #29 and added comment as suggested in #33.

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

I am not sure if it is related to this issue but Entities are not being deleted when the tests are failed.

Let's say you have created new user, new taxonomy term and new node in a test , the test is failed. Ideally all the data created should be cleaned up but it is not the case, the newly created entities are present in the database.

As a work around we need to clean up the entities in teardown manually.

Applying the patch from this issue still produces the same result.

bhanu951 → created an issue.

Reopening this issue as the core issue ✨ Use one-time login link instead of user login form in BrowserTestBase tests Fixed is fixed.

For compatibility I have downgraded the PHP version and merged the code.

@jibus Sure will be waiting for your feedback.

@jibus Thanks for reporting. I downgraded the required PHP version.

Are you using this module in one of your projects ? or you just came across this module ?

I wanted to know if you have faced any issues in TFA validation after migration, in case you are using this module.

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

I too am also able to reproduce this by Navigating to a user with TFA (at least the trusted browser plugin), clicking on the TFA tab, and than clicking on "Configure trusted browsers" as mentioned in #7 🐛 The "tfa_trusted_browser" plugin does not exist. Needs work

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

Uploading changes as patch before changing target branch.

Bumping priority after 3 weeks.

Adding changes in MR as patch before rebasing to 11.x

Added strict_types on new files and rebased to head.

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

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

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

Bhanu951 → changed the visibility of the branch 3437552-fixes-for-opt-in-for-security-review to hidden.

Changes done please re-review.

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

Bhanu951 → changed the visibility of the branch 11.x to hidden.

Bhanu951 → changed the visibility of the branch 2607920-breadcrumb-render-cache-10.x to active.

Bhanu951 → changed the visibility of the branch 2607920-breadcrumb-render-cache-10.x to hidden.

> Can you specify which missing configs the theme is complaining about?

Theme is not complaining about any missing configs. But the site is not being displayed properly due to missing blocks and views.

> Are you planning on using this theme somewhere else?

I am trying to use it for my personal site. With removing references to drupalcon and Drupal Logs as it is GPL.

>This theme is not meant to be installed on any other site

Do you mean to say I can not use this theme with stripped DrupalCon References ?

Bhanu951 → created an issue.

fjgarlin → credited Bhanu951 → .

Bhanu951 → changed the visibility of the branch 11.x to hidden.

Seems the test failure from #153 🐛 Notice: Undefined index: #type in Drupal\Core\Form\FormHelper::processStates() Needs work is a random error. Re-running test did not re-produce it.

  Error message
  Warning: Undefined array key "#type" in Drupal\Core\Form\FormHelper::processStates() (line 211 of core/lib/Drupal/Core/Form/FormHelper.php).
  Drupal\Core\Form\FormHelper::processStates(Array) (Line: 412)
  Drupal\Core\Render\Renderer->doRender(Array) (Line: 458)
  Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 204)
  Drupal\Core\Render\Renderer->render(Array) (Line: 474)
  Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 44)
  __TwigTemplate_335c6caddc004ce5ce1cc6eb323c5a5a->doDisplay(Array, Array) (Line: 394)
  Twig\Template->displayWithErrorHandling(Array, Array) (Line: 367)
  Twig\Template->display(Array) (Line: 379)
  Twig\Template->render(Array, Array) (Line: 40)
  Twig\TemplateWrapper->render(Array) (Line: 53)
  twig_render_template('core/themes/claro/templates/node-edit-form.html.twig', Array) (Line: 372)
  Drupal\Core\Theme\ThemeManager->render('node_edit_form', Array) (Line: 445)
  Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 204)
  Drupal\Core\Render\Renderer->render(Array, ) (Line: 238)
  Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 592)
  Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 231)
  Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 128)
  Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
  Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
  call_user_func(Array, Object, 'kernel.view', Object) (Line: 111)
  Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.view') (Line: 187)
  Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
  Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
  Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
  Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
  Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
  Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
  Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
  Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
  Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
  Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Steps to Reproduce :

Added a field visibility condition using `#states` API

  $form['body']['#states'] = [
      'visible' => [
        ':input[name="field_article_type"]' => ['value' => 14833],
      ],
    ];

    $form['field_local_video']['#states'] = [
      'visible' => [
        ':input[name="field_article_type"]' => ['value' => 14832],
      ],
    ];
   
 

Among the above fields

body --> Default Body Field

field_local_video --> Media Entity Reference Field

field_article_type --> Taxonomy term field.

Re-rolled Patch from #133 against 11.x branch.

But there seems a test failure. Might not be related to this change. Test failure in https://git.drupalcode.org/issue/drupal-2700667/-/blob/2700667-notice-undefined-index/core/modules/file/tests/src/Functional/DownloadTest.php#L138

    1)
    Drupal\Tests\file\Functional\DownloadTest::testPrivateFileTransferWithoutPageCache
    Correctly denied access to a file when file_test sets the header to -1.
    Failed asserting that 200 is identical to 403.
    
    /builds/issue/drupal-2700667/core/modules/file/tests/src/Functional/DownloadTest.php:138
    /builds/issue/drupal-2700667/core/modules/file/tests/src/Functional/DownloadTest.php:76

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

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

Rerolled patch from #26 against 11.x branch.

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

Bhanu951 → changed the visibility of the branch 11.x to hidden.

For anyone looking for solution here is the sample code . Basically you need to use Derivatives to create multiple blocks in Drupal 8+ .

https://git.drupalcode.org/sandbox/Bhanu951-3103712/-/blob/8.x-dev/block...

For those who are looking for limit queue items or queue execution time, there are additional flags in drush commands to limit them.

From drush queue:run documentation page 

Run a specific queue by name.

Arguments¶

• name. The name of the queue to run, as defined in either hook_queue_info or hook_cron_queue_info.

Options¶

• --time-limit=TIME-LIMIT. The maximum number of seconds allowed to run the queue.
• --items-limit=ITEMS-LIMIT. The maximum number of items allowed to run the queue.
• --lease-time=LEASE-TIME. The maximum number of seconds that an item remains claimed.

Came from http://www.madirish.net/555

Re-Rolled patch from #46 against 11.x head.

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

Bhanu951 → created an issue.

Rebased to latest head but need to merge commits, a697018f and 78d40bad - Update /multilingual.tar.gz .

@claudiu.cristea by any chance do you remember what those changes are ?