Merged
Conversation
andrestejerina97
force-pushed
the
feature/audit-formatters-event-site
branch
from
09bcf55 to
2c02e2b
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive audit logging support for Summit and event-related entities while refactoring existing audit formatters to use a shared buildChangeDetails method. The changes reorganize presentation-related formatters into a subdirectory for better code organization.
- Adds 13 new audit log formatters for Summit entities (venues, locations, events, media types, etc.)
- Refactors 6 existing formatters to use the new
buildChangeDetailshelper method from AbstractAuditLogFormatter - Reorganizes presentation-related formatters into a PresentationFormatters subdirectory
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| config/audit_log.php | Registers new audit formatters and updates namespaces for moved presentation formatters |
| app/Audit/AbstractAuditLogFormatter.php | Adds shared methods for formatting change details: buildChangeDetails, formatFieldChange, formatChangeValue, and getIgnoredFields |
| app/Audit/ConcreteFormatters/EntityUpdateAuditLogFormatter.php | Adds return type hint to getIgnoredFields method |
| app/Audit/ConcreteFormatters/SummitAuditLogFormatter.php | New formatter for Summit entity audit logs |
| app/Audit/ConcreteFormatters/SummitEventAuditLogFormatter.php | New formatter for SummitEvent entity audit logs |
| app/Audit/ConcreteFormatters/SummitVenueAuditLogFormatter.php | New formatter for SummitVenue entity audit logs |
| app/Audit/ConcreteFormatters/SummitVenueRoomAuditLogFormatter.php | New formatter for SummitVenueRoom entity audit logs |
| app/Audit/ConcreteFormatters/SummitGeoLocatedLocationAuditLogFormatter.php | New formatter for SummitGeoLocatedLocation entity audit logs |
| app/Audit/ConcreteFormatters/SummitExternalLocationAuditLogFormatter.php | New formatter for SummitExternalLocation entity audit logs |
| app/Audit/ConcreteFormatters/SummitHotelAuditLogFormatter.php | New formatter for SummitHotel entity audit logs |
| app/Audit/ConcreteFormatters/SummitAirportAuditLogFormatter.php | New formatter for SummitAirport entity audit logs |
| app/Audit/ConcreteFormatters/SummitMediaUploadTypeAuditLogFormatter.php | New formatter for SummitMediaUploadType entity audit logs |
| app/Audit/ConcreteFormatters/SummitEventAttendanceMetricAuditLogFormatter.php | New formatter for SummitEventAttendanceMetric entity audit logs |
| app/Audit/ConcreteFormatters/PresentationFormatters/PresentationSpeakerAuditLogFormatter.php | Moved to PresentationFormatters subdirectory (namespace change only) |
| app/Audit/ConcreteFormatters/PresentationFormatters/PresentationUserSubmissionAuditLogFormatter.php | Moved to PresentationFormatters subdirectory and refactored to use buildChangeDetails |
| app/Audit/ConcreteFormatters/PresentationFormatters/PresentationTrackChairRatingTypeAuditLogFormatter.php | Moved to PresentationFormatters subdirectory and refactored to use buildChangeDetails |
| app/Audit/ConcreteFormatters/PresentationFormatters/PresentationTrackChairScoreTypeAuditLogFormatter.php | Moved to PresentationFormatters subdirectory and refactored to use buildChangeDetails |
| app/Audit/ConcreteFormatters/PresentationFormatters/PresentationSpeakerSummitAssistanceConfirmationAuditLogFormatter.php | New formatter replacing SpeakerAssistanceAuditLogFormatter with simplified implementation |
| app/Audit/ConcreteFormatters/PresentationFormatters/PresentationVideoAuditLogFormatter.php | New formatter for PresentationVideo entity audit logs |
| app/Audit/ConcreteFormatters/PresentationFormatters/PresentationSlideAuditLogFormatter.php | New formatter for PresentationSlide entity audit logs |
| app/Audit/ConcreteFormatters/PresentationFormatters/PresentationLinkAuditLogFormatter.php | New formatter for PresentationLink entity audit logs |
| app/Audit/ConcreteFormatters/PresentationFormatters/PresentationMediaUploadAuditLogFormatter.php | New formatter for PresentationMediaUpload entity audit logs |
| app/Audit/ConcreteFormatters/PresentationFormatters/PresentationActionTypeAuditLogFormatter.php | New formatter for PresentationActionType entity audit logs |
| app/Audit/ConcreteFormatters/SelectionPlanAuditLogFormatter.php | Refactored to use buildChangeDetails and removed custom formatDate method |
| app/Audit/ConcreteFormatters/SubmissionInvitationAuditLogFormatter.php | Refactored to use buildChangeDetails for cleaner update logging |
| app/Audit/ConcreteFormatters/SpeakerRegistrationRequestAuditLogFormatter.php | Refactored to use buildChangeDetails for cleaner update logging |
| app/Audit/ConcreteFormatters/SummitTrackChairAuditLogFormatter.php | Refactored to use buildChangeDetails for cleaner update logging |
| app/Audit/ConcreteFormatters/FeaturedSpeakerAuditLogFormatter.php | Refactored to use buildChangeDetails for cleaner update logging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+105
to
+106
| $fields_summary = count($changed_fields) . ' field(s) modified: '; | ||
| return $fields_summary . implode(' | ', $changed_fields); |
There was a problem hiding this comment.
The variable name 'fields_summary' is misleading because it contains both a count and a prefix string, not just a summary. Consider renaming to 'fields_prefix' or directly inline the concatenation to improve clarity.
Suggested change
| $fields_summary = count($changed_fields) . ' field(s) modified: '; | |
| return $fields_summary . implode(' | ', $changed_fields); | |
| return sprintf('%d field(s) modified: %s', count($changed_fields), implode(' | ', $changed_fields)); |
|
|
||
| class SummitEventAuditLogFormatter extends AbstractAuditLogFormatter | ||
| { | ||
| private string $event_type; |
There was a problem hiding this comment.
The variable name 'event_type' is confusing in this context because this formatter deals with Summit entities, and 'event_type' could be mistaken for a summit event type. Consider renaming to 'audit_event_type' or 'operation_type' to clarify this refers to the audit operation (creation/update/deletion).
| $summit = $subject->getSummit(); | ||
| $summit_name = $summit ? ($summit->getName() ?? 'Unknown Summit') : 'Unknown Summit'; | ||
|
|
||
|
|
There was a problem hiding this comment.
There is an unnecessary blank line here that should be removed to maintain consistency with the formatting of other audit log formatters in this PR.
Suggested change
smarcet
reviewed
Dec 22, 2025
| "[%s - %s]", | ||
| $this->formatDate($subject->getSubmissionBeginDate()), | ||
| $this->formatDate($subject->getSubmissionEndDate()) | ||
| $subject->getSubmissionBeginDate()?->format('Y-m-d H:i:s') ?? 'N/A', |
Collaborator
There was a problem hiding this comment.
please move this to an util and use it everywhere ( how to format the dates ) so we are consistent
smarcet
reviewed
Dec 22, 2025
|
|
||
| class SummitEventAttendanceMetricAuditLogFormatter extends AbstractAuditLogFormatter | ||
| { | ||
| private string $event_type; |
Collaborator
There was a problem hiding this comment.
this private string $event_type;
public function __construct(string $event_type)
{
$this->event_type = $event_type;
}
is repetead all over the code
lets move it to AbstractAuditLogFormatter
smarcet
approved these changes
Dec 22, 2025
Collaborator
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@andrestejerina97 please review comments
smarcet
requested changes
Dec 22, 2025
Collaborator
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@andrestejerina97 please review comments
andrestejerina97
force-pushed
the
feature/audit-formatters-event-site
branch
from
9831428 to
cbba14f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref: https://app.clickup.com/t/86b7thffg