search

DamienHarper / auditor

auditor, the missing audit log library
MIT License
164 stars 53 forks source link

Ignore attribute is not recognized on child classes #101

Closed adeanzan closed 2 years ago

adeanzan commented 2 years ago
Q A
auditor version 2.0.1 (auditor bundle 5.0.2)
PHP version 8.1.5
Database MariaDB

Summary

The Ignore attribute is not respected when it's on a private property in a parent class and a field was changed in a child class.

Current behavior

Here is my class hierarchy:

BaseOrder
  |- PizzaOrder

I have an updatedAt property defined as follows:

// Defined on BaseOrder
#[ORM\Column(name: "updatedAt", type: "datetime_immutable", nullable: true)]
#[Audit\Ignore]
private ?\DateTimeImmutable $updatedAt = null;

When making changes to a PizzaOrder the changes to updatedAt appear in the audit log.

How to reproduce

See above, please let me know if you'd like a working code sample.

Expected behavior

Ignore annotation can be defined in parent classes on private variables. Note that it does work as expected for protected variables.

DamienHarper commented 2 years ago

@adeanzan Thanks for the report, I'll have a look at this issue

DamienHarper commented 2 years ago

@adeanzan Would you mind sharing a working example that reproduce the issue please? That would speed up the fix.

adeanzan commented 2 years ago

@DamienHarper - Sure thing, I'll work on getting something together!

adeanzan commented 2 years ago

@DamienHarper - I've created a fork of the demo application that reproduces the problem: https://github.com/adeanzan/auditor-bundle-demo/tree/auditor-issue-101

  1. Check out the above repository
  2. Switch to branch auditor-issue-101
  3. composer install and ./reload.sh
  4. View the Audit logs for "Author"

Expected: The most recent Author entry doesn't have a change for updatedAt since it's supposed to be ignored

Actual: updatedAt field appears in the audit log

If I copy the updatedAt property and accessors into Author.php then the ignore works like I expect it to.

Relevant Files:

Thanks for looking into it!

DamienHarper commented 2 years ago

@adeanzan could you please test PR #107

DamienHarper commented 2 years ago

@adeanzan I finally merged #107 which fixes the issue. Feel free to re-open if needed. Anyway, thanks for the fork, it helped ;)

adeanzan commented 2 years ago

@DamienHarper Thank you for the fix!