iTop
iTop copied to clipboard
FAF: Add counter & triggers on file attributes / attachments downloads
Not for iTop 2.7 obsviously, but maybe iTop 2.8? 😁
What it does
- [X] Add a downloads counter in the ormDocument object (file attributes, attachments)
- [X] Add triggers on file attributes and attachments downloads
What needs to be done
- [x] Update PHPDoc to tell iTop version it was introduced
- [ ] Find a way to use the counter in lists, notifications, ...
Limitations
- When file / attachment is retrieved directly from browser's cache, counter is not increased and trigger not triggered.
You may use the new KeyValueStore class to store the count ? It might be easier, and more flexible ?
Since there is one counter per row iTop counter doesn't seems appropriate to me. (For the ref, there is only one counter per root class)
Tracked in Combodo's system under N°2889
I've just removed the milestone as 3.0.0 was delivered some time ago ! @Molkobain is this PR really in the pending Combodo state ?
Yes, waiting for the events service to come out.
Rebased on develop and forced push. Starting rework to use the new event system.
Ready for review!
Add a trigger on file attributes (eg. file on the DocumentFile class) "download"
Without the last word, I am lost, what does it mean exactly, when is it triggered, on file download? If yes, can it be filtered on some AttributeFile(s) like a TriggerOnObjectUpdate?
If I have on a CustomerContract class, defined two AttributeFile with code "contract" and "appendix", my placeholder for the first one would be: $this->contract_downloads_count$ or $this->document_downloads_count$ Which depending on the trigger may be guessable or not.
For attachment, there is no doubt as there aren't any field code.
Discussed with Vincent:
Add a trigger on file attributes (eg. file on the DocumentFile class) "download"
Without the last word, I am lost, what does it mean exactly, when is it triggered, on file download? If yes, can it be filtered on some AttributeFile(s) like a TriggerOnObjectUpdate?
It is trigger on file download, which means when the user clicks the hyperlink. It cannot be filtered yet in the trigger, that one of the pending questions. We decided to add the filtering like on other triggers.
If I have on a CustomerContract class, defined two AttributeFile with code "contract" and "appendix", my placeholder for the first one would be: $this->contract_downloads_count$ or $this->document_downloads_count$ Which depending on the trigger may be guessable or not.
For attachment, there is no doubt as there aren't any field code.
The action is always executed for 1 AttributeFile (when the clicks on the hyperlink), so I'll add placeholders for both attribute code / label so we can display then in the Action.
Accepted during technical review.
Off this PR, a discussion should take place with Eric and Pierre to discuss about where the triggers should be (within the listener?) and how to handle their errors.
Discussed during functional review:
- [x] Erwan and Guillaume will check impact of adding a empty column to a table with a BLOB column.
- [x] Add downloads count in the AttributeBlob rendering dict entry (with "Open / Download")
- [x] Add downloads count column in the portal attachments table
- [x] Change placeholders in the actions so
-
$document_xxx$
become$attachment->xxx
for the attachment trigger -
$document_xxx$
become$file->xxx
for the AttributeBlob trigger
-
- [x] Add downloads count in the AttributeBlob rendering dict entry (with "Open / Download")
I did several mockups, none were really great, this is what I came up with.
Accepted during functional review.
Regarding the interrogation on adding a new column in tables with a lot of blobs: Since MySQL 5.7 and MariaDB 10.3.7 the algorythm changed and there no temporary table created. It was tested on a very big DB (10GB+), the alteration (during the setup) took 62ms.
Nevertheless, a warning will be added in the migration notes to warn about this column addition.
Also that the default parameter ALGORYTHM=INPLACE
in MariaDB can be overload in the MariaDB conf (ALGORYTHM=COPY
)`, in that case the migration will take longer.
Off this PR, a discussion should take place with Eric and Pierre to discuss about where the triggers should be (within the listener?) and how to handle their errors.
We didn't took time to have this conversation :) I have an idea of a TriggerOnEvent, I'll try to make a PR !