Expiration Date #194
Expiration Date #194
Conversation
paperless-ngx-secretary
bot
added
backend
frontend
non-trivial
|
Hello @MacWale, thank you very much for submitting this PR to us! This is what will happen next:
Please allow up to 7 days for an initial review. We're all very excited about new pull requests but we only do this as a hobby. |
|
@MacWale Thanks, can you please update this with some explanation of the code / feature? Rather than us trying to reverse-engineer your PR =) Edit: to me just looks like this adds a field that the user can set after which the document is marked "expired" and then daily they are deleted? |
|
Also you should not be updating any of the lang files directly. Only the main |
Thanks for the note |
Yes, a simply feature to remove old documents |
| operations = [ | ||
| migrations.AddField( | ||
| model_name='document', | ||
| name='expired', |
There was a problem hiding this comment.
expired sounds like a boolean. Kind of "is it expired or not?". I think the field should be called expires or expires_at, because it is nearly always a date in the future.
| def test_remove_expired_documents(self): | ||
| self.assertEqual(tasks.remove_expired_documents(), "0 expired documents deleted.") | ||
|
|
||
| def test_remove_expired_one_from_one(self): | ||
| Document.objects.create(title="test", content="my document", checksum="wow", added=timezone.now(), | ||
| created=timezone.now(), modified=timezone.now(), expired=timezone.now()+timezone.timedelta(days=-1)) | ||
| self.assertEqual(tasks.remove_expired_documents(), "1 expired documents deleted.") | ||
|
|
||
| def test_remove_expired_zero_from_one(self): | ||
| Document.objects.create(title="test", content="my document", checksum="wow", added=timezone.now(), | ||
| created=timezone.now(), modified=timezone.now(), expired=timezone.now()+timezone.timedelta(days=1)) | ||
| self.assertEqual(tasks.remove_expired_documents(), "0 expired documents deleted.") | ||
|
|
||
| def test_remove_expired_one_from_three(self): | ||
| Document.objects.create(title="test", content="my document", checksum="wow1", added=timezone.now(), | ||
| created=timezone.now(), modified=timezone.now(), expired=timezone.now()+timezone.timedelta(days=-1)) | ||
| Document.objects.create(title="test", content="my document", checksum="wow2", added=timezone.now(), | ||
| created=timezone.now(), modified=timezone.now(), expired=timezone.now()+timezone.timedelta(days=1)) | ||
| Document.objects.create(title="test", content="my document", checksum="wow3", added=timezone.now(), | ||
| created=timezone.now(), modified=timezone.now()) | ||
| self.assertEqual(tasks.remove_expired_documents(), "1 expired documents deleted.") | ||
|
|
There was a problem hiding this comment.
This tests to check for an string return. But I think they also should test, that the documents actually are removed.
|
@MacWale Is there a chance that the auto-removal could be disabled? I'd like to review and delete the documents manually myself when they expire, rather than have it run as an unattended job. |
|
I like this idea, and it aligns with what I was thinking of when I submitted #808 - a way to "bubble up" things that have expire dates, or even just "tickle" dates. An example use case would be the ability to add "is expired" stuff to a view (and thus a dashboard) is important, to me, and I think useful to the larger userbase. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Adding my view of why this would be a useful addition to functionality. In my opinion an initial extra field to add expiration date (with a default date that would never expire) would be sufficient. Users could filter and then manually review and delete documents as they please. Automation of this could be a further development. |
|
I agree with @neilcrew, an extra field with an expiration date would be sufficient. If users can filter on this expiry date, like on the created and added date, they can find the documents, review them and decide their fate. The 'automatic' deletion of document should be optional and default off. Personally I do not see any usecase when I would use automatic deletion.
I really believe an expiration date is a very good idea with a lot of potential. But only if the implementation is like the 'added' and 'created' date fields, allowing users to filter on them. |
|
I like the idea of an expiration of documents, but I think there is a far bigger conversation to be had here. As @nickybulthuis points out, you might want a document that has an expiration date around the time of a warranty expiration. Documents might have also passed their legally required retention period. So perhaps in addition to an expired date field (which would work the same as the current added and created date fields), a policy for retention could be implemented. Of course then what to do with a document that falls out of its retention period? Do you auto-delete, send it to a recycle bin for a period of time before perma-deletion, or do nothing? |
|
Would it be possible, even if it's in a future release, to say "expire this document in X days/months/weeks/years"? That's useful for legal documents. I implemented it the quick&dirty way on paperless-ng but would love to see it coming from the official release. |
|
I would think there should be a difference between "expiration" and "retention". It would be a bad idea to confuse the two. "Expiration" describes the content of the document. Could be a warranty expiration, could be a contract expiration, etc. Users likely won't want this to be the date the document gets purged as even a document about an expired contract has value years after the fact. "Retention" would simply be how long the document is kept for. For my jurisdiction, you usually don't need to keep documents more than 7 years, so a retention policy of 7 years could be applied to the document - after which an action could be applied to the document (such as delete, move to recycle bin, archive, apply a tag, etc). |
|
@MacWale this PR has been stale for a bit, it looks like there are some questions about tests, some conflicts and I think this feature would need at least a little documentation (perhaps in the ui and docs). If youre still interested in working on this of course please let us know and we're happy to re-open but given how long its been I'm going to close this for now. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns. |
No description provided.