I am working on a project that involves storing/cataloging lots of links. The people in charge of gathering information and storing/updating links will also be checking for and fixing broken links. They won't necessarily be tech-y people, though, and so I wanted to be able to customize the Broken Links report so that I could show only certain errors/response codes, or even translate some of it into English (maybe show the word "Redirect" instead of "301," for example). I also thought it would be useful to filter the report to only show certain content types, in case someone is only in charge of administering links within a certain content type but not another. And if I go this route, I thought it would be nice to be able to create multiple, different Broken Links reports, each of which shows different types of information.

To make it easier to do all the customization I wanted, I wrote a simple module that integrates the Linkchecker tables with Views via hook_views_data and then provides a default view set up to look exactly like the Broken Links report, via hook_views_default_views. Since these hooks are contained in one file, the only action that would be needed to include Views integration in the module would be to include linkchecker.views.inc (which I've attached to this post - you'll need to rename it to linkchecker.views.inc) in the module directory, and then add hook_views_api to the .module file as follows:

function linkchecker_views_api(){
      return array(
            'api' => 2.0,
      );
}

I don't know if my needs were so specific that no one else will find this useful -- in which case I'll just keep this code in my own custom module -- or if anyone thinks this would be a good idea to include in the linkchecker module by default. Also, I'm sorry for not writing a patch - I don't really know anything about how to go about that, or how CVS works, or anything, and I figured that, since 99% of this code is in its own, separate file, that it wouldn't really be worth it to write a patch that just implements hook_views_api.

Anyway - let me know what you think. Thanks!

Files: 
CommentFileSizeAuthor
#116 linkchecker-views-integration-965720-116.patch50.49 KBjames.williams
#113 linkchecker-views-integration-965720-113.patch44.58 KBkafmil
#112 linkchecker-views-integration-965720-112.patch52.34 KBkafmil
#102 linkchecker-views-integration-965720-102.patch50.4 KBkafmil
#101 linkchecker-views-integration-965720-101.patch52.15 KBkafmil
#96 linkchecker-views-integration-965720-96.patch52.01 KBkafmil
#95 linkchecker-views-integration-965720-95.patch45.87 KBhelmo
#93 linkchecker-views-integration-965720-93.patch45.75 KBMrHaroldA
#89 linkchecker-views-integration-965720-89.patch45.96 KBjantoine

Comments

hass’s picture

It would be great if you could add your views module. I'm interested to get the functionality you are writing about into the module. There is also one request to implement link checker reports with user defined filtering...

nkschaefer’s picture

To be clear - you want me to upload the (separate) module I've written to drupal.org as its own "project" -- right? I can do that, but I need to learn how (and apply for a CVS account) first. I just want to make sure I'm going about this the right way, since I'm somewhat new to Drupal and haven't contributed anything here yet. If there's some other way I should go about this -- i.e. submitting my code here, as a proposed change to this module -- I can figure out how to go about doing that instead.

Thanks for your help with this.

hass’s picture

Let's attach a zip with your module here first. We may integrate it completly into the linkchecker module. I see no reason why we should maintain this features in a seperate module.

nkschaefer’s picture

FileSize
6.84 KB

All right -- here's a zip of the module directory. All the Views fields should appear under the group "Broken Links" in the Views UI. And the default View this module creates is called "broken_links;" as far as I know, it should look just like the Broken Links report that the Linkchecker module creates.

In addition to the fields in the linkchecker_links table, I've exposed two custom fields to Views. These are "Edit Settings Link," which provides a link to edit the Linkchecker settings for the given URL, and "Status Code Meaning," which provides a rough human-readable translation of what the status code means. I lifted all the status code meanings from the page at http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html.

Let me know what you think. Thanks!

_vid’s picture

Subscribing...
This sounds like a great idea. I've been making a number of my links a separate content type that I can display them in a view organized by taxonomy. But that's overkill in a number of cases and having a simple report that show's all the links on the site is great!
Vid

waverate’s picture

This sounds fantastic however perhaps I missed something in nkschaefer's instructions.

1. I put the original attachment in /modules/linkchecker/includes and renamed it linkchecker.views.inc

2. I added his original function linkchecker_views_api() to the bottom of linkchecker.module

If this is correct, where should I be seeing the linkchecker content in views?

nkschaefer’s picture

Sorry for the confusion; if you just want to get the broken links Views integration, that zip file I uploaded is a stand-alone module: just put it in your modules folder and enable it; it should work (although you might need to clear your cache).

The thing about adding the hook into the linkchecker module, etc. is for if linkchecker's maintainers want to integrate this into the main module -- but for now, I thought it would be best to keep my code in its own module, so you can keep updating linkchecker without needing to worry that you're overwriting anything.

nkschaefer’s picture

I should add that, once you enable the module, you'll see a new View available called "broken_links." It should be set up to look just like linkchecker's "Broken Links" report, and if you edit it, you can see how to customize it or build your own Views from linkchecker data.

waverate’s picture

nkschaefer,

Thanks for the clarification. I was hoping that your instructions at #1 were how to added the Views capability to the existing Linkchecker module without having to load another module.

That being said, your file at #4 works perfectly.

I did not allow permission for users to access own broken links report. Instead, I used the broken_links View to create a My Broken Links tab in the /user page. It allowed me to display the table of broken links as I see fit.

I wouldn't want to make Views a dependency for the Linkchecker module however this should be an addon.

d0t101101’s picture

Hey nkschaefer,

Great work, thanks for contributing this. Hope to see views support included in Link Checker based on your code.

.

d0t101101’s picture

I'm finding duplicates in both the standard broken_links view, and my own custom views cloned from this report. Selecting 'distinct' has incorrect results, the count is right but duplicates remain.

Any ideas on how to solve this? I'm by no a views filter guru, but have tried everything I could think of to fix - adding more filters, selecting distinct, looking at the code, etc...

Thanks,
.

nkschaefer’s picture

Hi -

Sorry I haven't checked here lately. Are you still having the same problems? I'll look into it once I get a chance -- but the code in that module I uploaded relies on preexisting views filters, so I'm not sure what could be causing duplicate rows.

I don't know if you've had a look at your database tables, but one quick idea that comes to mind is that you might have multiple links per node. The linkchecker module provides two tables (linkchecker_nodes and linkchecker_links) -- and the base element of the main table (linkchecker_links) is the URL of a link, not the node that contains it. The base table of the view I provided is node -- so if one node has multiple links stored in it, it might show up in that report multiple times. This issue didn't come up for me, because I'm storing URLs in a single-value CCK field, so each node has one URL.

How are your URLs related to nodes -- can one node have multiple URLs? Can one (the same) URL be stored in multiple nodes?

Once I get some time, I'll have to look back over this to remind myself how it works - and then I might be a little more helpful.

greg.harvey’s picture

@hass Any plans to include this, or should nkschaefer create a project for it?

Either way, @nkschaefer - perfect! Just what I need! =)

hass’s picture

It's planed to get included sometimes...

greg.harvey’s picture

Status:Active» Needs review

Great! Setting to 'needs review', since the module is attached in #4. Would you someone to re-roll that as a patch to be applied to 6.x-2.x-dev to make it easier for you to apply once tested?

hass’s picture

Status:Needs review» Needs work

Yes, patch is required.

nkschaefer’s picture

Well, I've never created a patch and have no experience with Git/SVN/CVS (hopefully will soon), but if anyone wants to give it a shot, all you have to do is add the hook_views_api() (linkchecker_views_api()) to linkchecker.module (just copy and paste from the initial post), then add the .views.inc file to the module directory and rename to linkchecker.views.inc. I'd imagine that would just take a minute or two.

polynya’s picture

Version:6.x-2.4» 7.x-1.x-dev
Status:Needs work» Needs review
FileSize
13.7 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in 965720-linkchecker-views.patch. View

This patch for Drupal 7 is based on nkschaefer's work.

hass’s picture

Title:Views integration for Broken Links report - code attached» Views integration for Broken Links report
hass’s picture

hass’s picture

Status:Needs review» Needs work

There are several bugs in this patch (I have not reviewed the module if this is the same):

  • Status code has missunderstood - it's not a status of broken or not
  • Security issue: Everyone has access to the view - I hope you are not running this code with non-admins!
  • Filters are not correct, last checked is not of type date or I'm at least cannot create a filter with a value of 0 to exclude unchecked links
  • The report is for node links only, blocks and comments are missing
  • Pager items is 10, should be 50
  • We cannot sort by columns that have no index. Major performance issues will occur and on TEXT fields we are technically not able to add indexes
  • We need to add some indexes, at least to status code, status and wherever it's possible...
  • View should be disabled by default
  • This is not an view that has been exported from D7!? How have you created the view?
  • D7 Views 3.0 seems to support translation of string *yeah*
  • I'm wondering that the base_table = 'node' and not linkchecker_link... not sure what this does

I have fixed a few of them, but still playing with the views GUI...

hass’s picture

FileSize
26.14 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in Views_integration_for_broken_node_links_report-D7.patch. View

This patch adds indexes to linkchecker_link table, fixes some of above issues and adds some descriptions to make it more pretty.

OPEN:

  • I'm wondering that the base_table = 'node' and not linkchecker_link... We don't need the node table at all. This seems to be wrong from the table joins I see in views.
  • The report is for node links only, blocks and comments are missing
  • last_checked is a unix timestamp and seems not supported by views, see #1266398: Creating filter for Unix Timestamp field. A workaround has been implemented with type numeric.
  • If a link is used in more than one node the Operation links need to show all nodes with these broken link.
  • Security: If link settings are shown, must depend on permissions.
hass’s picture

Status:Needs work» Needs review
FileSize
26.14 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in D7-Views_integration_for_broken_node_links_report.patch. View

Re-attach for testing bot.

Status:Needs review» Needs work

The last submitted patch, D7-Views_integration_for_broken_node_links_report.patch, failed testing.

hass’s picture

FileSize
25.69 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch D7-Views_integration_for_broken_node_links_report_0.patch. See the log in the details link for more information. View

+

hass’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, D7-Views_integration_for_broken_node_links_report.patch, failed testing.

hass’s picture

Status:Needs review» Needs work
FileSize
25.93 KB
PASSED: [[SimpleTest]]: [MySQL] 101 pass(es). View

git patch format

hass’s picture

Status:Needs work» Needs review
hass’s picture

Status:Needs work» Needs review
FileSize
24.26 KB
PASSED: [[SimpleTest]]: [MySQL] 101 pass(es). View

Committed the index changes to latest DEV as they are not only for views.

hass’s picture

Status:Needs review» Needs work

Ok, nkschaefer's module has small differences. There are at least a few interesting additions we should also implement.

I'd also planed to make the status code messages more user friendly. He already implemented something like this to show the standardized error messages. My idea was to change the way how the current _linkchecker_isvalid_response_code() function works. It should become reusable for other use cases like his. The main idea was to extend/change the $responses array by more information that also allows translation and can tell a user what they can/must do if they see a specific code.

  $responses = array(
    100 => array(
      'title' => t('Continue'),
      'help' => t('Please do foo, if you see this error code. The reasons for this message could be foo and bar and can be solved by doing foo.')
    ),
    ...

@nkschaefer: Do you like to work more on this views patch to get it in? Currently it really has some open to do's we must solve first. I would also love to see a views block as this was another request. After the 'page' view is ready this seems to be a 3 minutes job, we should just integrate.

Setting CNW for the todo's listed in #22.

mgifford’s picture

Status:Needs work» Needs review
hass’s picture

Status:Needs review» Needs work
hass’s picture

Marked #726006: Filtering results in admin/reports/linkchecker by link content / status / node type / etc as duplicate. We can implement filters with views exposed filters.

hass’s picture

FileSize
18.95 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

A few steps closer to this feature. UNION support is not available in views per #1873796: UNION support. Implemented a workaround for both queries.

+      'filter' => array(
+        'handler' => 'views_handler_filter_in_operator',
+        'options callback' => 'linkchecker_views_get_ignore_response_codes',
+      ),
...
+  $handler->display->display_options['filters']['code']['value'] = array(
+    drupal_map_assoc(preg_split('/(\r\n?|\n)/', variable_get('linkchecker_ignore_response_codes', "200\n206\n302\n304\n401\n403")))
+  );

Requires it's own handler in linkchecker or we cannot use the existing linkchecker_ignore_response_codes variable.
  • We also need to write our own permission handler. Just checking for 'access broken links report' is not ok.
  • Empty result set text t('No broken links have been found.') need to be added
  • +, +, +
hass’s picture

FileSize
22.51 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

Just posting latest code.

hvalentim’s picture

Wonderful functionality. Should we start testing it?

Rob_Feature’s picture

tried to apply this without success on the latest dev....I ended up manually applying it.

It seems to me the biggest piece missing is any reference to the node...the [nid] in the view does nothing because there's no node data in the view itself.

bago’s picture

Status:Needs work» Needs review
FileSize
25.72 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch linkchecker-viewsintegration-965720-39.patch. Unable to apply patch. See the log in the details link for more information. View

So, here is my attempt to finally have this issue fixed.

here is what I did:

  • I changed the last_updated column to date field, so that it can be better displayed/customized
  • I added an "operations" custom field that will display the list of operations like the build in report and replaced the "Custom text" field with this one
  • Changed the default status to disabled and changed the URLs to the built-in URLs so you just have to enable it and alter the view definition to use views instead of the build in

Note: I had to almost clone some of the code from _linkchecker_report_page in order to regenerate links and maybe this could be refactored so to be written only once. Also in that "cloning" I lost the $account feature as I don't know what it is about... I think this $account stuff is the only missing stuff in order to views to perfecly replace the build in views.

hass’s picture

Thanks for following up here. We should write up an issue summary. There are still some critical things to do and nobody should run this code without except you are site admin. The code should not used with any normal user with limited permissions (access bypass security issues exists).

Just from memory

  • Custom access handler (cannot committed without)
  • Split the one view with two displays into two independed views to cleanup the pre execute hook and allow more flexibility in custom displays
  • Empty result set text
  • + other things I may missed
bago’s picture

So, let's focus on the blocker: "Custom access handler". Can you explain? (I don't know the linkchecker module and I just broadly know views api)

hass’s picture

I see you may solved it a bit in _linkchecker_operations(), but I thought we need something different as every single link need to be checked if the user has access to it. In views UI the user is able to select a permission. This is not ok and need to work like the menu access callback. I'm not sure about the permission checks we have on the links shown in the reports table. It was really complex to implement this. Without a code review I cannot say for sure if your code works like current report page.

bago’s picture

So, I think this is too hard for me, expecially considering I just need the last_checked column in the main report page and I don't use user's report pages.

I propose another solution: what about splitting this task in "views integration for main report" and "views integration for user's report" so that we can simply remove from my patch the user report stuff and try to have this code committed? I think this would be a good option for users while we wait for someone with better views/linkchecker knowledge to also support the user's report "tweaks".

If you agree I will submit a new patch removing the user/*/brokenlinks view.

This issue has 2.5 years, so I think it is better to narrow it down and make progresses instead of looking for the perfect world.

hass’s picture

If it would be a real requirement for someone I guess it would have been done already. As I know implementing one or the other view is no difference in complexity.

bago’s picture

Well, it is a real requirement for me. I need to show the last checked column. You closed my feature request pointing to this issue, but this issue is not solving my problem (as it had no working patches), so I tried to fix it to help it being committed.

I think the admin report is simpler because it doesn't need to check for user permissions, that is the missing / blocking brick for this issue.

I need a solution for the last_checked in the admin report and I would have liked to have it committed because I don't like to branch modules. That's all. You are free to reject feature requests or patches, but don't tell me that this is not a requirement for anyone ;-)

hass’s picture

Above patch works, but has security issues. If you only run it as admin it's fine.

bago’s picture

What I say is that the security/permission issue is mainly related tp the view for user/*/brokenlinks.
THe admin view is currently tied to a permission "Access broken links report" so it doesn't have security issues. Even if you assign this permission to anonymous at most you see links but don't see node, comments or blocks you don't have access to (only the links). I think this is an expected/lecit behaviour that doesn't raise security concerns.

Instead I agree that the user broken link page would need some more work to be reproduced with views and I agree this could be a blocker for the view to be committed, that's why I proposed to push the admin report part of the patch and then leave to someone else the user report part.

If you are willing to review and apply a patch involving only the admin report I will provide a clean patch for that.

hass’s picture

"Access broken links report" is a security issue!

bago’s picture

Can you elaborate? I don't understand the security issues with that permission and the content we display.

If we change it to "administer content" & "access broken link report" or to "role is administrator" would this fix your security concerns?

I just want to see some of this commited and then we can talk about enhancement: also more developers will be attracted once part of the feature they want is there.

hass’s picture

Not sure as we kept this access bypass secret in past. Not sure if security team allows this. Only safe permission is administer site.

bago’s picture

OK, so what do you think if I clean the patch to leave only the admin report view, disabled by default and with the "administer site" permission? This would work for me. If this is acceptable for a first commit then I'm happy to prepare and test the patch.

hass’s picture

Status:Needs review» Needs work
bago’s picture

Status:Needs work» Needs review
FileSize
22.76 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch linkchecker-viewsintegration-965720-52.patch. Unable to apply patch. See the log in the details link for more information. View

Not sure if your answer to my last question was yes or no... btw I removed the user report and changed the admin report to require "administer site" permission.

hass’s picture

Status:Needs review» Needs work

I do not commit broken code, feature regressions, feature reductions or unready code. Sorry.

bago’s picture

It's hard to collaborate to this project :-(

I don't agree with you about this patch having broken code, feature regressions, feature reductions or unready code. I will simply go back to my simple one liner local change to enable the last_changed column.

My only criticism is that you wasn't clear in your messages about what is acceptable and what is not and so I wasted hours trying to produce a patch you liked.

Thank you anyway.

hass’s picture

Not really.

I cannot commit any of these patches as it reduces features (admins cannot allow users to see the global broken link report) and the previously mentioned access handler is also missing (security issue, unready code). The patch may add the last_checked column and this is not what the most of the users need and we have very limited space in the already overloaded table. Such a column need to be removed and you can add it on your server manually. I also tried using last_checked with date format and this does not allow excluding "-1" values if I remember correctly (possibly broken patch). However I'm willingly to learn from your code; if you found a way to implement this as I failed to implement it myself.

The only patch that is acceptable is a 1:1 replacement of the current reports. I see the reasons why this feature is useful for some people to customize the output and that's why I accept this community effort. However the interest seems to be very limited as nobody really try to complete the outstanding todo's and I have personally no need to move all this stuff to views. If you need the views integration you can complete the outstanding work and I will review the complete patch for sure.

bago’s picture

I accept your opinion. There's nothing I can do to convince you of my opinion as you are the owner of this project and it is your right to decide what to commit and what to not commit.

This does not remove a single feature from the module as the view is optional and if you don't enable it the code is unchanged. If you enable it you don't get any security issue because the view has "perm = administer site".

This is not a replacement as it doesn't remove the core reports. I would agree with you if this we removed the core code.

My needs do not worth the work you ask in order to be committed (I tried to provide a patch simply because I misunderstood your position and I thought we could have it committed with the changes I added): at most I will create a new project with the view integration code, so to share with other something useful.

Thank you for the great module!

bago’s picture

For anyone interested in testing using some limited view integration I created an integration project: Link checker Views.

I hope we will be able to complete the requirements for this integration to be merged back to linkchecker project so to close that child project, but if this won't be the case the few users needing this integration have something to work with, without relying on patches.

Leaving this issue opened, closing it, moving it to linkchecker_views is up to you: as you feel it's better.

hass’s picture

It would be a lot more useful for all not wasting time with duplicating efforts by working here on the issue.

jelo’s picture

I did not read the full discussion. However, I am very much interested in this feature. Right now the link checker report seems to include all nodes, published and unpublished. With views integration an admin could easily create a new report that only shows broken links on published nodes.

My use case: I have a huge archive of unpublished content (news etc.) that I need to keep, but which is inaccessible to the general website visitor. However, I need to keep the published nodes free of dead links. The current report is not helpful to me because I cannot easily identify the nodes that need my attention (= are still published)...

hass’s picture

Right now the link checker report seems to include all nodes, published and unpublished.

This is incorrect. Links from unpublished nodes will be removed.

jelo’s picture

Thanks for the clarification. When is the update triggered because it does not seem to be instantly. I cleared caches etc. I think I have configured cron to only run linkchecker once at night so if it triggers on a cron run then I might just have to wait a little longer...

hass’s picture

Immediatly after the node is unpublished it is no longer shown in the reports.

jelo’s picture

Unfortunately, that is not the case in my site. I just tested a variety of scenarios and links do not get removed from the report when nodes are unpublished. They do get removed however when the node is deleted. I tried nodes with only one revision, multiple revisions, nodes that only had one dead link on it and the same dead link was not included in any other nodes etc.

I took a look at /includes/linkchecker.pages.inc and the function linkchecker_admin_report_page does not distinguish in the sql query if a node is published or not. The table linkchecker_links has a status field in it, but all my entries have status 1. I do not see how that table should work though either because a dead link recorded in that table could appear on multiple pages that are recorded in linkchecker_nodes. So if a broken link is recorded in linkchecker_links, where do you determine if it appears in a node that is active/published?

Sorry if I am missing something obvious. I am normally just working on the frontend and not backend.

J.

hass’s picture

The entries must be removed in hook_node_update(). There is a condition that matches unpublish state and removes all references. Are you using any revisioning modules? Workbench only works in the latest release.

jelo’s picture

Hmh, that might explain it. I am using Revisioning (6.x-3.15) and Workflow (6.x-1.5). I guess these modules are not supported then. Which brings me back to how great views integration would be ;-)

hass’s picture

Please open a new request. Workbench Moderation module IS supported by linkchecker. Revisioning module has never been tested and may need to be added, too. This has nothing to do with this issue here.

jelo’s picture

Thanks hass. Opened under https://drupal.org/node/2067317. I hope that the description covers the issue we discovered here reasonably well...

koppie’s picture

Will this ever be added to D6?

tommybyskovlund’s picture

Integration with views would be great, and I'd hope not to need a separate module

awolfey’s picture

I've been looking and playing with this. I think the access problem can be solved by using node as the base table and adding a relationship to the linkchecker_link table. That way the node_access tag is added to the query.

This requires a hook_views_data_alter() of the node table to add the relationship to linkchecker_node, and linkchecker_link is joined to linkchecker_node in hook_views_data() from the linkchecker_views module.

Is there still interest in bringing this back in to linkchecker?

hass’s picture

I think the access problem can be solved by using node as the base table and adding a relationship to the linkchecker_link table. That way the node_access tag is added to the query.

No, that is not possible. See my comments #21 and #22, please.

Is there still interest in bringing this back in to linkchecker?

Yes, highly interrested in getting this feature implemented before the D8 port starts.

The latest patch you should work on is #36. The later ones are invalid.

hass’s picture

hass’s picture

awolfey’s picture

This is simply a reroll of #36 against the current dev.

awolfey’s picture

Here is what I think can be done with the patch from #75 (originally #36).

If we add the 'node_access' tag in linkchecker_views_pre_execute() then for the user report, nodes will be filtered out by modules implementing hook_node_grants().

    $query->addTag('node_access');

The view itself would need to further control access to unpublished nodes, probably with the view own unpublished content permission, if needed.

The subquery there limits comments to those created by the current user, so I don't think there's anything to do with comments, and if the node is no longer accessible then the node_access control will prevent the comment owner from seeing the links on the comment.

The limitations of this approach are

  • That the name of the view and display are hardcoded into the pre_execute hook, so you're forever limited to the one view and the two displays.
  • From what I can tell, there is no field-level access control. So even if we could prevent access to restricted nodes, there may be available nodes that hide fields with the field permissions module or other means. These fields are still scanned and there's nothing in place to hide any links they contain. This is true in the current implementation of the module. I could create a node, and admin could add comments and links in a field I don't have access to, and then I could see the links in the user report even though I can't see the field. I'm not sure where this case would ever come up. You do have to trust the site admins to know what they are doing.
  • Additionally, the union of the node, comment and block tables make it pretty hard to do much with the view for the admin report. The same goes for the user report with the node and comment tables.

The alternate approach suggested by @bago that uses the node, comment and block(?) tables as base tables has a larger but cleaner views integration, and makes the views much more flexible and extensible. It still has the field access limitations, and therefore like the current implementation and hass's approach has to limit access to only users trusted to view all fields on the content they have access to. Also, you'd probably need separate views to display node, comment and block links.

Personally I like bago's approach because it makes the node object available in the view.

Feedback?

TDI007’s picture

I implement the patch of #75 but no group "broken links" appears for the field "show" in ".../views/add". How I can start with this great idea.
I cant see any error messages in recent logs or somewhere.
Please give me a hint.
(I have installed 7.x-1.2+2-dev of link checker)

jantoine’s picture

FileSize
45.81 KB
PASSED: [[SimpleTest]]: [MySQL] 159 pass(es). View

The attached patch used the patch from #30 as a starting point. I've read through the entire thread and addressed all the issues that were raised along with many other issues that I identified while working on the issue. Here is a summary of all changes this patch makes to the module:

  • Defined all views handlers and plugins in the .info file.
  • Updated the admin and user report menu items to use a new _linkchecker_access() function for the access callback.
  • Updated the linkchecker edit link settings menu item to use a new _linkchecker_access_edit_link_settings() function for the access callback.
  • Renamed the _linkchecker_user_access_account_broken_links_report() function to _linkchecker_access().
    This was done to follow similar access related functions found in core (node_access, user_access, etc.)
  • Updated the _linkchecker_access() function to grab a user object from the path if one exists.
    This was done in order to validate access for pages generated by views.
  • Updated the _linkchecker_access() function to work for both admin and user reports.
    This was done so a single function could handle page level access for consistent permissions and ease of updating.
  • Renamed the _linkchecker_user_access_edit_link_settings() function to _linkchecker_access_edit_link_settings().
    This was done for naming consistency among access related functions.
  • Updated the _linkchecker_access_edit_link_settings() function allowing users with the 'access broken links report' permission.
    This was done for consistency since edit link settings links were displayed on the broken links admin report without any other checks. We could change this so that both the 'access broken links report' and 'edit link settings' permissions are required if desired.
  • Implemented hook_views_api().
  • Added a _linkchecker_response_codes() function that returns an associative array of HTTP status code messages keyed by the HTTP status code.
    This was done so that we could leverage this list of options within views filters.
  • Updated the _linkchecker_isvalid_response_code() function to get the list of HTTP status codes from the _linkchecker_response_codes() function.
  • Moved the _linkchecker_is_internal_url() function to the .module file.
    This was done so the function could be used by views without having to include another file.
  • Added a views handler for an edit block link field.
    This was done so that the edit block link only appears for users with access.
  • Added a views handler for an edit link settings link field.
    This was done so that the edit link settings link only appears for users with access.
  • Added a views handler for an add redirect link field.
    This was done so that the add redirect link only appears for users with access.
  • Added a views handler for the URL field to display a access denied message.
    This was done to simulate the entire row being replaced with an access denied message, but that is not easily accomplished in views. The best way to handle row level access is to create a linkchecker access system similar to how the node access system works. In drupal 8, I think it would be best to make links entity and utilize the entity access system. This would also make everything natively available in views reducing much of this code.
  • Updated the views handler for the code filter adding a list of code options to filter on.
    It seemed silly to have a views filter pull filter values from a variable. Plus, that also limited all views to sharing the same filter and prevented doing exposed filters.
  • Implemented hook_views_data().
    • I declared the linkchecker_link table as a base table.
      This was done so views can be created via the UI.
    • I've added relationships to nodes and comments.
      This was done so that nodes and comments can be fully leveraged.
  • Implemented hook_views_plugins().
  • Implemented hook_views_pre_render().
    This was done to add the status message related to the number of unchecked links that is seen on the current reports to views pages that have the linkchecker_link table as the base table.
  • Implemented hook_views_default_views().
    This adds default page views similar to the current admin and user reports. I customized the views export adding conditions around components dependant on other modules such as comments and redirects. This prevents the view from showing the 'broken or missing handlers' error if one of the modules is not installed.
  • Added a Linkchecker access plugin for deterining if the views can be accessed by the user.
    This essentially just calls the _linkchecker_access() function.

@todo
There are a few things that are done in the current reports that are difficult to replicate in views and that time didn't allow me to tackle.

  • The user report is not currently filtered, so the user mostly sees access denied messages.
    Author contextual filters can be added to views and do work, but there isn't currently an OR opperator for contextual filters, so a link would have to belong to a node AND a comment to show up. There are a few issues in the Views issue queue for adding an OR opperator or somehow leveraging contextual filter values in normal views filters, but these solutions would require a patch or dependency on another module. I think the best option forward is to have three separate tables on the user report for node, comment and block links.
  • Results are not currently grouped by URL.
    Grouping by URL can be done in Views and works, but it's not the same as the current report and other fields are not aggregated together. Views aggregation is another possible solution.
  • Performance and row filtering.
    Performance doesn't seem to be an issue on my system, but a lot of access queries could be reduced by creating a linkchecker access system similar to how the node access system works. This would allow for row filtering at the database level.
jantoine’s picture

I forgot to mention that there is a bug with the comment_access() function that causes comment edit links to appear when no comment exists. #2542798: Edit comment link causes PHP notices and is visible by unauthroized users if no comment exists and #2558435: comment_access() doesn't validate the $comment object both have patches that address this issue.

helmo’s picture

Status:Needs work» Needs review

Thanks, I just did a quick test with the patch from #78 and it looks OK so far.

What would the test bots say...

hass’s picture

Status:Needs review» Needs work

per #78 there are todos.

jantoine’s picture

@hass,

I'd love to hear your feedback on the three todo's and my suggestions for fixes. Note that adding a linker_access table would also solve both the first and third issues. I'd love to have an agreed way forward before putting in any more time.

MrHaroldA’s picture

@jantoine: thank you for your contribution! This made it possible to add a broken links block to the editor dashboard.

I did a quick review too, and ran into these notices:

Notice: Undefined variable: comment in linkchecker_views_default_views() (line 57 of /home/harold/.../modules/contrib/linkchecker/views/linkchecker.views_default.inc).
Notice: Undefined variable: comment in linkchecker_views_default_views() (line 118 of /home/harold/.../modules/contrib/linkchecker/views/linkchecker.views_default.inc).
Notice: Undefined variable: comment in linkchecker_views_default_views() (line 179 of /home/harold/.../modules/contrib/linkchecker/views/linkchecker.views_default.inc).
Notice: Undefined variable: comment in linkchecker_views_default_views() (line 228 of /home/harold/.../modules/contrib/linkchecker/views/linkchecker.views_default.inc).
Notice: Undefined variable: comment in linkchecker_views_default_views() (line 281 of /home/harold/.../modules/contrib/linkchecker/views/linkchecker.views_default.inc).
Notice: Undefined variable: comment in linkchecker_views_default_views() (line 380 of /home/harold/.../modules/contrib/linkchecker/views/linkchecker.views_default.inc).
Warning: Invalid argument supplied for foreach() in menu_unserialize() (line 400 of /home/harold/.../includes/menu.inc).

I guess this has to do with the comment module being disabled?

jantoine’s picture

@MrHaroldA,

Glad everything worked for you. Yes, it appears you're suffering from the bugs I ran into with the comment module and views. The issues listed in #79 contain patches that should eliminate these messages.

MrHaroldA’s picture

I found another one today. Users with access to the view get a "Permission restrictions deny you access to this broken link." text instead of the URL. This can be "fixed" by granting the "Access broken links report" permission, but I don't want our editors to access the default broken links report page.

The (error) messages, HTTP codes and other information are available, it's just the URL which is blocked.

... but then again, even the root user gets the "Permission restrictions deny you access to this broken link." message in the broken links report?

jantoine’s picture

@MrHaroldA,

This isn't that different than the default user report. The main difference is that it includes many links the user doesn't have access too, hence the message. One way I have cleaned this up was to add a filter on the URL field filtering out any records that begin with "Permission restrictions". This should help clean up the user report. What we really need to do here, I think, is implement and access table for links similar to node access and entity access. Waiting to hear back from Hass on this issue.

MrHaroldA’s picture

What we really need to do here, I think, is implement and access table for links similar to node access and entity access.

I'm not quite grasping this. Why are we checking access to a link? Shouldn't we check view permission for the page where the link is found instead?

The dependency on the "Access broken links report" permission should be removed too; this should only affect if you're able to visit admin/reports/linkchecker, not if you can see URLs in a view.

On a side note, I can't seem to be able to translate the messages.

hass’s picture

Every link need to be checked. Otherwise field permissions may not verified and users may get access to urls they are not allowed to access.

jantoine’s picture

I have re-rolled the patch in #78 against the latest dev release instead of against the 7.x-1.x. This is due to changes the patch makes to the linkchecker.info file that conflict with modifications Drupal's release process makes to the file.

I've also gotten approval to tackle the remaining @todo items mentioned in #78. It may be a bit before I actually get to them, but now would be a great time to get feedback before I start implementation.

MrHaroldA’s picture

This is also bugged:

-    'access arguments' => array('access broken links report'),
+    'access arguments' => '_linkchecker_access',

I'd guess the access callback should be '_linkchecker_access' and the arguments should be empty.

MrHaroldA’s picture

Here's a little rewrite of the #78 patch that fixes the issue in #90.

MrHaroldA’s picture

FileSize
45.75 KB

Here's another patched patch that fixes these undefined indexes:

+  $comment = module_exists('comment')) {
+    $comment = TRUE;
+  }
+  if (module_exists('block')) {
+    $block = TRUE;
+  }
+  if (module_exists('redirect')) {
+    $redirect = TRUE;
+  }

+  $comment = module_exists('comment');
+  $block = module_exists('block');
+  $redirect = module_exists('redirect');
MrHaroldA’s picture

welly’s picture

What version of the module is this patch created against because I'm finding it is unable to run against the dev and stable versions. Running the simplytest.me build fails to run also, reporting an error in patching the project.

helmo’s picture

It was against this commit:

commit 5918e55bc40964e516da06fc9a5a913750ec7f73
Author: hass <hass@85918.no-reply.drupal.org>
Date:   Sat Jan 17 01:48:27 2015 +0100

    Issue #2303177: "Back to Top" Page anchor being reported as a 404 error

Here's an updated version.

kafmil’s picture

I've made a couple of minor adjustments to @hemlo's latest patch. I was getting errors around the comments edit button. I've also moved the default view to an "Advanced view" and added exposed filters. The patch is run on the 1.2 tag.

Status:Needs review» Needs work

The last submitted patch, 96: linkchecker-views-integration-965720-96.patch, failed testing.

The last submitted patch, 96: linkchecker-views-integration-965720-96.patch, failed testing.

The last submitted patch, 96: linkchecker-views-integration-965720-96.patch, failed testing.

The last submitted patch, 96: linkchecker-views-integration-965720-96.patch, failed testing.

kafmil’s picture

I made a minor update to the default view name. This patch still runs against 1.2 so it will fail automated testing.

kafmil’s picture

Here's the same patch as above applied to commit 10ffeaf0bd8c8ba9102d10187e49f57acec2b4ad

kafmil’s picture

Status:Needs work» Needs review

The last submitted patch, 89: linkchecker-views-integration-965720-89.patch, failed testing.

The last submitted patch, 89: linkchecker-views-integration-965720-89.patch, failed testing.

The last submitted patch, 89: linkchecker-views-integration-965720-89.patch, failed testing.

The last submitted patch, 89: linkchecker-views-integration-965720-89.patch, failed testing.

The last submitted patch, 93: linkchecker-views-integration-965720-93.patch, failed testing.

The last submitted patch, 93: linkchecker-views-integration-965720-93.patch, failed testing.

The last submitted patch, 93: linkchecker-views-integration-965720-93.patch, failed testing.

The last submitted patch, 93: linkchecker-views-integration-965720-93.patch, failed testing.

kafmil’s picture

Removed the redirect column from the report table. This is for the 1.2 tag.

kafmil’s picture

Yuri’s picture

The patch #113 does not work on the latest dev

mgifford’s picture

Status:Needs review» Needs work
james.williams’s picture

Patch #113 was missing hook_views_api() whilst patch #112 actually included other changes in the latest dev snapshot (when compared to 7.x-1.2). So here's an actual re-rolled patch made against latest dev, including the changes from #113 and hook_views_api() (which was in patch #112 and everything else before!).