Skip to content

Conversation

@dozsan
Copy link
Contributor

@dozsan dozsan commented Jan 30, 2025

The point of the development is that the entity can be narrowed down in terms of fields. You can include and extend it, which can put fields into it in a meaningful way. This way we can make sure that the password on a user entity is not written to the audit log

@indjeto
Copy link
Collaborator

indjeto commented Feb 3, 2025

Can you reuse the already existing config audited_entities and not adding new entities config?

audited_entities:
    App\Entity\Test:
        mode: include
        fields:
            - name
            - createdAt
            - modifiedAt

Make mode = include default and not required
Make fields = ~ default and not required
This way it will stay backward compatible and no need of adding new config that will make skip the old one.

@dozsan
Copy link
Contributor Author

dozsan commented Feb 3, 2025

I put it in a separate config because my goal was to keep it running smoothly.
If I put it into audited_entities I think it will be misleading because of the mode: exclude because there is already an unaudited_entities.
The other reason is that it was easier to distinguish between the two types of data structures.

Anyway, it will not be backwards compatible because the logic is a single array with numeric key value and the new logic is a multi-dimensional array with fqcn key value. And I don't know how to check in Configuration if the array key is numeric or string.
In fact, as I think about it now, I don't think symfony will accept this solution:

audited_entities:
    - App\Entity\User
    App\Entity\Test:
        mode: include

PHPStorm answer: Invalid child element in a block sequence

All kinds of things should be touched after the update:(

@indjeto
Copy link
Collaborator

indjeto commented Feb 4, 2025

Maybe you're right. This way the confusion will be smaller.
We can keep it separate and mark the old config as deprecated.
I think it will be better if both configs used to raise an exception that you cannot use both, to be clear what the user wants and what will get. You can do this with configs validation.

add deprecation and validate to Configuration.php
phpunit fixes
@dozsan
Copy link
Contributor Author

dozsan commented Feb 4, 2025

We have what you asked for!
The PHPUnit tests have run successfully
I put v1.2 in the deprecated, as I look at the next tag
Plus I added php8.4 to the pipeline

@indjeto indjeto merged commit 75cf8d7 into DATA-DOG:master Feb 5, 2025
4 checks passed
@indjeto
Copy link
Collaborator

indjeto commented Feb 5, 2025

Great job @dozsan ! This project was abandoned for some time. But I think it has potential and it's good it's alive again. For me is the best audit bundle and very useful for investigation.

@dozsan
Copy link
Contributor Author

dozsan commented Feb 5, 2025

@indjeto
I have a question. In the branch and tags selector, is the list of tags the list of tags in git? Because if so, I made a mistake and didn't tag it in git. Unfortunately, I'm not that knowledgeable about github, and I don't know if this has to be manually entered in the release or in git.
Should I add another PR with v1.2 in it or do you add it?
Thanks and sorry!

@indjeto
Copy link
Collaborator

indjeto commented Feb 6, 2025

No worries. I'll fine tune some things and will push release.
I'm also new to github, but will try

@indjeto
Copy link
Collaborator

indjeto commented Feb 10, 2025

@dozsan I made a release, but then deleted.
I tried to use the new setting in an app, but I got confused. Is this "mode" for the entity or for the fields?

How can I add one entity but excluding some of it's fields?
I think this is very valid case.

An option could be to have one more field - "excludedFields"?

@dozsan
Copy link
Contributor Author

dozsan commented Feb 11, 2025

Basically, the mode determines what we want to do.
If mode: include, then the fields field lists the things you want to log on the entity.
If mode: exclude then the fields field tells us what we want to exclude from the entity

data_dog_audit:
    blame_impersonator: true
    entities:
        App\Entity\Account:
            mode: 'exclude'
            fields:
                - password
                - createdAt
                - updatedAt

In this case, we exclude password, createdAt, updateAt from the Account entity. They will not be in the log

@indjeto
Copy link
Collaborator

indjeto commented Feb 11, 2025

Ok, so this mode is for the fields, and if no fields specified - for the whole entity.
I released v1.2.0 and opened a discussion.

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.

2 participants