Add dkan linkchecker feature #2339

janette · 2018-01-19T23:52:48Z

User Story

As a user in charge of data integrity, I want to be able to see where links are failing in my data catalog.

See broken URLs report displayed in a human-readable format
Find/exclude intentionally non-public results (public access level)
Find/exclude different error codes/types (i.e. redirects vs. file not found)
See, along with broken URLs, the data resource URL so that they can easily find the resource
See, along with broken URLs, the dataset, so that they can group multiple-bad-URLs that are from a single dataset
See contact info for the dataset so that they can reach out to the data source
Know if a bad link is from a JSON harvest, so that they can change it there.

QA steps

Reminders

There is test for the issue.
CHANGELOG updated.
Coding standards checked.
Review docs.getdkan.com (or in /docs) to see if it still covers the scope of the PR and update if needed.

Connects https://github.com/NuCivic/client-usva-data/issues/69

erogray · 2018-01-23T17:43:07Z

connects https://github.com/NuCivic/client-usva-data/issues/69

janette · 2018-01-24T05:52:50Z

views_join
https://api.drupal.org/comment/54213#comment-54213
https://stackoverflow.com/questions/1340423/or-operator-in-drupal-view-filters
https://www.drupal.org/forum/support/post-installation/2012-06-28/how-to-add-a-field-on-field-condition-to-on-clause-in

janette · 2018-03-13T17:15:45Z

Fails linting

Generating autoload files
Config value "installed_paths" added successfully
Diff URL: https://github.com/GetDKAN/dkan/pull/2339.diff
Linting: modules/dkan/dkan_dataset/modules/dkan_dataset_content_types/dkan_dataset_content_types.info
modules/dkan/dkan_dataset/modules/dkan_dataset_groups/dkan_dataset_groups.info
modules/dkan/dkan_dataset/modules/dkan_dataset_rest_api/dkan_dataset_rest_api.info
modules/dkan/dkan_dataset/modules/dkan_dataset_voting/dkan_dataset_voting.info
modules/dkan/dkan_datastore/modules/dkan_datastore_api/dkan_datastore_api.info
modules/dkan/dkan_datastore/modules/dkan_datastore_fast_import/dkan_datastore_fast_import.info
modules/dkan/dkan_linkchecker/dkan_linkchecker.features.inc
modules/dkan/dkan_linkchecker/dkan_linkchecker.features.user_permission.inc
modules/dkan/dkan_linkchecker/dkan_linkchecker.info
modules/dkan/dkan_linkchecker/dkan_linkchecker.install
modules/dkan/dkan_linkchecker/dkan_linkchecker.module
modules/dkan/dkan_linkchecker/dkan_linkchecker.strongarm.inc
modules/dkan/dkan_linkchecker/dkan_linkchecker.views_default.inc
modules/dkan/dkan_sitewide/modules/dkan_sitewide_panelizer/dkan_sitewide_panelizer.info
modules/dkan/dkan_topics/modules/dkan_default_topics/dkan_default_topics.info
+linkchecker.module
test/dkanextension/src/Drupal/DKANExtension/Context/LinkcheckerContext.php
themes/nuboot_radix/nuboot_radix.info
ERROR: The file "+linkchecker.module" does not exist.

fmizzell · 2018-04-04T19:14:12Z

modules/dkan/dkan_linkchecker/dkan_linkchecker.module

+ * Implements hook_preprocess_page().
+ */
+function dkan_linkchecker_preprocess_page(&$vars) {
+  if (arg(2) == 'dkan-linkchecker-report' || arg(4) == 'dkan_linkchecker_reports') {


Why the check on arg(4)?

The size of the table is so large that I'm reducing font size so it does not require an annoying amount of scrolling. The arg(4) is to include this small font size on the views UI page for the same reason.

Ok, lets add a comment to that effect. I think the arg(2) is self explanatory.

fmizzell · 2018-04-04T19:18:14Z

modules/dkan/dkan_linkchecker/dkan_linkchecker.module

+ */
+function dkan_linkchecker_menu_alter(&$items) {
+  // Remove normal linkchecker report link.
+  $items['admin/reports/linkchecker']['access callback'] = FALSE;


I am assuming that this report is the default view you disabled previously. In that case, is this necessary?

If you don't disable the original view there will be multiple report pages and confusion in my opinion.

What does the code in dkan_linkchecker_enable do?

That is removing the link to the disabled view from the admin menu.

fmizzell · 2018-04-04T19:29:45Z

test/dkanextension/src/Drupal/DKANExtension/Context/LinkcheckerContext.php

+class LinkcheckerContext extends RawDKANContext {
+
+  protected $old_global_user;
+  public static $modules_before_feature = array();


Is there a particular reason for this variables to be public? It looks like we (this class) are managing them through the whole cycle of the use of this context.

I based this code on what is being used in the workbench context, I have no reasoning beyond that.

Lets make them private then. public properties are generally not good as we can easily lose control of their state.

fmizzell · 2018-04-04T19:35:34Z

test/features/linkchecker.feature

+      | name    | mail                | roles                |
+      | John    | john@example.com    | site manager         |
+
+    Given groups:


We are not using the groups or group memberships on the test, I think? Why are we creating them?

This is so the test passes on client sites that require group assignment on their datasets

Boooo... Lets add a comment then. Without that knowledge I would have deleted that during clean up.

ok after looking again I remember I removed the test that created a dataset with a bad link because too much needs to happen to get that bad link into the report. So since we are not creating a dataset I've removed the group background setup.

fmizzell · 2018-04-04T19:37:28Z

test/dkanextension/src/Drupal/DKANExtension/Context/LinkcheckerContext.php

+ */
+class LinkcheckerContext extends RawDKANContext {
+
+  protected $old_global_user;


At some point we should discuss OOP style standards as a group, but I personally like proper camel case both for methods and properties of a class.

I looked at the PSR standards and they do not say anything about how to name properties, so I will back down for now :)

fmizzell · 2018-04-04T20:24:18Z

modules/dkan/dkan_linkchecker/dkan_linkchecker.module

+ * Implements hook_preprocess_page().
+ */
+function dkan_linkchecker_preprocess_page(&$vars) {
+  if (arg(2) == 'dkan-linkchecker-report' || arg(4) == 'dkan_linkchecker_reports') {


Ok, lets add a comment to that effect. I think the arg(2) is self explanatory.

fmizzell · 2018-04-04T20:25:58Z

modules/dkan/dkan_linkchecker/dkan_linkchecker.module

+ */
+function dkan_linkchecker_menu_alter(&$items) {
+  // Remove normal linkchecker report link.
+  $items['admin/reports/linkchecker']['access callback'] = FALSE;


What does the code in dkan_linkchecker_enable do?

fmizzell · 2018-04-04T20:37:41Z

test/dkanextension/src/Drupal/DKANExtension/Context/LinkcheckerContext.php

+ */
+class LinkcheckerContext extends RawDKANContext {
+
+  protected $old_global_user;


I looked at the PSR standards and they do not say anything about how to name properties, so I will back down for now :)

fmizzell · 2018-04-04T20:38:43Z

test/dkanextension/src/Drupal/DKANExtension/Context/LinkcheckerContext.php

+class LinkcheckerContext extends RawDKANContext {
+
+  protected $old_global_user;
+  public static $modules_before_feature = array();


Lets make them private then. public properties are generally not good as we can easily lose control of their state.

fmizzell · 2018-04-04T20:39:54Z

test/features/linkchecker.feature

+      | name    | mail                | roles                |
+      | John    | john@example.com    | site manager         |
+
+    Given groups:


Boooo... Lets add a comment then. Without that knowledge I would have deleted that during clean up.

kimwdavidson · 2018-04-23T20:57:46Z

@janette We QA'd this, but got stuck on the last step!

After running cron, there's no Broken Links Report under 'Reports' for admins and no Broken Links Report in the admin menu for site managers.

janette · 2018-04-23T21:11:40Z

@kimwdavidson @dafeder did you run cron?
I cleared cache

logged in a admin:

logged in as sitemanager:

kimwdavidson · 2018-04-23T21:34:25Z

@janetteday, yup, ran cron

…

On Mon, Apr 23, 2018 at 5:11 PM, Janette Day ***@***.***> wrote: @kimwdavidson <https://github.com/kimwdavidson> @dafeder <https://github.com/dafeder> did you run cron? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2339 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AVe-LDBZX9PD_leapIoyNB0TuIf0n13bks5trkOOgaJpZM4RlM2f> .

-- Kim Davidson Project Manager | CivicActions <http://civicactions.com> e: kim.davidson@gmail.com Skype: kwdavidson

janette · 2018-04-23T21:35:43Z

@kimwdavidson ok I guess the trick was clearing cache

fmizzell · 2018-04-24T04:53:46Z

@janette yes we did

kimwdavidson · 2018-04-27T14:11:56Z

@janette: QA'd and this all looks great, except when I'm logged in as a site manager, I can find the broken links report if I go into Site Configuration -> Link Checker Settings, but is it supposed to just be an item at the top of the menu?

janette · 2018-04-27T14:19:22Z

@kimwdavidson do you want to screen share? the link is under Site Configuration if you are a site manager

kimwdavidson · 2018-04-27T18:35:25Z

Checked that last thing, and it's working as expected, so this is all ready!

* Adds a linkchecker to DKAN.

janette added In Progress labels Jan 19, 2018

janette force-pushed the dkan-linkchecker branch from a8bc3ca to 8cc5235 Compare January 20, 2018 05:42

janette force-pushed the dkan-linkchecker branch from e75a05e to 88485ce Compare January 25, 2018 00:19

kimwdavidson assigned janette Jan 30, 2018

kimwdavidson added ready and removed In Progress labels Jan 30, 2018

janette force-pushed the dkan-linkchecker branch 2 times, most recently from e8dda20 to 9b2ff14 Compare February 6, 2018 02:23

kimwdavidson added In Progress and removed ready labels Feb 6, 2018

kimwdavidson mentioned this pull request Feb 6, 2018

DKAN link checker #2354

Closed

janette force-pushed the dkan-linkchecker branch 5 times, most recently from 32cbb50 to 6da7289 Compare February 13, 2018 19:51

janette added circleci/chrome issue Blocked and removed In Progress circleci/chrome issue labels Mar 23, 2018

fmizzell self-requested a review April 4, 2018 19:04

fmizzell suggested changes Apr 4, 2018

View reviewed changes

erogray assigned fmizzell and unassigned janette Apr 10, 2018

fmizzell force-pushed the develop branch from cd7f524 to 6705575 Compare April 13, 2018 22:46

fmizzell force-pushed the dkan-linkchecker branch from 3a7c35e to 5d83598 Compare April 18, 2018 16:04

fmizzell added 3 commits April 18, 2018 11:27

Adds the dkan linkchecker module.

b735893

Includes some test clean up.

3c3cf5f

Improves the linter.

8598262

fmizzell force-pushed the dkan-linkchecker branch from 5d83598 to 8598262 Compare April 18, 2018 16:36

fmizzell approved these changes Apr 18, 2018

View reviewed changes

Update linkchecker patch. Improve menu links.

69cea70

kimwdavidson added In Progress and removed Blocked labels Apr 23, 2018

kimwdavidson assigned janette and unassigned fmizzell Apr 23, 2018

kimwdavidson added Internal QA and removed In Progress labels Apr 25, 2018

kimwdavidson assigned kimwdavidson and unassigned janette Apr 25, 2018

fmizzell merged commit 6ee39a2 into develop Apr 28, 2018

fmizzell removed the Internal QA label Apr 28, 2018

fmizzell deleted the dkan-linkchecker branch April 28, 2018 23:41

fmizzell pushed a commit that referenced this pull request Apr 28, 2018

Add dkan linkchecker feature (#2339)

3150933

* Adds a linkchecker to DKAN.

fmizzell pushed a commit that referenced this pull request Apr 29, 2018

Add dkan linkchecker feature (#2339)

e289395

* Adds a linkchecker to DKAN.

dafeder pushed a commit that referenced this pull request Apr 24, 2020

Add dkan linkchecker feature (#2339)

a7f503c

* Adds a linkchecker to DKAN.

dafeder pushed a commit that referenced this pull request Apr 24, 2020

Add dkan linkchecker feature (#2339)

d574b42

* Adds a linkchecker to DKAN.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dkan linkchecker feature #2339

Add dkan linkchecker feature #2339

janette commented Jan 19, 2018 •

edited

Loading

erogray commented Jan 23, 2018

janette commented Jan 24, 2018

janette commented Mar 13, 2018

fmizzell Apr 4, 2018

janette Apr 4, 2018

fmizzell Apr 4, 2018

fmizzell Apr 4, 2018

janette Apr 4, 2018

fmizzell Apr 4, 2018

janette Apr 5, 2018

fmizzell Apr 4, 2018

janette Apr 4, 2018

fmizzell Apr 4, 2018

fmizzell Apr 4, 2018

janette Apr 4, 2018

fmizzell Apr 4, 2018

janette Apr 5, 2018

fmizzell Apr 4, 2018

janette Apr 4, 2018

fmizzell Apr 4, 2018

fmizzell Apr 4, 2018

fmizzell Apr 4, 2018

fmizzell Apr 4, 2018

fmizzell Apr 4, 2018

fmizzell Apr 4, 2018

kimwdavidson commented Apr 23, 2018

janette commented Apr 23, 2018 •

edited

Loading

kimwdavidson commented Apr 23, 2018 via email

janette commented Apr 23, 2018

fmizzell commented Apr 24, 2018

kimwdavidson commented Apr 27, 2018

janette commented Apr 27, 2018

kimwdavidson commented Apr 27, 2018

Add dkan linkchecker feature #2339

Add dkan linkchecker feature #2339

Conversation

janette commented Jan 19, 2018 • edited Loading

User Story

QA steps

Reminders

erogray commented Jan 23, 2018

janette commented Jan 24, 2018

janette commented Mar 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kimwdavidson commented Apr 23, 2018

janette commented Apr 23, 2018 • edited Loading

kimwdavidson commented Apr 23, 2018 via email

janette commented Apr 23, 2018

fmizzell commented Apr 24, 2018

kimwdavidson commented Apr 27, 2018

janette commented Apr 27, 2018

kimwdavidson commented Apr 27, 2018

janette commented Jan 19, 2018 •

edited

Loading

janette commented Apr 23, 2018 •

edited

Loading