'Last modified' for adlists should display modification date of adlist instead of database modification

Since v5.0 pihole displays a mouseover tooltip for each adlist in group management containing information about "Last modified" (field date_modified in gravity.db).

From a user's perspective I would expect this date corresponds to the date the content of the adlist has changed. Instead, it shows when the database entry for that adlist changed.
This can easily seen by enabling/disabling an adlist and the "Last modified" will change.

During a gravity run, pihole already reports whether the content of an adlist has changed or not - so the information is already available to pihole .

Bildschirmfoto zu 2020-07-05 11-48-07

I therefor request to change the date_modified in gravity.db only then the content of an adlist has changed and not the database entry.

I know the current behavior of "Last modified" is the same for groups/clients/domains, but in that cases there is no additional "layer of content" besides the database entry and the behavior should not change.

If changing it for adlists only would cause irritation for users an additional field "Adlist content last modified" might be helpful.

This will need an additional field in the gravity database. The current modified timestamps are automatically added by SQL triggers on any database modifications.

The branches new/adlist_date_updated address this. Please test if they are doing what you expect them to do.

pihole checkout core new/adlist_date_updated
pihole checkout web new/adlist_date_updated

Screenshot at 2020-07-13 22-10-53

Thank you for the implementation.

Works as intended but not (yet) as desired. Two remarks

  1. Don't set the default value to now. I have some adlists that were disabled during the update but they still got an timestamp for date_updated. But this information is wrong (in fact, pihole doesn't know when the adlist was updated). This would also fail if I add a new adlist, disable it before running gravity the first time. Adlist content was never updated in this case. I would recommend to set the default to NULL.

  2. If been thinking about the database trigger for date_modified a bit. While it is formally correct to trigger when adlist content was changed and therefor date_updated (and update date_modifiedas well), it does remove some valuable information. With the introduction of date_updated I'm interested to see date_modified only reflecting changes to the database entry other than due to adlist content changes. E.g., last time I changes to comment or enabled the adlist. This information is lost if it is overwritten by each adlist update.

P.S. What was the original intention for date_modified?

I agree

It tells you you have last enabled/disabled it or changed its comment. It is the same for all tables (clients, domains, adlists).

I guessed so - that encourages me to think the behavior of the trigger should be modified to not update when date_updated changed.

Sorry for the delay, I rebased above's commits onto the current development branches and did the following fine-tuning as suggested:

  • the default value of date_updated is NULL which results in N/A on the dashboard tooltip
  • the date_modified trigger only fires on changes of address, enabled and/or comment
1 Like