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

Support broadcast in FDC3 Action #1353

Closed
symphony-jean-michael opened this issue Sep 13, 2024 · 6 comments · Fixed by #1368
Closed

Support broadcast in FDC3 Action #1353

symphony-jean-michael opened this issue Sep 13, 2024 · 6 comments · Fixed by #1368
Labels
enhancement New feature or request
Milestone

Comments

@symphony-jean-michael
Copy link
Contributor

symphony-jean-michael commented Sep 13, 2024

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:

  • raiseIntent : If no action or raiseIntent is specified, then the current default behaviour would be used with fdc3.rainseIntent / fdc3.raiseIntentForContext
  • broadcast : If broadcast action is specified, it will call fdc3.getOrCreateChannel('Channel 1')to retrieve the channel, then broadcast the context to that channel with channel.broadcast(context). If no, channel has been specified, it will try to broadcast to the current channel (fdc3.getCurrentChannel())

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

@symphony-jean-michael symphony-jean-michael added the enhancement New feature or request label Sep 13, 2024
@kriswest
Copy link
Contributor

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

However:

  • 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.)
  • The channel element only has one property, might it be simpler to have just a channelId property?

@kriswest kriswest added this to the 2.3 candidates milestone Sep 20, 2024
@symphony-jean-michael
Copy link
Contributor Author

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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

Hi @kriswest
I've juste created the PR #1368
Could you please have review it ?

Thank you

@kriswest kriswest linked a pull request Sep 27, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants