finos / FDC3

An open standard for the financial desktop.
https://fdc3.finos.org
Other
202 stars 132 forks source link

Support broadcast in FDC3 Action #1353

Closed symphony-jean-michael closed 1 month ago

symphony-jean-michael commented 2 months ago

Enhancement Request

Use Case:

Today, in Symphony, the user can send a chat message using the FDC3 StartChat intent. ℹ️ The format of the message can be found here : https://fdc3.finos.org/docs/context/ref/Message.

A great FDC3 feature is the ability to attach entities to the message. This is defined in the FDC3 action.

Example: The user can send an FDC3 message containing a button. When the user clicks on the button, it will trigger the FDC3 action attached to the button. This is the entity defined in FDC3.action. See example in the standard: https://fdc3.finos.org/docs/context/ref/Action#example

This works perfectly. But today, some of our customers would also like to broadcast a context instead of raising an intent. But the current FDC3 action format doesn't allow this for now.

In the FDC3 specification, you can read :

The action may be completed by calling fdc3.raiseIntent() with the specified Intent and Context, or, if only a context is specified, by calling fdc3.raiseIntentForContext()

(From https://fdc3.finos.org/docs/context/ref/Action)

Workflow Description

So, we would like to suggest an update of the FDC3 action to support the broadcast.

We propose to modify the current table of FDC3 Action properties. Our modifications are shown in bold.

Property Type Required Example Value
type string Yes 'fdc3.action'
action string No 'broadcast' or 'raiseIntent'
title string Yes 'Click to view Chart'
intent string No 'ViewChart'
context string Yes See Below
channel object No myChannel
channel.id string Yes 'Channel 1'
app object No 'myApp'
app.appId string Yes 'app1'
app.instanceId string No 'instance1'

The action field indicates the type of action:

Workflow Examples

Here is the Action example updated :

const action = {
    type: 'fdc3.action',
    title: 'Click to view Chart',
    action: 'broadcast',      <========= NEW
    channel: {      <========= NEW
      id : 'Channel 1'
    },
    context {
        type: 'fdc3.chart',
        instruments: [
            {
                type: 'fdc3.instrument',
                id: {
                    ticker: 'EURUSD'
                }
            }
        ],
        range: {
            type: 'fdc3.dateRange',
            starttime: '2020-09-01T08:00:00.000Z',
            endtime: '2020-10-31T08:00:00.000Z'
        },
        style: 'candle'
    },
    app {
        appId: 'MyChartViewingApp',
        instanceId: 'instance1'
    }
}

What do you think ? Thank you

kriswest commented 2 months ago

Seems a reasonable addition. As neither of the new properties, action or channel, are required I think this is non-breaking.

However:

symphony-jean-michael commented 2 months ago

Hi @kriswest

Thank you for your comments.

app and channel seem to be mutually exclusive, but you provide both in your example... It's hard to do so via a non-breaking change but a better design for the context might have been to have nested properties under the action so raise intent has optional intent and app properties, while broadcast has a channel property. We could only do that now via a breaking change or adding new properties and deprecating old ones, hence, I suggest we just handle it via documentation (e.g. "the channel property is ignored unless the action is broadcast", etc.)

I totally agree with your remarks 👍

The channel element only has one property, might it be simpler to have just a channelId property?

I suggested using an object because I noticed that a channel can have other properties such as type and displayMetadata. In my case, I only need the ID when broadcasting, but maybe this information will be useful in another case. 🤔 getOrCreateChannel needs only the channel id, so I guess we can leave the other parameters optional.

kriswest commented 2 months ago

@symphony-jean-michael the additional properties of a channel are not used by the API (which only ever takes the channelId as an argument). The additional values are only provided for display purposes.

The one exception is a private channel, which can't be retrieved by id, you have to raise an intent and receive that back. While the additional type property could indicate that its a private channel that you already have to have a refernece to to use, you could also just figure that out using the id if private channels end up involved in your use case - you would have had to receive the channel via a previous interaction where you raised an intent so that should be easy to recognise. Hence, I'd suggest dropping the additional channel params.

kriswest commented 2 months ago

P.S. we're about to try and close the FDC3 2.2 scope - this is not a big change but would need to go onto a PR and the change be agreed this Thursday. Otherwise, it can be agreed and merged post 2.2 and you can start using before it comes out in a later standard version.

symphony-jean-michael commented 2 months ago

P.S. we're about to try and close the FDC3 2.2 scope - this is not a big change but would need to go onto a PR and the change be agreed this Thursday. Otherwise, it can be agreed and merged post 2.2 and you can start using before it comes out in a later standard version.

Thanks @kriswest for the explanation. I will create the PR this afternoon.

symphony-jean-michael commented 2 months ago

Hi @kriswest I've juste created the PR https://github.com/finos/FDC3/pull/1368 Could you please have review it ?

Thank you