Skip to content

feat: add missing formatters in config and UT#508

Open
andrestejerina97 wants to merge 1 commit intomainfrom
feature/add-missing-ut-formatters
Open

feat: add missing formatters in config and UT#508
andrestejerina97 wants to merge 1 commit intomainfrom
feature/add-missing-ut-formatters

Conversation

@andrestejerina97
Copy link
Copy Markdown
Contributor

@andrestejerina97 andrestejerina97 commented Feb 24, 2026

@andrestejerina97 andrestejerina97 marked this pull request as ready for review February 24, 2026 23:22
@andrestejerina97 andrestejerina97 force-pushed the feature/add-missing-ut-formatters branch from 953ae62 to 793b827 Compare February 25, 2026 19:31
@andrestejerina97
Copy link
Copy Markdown
Contributor Author

@martinquiroga-exo ready to review

@andrestejerina97
Copy link
Copy Markdown
Contributor Author

@martinquiroga-exo

Copy link
Copy Markdown
Contributor

@martinquiroga-exo martinquiroga-exo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrestejerina97 please see comment

$formatter = new PresentationTypeAuditLogFormatter(IAuditStrategy::EVENT_ENTITY_UPDATE);
$formatter->setContext(AuditContextBuilder::default()->build());
$changeSet = [
'name' => ['Keynote', 'Opening Keynote']
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be type not name

Copy link
Copy Markdown
Contributor

@caseylocker caseylocker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blockers

  1. Inheritance gap — RSVPQuestionTemplateAuditLogFormatter is dead code in production. Traced the path: AuditEventListener.php:45-58 hands Doctrine's concrete entity to the strategy, AuditLogOtlpStrategy.php:59 passes it through unchanged, AuditLogFormatterFactory.php:138 does exact get_class($subject) lookup. No parent walk anywhere in app/Audit — grep for getRootEntityName|class_parents|is_subclass_of|getParentClass is empty. And SummitRSVPTemplateQuestionFactory.php:38-80 only ever instantiates concrete subtypes (9 cases, no default) — there's no production path that creates a bare RSVPQuestionTemplate. So every real audit event arrives as RSVPDropDownQuestionTemplate, RSVPCheckBoxListQuestionTemplate, etc.; the parent-class config key at audit_log.php:283-286 never matches. Same shape for PrivatePresentationCategoryGroup per PresentationCategoryGroup.php:31. Fix: register each concrete subclass individually — that's the precedent already in this file (SummitVenue/SummitVenueRoom/SummitExternalLocation/SummitHotel/SummitAirport at lines 107,111,135,139,143,147) — or have getFormatterByContext fall back to class_parents() on a miss.

  2. Martin's 'name''type' comment is still sitting there 6 weeks later. tests/OpenTelemetry/Formatters/PresentationTypeAuditLogFormatterTest.php:73 uses change-set key 'name', but the Doctrine property is $type (SummitEventType.php:37-38, #[ORM\Column(name: 'Type')]). Passes only because buildChangeDetails doesn't validate keys. One line.

  3. 52 commits behind main, GitHub flags CONFLICTING. The conflict surface is just the new block at config/audit_log.php:259-289. Rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants