"Don't treat these response codes as errors" not used

Created on 30 June 2019, over 5 years ago
Updated 22 January 2024, 11 months ago

The list is not used in the broken report view to only list other status codes. Therefore currently everything is shown including the good and ignored status codes.

🐛 Bug report
Status

Needs review

Version

2.0

Component

Code

Created by

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • You can use a hook to alter the view. Or you can add a custom filter plugin?

  • I think this would be an important functionality to figure out. As it is right now, the Broken Links report is so inconvenient compared to before, that as an admin I consider it nearly worthless.

    Using the "failed links" filter shows huge amounts of valid links, such as 307 redirects from URL redirects on nodes. The report also shows links even if "test link" is set to false.

  • 🇳🇿New Zealand Gold 20 minutes in the future

    Is there any reason to just not record the results if the response code is in the list?

  • It's a "Broken Links" report. It doesn't make sense to show all links. I don't know how to fix it, though.

  • 🇳🇿New Zealand Gold 20 minutes in the future

    I've never written a Views filter but am currently giving it a go.

    The intention is to just add the filter to the View and have it add and code not in ("this list") to the query. The filter config form will just have a link to the module config form. Then add this filter and export the view again.

  • 🇳🇿New Zealand Gold 20 minutes in the future

    I've got a branch that's not ready for an MR yet.

    https://git.drupalcode.org/issue/linkchecker-3065056/-/commit/237e7dd35a...

    This is on 2.0.x and looks to be working, right up until it doesn't.

    The query() method is being called. I can see my WHERE clause being added to the query object. If I use the exposed filters to add a filter on only one Status Code I can see that being added as well and it is going into the same place in the object my stuff is going. The only difference is that my filter is not being used.

    I've tried a couple of approaches.

    I used $this->query->addWhere($this->options['group'], "{$this->tableAlias}.{$this->realField}", $ignored_response_codes, 'NOT IN') and even walked over $ignored_response_codes adding each as a "!=". I also tried addWhereExpression($this->options['group'], "{$this->tableAlias}.{$this->realField} NOT IN (" + implode(',', $ignored_response_codes).

    While all are going into the query object, none of these worked.

    All we appear to need to do is get this query() to work and we've got it.

    If people want to patch their local version you can add .patch to the commit link above. Once patched you can edit your local Broken Links Report to add the filter.

    Input requested. I'm hoping that there's something about Views Filters that I'm just missing, but the things that I've been referencing all suggest this should be working.

  • 🇳🇿New Zealand Gold 20 minutes in the future

    Gold changed the visibility of the branch 3065056-dont-treat-these to hidden.

  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update 11 months ago
    Not currently mergeable.
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    13:17
    9:23
    Running
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.1 + Environment: PHP 8.0 & MySQL 5.7
    12:19
    8:42
    Running
  • Status changed to Needs review 11 months ago
  • 🇳🇿New Zealand Gold 20 minutes in the future

    I've got a working MR. At least it's working locally for me.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.2 & MySQL 8
    last update 11 months ago
    Build Successful
  • Status changed to Needs work 11 months ago
  • It does filter. However, the filter in the dropdown for "Failed" is no longer needed. And the filter for "Success (2xx)" should be removed, as it will never return results.

  • I also get an error. Edit the linkchecker view with the patch applied.

    Warning: Undefined array key "ignored_response_codes" in Drupal\linkchecker\Plugin\views\filter\IgnoredResponseCodes->adminSummary() (line 44 of modules/contrib/linkchecker/src/Plugin/views/filter/IgnoredResponseCodes.php).

    $this->options['ignored_response_codes'] doesn't exist and is never set. Also, the return value isn't supposed to be an array. Look at examples from Core or the Views module. The solution is this:

      public function adminSummary() {
        $ignored_response_codes = $this->getIgnoredResponseCodes();
        if (!empty($ignored_response_codes)) {
          return $this->t('Ignored response codes: @codes', ['@codes' => implode(', ', $ignored_response_codes)]);
        }
        return '';
      }
    
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    last update 11 months ago
    85 pass, 2 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.0 & MySQL 5.7
    last update 11 months ago
    Composer require failure
  • I fixed the error. Still needs changes per #15

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    last update 11 months ago
    85 pass, 2 fail
  • Status changed to Needs review 11 months ago
  • Question, should the report also exclude links that have status checking disabled?

  • 🇳🇿New Zealand Gold 20 minutes in the future

    #15 It does filter. However, the filter in the dropdown for "Failed" is no longer needed. And the filter for "Success (2xx)" should be removed, as it will never return results.

    I don't think this is the case. If the ignored response codes was intentionally set to be empty all response codes would be returned and the existing selectors would be useful. I would revert commit bfc4fdad. If we forced the contents of this to include "success" values this would be a reasonable edit.

    #16 re: adminSummary()

    Good catch. Cheers. :)

    #19 Question, should the report also exclude links that have status checking disabled?

    I'm not sure where/what this is. Either way though, that's out of scope for this issue and should be added as a new issue so we can get this one over the line.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    last update 11 months ago
    86 pass
  • Okay, reverted. Though it still doesn't make 100% sense, since the report is called "Broken Links".

  • 🇳🇿New Zealand Gold 20 minutes in the future

    Okay, reverted. Though it still doesn't make 100% sense, since the report is called "Broken Links".

    Absolutely agree with this. It's not a task that this ticket is for though. :) And you're right, with it being a standard view the end user can modify it to their specific needs.

    I'm feeling like this is RTBC but as one of the people that did the code part of this I don't think it's my place to do that.

Production build 0.71.5 2024