Set last_access->value to 0 or NULL for new redirects

Updated patch to be compatible to latest dev version.

Thanks! How is the patch working for you? It would be nice to get the maintainer to take a look at this issue.

Patch #22 is 100% identical to #20 (which means there was no need to reroll it) and should be ignored.

NULL would probably be better than zero so that we can use the default views output for empty instead of rewriting the field.

Also, an update script would be good to set last_access values with an access_count of 0 to NULL.

Here is a patch with the function. The maintainer should decide what way this should be implemented in views so that those can be reworked and so that tests can be rewritten.

Here are the basic tests added with NULL value.

The maintainer should decide what way this should be implemented in views so that those can be reworked and so that tests can be rewritten.

Whoops. here you go.

+ // Initial field values should be 0. Redirects should be considered

0 should be changed to NULL

This is better for the reports test: (lines 51-57)

    // The popular report will show all aliases, ordered by the most used one.
    $this->drupalGet('admin/config/search/redirect/popular');
    $this->assertSession()->elementContains('css', 'tbody tr:first-child td:nth-child(2)', 'bar');
    $this->assertSession()->elementContains('css', 'tbody tr:first-child td:nth-child(6)', '5');
    $this->assertSession()->elementContains('css', 'tbody tr:last-child td:nth-child(2)', 'foo');
    $this->assertSession()->elementContains('css', 'tbody tr:last-child td:nth-child(5)', 'Never');
    $this->assertSession()->elementContains('css', 'tbody tr:last-child td:nth-child(6)', '0');

tonytheferg → changed the visibility of the branch 3213927 to hidden.

tonytheferg → changed the visibility of the branch 3213927 to active.

This looks great, I needed to uninstall/re-install the module to get the new view code after patching (expected). And ran into 🐛 The field access_count has already been deleted and it is in the process of being purged. Active while doing the uninstall, but running cron to clean up the fields worked.

Thanks for the review!
Yeah, I should write an update hook.

Bumping back to NR for the update hook.

Should probably have a couple of people test it.

You have to install the module without the patch, then apply the patch, then run drush updb etc.
All redirects that have an access count of 0 will have the last_access timestamp set to NULL.

tonytheferg → changed the visibility of the branch 3213927- to hidden.

Here is a patch for those who prefer patches.

I reviewed the addition, while it doesn't affect me it could help some scenarios. Thanks again @tonytheferg

PR and the implementation approach looks good to me, if possible can we address the phpcs related issues?
I understand that those are not related to the changes we did, however it would be good to address since those came across.
I can do it before merge otherwise.

Thanks everyone for the inputs and feedback.
I've merged this to 2.x and tagged for the releaes 2.0.2-rc1 https://www.drupal.org/project/redirect_metrics/releases/2.0.2-rc1 →

Automatically closed - issue fixed for 2 weeks with no activity.