Closed roman-eonx closed 2 years ago
@roman-eonx
I think that data
structure should be "free", your example is based on an ActivityLog related to actions taken on DB entities/models, and we can/should of course come up with a consistent structure for those cases, but that should be done at the code level I believe. This way we don't need extra
, any additional information can be added in data
directly.
And we can start creating ActivityLog for other types of actions that don't involve DB entities/models.
My point is that the persistence layer of this package shouldn't be too much opinionated on what to persist π
In the ActivityLog entity attributes I can see you have a polymorphic relationship with "subject", is that optional? Not sure if every would have a subject..
Otherwise I really like the idea:
@natepage thanks for the feedback. I've updated my post:
data
structureextra
propsubjectId
as optional, leaving subjectType
as required (I think we always have some "type" of a subject, even if it is not a DB entity)Hi @roman-eonx , would this also be great if the subjectType
we use is a string representation of an entity instead of the FQN of an entity class, eg. user
instead of App\Entity\User
? so if it happens that one of the project decided to move an entity class we won't messed up with the current record saved in ActivityLog? I think this idea is common for polymorphic relationships and keeping the integrity of the data.
@albertlabarentojr yes, I think we shouldn't use FQCNs here. But maybe a short class name (eg User
), not an abstract string (eg user
). Because in bridges for Eloquent/Doctrine we could have listeners for DB entities that determine a subject type from a DB entity object by simply getting its short class name.
@albertlabarentojr Great point! π
@roman-eonx I think the main thing is to get everybody to agree to a convention and stick to it, because if the process can resolve the short class name then it could also apply lowercase to it π But I agree on a defining a convention to allow to have automated processes! π
@natepage sure thing. But if we decide to transform a class name, it shouldn't be just a lowercase. It should be either kebab-case
or snake_case
. Because otherwise, you will have to "map" those types to the original class names if you need to "resolve" a subject entity from subjectType
+subjectId
:wink:
@roman-eonx yes but I think that regardless of what we decide each app will still need to have mapping for subjects that are not necessarily classes, just thinking out loud here π
@natepage @roman-eonx I think the idea of an Actor here needs to be separated, Actor is also needed for entities that has createdBy, updatedBy (Blameable for some projects). Thus a new package for EasyActor an appropriate solution? Actor can also have it's own context, and it's up to the application how will they resolve this context
@albertlabarentojr I suppose for blameable entities you always have to introduce a DB entity to store the Actor's data. Otherwise, how would you implement this relationship? And this is, probably, a very project-specific thing that would be hard to abstract in a package.
Thank you @roman-eonx , I've created a POC for storing and resolving Actors in different scenarios, we'd like to get a feedback from you, the idea is, Actor can be used in many scenarios like replacing blameable relationships to use actor, one example on our case, OrderHistory requires triggeredBy this is where we implemented the idea of an actor
We do have ActorTypes which are resolved (ActorContext ) within the application level:
system:command - Resolved from ResolveSystemActorCommandEventListener
which listens to ConsoleCommandEvent
system:async_job - Resolved from ResolveAsyncJobActorMiddleware
messenger middleware
user:jwt and user:api_key - Resolved from Easy Security configurators
Here is the POC for that service:
@albertlabarentojr I like your PoC. It definitely can be used as a reference.
But as I told above, it would be really hard to abstract this whole structure in a package:
ActorInterface
could go to the EasyActivity package, for sureActorTypesInterface
could be merged with ActorInterface
, as those types are closely related to an ActorActorContextInterface
, ActorContext
, and NoActorInContextException
are really specific to the project. For example, in our projects, we'd prefer to use our own exception class (right now we are integrating EasyErrorHandler, so have the new exception structure). And probably, we could reuse SecurityContext
(EasySecurity) for storing an Actor instead of an ActorContext
SystemAsyncJobActor
is specific to Symfony (because of Messenger), so we should either have two different Actors for Symfony/Laravel or keep this Actor implementation for projectsUserApiKeyActor
relies on an ApiKey
entity that is specific to the projectUserJwtActor
relies on a User
entity that is specific to the projectSo, we can think about adding ActorInterface
to the package (to define a contract to have consistent Actor structure across all the projects). And leave other classes implementation to the projects themselves.
Hi @roman-eonx , thank you for the feedback, that was really clear and on detail. looking forward to get this started and more discussions with you soon.
I'm glad to announce that we've implemented the EasyActivity package. However, it would be better to consider it as a beta version for now. Once we start using it in our projects, collect feedback, fix possible issues and polish it, I will let everyone know the package is ready to use.
@roman-eonx thank you for this! Amazing work!
@roman-eonx should we close this issue now?
@natepage I was going to do this once we finish the documentation. Will be done very soon.
We have business requirements for "Activity/Action Logs" functionality in different projects. In some projects, this is implemented already. It would be helpful to agree on such logs structure and common ideas and introduce a new package to simplify the development.
Decisions/ideas:
ActivityLog
.ActivityLog
we decided to use anactor
JSON attribute. The proposedactor
structure is the following:type
(mandatory): the way a person is authenticated or the background process type (JWT / ApiKey / System / CronJob / etc.)name
(mandatory): an "acting" person/process name (a full name of the authenticated User, a Provider name, an ApiKey name/description, a background process name/identifier, etc.)id
(optional): a person-representing entity identifier that helps a frontend to render a link to the person profile page (depending on a project, it can be a User id, an Employee id, a Provider id, etc.)data
JSON object with the information required by the business. For the DB entity create/update/delete actions, we may store a list of a subject entity attributes (for thecreate
action), lists of original and changed entity attributes (for theupdate
action), or a list of original entity attributes (for thedelete
action). This way a frontend will be able to render abstract Activity Log descriptions, depending on the business needs. For example:An object was created with the following attribute values: ...
The following attribute changes were applied during the update: ...
The name was changed from "..." to "..."
The proposed
data
structure for a DB subject entity is the following (wherechanged
is an optional property):updatedAt
or some sensitive data).id
createdAt
actor
action
:create
,update
,delete
(more possible options can be added depending on a project needs)subjectType
(a type/class of a "loggable" entity the activity is applied to)subjectId
(an optional identifier of a DB entity the activity is applied to)data