The `AppIntent` interface incorrectly describes an intent

[ggeorgievx]

Minor Issue

The AppIntent interface incorrectly describes an intent. The displayName of the AppIntent's intent is ambiguous.

Area of Issue

[ ] App Directory [X] API [ ] Context Data [X] Intents [ ] Use Cases [ ] Other

Issue Description:

Let's say we have the following FDC3 application definitions (this is just an example to illustrate the issue - some required properties are missing):

[
    {
        "name": "ChartA",
        "intents": [
            {
                "name": "fdc3.ViewChart",
                "displayName": "Chart A's View Chart Intent",
                "contexts": [
                    "fdc3.instrument"
                ]
            }
        ]
    },
    {
        "name": "ChartB",
        "intents": [
            {
                "name": "fdc3.ViewChart",
                "displayName": "Chart B's View Chart Intent",
                "contexts": [
                    "fdc3.instrument"
                ]
            }
        ]
    }
]

After calling fdc3.findIntent('fdc3.ViewChart') we would get the following AppIntent:

{
    intent: {
        name: 'fdc3.ViewChart',
        displayName: '...'
    },
    apps: [
        {
            name: 'ChartA'
        },
        {
            name: 'ChartB'
        }
    ]
}

The issue is that the display name is ambiguous - it could be Chart A's View Chart Intent or Chart B's View Chart Intent.

Going through the existing open-source FDC3 implementations we noticed that both Nick Kolba's desktop-agent, as well as Finsemble's FDC3 implementation would return an AppIntent with the intent display name of the first application.

Additional Context:

A PR for this issue would require a change to the existing AppIntent interface.

[rikoe]

This is a good catch, we need to fix this, and 2.0 will be a good place to do this, as it may require breaking changes to the AppIntent interface.

[kriswest]

Should we just drop the displayName field from IntentMetadata (which leaves it as a rather pointless class containing only a single string name) ? After all intents should be:

• Recognizable
  • Generally self-evident what the thing is according to the intents spec.

We could then also drop displayName from the appD intents object.

An alternative would be to add some form of reference for intents (other than the config of individual apps), perhaps as part of the appD, where the display name could be drawn from.

[kriswest]

@ggeorgievx any thoughts on how to resolve this issue? I can see how this flaw might have come about:

• There exists suggested display names for the standard intents: https://github.com/finos/FDC3/blob/master/src/intents/standard%20intents.json
  • However, the spec/docs don't point to this AND there's no such reference for a custom intent, apart from what an individual app names it in their intent config.

When Citi showed us their AppD portal, it included a reference for both intents and context types. Perhaps we should go that route, add it as a new record type to the appD and remove the displayName ?config?? Perhaps with a suggested fail-over to the raw intent name?

[ggeorgievx]

IMO the cheapest solution would have us add an optional displayName property to the AppMetadata interface similarly to how we dealt with application instances in #509.

Another solution would be to change the AppIntent interface's apps property to a singular from (app). That way we would have the AppIntent interface describe a single instance/implementation of an intent with the correct metadata like the displayName (and would truly be an AppIntent as opposed to an AppsIntent). We would then also need to change the signature of findIntent() (and possibly the name to findIntent**s**()) to return a promise resolving with an array of AppIntent (Promise<AppIntent[]>).

Thoughts?

[rikoe]

I personally like this suggestion @ggeorgievx - I think it is the "best" way to solve the problem, and the most flexible for the future. An AppIntent represents the relationship between a single app and intent, and you could have multiple of them returned...

[ggeorgievx]

@kriswest @mattjamieson Team, what are your thoughts on this one? If you agree with Riko I am more than happy to raise a PR.

Alternatively we could have this added to the mtg agenda for the next Standards WG.

[kriswest]

I agree that this change makes sense. The semantics of AppIntent have always bothered me as has the hack you have to use to pick a display name for intents.

The affected functions are just these two, right?

  // intents
  findIntent(intent: string, context?: Context): Promise<AppIntent>;
  findIntentsByContext(context: Context): Promise<Array<AppIntent>>;

We could/should switch both to return Promise<Array<AppIntent>>, with each entry then corresponding to single app - there can be a mix of intents in the array in the findIntentsByContext response, but I'd be inclined to be consistent over the return type rather than goto a 2d array (particularly as every entry in the response to findIntent could have a different displayName even when the intents are all the same).

The (breaking) change will of course need approval by the SWG, but I think it's worth implementing the PR to present to the group. Don't forget that it will also need fixing in the Methods.ts.

@thorsent are you also happy with this change?

P.S. apologies for the delay in responding @ggeorgievx, I missed the original notification.

[kriswest]

One more thing to consider... Rex suggested this in the deprecation policy (see #532):

Rather than quietly change the behavior of an existing feature, consider deprecating it and replacing it with something with a different name/syntax.

This is a breaking change without a prior deprecation. Should we consider revising the function and type names and preserve the old ones as deprecated? Implementing a fallback/conversion in Methods.ts is possible.

Not sure what I would call the replacement for AppIntent... IntentApp? Implies findIntentApp and findIntentAppByContext

[thorsent]

@kriswest I like the change.

I think it's worth discussing how to deal with breaking changes more elegantly. Perhaps namespacing offers a more discrete approach:

//Maintain separate namespaces for each official version
export namespace FDC3_1 {
	export type FDC3 = {
		findIntentsByContext(context: string): Promise<AppIntent>
	}
}
export namespace FDC3_2 {
	export type FDC3 = {
		findIntentsByContext(context: string): Promise<Array<AppIntent>>
	}
}

// Default to the most recent version 
export type FDC3 = FDC3_2.FDC3 & {
	fdc3_1 ?: FDC3_1.FDC3, // But provide access to prior versions - if the desktop agent supports it.
	fdc3_2 ?: FDC3_2.FDC3
}

Since fdc3 is a declared global though, this would force developers to be a little more creative:

myapp.ts

import { FDC3_1 } from "@types/@finos/fdc3";

let myFDC3 : FDC3_1 = fdc3.fdc3_1;

const appIntent = await myFDC3.findIntentsByContext("chart");

[kriswest]

@thorsent @ggeorgievx @rikoe For the System channels -> User channels rename, where functions changed name (e.g. getSystemChannels() -> getUserChannels()) we implemented an automated fallback in the NPM Module's Methods.ts. There's another for the addContextListener signature that was deprecated here.

Not everyone uses that NPM module, but it does provide an alternative to namespacing that we could use... It does work best if we change the name of the function and types when making breaking changes (as its easy to detect that the new function is not present and we need to adapt).

Finally, my assumption is that it will usually be easier to update the NPM module in an application than it will be to update the desktop container (which might only provide FDC3 1.2, where your app has been updated to use FDC3 2.0).

[thorsent]

The fallback works fine for JS (and desktop agents could also offer backward compatibility).

The only issue is that existing TS implementations would break during the compile cycle when the published types change. So I think it depends on whether that is an acceptable compromise.

[kriswest]

On returning to this issue, I am less sure about the proposed change (allowing for a display name per intent/app combo). There are situations (e.g. a button to raise an intent in an app, which can then be resolved by multiple other apps) and API calls (findIntentsByContext and raiseIntentForContext and resolver responses/UI) where we need a single display name for an intent. There is a use-case for data unique to an intent/app combo, but its perhaps descriptive rather than nominative (what precisely will the app do when it receive the roughly descriptive intent).

The /intents appD endpoint proposed in #719 also doesn't provide a solution where multiple appDs are in use as you could still end up with multiple display names per intent. We could apply the display names in the standard itself; there exists a source file with display names although I think these are rarely used and not necessarily correct, e.g.:

    {
      "name": "ViewInstrument",
      "displayName": "Details"
    },
    {
      "name": "ViewAnalysis",
      "displayName": "Analyze"
    },
    {
      "name": "ViewHoldings",
      "displayName": "Holdings"
    }

It also doesn't provide a solution for proprietary or as yet unstandardized intents.

Hence, I am again leaning towards simply dropping the display name, in favour of just using the intent name (and living with PascalCase) as that provides a simple solution for all cases.

[kriswest]

Notes from #802:

• The AppIntent interface only allows for a single display name to be returned. However, the main source for the display name is the interop/intents configuration of apps that can resolve the intent - hence, multiple different display names maybe returned, forcing the desktop agent to pick one.
• This was less of a problem in FDC3's early days when a Desktop Agent would typically be working with a single appD in the control of the desktop developer, allowing them to ensure consistency in the names.
• However, in situations where the Desktop Agent develop, (multiple) app developers and the desktop owner all differ it becomes far more challenging.
• Proposed solutions:
  • Alter the AppIntent type to allow a display name per app returned
    • Challenging for a resolver UI or applications using findIntent as they are likely to need a single display name
    • Also a breaking change...
  • Advise that display names are drawn from the 'standard intents.json' file in the standard
    • The display names in that file have not been well curated and vary in form
    • Does not provide a solution for proprietary intents.
  • Deprecate the displayName field and rely on the intents name instead.
    • Intents are already required to be 'recognizable' and their meaning to be 'generally self-evident'
    • Non-breaking (until field is removed in a later version)
• However, there is a use-case for some descriptive information about a specific apps handling of an intent: describe the precise behavior of the application.
  • This could be handled through by allowing an app to configure a description field in its interop.intents.listensFor metadata (breaking change) and a type that extends AppMetadata in the Desktop Agent API to add the description field (non-breaking).
  • A breaking change after the 2.0 release is undesirable.
• PR for the change (deprecate display name, explain in documentation that intent name should be used for display) to be worked up and brought back for further review