🇺🇦Ukraine @quadrexdev

Just updating the status of the issue: Contacted @ eiriksm → via contact form today (26.06.2024). Waiting for some reply

Done and ready to be reviewed :)

quadrexdev → created an issue.

Prepared a merge request with a config form. Please review.

It could be reviewed now :)

We used the patch from #9 in our project and it worked fine except for supporting the includes tree.

Our case:

Node with layout builder, inside layout builder layout added some blocks with paragraphs (just a basic block type with a paragraph field) -> paragraphs were not included even after configuring default includes.

What helped:

Adding "include" parameter before

    $this->rootParser->parse($response);

Attaching patch file

a) Removed core key from info.yml file since it is deprecated starting from 8.7.7 (and D8 already reach EOL)
b) Added minimum core_version_requirement to 9.2 (so we can use new once approach)
c) Updated js behavior to use drupal/once library instead of jquery/once

quadrexdev → created an issue.

quadrexdev → created an issue.

quadrexdev → created an issue.

quadrexdev → created an issue.

Implemented and ready for review

After a short talk with a maintainer - we are adding 📌 Configuration form with a list of content types and taxonomy vocabularies Active as a related issue because we want to use the result of this task here.

Fixed everything and prepared MR. All green now, please review.

quadrexdev → created an issue.

It could be reviewed now.

quadrexdev → created an issue.

It could be reviewed now

quadrexdev → created an issue.

Maybe there could be a more elegant solution to this problem, but this one at least resolves the issue for my project.

Please review.

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

"isset" check should be enough even though

$variables['breadcrumb']

is supposed to be an empty array by default. Please see

template_preprocess_breadcrumb()

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

Whoops, forgot that it's already fixed within this issue - https://www.drupal.org/project/html_title/issues/3416142 📌 Tests pass but with warnings Needs review

quadrexdev → created an issue.

Moved the changes from #2 in a merge request, and all tests passed.

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

So I managed to reproduce the issue and created a merge request with a fix + simple test that fails without adding cache tags for rendered titles.

Please review.

All green now, please review

Applied patch from #4 + fixed tests + fixed some changes from the patch -> moved all these in the MR.

Please review.

quadrexdev → created an issue.

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

quadrexdev → created an issue.

@joelpittet I did reproduce this issue on 2.0.x branch and this one should be fixed within https://www.drupal.org/project/linkchecker/issues/3247797 🐛 Object of class Drupal\user\Entity\User could not be converted to string Needs work .

So it's not being triggered because of a fatal error that should be fixed within the 3247797 issue.

Applied suggestions

Applied solution from https://www.drupal.org/project/linkchecker/issues/3441943 📌 Remove validation of "Invalid response code" for administrators Needs review so that we're replacing internal class with status codes in favor of Symfony Response class.

This way we should avoid 99% of problems caused by the incompleteness of allowed status codes in the internal class.

Please review

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

Yep, this one was tested on 404 handler :)

So this issue needs to be reviewed once again after #3386603 is finished

@joelpittet Yep, sounds reasonable to work on replacement in #2958196

I'm wondering why \Symfony\Component\HttpFoundation\Response::$statusTexts is not being used instead of \Drupal\linkchecker\LinkCheckerResponseCodes::$responseCodes

The Symfony response class has a 418 status code as an allowed response code so using it should resolve the problem of this issue.

I reproduced this issue and prepared a fix here - https://git.drupalcode.org/project/linkchecker/-/merge_requests/51

a) Added post_update hook to fix orphaned entries in queues
b) Added cleanUpQueues method that is being called in linkchecker_entity_delete for instances of LinkCheckerLinkInterface.
c) Updated/added new tests

Please review

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

quadrexdev → created an issue.

I think it could be reviewed now

I guess it could be reviewed now.

P.S. in LinkcheckerUnpublish404Test and LinkcheckerRepair301Test I have added the 'system' module in $this->installConfig so that system configs will be installed and the check

$this->currentUser->isAuthenticated()

will be called here: \Drupal\system\TimeZoneResolver::getTimeZone

  protected function getTimeZone() {
    $config = $this->configFactory->get('system.date');
    if ($config->get('timezone.user.configurable') && $this->currentUser->isAuthenticated() && $this->currentUser->getTimezone()) {
      return $this->currentUser->getTimeZone();
    }
    elseif ($default_timezone = $config->get('timezone.default')) {
      return $default_timezone;
    }
    return NULL;
  }

Ok, so I found a place where the error happens: module system, \Drupal\system\TimeZoneResolver::getTimeZone.

Since here we're setting the user object as uid (idk why) - \Drupal\linkchecker\Plugin\LinkStatusHandlerBase::switchSession ->

$this->uid > 0

check in \Drupal\Core\Session\UserSession::isAuthenticated method leads to the error (called in TimeZoneResolver event subscriber)

More details in the attached screenshot

I did reproduce the issue on the 2.0.x release as well.

To summarize steps from #10 - this error is reproducible if any status handler plugin is enabled. It happens here - \Drupal\linkchecker\Plugin\LinkStatusHandlerBase::switchSession because the user object is passed instead of the user ID.

I do believe it should be detected and tested in LinkcheckerRepair301Test and LinkcheckerUnpublish404Test but for some reason, tests are passing... I'll try to investigate it and fix

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

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

I guess it could be reviewed now

quadrexdev → changed the visibility of the branch 2.0.x to hidden.

quadrexdev → changed the visibility of the branch 8.x-1.x to hidden.

Invalid internal links are unrouted ones so I think ->isRouted() check is not what we need, looks like just adding !$url_obj->isExternal() should be enough.

It worked fine on my local using the 9.5.11 core release. I do believe it should be a part of 2.0.x release as well.

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

Ok, now it's passing.

So after applying a patch from #3 all the users had access to view the linkchecker entity (and that was the issue why LinkCheckerLinkAccessTest was failing). I reverted this part cuz the viewing operation is not supposed to be allowed for all the users, right?

Please review

Oops, changed status to needs review before tests passed so reverting it back

Added a merge request with the changes from #3 + added a simple Kernel test to verify it's working.

Please review

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

Moved changes from #2 in a merge request + applied suggestion from #4

Please review

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

I did not reproduce this issue, "link_to_entity" is always of boolean type, not integer.

Please see the schema: https://git.drupalcode.org/project/drupal/-/blob/8.8.x/core/config/schem...

Looks like we need more info to understand/reproduce the problem (if still reproducible)

Implemented a solution by extending the default selection plugin + entity autocomplete element. Please review.

P.S. For sure I'm gonna use it on my projects until it is merged&included in the next release :)

I think we don't need to test the revision revert/revision delete forms In testRevisionsPage(), just because on these forms there is no node title by default.
For reference - https://git.drupalcode.org/project/drupal/-/blob/8.8.x/core/modules/node....

And this - https://git.drupalcode.org/project/drupal/-/blob/8.8.x/core/modules/node...

But, it was there because of this line: $title = \Drupal::service('html_title.filter')->decodeToMarkup($node->label()); in html_title_preprocess_page_title. That's why the testRevisionsPage() was passed before the changes within this issue.

Prepared all the changes in the pull request - https://git.drupalcode.org/project/html_title/-/merge_requests/20.

Please review

Changing version in favor of dev release

I saw some cspell warning while working on this issue - https://www.drupal.org/project/html_title/issues/3272950 🐛 Special characters in RSS item titles are double-encoded Needs review , so decided to fix it here as in the appropriate issue.

All green now, please review

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

Added test coverage in an existing merge request, please review

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

I can't reproduce this issue as well, works just fine

Selecting HtmlTitleFormatter for the title field should be enough to resolve the issue. I do believe this issue should be "Closed (works as designed)"

Prepared a merge request that adds explanation text instead of just a blank page. Please review.

Yeah, it's reproducible in case you just created an export entity but did not assign any exportable entities yet. I think the best solution would be to add some explanation text here so users won't be confused.

Changing the target version in favor of dev release and status to active. "Needs work" is usually set after some initial changes on the issue that require some adjustments/tests.

So I finished with the implementation of translations.

Currently, there are three ways of how can we translate flexible descriptions:

1. Using default translate routes so from pages like /flexible-description/{id}/translations, /uk/flexible-description/{id}/translations/add/en/uk
2. Using management form - /admin/structure/manage-flexible-descriptions. Just switch on the desired language and visit this form.
3. Using HTMX form - visit any entity form that supports flexible descriptions and click add/edit description button, flexible descriptions will be handled depending on the current language

About tests: some assertions fail when tests run within ci, but locally it works just fine. Not sure what is the reason for that, increasing delays in waitFor methods didn't help here and the HTML looks fine in the test artifacts.

I added some todo's here so it should be investigated and fixed in the future (I will prepare a separate issue for that).

Finally got all tests green, merged in 1.0.x-dev

Finally get it all green, except cspell. Not sure why it happens but I don't think it's a blocker now, to be investigated later.

Pushed an implementation of gitlab ci pipelines. Now we need to fix all the warnings/errors.

quadrexdev → changed the visibility of the branch 3425192-inline-operations to active.

quadrexdev → changed the visibility of the branch 3425192-inline-operations to hidden.

quadrexdev → created an issue.

Also, we should avoid using htmx as an asset directly in the module, use the same approach as the "webform" module does - https://www.drupal.org/docs/8/modules/webform/webform-frequently-asked-q... →

quadrexdev → created an issue.

Opened a merge request with inline descriptions editing functionality. This one still needs test coverage at least. If anyone wants to review it and improve it somehow - feel free

Hi @heyyo,

I think it's a great idea to provide the possibility to create/edit flexible descriptions inline. I'm gonna investigate and do it

Tested it on my project and it worked well, Drupal 10.2 +1 to RTBC

Merged, thanks!

I checked it and added some comments in the MR.

Thanks, that sounds reasonable.

Just two points to finish this issue:

1. Please create a merge request using forks.
2. Let's combine if checks within one statement:

Before:

 if(!empty($form[$entity_type_id])){
                 if (empty(Element::children($form[$entity_type_id]))) {
                     unset($form[$entity_type_id]);
             }

After:

 if(!empty($form[$entity_type_id]) && empty(Element::children($form[$entity_type_id]))) {
                   unset($form[$entity_type_id]);
             }

Just fixed a small issue in tests and it seems to work fine.

Thanks!

Pushed the current state of the issue.

What was added/changed:

1. Subfolders for the Functional tests of a submodule.
2. Policy tests that extend Global tests of a submodule.
3. trait RerouteEmailPolicyTestTrait with a configureRerouteEmailPolicy method.
4. Fixed an issue with the validateMultipleEmails method in SettingsForm. The problem was with the usage of ->getValue($element['#name'])

In case we're configuring a rerouting on the policy form - the $element['#name'] was like that: "config[reroute_email][container..." so $addresses variable is always null. Fixed by changing this line to $element['#parents']
5. Added getRerouteConfiguration() method that returns reroute configuration depending on the $rerouteConfigurationStorage property (configureRerouteEmailPolicy or configureRerouteEmailGlobal)
6. Updated configureRerouteEmail() method so it executes a correct method depending on the $rerouteConfigurationStorage property + some required stuff like prefixes for post data.

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

Added ContactTest for a submodule that extends the test from a main module. Also, renamed test classes so we have no SymfonyMailer beginning (so just better naming).

Please review.

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

Thanks for your help @bohart!

I guess now it's ready for the review.

A summary of what was done:

1. The reroute_email_extract_addresses function is moved as the extractEmailAddresses method in the new trait - RerouteEmailHelpers.
2. All to/cc/bcc-related functions use the extractEmailAddresses method to process addresses.
3. New test - SymfonyMailerMultipleRecipientsTest.
4. Associative array keys are used for datasets in SymfonyMailerTestEmailFormTest (so it's easier to find problematic places in case the test fails).
5. New test - InvalidAddressesTest.
6. Updated tests that were failing because of spaces between e-mail addresses in the string. F.e - link.
7. Added clearing of mail collector state due to the following error:

1) Drupal\Tests\reroute_email_symfony_mailer\Functional\SymfonyMailerMultipleRecipientsTest::testMultipleRecipients
All emails have been checked.
Failed asserting that actual size 1 matches expected size 0.

Explained here - link.

I do believe it should be moved to the issues queue of Symfony mailer to add support for multiple e-mails in the collector state.