🇺🇦Ukraine @quadrexdev

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

Thanks everyone for putting some effort here. I have created a merge request with changes from the attached patches.

We have some failing jobs and appropriate issues to resolve:

1. Drupal 11 compatibility (PHPUnit) - https://www.drupal.org/project/domain_path/issues/3429923 📌 Automated Drupal 11 compatibility fixes for domain_path Needs review
2. Validation jobs (PHPUnit, phpcs, eslint) - https://www.drupal.org/project/domain_path/issues/3409106 🐛 Resolve phpcs failing test Active

Those two should be resolved first

Let's fix all the failing validation stuff over there :)

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

Actually, we noticed that it was related to a contrib module - https://www.drupal.org/project/multiple_fields_remove_button →

So I guess nothing to fix in the Tagify module. Probably could be closed (works as designed)

I can confirm that this issue is reproducible. My setup: Drupal core 10.3.1, tagify - 1.2.20.

I'll try to investigate and resolve it

+1 to RTBC, I just tested it and it worked pretty well!

quadrexdev → created an issue.

It could be reviewed now.

Please note that this task uses changes from 📌 Add queue processing for nodes/terms/routes similarly to views Needs review so the parent issue should be merged first.

I have finished this issue, please review :)

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

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

Re-checked and committed on 2.0.x, thanks everyone for your contribution ❤️

Double-checked, seems to be fine.

Committed on 2.0.x.

Committed on 2.0.x, thanks everyone for your contribution!

Merged in 2.0.x

Thanks for reporting, this one was already resolved within this issue - https://www.drupal.org/project/linkchecker/issues/3465025 📌 Fix cspell, upgrade status and phpstan issues Fixed

Alright, everything seems to be resolved.

Thanks @eiriksm for reviewing the merge request!

Alright, the tests are fixed. Remaining stuff:

a) cspell issues
b) eslint issues
c) phpstan issues
d) upgrade status issues
e) revert allow_failure for the cspell, eslint, phpstan, upgrade status so it should be equal to false

1) We'll allow failing for such jobs as phpstan, cspell, and upgrade status so it won't prevent us from further dev release updates.

@todo - revert it once this issue is fixed.

2) Still, some tests failed within the PHPUnit job so it should be fixed too.

quadrexdev → created an issue.

Okay, so I added two base fields: parent_entity_id and parent_entity_type_id instead of using DER field or a link field.

Adding two base fields for storing entity_type_id and entity_id looks like a simpler and more reliable solution for me.

I'll spend some more time testing this one. If someone else can test it and/or provide some feedback - feel free :)

I have started working on this and I hope to finish it within the next few days

Thanks for your quick reply!

Contacted you in Slack for further communication :)

Thanks @vladimiraus → !

I also contacted @c-logemann → to speed up the process.

Created a MR with a fix, which works for me with D10.3 && advagg 6.0.0-alpha1

quadrexdev → created an issue.

Yep, It would be nice to have this feature

Looks good to me

Added some suggestions

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