From 923e40ed8f80c10512c0509ed3c0b7740c78f91b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Sun, 14 May 2023 21:41:00 +0200 Subject: [PATCH] Add the data after the change to a element edited log entry, so you can easily view the changes in log detail pages --- .env | 3 ++ config/parameters.yaml | 6 +++ config/services.yaml | 1 + docs/configuration.md | 3 +- src/DataTables/Column/RevertLogColumn.php | 4 +- src/DataTables/LogDataTable.php | 2 +- .../Contracts/LogWithNewDataInterface.php | 41 +++++++++++++++++ src/Entity/Contracts/TimeTravelInterface.php | 2 +- .../LogSystem/ElementDeletedLogEntry.php | 2 +- .../LogSystem/ElementEditedLogEntry.php | 31 +++++++++++-- .../LogSystem/EventLoggerSubscriber.php | 46 +++++++++++++++---- src/Services/LogSystem/TimeTravel.php | 2 +- .../log_system/details/helper.macro.html.twig | 9 +++- translations/messages.en.xlf | 6 +++ 14 files changed, 136 insertions(+), 22 deletions(-) create mode 100644 src/Entity/Contracts/LogWithNewDataInterface.php diff --git a/.env b/.env index 7db81e46..e969be4a 100644 --- a/.env +++ b/.env @@ -71,6 +71,9 @@ HISTORY_SAVE_CHANGED_FIELDS=1 HISTORY_SAVE_CHANGED_DATA=1 # Save the data of an element that gets removed into log entry. This allows to undelete an element HISTORY_SAVE_REMOVED_DATA=1 +# Save the new data of an element that gets changed or added. This allows an easy comparison of the old and new data on the detail page +# This option only becomes active when HISTORY_SAVE_CHANGED_DATA is set to 1 +HISTORY_SAVE_NEW_DATA=1 ################################################################################### # Error pages settings diff --git a/config/parameters.yaml b/config/parameters.yaml index a7a23db3..ba63a202 100644 --- a/config/parameters.yaml +++ b/config/parameters.yaml @@ -125,3 +125,9 @@ parameters: env(DEFAULT_URI): 'https://partdb.changeme.invalid/' env(SAML_ROLE_MAPPING): '{}' + + env(HISTORY_SAVE_CHANGED_DATA): 1 + env(HISTORY_SAVE_CHANGED_FIELDS): 1 + env(HISTORY_SAVE_REMOVED_DATA): 1 + env(HISTORY_SAVE_NEW_DATA): 1 + diff --git a/config/services.yaml b/config/services.yaml index 360963c3..84b80fdd 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -78,6 +78,7 @@ services: $save_changed_fields: '%env(bool:HISTORY_SAVE_CHANGED_FIELDS)%' $save_changed_data: '%env(bool:HISTORY_SAVE_CHANGED_DATA)%' $save_removed_data: '%env(bool:HISTORY_SAVE_REMOVED_DATA)%' + $save_new_data: '%env(bool:HISTORY_SAVE_NEW_DATA)%' tags: - { name: 'doctrine.event_subscriber' } diff --git a/docs/configuration.md b/docs/configuration.md index 1132f6a7..39a714e7 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -46,8 +46,9 @@ The following configuration options can only be changed by the server administra ### History/Eventlog related settings The following options are used to configure, which (and how much) data is written to the system log: * `HISTORY_SAVE_CHANGED_FIELDS`: When this option is set to true, the name of the fields which are changed, are saved to the DB (so for example it is logged that a user has changed, that the user has changed the name and description of the field, but not the data/content of these changes) -* `HISTORY_SAVE_CHANGED_DATA`: When this option is set to true, the changed data is saved to log (so it is logged, that a user has changed the name of a part and what the name was before). This can increase database size, when you have a lot of changes to enties. +* `HISTORY_SAVE_CHANGED_DATA`: When this option is set to true, the changed data is saved to log (so it is logged, that a user has changed the name of a part and what the name was before). This can increase database size, when you have a lot of changes to entities. * `HISTORY_SAVE_REMOVED_DATA`: When this option is set to true, removed data is saved to log, meaning that you can easily undelete an entity, when it was removed accidentally. +* `HISTORY_SAVE_NEW_DATA`: When this option is set to true, the new data (the data after a change) is saved to element changed log entries. This allows you to easily see the changes between two revisions of an entity. This can increase database size, when you have a lot of changes to entities. If you wanna use want to revert changes or view older revisions of entities, then `HISTORY_SAVE_CHANGED_FIELDS`, `HISTORY_SAVE_CHANGED_DATA` and `HISTORY_SAVE_REMOVED_DATA` all have to be true. diff --git a/src/DataTables/Column/RevertLogColumn.php b/src/DataTables/Column/RevertLogColumn.php index 2b71d8f0..16f3365a 100644 --- a/src/DataTables/Column/RevertLogColumn.php +++ b/src/DataTables/Column/RevertLogColumn.php @@ -73,13 +73,13 @@ class RevertLogColumn extends AbstractColumn { if ( $context instanceof CollectionElementDeleted - || ($context instanceof ElementDeletedLogEntry && $context->hasOldDataInformations()) + || ($context instanceof ElementDeletedLogEntry && $context->hasOldDataInformation()) ) { $icon = 'fa-trash-restore'; $title = $this->translator->trans('log.undo.undelete'); } elseif ( $context instanceof ElementCreatedLogEntry - || ($context instanceof ElementEditedLogEntry && $context->hasOldDataInformations()) + || ($context instanceof ElementEditedLogEntry && $context->hasOldDataInformation()) ) { $icon = 'fa-undo'; $title = $this->translator->trans('log.undo.undo'); diff --git a/src/DataTables/LogDataTable.php b/src/DataTables/LogDataTable.php index b94d3c57..bef88955 100644 --- a/src/DataTables/LogDataTable.php +++ b/src/DataTables/LogDataTable.php @@ -236,7 +236,7 @@ class LogDataTable implements DataTableTypeInterface 'href' => function ($value, AbstractLogEntry $context) { if ( ($context instanceof TimeTravelInterface - && $context->hasOldDataInformations()) + && $context->hasOldDataInformation()) || $context instanceof CollectionElementDeleted ) { try { diff --git a/src/Entity/Contracts/LogWithNewDataInterface.php b/src/Entity/Contracts/LogWithNewDataInterface.php new file mode 100644 index 00000000..0ecad0f2 --- /dev/null +++ b/src/Entity/Contracts/LogWithNewDataInterface.php @@ -0,0 +1,41 @@ +. + */ + +namespace App\Entity\Contracts; + +interface LogWithNewDataInterface +{ + /** + * Checks if this entry has information about the new data. + * @return bool + */ + public function hasNewDataInformation(): bool; + + /** + * Returns the new data for this entry. + */ + public function getNewData(): array; + + /** + * Sets the new data for this entry. + * @return $this + */ + public function setNewData(array $new_data): self; +} \ No newline at end of file diff --git a/src/Entity/Contracts/TimeTravelInterface.php b/src/Entity/Contracts/TimeTravelInterface.php index 22841cc8..c756f53e 100644 --- a/src/Entity/Contracts/TimeTravelInterface.php +++ b/src/Entity/Contracts/TimeTravelInterface.php @@ -31,7 +31,7 @@ interface TimeTravelInterface * * @return bool true if this entry has information about the changed data */ - public function hasOldDataInformations(): bool; + public function hasOldDataInformation(): bool; /** * Returns the data the entity had before this log entry. diff --git a/src/Entity/LogSystem/ElementDeletedLogEntry.php b/src/Entity/LogSystem/ElementDeletedLogEntry.php index a25f9bf7..c20bf87c 100644 --- a/src/Entity/LogSystem/ElementDeletedLogEntry.php +++ b/src/Entity/LogSystem/ElementDeletedLogEntry.php @@ -88,7 +88,7 @@ class ElementDeletedLogEntry extends AbstractLogEntry implements TimeTravelInter return $this; } - public function hasOldDataInformations(): bool + public function hasOldDataInformation(): bool { return !empty($this->extra['o']); } diff --git a/src/Entity/LogSystem/ElementEditedLogEntry.php b/src/Entity/LogSystem/ElementEditedLogEntry.php index ec5b9f80..fb5f2e5c 100644 --- a/src/Entity/LogSystem/ElementEditedLogEntry.php +++ b/src/Entity/LogSystem/ElementEditedLogEntry.php @@ -25,6 +25,7 @@ namespace App\Entity\LogSystem; use App\Entity\Base\AbstractDBElement; use App\Entity\Contracts\LogWithCommentInterface; use App\Entity\Contracts\LogWithEventUndoInterface; +use App\Entity\Contracts\LogWithNewDataInterface; use App\Entity\Contracts\TimeTravelInterface; use Doctrine\ORM\Mapping as ORM; use InvalidArgumentException; @@ -32,7 +33,7 @@ use InvalidArgumentException; /** * @ORM\Entity() */ -class ElementEditedLogEntry extends AbstractLogEntry implements TimeTravelInterface, LogWithCommentInterface, LogWithEventUndoInterface +class ElementEditedLogEntry extends AbstractLogEntry implements TimeTravelInterface, LogWithCommentInterface, LogWithEventUndoInterface, LogWithNewDataInterface { protected string $typeString = 'element_edited'; @@ -49,7 +50,7 @@ class ElementEditedLogEntry extends AbstractLogEntry implements TimeTravelInterf */ public function hasChangedFieldsInfo(): bool { - return isset($this->extra['f']) || $this->hasOldDataInformations(); + return isset($this->extra['f']) || $this->hasOldDataInformation(); } /** @@ -59,7 +60,7 @@ class ElementEditedLogEntry extends AbstractLogEntry implements TimeTravelInterf */ public function getChangedFields(): array { - if ($this->hasOldDataInformations()) { + if ($this->hasOldDataInformation()) { return array_keys($this->getOldData()); } @@ -92,7 +93,29 @@ class ElementEditedLogEntry extends AbstractLogEntry implements TimeTravelInterf return $this; } - public function hasOldDataInformations(): bool + public function hasNewDataInformation(): bool + { + return !empty($this->extra['n']); + } + + public function getNewData(): array + { + return $this->extra['n'] ?? []; + } + + /** + * Sets the old data for this entry. + * + * @return $this + */ + public function setNewData(array $new_data): self + { + $this->extra['n'] = $new_data; + + return $this; + } + + public function hasOldDataInformation(): bool { return !empty($this->extra['d']); } diff --git a/src/EventSubscriber/LogSystem/EventLoggerSubscriber.php b/src/EventSubscriber/LogSystem/EventLoggerSubscriber.php index d9c1173d..eed5953d 100644 --- a/src/EventSubscriber/LogSystem/EventLoggerSubscriber.php +++ b/src/EventSubscriber/LogSystem/EventLoggerSubscriber.php @@ -78,11 +78,12 @@ class EventLoggerSubscriber implements EventSubscriber protected bool $save_changed_fields; protected bool $save_changed_data; protected bool $save_removed_data; + protected bool $save_new_data; protected PropertyAccessorInterface $propertyAccessor; public function __construct(EventLogger $logger, SerializerInterface $serializer, EventCommentHelper $commentHelper, - bool $save_changed_fields, bool $save_changed_data, bool $save_removed_data, PropertyAccessorInterface $propertyAccessor, - EventUndoHelper $eventUndoHelper) + bool $save_changed_fields, bool $save_changed_data, bool $save_removed_data, bool $save_new_data, + PropertyAccessorInterface $propertyAccessor, EventUndoHelper $eventUndoHelper) { $this->logger = $logger; $this->serializer = $serializer; @@ -93,6 +94,8 @@ class EventLoggerSubscriber implements EventSubscriber $this->save_changed_fields = $save_changed_fields; $this->save_changed_data = $save_changed_data; $this->save_removed_data = $save_removed_data; + //This option only makes sense if save_changed_data is true + $this->save_new_data = $save_new_data && $save_changed_data; } public function onFlush(OnFlushEventArgs $eventArgs): void @@ -150,6 +153,7 @@ class EventLoggerSubscriber implements EventSubscriber $log->setTargetElementID($undoEvent->getDeletedElementID()); } } + $this->logger->log($log); } } @@ -301,6 +305,24 @@ class EventLoggerSubscriber implements EventSubscriber }, ARRAY_FILTER_USE_BOTH); } + /** + * Restrict the length of every string in the given array to MAX_STRING_LENGTH, to save memory in the case of very + * long strings (e.g. images in notes) + * @param array $fields + * @return array + */ + protected function fieldLengthRestrict(array $fields): array + { + return array_map( + static function ($value) { + if (is_string($value)) { + return mb_strimwidth($value, 0, self::MAX_STRING_LENGTH, '...'); + } + + return $value; + }, $fields); + } + protected function saveChangeSet(AbstractDBElement $entity, AbstractLogEntry $logEntry, EntityManagerInterface $em, bool $element_deleted = false): void { $uow = $em->getUnitOfWork(); @@ -314,20 +336,24 @@ class EventLoggerSubscriber implements EventSubscriber } else { //Otherwise we have to get it from entity changeset $changeSet = $uow->getEntityChangeSet($entity); $old_data = array_combine(array_keys($changeSet), array_column($changeSet, 0)); + //If save_new_data is enabled, we extract it from the change set + if ($this->save_new_data) { + $new_data = array_combine(array_keys($changeSet), array_column($changeSet, 1)); + } } $old_data = $this->filterFieldRestrictions($entity, $old_data); //Restrict length of string fields, to save memory... - $old_data = array_map( - static function ($value) { - if (is_string($value)) { - return mb_strimwidth($value, 0, self::MAX_STRING_LENGTH, '...'); - } - - return $value; - }, $old_data); + $old_data = $this->fieldLengthRestrict($old_data); $logEntry->setOldData($old_data); + + if (!empty($new_data)) { + $new_data = $this->filterFieldRestrictions($entity, $new_data); + $new_data = $this->fieldLengthRestrict($new_data); + + $logEntry->setNewData($new_data); + } } /** diff --git a/src/Services/LogSystem/TimeTravel.php b/src/Services/LogSystem/TimeTravel.php index 99bda007..daeed7ea 100644 --- a/src/Services/LogSystem/TimeTravel.php +++ b/src/Services/LogSystem/TimeTravel.php @@ -194,7 +194,7 @@ class TimeTravel public function applyEntry(AbstractDBElement $element, TimeTravelInterface $logEntry): void { //Skip if this does not provide any info... - if (!$logEntry->hasOldDataInformations()) { + if (!$logEntry->hasOldDataInformation()) { return; } if (!$element instanceof TimeStampableInterface) { diff --git a/templates/log_system/details/helper.macro.html.twig b/templates/log_system/details/helper.macro.html.twig index d867c6d1..97d776b8 100644 --- a/templates/log_system/details/helper.macro.html.twig +++ b/templates/log_system/details/helper.macro.html.twig @@ -22,12 +22,19 @@ {% endif %} {# For log entries, where we know the old data, this is the last exectuted assignment #} - {% if attribute(entry, 'oldDataInformations') is defined and entry.oldDataInformations %} + {% if attribute(entry, 'oldDataInformation') is defined and entry.oldDataInformation %} {# We have to use the keys of oldData here, as changedFields might not be available #} {% set fields = entry.oldData | keys %} {% set old_data = entry.oldData %} {% endif %} + {# For log entries, where we have new data, we define it #} + {% if attribute(entry, 'newDataInformation') is defined and entry.newDataInformation %} + {# We have to use the keys of oldData here, as changedFields might not be available #} + {% set fields = entry.newData | keys %} + {% set new_data = entry.newData %} + {% endif %} + {% if fields is not empty %} diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index 13364c1e..d27d2eba 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -11349,5 +11349,11 @@ Element 3Invalid Regex expression + + + log.element_changed.data_after + Data after change + +