search

mihaifm / HIBPOfflineCheck

Keepass plugin that performs offline and online checks against HaveIBeenPwned passwords
Other
317 stars 17 forks source link

[feature request] Introduce a new setting to be able to control whether the Last Modification Time is updated or not #53

Closed cristianst85 closed 1 year ago

cristianst85 commented 1 year ago

Hello.

I would like to have a new setting to be able to control whether the Last Modification Time is updated or not when an entry is Checked/Cleared/Excluded.

It can be argued that the information this plugin provides (and also stores for convienience) for an entry in the Have I been pwned? column it's not an intrinsic part of the entry, but it's derived based on some external 3rd party information. To put this in simpler words, for example - and I think this is a very common scenario - when checking all of the entries (or many of them at once) in the database, all have their Last Modification Time field updated to about same moment, which makes it harder to determine when the actual entry's information (title, username, password, etc.) was modified based on the Last Modification Time.

The argument I am trying to make is similar to the one made here.

Unfortunately, I am looking at the code, and I don't see any nice way to implement this but to modify the TouchEntry method to something like this (the setting control code was omitted for verbosity).

var lastModificationTime = pe.LastModificationTime; pe.Touch(true); pe.LastModificationTime = lastModificationTime;

I am happy to submit a PR for this, but let's discuss it first.

mihaifm commented 1 year ago

This sounds good to me. If you can give it a quick test too, that would be great.

cristianst85 commented 1 year ago

A few clarifications, do you think we need granularity for each operation (Check/Clear/Exclude) or one setting to drive all of them? I would prefer to keep it simple.

Secondly, what should be the default for this new setting, should it be Enabled by default not to introduce a breaking change in behavior?

Thank you!

mihaifm commented 1 year ago

I don't think we need a setting, sounds like a reasonable behavior to have by default, we shouldn't be altering Last Modification Time

cristianst85 commented 1 year ago

Oh. While I agree with you that this should have been the default behavior from the start, others might not (if so, they can voice their opinion), also because it's introducing a breaking change, etc. But we can keep it simple for now.