Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PrimaryAccountable of resource with specific id is not updated after a transfer event #3

Open
ocataco opened this issue Sep 13, 2021 · 14 comments

Comments

@ocataco
Copy link

ocataco commented Sep 13, 2021

Context

I'm working on a simulation of a flow regarding medical gowns. Each gown has their own trackingIdentifier.
At some point the 'hospital agent' produces a lot (with a manifest that lists the gowns it contains).

This lot (a resource which has it's own id), is at some point transferred to another 'cleaner' agent.

Screenshot 2021-09-13 at 16 11 42

The EconomicEvent is created succesfully in reflow os as you can see from the ui, but the ownership and the location of the lot resource are not updated to the receiver;

Screenshot 2021-09-13 at 16 15 28

Issue

This is a problem because when i try to consume the resource as the receiver after the transfer i get the error message:
"You cannot do this since the provider is not accountable for the resource."

Below are the parameters for the transfer event i use in the client code.

event: {
        note: event_note,
        action: "transfer",
        provider: provider_id,
        receiver: receiver_id,
        hasPointInTime: ts,
        resourceInventoriedAs: lot_id,
      }

And this is when i view the state of the transfer event:

{
  "data": {
    "economicEvent": {
      "action": {
        "id": "transfer",
        "label": "transfer"
      },
      "id": "01FFFGYYJFA4V17WZZ2J1MWYW4",
      "note": "Gown Lot Transfer",
      "provider": {
        "id": "01FF84K296NSPKGQ39QD5EY30C",
        "name": "olvg",
        "note": "hospital user"
      },
      "receiver": {
        "id": "01FF84PJFC845FTTP59B3SBS6P",
        "name": "cleaner",
        "note": "I clean gowns"
      },
      "resourceInventoriedAs": {
        "accountingQuantity": {
          "hasNumericalValue": 0,
          "hasUnit": {
            "label": "om2:one",
            "symbol": "#"
          }
        },
        "currentLocation": {
          "id": "01FF84V8MWSEKNQ3ZQCRT8CPM8",
          "name": "OLVG, locatie Oost"
        },
        "id": "01FFFGYY8WE8KHM5BJ1FAAD7R3",
        "name": "Gown Lot",
        "onhandQuantity": {
          "hasNumericalValue": 0,
          "hasUnit": {
            "label": "om2:one",
            "symbol": "#"
          }
        },
        "primaryAccountable": {
          "id": "01FF84K296NSPKGQ39QD5EY30C",
          "name": "olvg"
        }
      },
      "resourceQuantity": null
    }
  }
}

Note that the name field of the primaryAccountable shows the name of the provider: "olvg" (the hospital agent) and not the name of the receiver "cleaner". Also the location is not updated to the location of the receiver.

For more context see: dyne/zenpub#58

@pral2a
Copy link

pral2a commented Sep 16, 2021

@ocataco I think your issue is related that ZenPub issue but has no answer, neither dyne/zenpub#60

@denizenging
Copy link
Member

Hi @ocataco,

You should use the field toResourceInventoriedAs instead of resourceInventoriedAs when you want to use the actions transfer and move. Also, you should ask for toResourceInventoriedAs instead of resourceInventoriedAs in the response you'll get as well.

Cheers,
srfsh

@ocataco
Copy link
Author

ocataco commented Sep 21, 2021

Hi Srfsh,

Can confirm this works!

Screenshot 2021-09-21 at 13 03 11

the resources is produced by the provider, transferred and consumed by the receiver

Screenshot 2021-09-21 at 13 01 44

and the transfer shows in the ui on the material passport.
when i try to click on "view resource" it says unrecognized, probably expected behaviour because it was consumed?

Thanks for the help srfsh :-)

@pral2a
Copy link

pral2a commented Sep 21, 2021

Thank you @srfsh! That's also helpful for ZenPub and WeLoop

@adam-burns I think it's super important to update the Reflow OS documentation with it to help Reflow pilots move further

@denizenging
Copy link
Member

Hi Srfsh,

Can confirm this works!

Screenshot 2021-09-21 at 13 03 11

the resources is produced by the provider, transferred and consumed by the receiver

Screenshot 2021-09-21 at 13 01 44

and the transfer shows in the ui on the material passport.
when i try to click on "view resource" it says unrecognized, probably expected behaviour because it was consumed?

Thanks for the help srfsh :-)

It seems like a UI bug, where it only uses the ID of resourceInventoriedAs.

Will work on that.

@ocataco
Copy link
Author

ocataco commented Sep 21, 2021

Oh one more thing, i see in the screenshot that location of the lot is still OLVG and not the location of the receiver, do we need to do a seperate "move" for that?

Update:

  • I have tried to pass the "atLocation" field with the id of the receiver to the "transfer" event, but the resulting location remains the location of the provider.
  • I have tried to create an extra "move" event (copying the parameters of the "transfer" action, thus providing toResourceInventoriedAs, and atLocation with id of the receiver, but the resulting location remains the location of the provider.

The documentation of EconomicEvent.atLocation states: "The place where an economic event occurs. Usually mappable."
So from the description this is not the right field to specifiy where it needs to update, but it seems to be the only field available to specify a location in the EconomicEvent.

@adam-burns
Copy link
Member

@srfsh @ocataco can you determine if is this a usage misunderstanding or if it is a separate but related uncovered issue?

Oh one more thing, i see in the screenshot that location of the lot is still OLVG and not the location of the receiver, do we need to do a seperate "move" for that?

Update:

* I  have tried to pass the "atLocation" field with the id of the receiver  to the "transfer" event, but the resulting location remains the location of the provider.

* I have tried to create an extra "move" event (copying the parameters of the "transfer" action, thus providing toResourceInventoriedAs, and atLocation with id of the receiver, but the resulting location remains the location of the provider.

The documentation of EconomicEvent.atLocation states: "The place where an economic event occurs. Usually mappable." So from the description this is not the right field to specifiy where it needs to update, but it seems to be the only field available to specify a location in the EconomicEvent.

@fosterlynn
Copy link

The documentation of EconomicEvent.atLocation states: "The place where an economic event occurs. Usually mappable." So from the description this is not the right field to specifiy where it needs to update, but it seems to be the only field available to specify a location in the EconomicEvent.

You are right, it is not the right field, but we "hacked" it in to use for updating resource location in move events, when we tightened up what resource properties can be modified by an update, and what can't be without an event. It doesn't really conflict for move we think, since move is internal to an agent, and tends to be close by, otherwise there would probably be a transportation process. But VF would be happy to fix that hack whenever the timing works for people. (We've been really careful about changes during this period of active software development, especially changes that might have some ripple effects, even just a lot of discussion time when people are busy with more important issues, and changes that are not particularly backwards compatible. But we only need to be as careful as you all want us to be.)

Also possibly relevant, the question came up should transfers also be able to update the location at the toResourceInventoriedAs. We don't think they need it. If there is a new resource created as a result of the transfer, currentLocation can be set at that time. If the transfer increments an existing resource, it probably already has its currentLocation if the agent cares about location.

@adam-burns
Copy link
Member

Thanks Lynn, @ocataco does currentLocation usage work for you?

@ocataco
Copy link
Author

ocataco commented Nov 22, 2021

Hi @adam-burns I understand that you could specify currentLocation when creating a new resource, but in my test I was explicitly trying to transfer these container resources without creating a new resource which I think is a valid way in use cases where cooperation is important. It's just seems to make sense to give things the same name when you are working together to get insight into the flow of resources.

As srfsh found out, the implementation allows for this by making the resourceInventoriedAs and toResourceInventoriedAs have the same id. I noticed that there was no way to update the location in this kind of transfer. the owner changed, the location did not.
and when I tried the move event to adjust for this it didn't seem to update the location either.

there is no usage misunderstanding as far as I can tell.
for both strategies I tried, it did not seem to update the location of the resource in the implementation, at the time of writing.

@fosterlynn
Copy link

It's just seems to make sense to give things the same name when you are working together to get insight into the flow of resources.

@ocataco thanks, seems like a valid (and nice) use case, just not one VF has run into. You mean the same ID too, I assume, meaning it is the same resource instance in the software. (Just to note for future reference, you can follow the flows of resources in either case, by following them through the input-process-output and transfer activities, irrespective of agents and locations. But still, good use case.)

What is you all's preference? You could use transfer event atLocation to change the resource currentLocation, if you don't need atLocation for its original purpose, which was requested by people who thought they might want to track distances for climate accounting, but afaik have not yet done so. Or we could add a property to event only for explicitly changing the resource currentLocation, if you want at this moment in your development. If we add it, when move is fixed, it could use the new field.

To me, it has become abundantly clear and we should go ahead and add the field and fix the hack, from the VF point of view. Getting it into the graphql spec, and using it in projects, can be coordinated as needed, and at projects' convenience. Opinions welcome. I'll put together a MR/PR in VF though, to get things moving, some time today.

@fosterlynn
Copy link

Added a property in VF called toLocation on EconomicEvent, that can be used to update currentLocation on the EconomicResource. Not yet in the graphql spec, probably not yet showing in the website gitbook.

https://lab.allmende.io/valueflows/valueflows/-/merge_requests/663/diffs

@ocataco
Copy link
Author

ocataco commented Nov 25, 2021

That sounds like a very nice addition, thanks Lynn!

Added a property in VF called toLocation on EconomicEvent, that can be used to update currentLocation on the EconomicResource. Not yet in the graphql spec, probably not yet showing in the website gitbook.

https://lab.allmende.io/valueflows/valueflows/-/merge_requests/663/diffs

@sbocconi
Copy link

Is the new property toLocation on EconomicEvent scheduled to be implemented in Bonfire?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants