"might_have" fields not fleshed out in action trigger event definition template output

Bug #850160 reported by Jeff Davis
32
This bug affects 9 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Undecided
Unassigned

Bug Description

EXECUTIVE SUMMARY: Apparently linked fields with a reltype of "might_have" in the fieldmapper do not get fleshed out in action trigger event definition template output.

DETAILS:
I'm using Evergreen 2.0.8. I've got an action trigger event definition that uses a hook with bresv (booking::reservation) as its core type. It generates output for the SendEmail reactor using this template:

Hello, [%- target.usr.first_given_name -%]!
The following item is ready for pickup: [%- target.target_resource_type.name -%]
Requested at: [%- target.request_lib.name -%]
Pickup at: [%- target.pickup_lib.name -%]

My event environment includes usr, target_resource_type, request_lib, and pickup_lib. When the event runs, the output looks like this:

Hello, Jeff!
The following item is ready for pickup: Test Item
Requested at: Anytown Public Library
Pickup at:

That last line ought to say "Pickup at: Anytown Public Library." In other words, pickup_lib is not being fleshed out, yet the other elements of my environment are. (And yes, there is a valid actor.org_unit.id in the booking.reservation.request_lib record.)

The only difference I can see between pickup_lib and the others is the reltype as defined in the fieldmapper:

<class id="bresv" controller="open-ils.cstore open-ils.pcrud" oils_obj:fieldmapper="booking::reservation" oils_persist:tablename="booking.reservation" reporter:label="Reservation">
  <links>
    <link field="usr" reltype="has_a" key="id" map="" class="au"/>
    <link field="target_resource_type" reltype="has_a" key="id" map="" class="brt"/>
    <link field="request_lib" reltype="has_a" key="id" map="" class="aou"/>
    <link field="pickup_lib" reltype="might_have" key="id" map="" class="aou"/>
  </links>
</class>

Here, pickup_lib's reltype is "might_have" where the others are "has_a". I tried adding some other "might_have" fields to my environment (e.g. capture_staff), and they didn't get fleshed out either, whereas "has_a" fields always get fleshed out. So I'm fairly confident that the reltype is somehow the crux of the problem.

I don't fully understand the fleshing-out process, so I don't have a bugfix for this yet.

Revision history for this message
Jeff Davis (jdavis-sitka) wrote :

"(And yes, there is a valid actor.org_unit.id in the booking.reservation.request_lib record.)"

There's also a valid ID in booking.reservation.pickup_lib, which is what I meant to say. :)

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Needs confirmation in 2.1+.

Changed in evergreen:
status: New → Incomplete
tags: added: actiontrigger
Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I've seen something like this happen on a 2.10.6 system, I'll try to test on 3.2 when I have a chance.

But the strange thing is that it isn't consistent.

For our lost/bills event definition, when I try to pull in the current account balance, with the environment field path = usr.money_summary, 30% of the time user.money_summary.balance_owed works, and 70% of the time user.money_summary works.

So I resorted to trying both of them. I really don't get how the user.money_summary.balance_owed gets moved to user.money_summary? How does it know to grab the one field I need?

[%- atotal = BLOCK -%]
[%- IF user.money_summary.balance_owed > 0 -%]
$[% user.money_summary.balance_owed | format('%.2f') %]
[%- ELSE -%]
$[% user.money_summary | format('%.2f') %]
[%- END -%]
[%- END -%]
[%- helpers.csv_datum(atotal) -%]

The FM for money_summary.balance_owed looks like:

<field reporter:label="Money Summary" name="money_summary" oils_persist:virtual="true" reporter:datatype="link"/>

<link field="money_summary" reltype="might_have" key="usr" map="" class="mus"/>

Here are some posts to the listserve about it from 2016.
https://georgialibraries.markmail.org/thread/gboysqav4lkyfabr

Josh

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I'm also curious if this same issue is the cause of the problem in LP#793627.

selfcheck receipt can break on processing hours of operation

I'll have to test to see if the problem comes up when and even number of bills is being processed vs and odd number.

Josh

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

Yep, any time there are and even number of circs grouped together in the notice, the user.money_summary get populated, and when there are an odd number then user.money_summary.balance_owed gets populated.

So something in the grouping and/or environment building code?
Josh

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

[2019-03-04 16:03:17] open-ils.trigger [DEBG:25373:Event.pm:621:] _object_by_path(): meth=search_money_user_summary, obj=, multi=1, step=money_summary, lfield=id
[2019-03-04 16:03:18] open-ils.trigger [DEBG:25373:Event.pm:621:] _object_by_path(): meth=search_money_user_summary, obj=Fieldmapper::money::user_summary=ARRAY(0x5b06e98), multi=1, step=money_summary, lfield=id
[2019-03-04 16:03:18] open-ils.trigger [DEBG:25373:Event.pm:621:] _object_by_path(): meth=search_money_user_summary, obj=290.00, multi=1, step=money_summary, lfield=id
[2019-03-04 16:03:18] open-ils.trigger [DEBG:25373:Event.pm:621:] _object_by_path(): meth=search_money_user_summary, obj=Fieldmapper::money::user_summary=ARRAY(0x5b06e98), multi=1, step=money_summary, lfield=id
[2019-03-04 16:03:19] open-ils.trigger [DEBG:25373:Event.pm:621:] _object_by_path(): meth=search_money_user_summary, obj=290.00, multi=1, step=money_summary, lfield=id
[2019-03-04 16:03:19] open-ils.trigger [DEBG:25373:Event.pm:621:] _object_by_path(): meth=search_money_user_summary, obj=Fieldmapper::money::user_summary=ARRAY(0x5b06e98), multi=1, step=money_summary, lfield=id

It looks like the first time an environment path gets processed, the object gets pulled correctly and cached, and then the next time it is run the object gets set to one of the values. Rinse and repeat. So odd numbers of events grouped together work correctly, since after all events are processed, the environment must be built from the cache.

I miss comments in code ;-(

https://git.evergreen-ils.org/?p=Evergreen.git;a=blob;f=Open-ILS/src/perlmods/lib/OpenILS/Application/Trigger/Event.pm;hb=HEAD#l619

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :
Download full text (4.4 KiB)

I've been adding logging to this to try and figure out what is going on.

I think part of the problem is that the might_have seems to cover might_have_one and might_have_many. I suspect that it would work fine for might_have_many, but in my use case, a user will only ever have one money.user_summary.

I'll need to test with a might_have that may have multiple rows to see what happens there.

So I wonder if there shouldn't be more relations types?

In Event.pm, the multi flag gets set for anthing other than has_a relations.

So the user_summary gets grabbed as an array of objects.

Then there is a line that says if the relation is might_have, then take the first object in the array and use that. The first time it runs it works fine, then the next time it runs, it grabs the first field in the object instead of the first object in an array.

What seems to work so far is to check to see if it is an array of objects before grabbing the first one.
$obj = $$obj[0] if $rtype eq 'might_have' and ref($obj) eq 'ARRAY';
vs
$obj = $$obj[0] if $rtype eq 'might_have';

Logs of original behavior.

So this is the second time the user.money_summary path gets processed for one bill, so the object has already been retrieved and is in the cache.

[2019-03-05 10:55:01] open-ils.trigger [DEBG:18699:Event.pm:503:] Running _object_by_path
[2019-03-05 10:55:01] open-ils.trigger [DEBG:18699:Event.pm:626:] _object_by_path(): meth=retrieve_actor_user, obj=185040, multi=0,
 step=usr, lfield=usr
[2019-03-05 10:55:01] open-ils.trigger [DEBG:18699:Event.pm:651:] _object_by_path() after fetch or cache: obj=Fieldmapper::actor::u
ser=ARRAY(0x5d5eb98), lval=185040, ffield=id, step=usr, fhint=au, str_path=money_summary, def_id=118
[2019-03-05 10:55:01] open-ils.trigger [DEBG:18699:Event.pm:659:] _object_by_path(): @$path code block
[2019-03-05 10:55:01] open-ils.trigger [DEBG:18699:Event.pm:669:] _object_by_path(): @$path code block - call by path again
[2019-03-05 10:55:01] open-ils.trigger [DEBG:18699:Event.pm:626:] _object_by_path(): meth=search_money_user_summary, obj=Fieldmapper::money::user_summary=ARRAY(0x5d85b08), multi=1, step=money_summary, lfield=id
[2019-03-05 10:55:01] open-ils.trigger [DEBG:18699:Event.pm:709:] _object_by_path(): Before Might Have: $VAR1 = bless( [
                 '200.00',
                 '200.00',
                 '0.0',
                 185040
               ], 'Fieldmapper::money::user_summary' );

[2019-03-05 10:55:01] open-ils.trigger [DEBG:18699:Event.pm:714:] _object_by_path() Might have: obj=200.00, objdump=$VAR1 = '200.00';

So the code "$obj = $$obj[0] if $rtype eq 'might_have';" grabs the first element of the results and sents that as the object, in this case '200.00'.

After adding "$obj = $$obj[0] if $rtype eq 'might_have' and ref($obj) eq 'ARRAY';"

I see this behavior.

[2019-03-05 11:04:13] open-ils.trigger [DEBG:18984:Event.pm:503:] Running _object_by_path
[2019-03-05 11:04:13] open-ils.trigger [DEBG:18984:Event.pm:626:] _object_by_path(): meth=retrieve_actor_user, obj=185040, multi=0,
 step=usr, lfield=usr
[2019-03-05 11:04:13] open-ils.trigger [DEBG:18984:Event.pm:651:] _object_by_path() after fetch or cache: obj=Fieldmapper...

Read more...

Changed in evergreen:
status: Incomplete → Confirmed
Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I found a definition of might_have that helps me out.

might_have” means the foreign table has the ID from my table, but with either zero or one objects (not multiple). Thus, it has the same linkage as “has_many” and the same usage as “has_a”.
https://library.calvin.edu/devdocs_project/doku.php

If that definition is accurate?

So if it has the same linkage as has_many, I get why the code was treating it like a has_many with the multi=1, and then was just trying to pull out the single value at the end.

Josh

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

Here is a branch with a possible fix for this issue.

user/stompro/lp850160_event_def_template_fleshing_might_have
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp850160_event_def_template_fleshing_might_have

I'm not sure if this fixes Jeff's original issue. It may not be the same issue since it looks like he was testing with a single, non grouped event. Sorry if I just hijacked this and it isn't the same issue.

This does solve the issue with self check receipts - hours of operation for me from bug #793627, as well as my issue with adding user.money_summary to a bill event def.

Josh

tags: added: pullrequest
Revision history for this message
Mike Rylander (mrylander) wrote :

Hrm... the cache /should/ have the array of one object, so it seems like we should always unwrap it. But, evidence being more persuasive than code-reading, it seems that we end up with a single object in the cache at the end of the call, so picking this commit for great justice. Awesome work, Josh.

Changed in evergreen:
assignee: nobody → Mike Rylander (mrylander)
status: Confirmed → Fix Committed
Revision history for this message
Mike Rylander (mrylander) wrote :

Picked to master through 3.3. Thanks again!

Changed in evergreen:
assignee: Mike Rylander (mrylander) → nobody
Revision history for this message
Terran McCanna (tmccanna) wrote :

Noting that this fix was released some time ago, but the status was not updated at the time.

Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.