Problem/Motivation

Logs currently dispatch events when entity operations are performed, but assets do not.

Proposed resolution

Let's add an AssetEvent that is dispatched on presave, insert, update, and delete.

Remaining tasks

Implement.

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork farm-3306227

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

m.stenta created an issue. See original summary.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Related issues: +#2551893: Add events for matching entity hooks

There is an upstream Drupal core issue to implement this for all entity types: #2551893: Add events for matching entity hooks

So this would be a stop-gap until that lands in core. Notably the Log module has @todos to use the core events too: https://git.drupalcode.org/project/log/-/blob/e0751d66501c68fcc3b0e78598...

m.stenta opened merge request !67

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Active » Needs review
m.stenta’s picture

m.stenta
he/him
Primary language English
commented

The reason I did this was to support https://github.com/farmOS/farmOS/pull/537, specifically:

Lastly, I want to raise one last idea/question: should we auto-assign asset owners in the same way we auto-assign log owners (when a log is saved, if no owner is assigned, it assigns it to the current user)? I actually went ahead and implemented this (which required also opening this MR: https://www.drupal.org/project/farm/issues/3306227) - but then in testing it I realized this means it's going to add "Owner" to all existing assets - and maybe we don't really want that. Maybe we should leave it up to the user to decide how ownership is assigned for assets. Although it feels weird for logs and assets to behave differently in this aspect, doesn't it? I kept this change out of the branch for now, but I'd like to hear what others think. We could also punt on this decision and open a followup issue for consideration (I can add my commit there).

If we decide NOT to do that, then maybe we don't actually need to merge this in now. If nothing is going to make use of this event yet, then maybe it would be better to punt on it, in hopes that the core issue upstream lands.

farmer-ed’s picture

farmer-ed commented

I like this one, events for assets could be useful, I'm trying to automate as much as possible synchronization of livestock assets between the the Department of Ag database and farmOS, could be useful for certain logs to be generated when a new Asset is created. Like a birth log if a new calf is added with a mother already in my herd, I can already achieve this externally but the simpler the better.

paul121’s picture

paul121 commented
Status: Needs review » Reviewed & tested by the community

  • m.stenta committed 7d6cb34 on 2.x 
    Issue #3306227: Dispatch events for asset presave, insert, update,...

  • m.stenta committed 8456962 on 2.x 
    Add CHANGELOG.md line for Issue #3306227.
m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks for the input @farmer-ed - I agree it's nice to have this. Merged!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.