Termination events

Yea, looks like you are right about the asset dependency.

Usually when I build modules with Asset and Log types I use the dependency - farm:farm_entity
as it is required by all asset and log types anyway.

1 Like

Tagged new 1.1.1 release that fixes this dependency issue.
Settled on using ‘farm_log’ as dependency, since it also depends on farm:asset. Although with it there also was at first similar issue as there is an old version on drupal.org, but it seems that if dependency in .info.yml is declared without project id only with the module id then drupal composer repo does not add it as composer dependency even when a module with the name exists on drupal.org and dependencies are still correctly resolved on module installation through drush or in drupal dashboard.

2 Likes

:hugs: Cool - @wotnak thanks so much for doing this! @mstenta is this kind of how you imagined the implementation?

Question questions / clarifications →

  • every termination log will have automatically assigned ‘Termination’ log category
    • This is nitpicky, but currently I used lower case t ‘termination’ to be consistent with my other log categories. We have lots of data with that, so just trying to stay consistent - is that possible?
  • you said “all referenced assets are archived?” - now that in 2.0 so many things are referenced assets… would that include equipment assets? Also, there may be other types (document assets have been thrown around). In this case, we may need to be specific about the archiving of assets.

Also, just to clarify expected behavior, after a termination log occurs, one could still un-archive an asset - that wouldn’t change anything else (btw this feels like the correct behavior to me). So the termination log just initiates a termination event and saves the date, but doesn’t have any other “sticky” behavior.

1 Like

The best option would be probably to add a machine name field to all taxonomy terms, so they can be unambiguously referenced by it in code, no matter what actual title formatting or language is used on a specific farmOS instance. But this should be probably handled in farmOS core.

For now, I could add configuration option that allows to select taxonomy term (category) that will be used to mark termination logs and move automatic marking of logs with termination category as termination logs from module install to manual action that can be executed from configuration page.

Currently, it archives all assets referenced in the termination log. But I could probably add an option to select which referenced assets will be terminated. Or maybe global configuration option that allows to limit asset types that will be terminated by termination logs.

Termination log on completion currently does two things:

  • archives assets referenced in the log,
  • assigns ‘Termination’ category to itself.

This is just one time action. (Although now that I’m thinking about it this would be also executed on every log change save if it is still marked as termination and completed. So I’ll add a check to execute this only on status change to completed.)

You can later un-archive the asset, and it won’t have any effects on asset termination.

Termination date displayed on the asset page is dynamically calculated by checking if the asset is referenced in any logs marked as termination. If so, it displays the found termination log timestamp and links to the log.

To un-terminate the asset, you would need to do one of the following:

  • mark the termination log as pending,
  • remove reference to asset from the termination log,
  • uncheck ‘Is termination’ field on the termination log,
  • delete the termination log.
2 Likes

Wow! Well done @wotnak! And welcome to the forum! :smile:

I like how consistent this with the way farmOS core handles movements, group membership, etc (with the is_termination boolean + computed field etc). Always a fan of consistency. :+1:

Ah yea we discovered this issue as well, and @Mixologic (renowned drupal.org expert :smile:) shared this workaround: https://www.drupal.org/project/farm_forest/issues/3190919

Tl;dr: don’t include farm: in the dependency. That doesn’t help in this case, as you pointed out, because you want to depend on farm:asset, and the asset project exists on drupal.org. :frowning:

so maybe a good option would be to rename asset module to farm_asset, so it can be declared as dependency by contrib modules.

Unfortunately we most likely won’t be doing this now that farmOS v2 is “stable”. It was farm_asset in v1, but we decided to make it “lower level” for consistency with the log module… with the thought that maybe we could split it out to a separate contrib one day (perhaps taking over the drupal.org/project/asset namespace one day… but that’s certainly not a high priority). Anywho… a bit of a tangent… it might be worth opening an issue in https://www.drupal.org/project/issues/farm_asset_termination to think more about this… happy to add more specific thoughts there! :slight_smile:

Not exactly! But this exists and my ideas don’t! So… :smile:

I was proposing a more general solution, which could be used to represent termination date. This module creates a more explicit concept of “termination”. I’m still thinking through the costs/benefits, but the whole point of having this “contrib” space for farmOS modules is for experimentation like this, so I’m thrilled to see it! Curious to hear what others think too!

(Perhaps the more general approach could be something we aim for in farmOS 3.x as well, and this module can serve until then… see related: Proposal for farmOS 3.x core datamodel improvements: Immutable Quantities & Soft Entity Deletion)

To take this idea a step further: one idea that has come up recently is to add some kind of “ontology reference” field to taxonomy terms, which could be used to link to an external ontology ID of some kind. The idea is the same: it would be more specific than the term’s “label” (which could also be represented in other languages perhaps!) Perhaps we should open a dedicated thread to discuss that?

Making this configurable would be nice IMO. To be honest it feels a bit unnecessary to add a category to the log if it already has is_termination boolean field which can be used for filtering. Seems like it would need a bunch of extra logic to keep those two things in sync if one gets changed but not the other.

I think what @gbathree was asking was whether it ONLY archives assets referenced in the log’s asset field, or if it also looks at the log’s equipment, location, and other asset reference fields. The core convention is that logs are only acting on assets in the asset field, so those are the only ones it should touch IMO.

1 Like

Yeah, I basically landed on the same workaround to remove farm: part of dependency declaration. Alongside asset, I was also declaring dependency on farm_log and since it already depends on asset I just removed explicit dependency on asset and currently using just farm_log without farm: part as a dependency.

It may be worth adding a note about this workaround and need for it on farmOS module development | farmOS for future reference.

I decided to add a category as it doesn’t require adding new filter type to filter termination logs and provides more compatibility with the workflow @gbathree already is using for asset termination.
Is termination field is treated as the source of truth, but the category is also added always when saving log changes and the log is completed and marked as termination, so it should be pretty consistently set when the log is treated as termination.

Ah, yes it already works that way, earlier when talking about referenced assets I meant assets added in the asset field.

2 Likes

It’s hard to cover everyone’s workflows, its the thing about farmOS is often workflow/conventions are left to the individual farms discretion. It feels like the category bit should be just left to the creator of the log anyway, of course it’s your contrib module so it’s entirely up to you what it should or shouldn’t do.

I still think farmOS should include Death and Sale Logs, I think these are important enough events to have their own log type, harvest or activity doesn’t really sound right for the job. I like the work @wotnak above has done and how universally it works for any log to be a termination log which potentially makes sense for lots of logs and asset types. I suppose it might make sense for certain logs like Death/Sale to be termination logs by Default.

Good idea! Care to start a pull request?

It feels like the category bit should be just left to the creator of the log anyway

I tend to agree with this, but I see the arguments for/against. Maybe making that configurable is the best middle ground?

Thinking bigger: I could imagine some kind of generalized “auto categorize” module that let’s you configure logic for what categories to apply when… that could be cool. Outside the scope of this thread tho. :slight_smile:

The farmOS Ledger module adds Sale logs.

@Farmer-Ed want to make a contrib Death log module? (There’s something about that question that feels macabre :skull:)

I suppose it might make sense for certain logs like Death/Sale to be termination logs by Default.

Ah this is interesting too. We have a similar mechanism for making certain asset types is_location by default. But it uses third_party_settings in the config YML which adds a hard dependency on the farm_location module. We wouldn’t be able to add that kind of hard dependency in the farm_ledger module. But maybe we could find another way to do it (eg: via a custom hook instead of config).

2 Likes

Can’t say I want one, but realistically Death is something we all have to deal with sooner or later!
As the old saying goes “Where there are Livestock there are dead stock”

But I have, it was one of my first ventures into Drupal/farmOS contrib dev,
Should probably combine it with some of the other modules I’m running for livestock just so I can give it a less morbid name. :skull_and_crossbones:

1 Like

Tagged new v1.2.0 release of the module that:

  • makes termination log category assignment optional and configurable,
  • adds option to mark existing logs tagged with configured category as termination logs,
  • adds option to configure default termination log types,
  • adds option to mark existing logs of configured logs types as termination logs,
  • adds option to disable asset archiving on termination log completion,
  • and removes automatic marking of logs with ‘Termination’ category as termination on module install.

6 Likes

Bravo @wotnak!! That module is shaping up quick! Very well done!

I’ll eat those words,
I think that covers it nicely now. Thanks @wotnak, I might just implement that now after a little testing of course, but it looks perfect.

I was to revisit my API flows for syncing my farmOS Herd database with the Department of Ag’s as they have grown unnecessarily convoluted due to bits being bolted on over time, at least this should reduce the complexity of the API call/s too.

2 Likes

I would really love to use this module like ASAP :slight_smile: I’m curious also Mike relevance to conventions we’re seeing in the USDA.

I know that from Pasa’s perspective, termination is a key concept that (as much as I tried to avoid) they really need to see and understand.

Is it possible to try this out on coffee-shop.farmos.dev @mstenta for me? Then I think I could give better feedback seeing the actually implementation.

1 Like

Hey all esp. @wotnak so I’m testing it out, specifically testing how it works compared to is_movement. An asset is only moved with is_movement once the date of the is_movement has passed… so if you set a future log, the movement will not occur until you hit that date.

Overall, while maybe a little confusing, it is accurate and reasonable.

I tested this behavior with the is_termination and found that it doesn’t work the same - a termination log placed in the future will terminate (archive) the asset today.

I think we want behavior that exactly mimics the is_movement, both because they are similar in the UI and because the concepts are the same.

What do you think?

1 Like

Released v1.2.1 with the fix for assets being archived when a termination log is marked as done with timestamp set in the future.

1 Like

Although when thinking about it some more I’m not sure if this should be fixed.
Marking log with timestamp set in the future as done seems to me as a bit of invalid action. It makes more sense to mark it as pending and update to done in the future when it is actually done.
One downside of the fix is that when time set in the log timestamp will pass the asset won’t be automatically archived since it currently is done on log save when log status or timestamp is changed and since both values presumably already have final values the automatic archiving won’t be executed.

2 Likes

This highlights the elegance of how the core location/group/inventory “fields” work. i.e. those fields on the asset are computed as a function of the current state of the logs, so they are always - by definition - consistent with the logs.

Doing it that way also allows “scheduled” movements / grouping / inventory adjustments to be created in the future and have them automatically applied when the time arrives without needing a (fragile) background job or manual step. Thus I’d argue allowing that is a feature, not really an “invalid action”.

2 Likes

I would consider a log marked as done with timestamp set in the future as being in invalid state. It is marked as done, but it is not actually done yet. In my opinion, either the timestamp should be adjusted to the moment of marking the log as done (or earlier) or the log should not be yet marked as done.
But if that is a supported workflow in farmOS I’m open to add full support for it in this module. I’m wondering what would be the best way to implement automatic archiving of terminated assets in such cases:

  • as a side effect of computing the termination field value?
    • it is probably the easiest way
    • but adding side effects to a function that should do only one thing (compute field value) is usually not considered a good programming practice
  • in a background job run in cron?
    • it looks better from the perspective of good programming practices
    • but since farmOS documentation recommends scheduling cron jobs once a week in many cases, it would probably take few days from the actual termination event for the asset to be archived
2 Likes

Yeah, if those are the only options you’re considering, I’d tend to agree that it’s best to just disallow creating status=done termination logs in the future.

The better option I was alluding to - and I’m not sure it is reasonable to try and do it in a contrib module - is to actually change the core asset status field to work like location or group does. That is, to make it not store any data directly, but always be computed as a function of the logs (in this case termination logs) that reference the asset.

Maybe there’s a hybrid strategy…

2 Likes

I’d say most important is that we consider a solution that makes sense / works for both seeding and termination, because they are so similar in structure and users will expect they work the same. And the concept driving them both is identical and hinges on the definitions and intentions behind status, and is_....

I do think I’ve been using logs willy nilly in the future, marking everything as done when in reality that’s not appropriate… at a minimum someone should be confirming that the action was completed (or not… or move the date if it was a different date, etc.) even through that’s somewhat of a pain if you’re using farmOS primarily as a data store and not an active management system (as we often are).

What if we had a ‘pending_until_date’ option in status. If you are actively using your farmOS instance for task management, you wouldn’t want to use pending_until_date because it would mark things as done that may not be done which is bad. But if you’re using it for data store, there are times when someone is sure they’re going to do something in the future (like I’m going to till field 6 in mid-December) but they may not come back and look at their farmOS instance until next spring… by marking pending_until_date you’re ensuring the user understands hey… you better actually do this or else your data will be wrong.

If we had that option, then we could have a consistent rule, which is

there are three log status:

  1. done - job is completed, and date is passed. Only a valid status if currentDate < logDate.
  2. pending - job is not completed. Any log date possible.
  3. pending_until_date - status will auto-change to done when log date is passed. Only a valid status if currentDate >= logDate

Another option is to have a separate boolean object in the log which serves this function, rather than having it as an option in the status field. Maybe that’s cleaner, dunno.

The process flow would then be.

  1. Logs cannot be marked as done unless the log date has passed
  2. If pending_until_date, log status auto-changes to done when date is passed
  3. If pending, log status stays pending and requires review and active marking of done.

This would allow me to add future known events with the pending_until_date safely, and ensure that actions (like archive, etc.) only happen when they are supposed to (when the action is done!).

1 Like