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

Context Equivalent of RaiseIntent - Issue 213 #268

Merged
merged 5 commits into from
Jan 26, 2021

Conversation

sgd2z
Copy link
Contributor

@sgd2z sgd2z commented Oct 19, 2020

PR for Issue #213

@mattjamieson
Copy link
Contributor

Is there any scenario where we would want to raiseContext with a target? I can see it being potentially useful where the target is known but the Desktop Agent has UI (or other) features for intent resolution which can be used prior to passing the intent+context to the target.

raiseContext(context: Context, target?: string);

@rikoe
Copy link
Contributor

rikoe commented Oct 22, 2020

Is there any scenario where we would want to raiseContext with a target? I can see it being potentially useful where the target is known but the Desktop Agent has UI (or other) features for intent resolution which can be used prior to passing the intent+context to the target.

raiseContext(context: Context, target?: string);

@mattjamieson I agree, it doesn't hurt, and is consistent with raiseIntent. Although to be consistent with #272 it should probably be raiseContext(context: Context, target?: string | AppMetadata) 😉

@mattjamieson
Copy link
Contributor

[...] to be consistent with #272 it should probably be raiseContext(context: Context, target?: string | AppMetadata) 😉

👍, might also be nice to define:

type Target = string | AppMetadata;

@@ -20,6 +20,7 @@ interface DesktopAgent {
findIntent(intent: string, context?: Context): Promise<AppIntent>;
findIntentsByContext(context: Context): Promise<Array<AppIntent>>;
raiseIntent(intent: string, context: Context, target?: string): Promise<IntentResolution>;
raiseContext(context: Context): Promise<IntentResolution>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if we can call this raiseIntentForContext.

There are two reasons for this:

  1. You can't raise a "context", that doesn't make sense intuitively.
  2. raiseIntent and raiseIntentForContext would follow the same pattern as findIntent and findIntentsByContext, one of which takes an intent name, and the other just a context. It's good symmetry.

Copy link
Contributor Author

@sgd2z sgd2z Oct 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good suggestion. Also makes explaining it simpler. I'll update the PR and docs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe 'raiseIntentsByContext' ?

```ts
raiseContext(context: Context): Promise<IntentResolution>;
```
Raises a context to the desktop agent to resolve. Raise context deals with the case where a context can have multiple associated intents. Similar to raiseIntent without a target, it provides the opportunity for Intent and target selection, which can result in a call to raiseIntent with a target under the hood to provide Intent resolution.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description doesn't make sense to me, mostly I think because it is not clear what "raising a context" means if you don't understand that under the covers the desktop is looking up intent(s) for the context and asking the user to pick one.

I think it is good if FDC3 (at this stage) only have one "thing" that can be "raised", but there are different ways of doing so, i.e. based on name, and based on context.

docs/api/spec.md Outdated
@@ -78,29 +78,31 @@ Examples of End Points include:
### Open an Application by Name
Linking from one application to another is a critical basic workflow that the web revolutionized via the hyperlink. Supporting semantic addressing of applications across different technologies and platform domains greatly reduces friction in linking different applications into a single workflow.

### Raising Intents
### Raising Intents or Contexts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be changed back to just Raising Intents - there is not an additional concept to understand.

docs/api/spec.md Outdated
Often, we want to link from one app to another to dynamically create a workflow. Enabling this without requiring prior knowledge between apps is a key goal of FDC3.

Intents provide a way for an app to request functionality from another app and defer the discovery and launching of the destination app to the Desktop Agent. There are multiple models for interop that Intents can support.
Intents provide a way for an app to request functionality from another app and defer the discovery and launching of the destination app to the Desktop Agent. There are multiple models for interop that Contexts and Intents can support.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here as well, this paragraph really refers to Intents, not to Contexts (which can't be raised in and of themselves) - just sticking to Intents as the thing that can be raised will make this clearer.

docs/api/spec.md Outdated

- **Chain**: In this case the workflow is completely handed off from one app to another (similar to linking). Currently, this is the primary focus in FDC3
- **Client-Service**: A Client invokes a Service via the Intent, the Service performs some function, then passes the workflow back to the Client. Typically, there is a data payload type associated with this intent that is published as the standard contract for the intent.
- **Remote API**: An app wants to remote an entire API that it owns to another App. In this case, the API for the App cannot be standardized. However, the FDC3 API can address how an App connects to another App in order to get access to a proprietary API.

A Context can be associated with multiple intents. For example, an instrument could be associated with ViewChart, ViewNews, ViewAnalysis or some other intent. Raising a context provides the opportunity for the system or the user to select the appropriate Intent for the selected Context and resolve that Intent.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Raising an intent by context provides" .... "for the provided Context" (the context isn't being selected, the intent is).

docs/api/spec.md Outdated
#### Intent Resolution
Raising an Intent will return a Promise-type object that will resolve/reject based on a number of factors.
Raising an Intent or a Context will return a Promise-type object that will resolve/reject based on a number of factors.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The title says "Intent Resolution", but it then talks about "Raising an Intent or Context" - removing the concept of raising "contexts" removes this ambiguity.

- Intent was resolved unambigiously and the recieving app was launched successfully.
- Intent was ambigious, a resolution was chosen by the end user and the chosen application was launched succesfully.
- Intent was resolved unambiguously and the receiving app was launched successfully.
- Intent was ambiguous, a resolution was chosen by the end user and the chosen application was launched successfully.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in this case the intent resolution return type should tell you which intent was ultimately selected by the user, otherwise this information is lost.

}
catch (er){
console.log(er.message);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest expanding IntentResolution with an intentName property, which will match the provided name when using raiseIntent, but will be populated with the selected intentName when resolved by the user or desktop agent based on context alone.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest expanding IntentResolution with an intentName property, which will match the provided name when using raiseIntent, but will be populated with the selected intentName when resolved by the user or desktop agent based on context alone.

Just to confirm, will consider this as a separate Issue/PR 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgd2z
Copy link
Contributor Author

sgd2z commented Oct 22, 2020

Is there any scenario where we would want to raiseContext with a target? I can see it being potentially useful where the target is known but the Desktop Agent has UI (or other) features for intent resolution which can be used prior to passing the intent+context to the target.

raiseContext(context: Context, target?: string);

Interesting idea. We should discuss in the meeting. I was thinking that raiseContext would eventually result in a raiseIntent call with a target and only added a raiseContext without a target. For drag and drop I was thinking of private temporary channel broadcast.

Copy link
Contributor

@kriswest kriswest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WG feedback: Addition to API makes sense, some changes requested for @sgd2z to make:

  • update function name to raiseIntentForContext
  • add optional target parameter for filtering purposes
  • ensure that docs are clear that IntentResolver UI should ask you to select an intent (which the resolver doesn't do at the moment), then an app to resolve it (unless a target was specified)

Consider:

  • naming: should intent be plural? IMHO no :-)
  • (Separate issue) extend IntentResolution to return the intent name (string) and target (AppMetadata) selected in the resolver (helps developers know where it should have gone, possibly log the action for product Intelligence).

@sgd2z
Copy link
Contributor Author

sgd2z commented Nov 17, 2020

PR has been updated with feedback and based on our discussions

docs/api/spec.md Outdated Show resolved Hide resolved
src/api/DesktopAgent.ts Outdated Show resolved Hide resolved
docs/api/ref/DesktopAgent.md Outdated Show resolved Hide resolved
docs/api/ref/DesktopAgent.md Outdated Show resolved Hide resolved
@rikoe rikoe mentioned this pull request Jan 7, 2021
19 tasks
@rikoe rikoe merged commit 3006545 into finos:master Jan 26, 2021
@rikoe rikoe added this to the 1.2 milestone Jan 26, 2021
@kriswest kriswest deleted the context-equivalent-of-raiseIntent branch October 8, 2021 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants