search

cds-hooks / docs

CDS Hooks website & specification
http://cds-hooks.org
Apache License 2.0
166 stars 61 forks source link

Strengthen documentation around app links #4

Open kpshek opened 8 years ago

kpshek commented 8 years ago

Our documentation around app links needs to be clarified. Questions that the documentation should answer:

isaacvetter commented 8 years ago

Hey Kevin, All,

An additional question - if a cds-service is granted an oauth access_token during the initial request, and the service then returns a link to a SMART on FHIR application, should the EHR append the standard iss and launch parameters to the app url?

That is, are the cds-service and the app assumed to be the same system? Thinking it through, I think that they should not be the same and would perhaps even have different scopes to the FHIR server. If this makes sense, it would be nice to clarify this - the EHR should launch the returned app url with standard SMART GET params.

Isaac

isaacvetter commented 8 years ago

Some thoughts:

Isaac

kpshek commented 8 years ago

An additional question - if a cds-service is granted an oauth access_token during the initial request, and the service then returns a link to a SMART on FHIR application, should the EHR append the standard iss and launch parameters to the app url?

That is, are the cds-service and the app assumed to be the same system? Thinking it through, I think that they should not be the same and would perhaps even have different scopes to the FHIR server. If this makes sense, it would be nice to clarify this - the EHR should launch the returned app url with standard SMART GET params.

Agreed. The OAuth access token definitely should not be reused between a CDS Service as a SMART app. While we haven't finalized our security plans yet (see #7), we're working towards a model where CDS Services use the SMART Backend Services model (B2B) whereas SMART apps do not.

app links should definitely be absolute urls. The app and service shouldn't be required to be the same system.

👍

How EHRs open apps is dependent upon user workflow and should be left to the EHR to configure.

👍

Definitely we should support customer URL schemes, this is the primary way that cds-hooks (and SMART on FHIR) work on mobile, right?

I'm pretty sure we should support custom URL schemes, too. I just want to actually implement this to work through any potential issues that may crop up. One of my questions is how does a CDS Service know to return, say, a native app link (myapp://) when running on a native platform (desktop, iOS, Android) vs their web (https://) app link? One way the CDS Service could do this today is by simply having a service endpoint that is only used in a native context. However, I can also see us communicating the environment on each CDS Service request.

Right now I'm personally fine leaving this as undefined until we can get some actual prototypes that want to launch a native app.

kpshek commented 8 years ago

Test Zulip integration...

kpshek commented 7 years ago

Originally I was thinking that SMART app links didn't have to be any special type of link in the CDS card. This is because the EHR has a known registry/whitelist of SMART app launch URLs. Thus, by comparing every CDS card link to this registry, if it was found to be contained within, the EHR could modify the link to add the SMART iss and launch parameters.

However, I have the following concerns with this approach:

  1. It assumes the EHR has a registry/whitelist of known SMART app launch URLs

While this is true of the EHR I'm intimately familiar with (Cerner), I don't know that this will hold true for all EHRs. Maintaining such a registry/whitelist is not necessary and in the case of Cerner, is done for business reasons (not due to technical or security reasons).

Additionally, in the not too distant future I can see where it may not be possible to maintain such a registry/whitelist for certain use cases (like for instance patient's bringing their own SMART app). While our use case of CDS Hooks has been focused on the practitioners in the EHR, I would not be surprised to see us venture into other venues like patients in their personal EHR or patient portal.

  1. It requires the EHR to re-write the card link

Only the EHR knows the appropriate values for the iss and launch parameters during a SMART app launch. As such, a CDS service can never supply a link to a SMART app that is absolute. Instead, the link must be made 'correct' by the EHR re-writing the link to add the appropriate parameters.

Based on this, I would like to define a first-class SMART app link within a CDS card. So, either an explicit SMART app launch URL field or some additional attribute on a link indicating that this is a SMART app. The former option is more explicit/clear at the cost of API verbosity where the latter (achieved through something like a type attribute on a link) is terse, but perhaps unnecessarily open-ended.

Right now I'm leaning towards that latter option of including a type attribute on each link (this is in addition the existing label and url attributes) with a value of smart. Non SMART app links could have a type of absolute.

I'd appreciate any thoughts from others on this.

kalyaniyerra commented 7 years ago

I like the option 2 to add the type attribute on each link to specify smart or absolute. This option adds for future extensibility for other types of apps also.

kpshek commented 7 years ago

Right now I'm leaning towards that latter option of including a type attribute on each link (this is in addition the existing label and url attributes) with a value of smart. Non SMART app links could have a type of absolute.

After offline conversation with @isaacvetter (Epic), @kalyaniyerra (Premier), the RxREVU team (Peregrin, Sean, Joel), and Elsevier (Alex DeJong), we decided to proceed with this proposal. This new type field will be required for each link returned in a card.

When the type is smart, the EHR will be responsible for appending valid and appropriate launch and iss parameters to the URL.

kpshek commented 7 years ago

I'd appreciate it if another set of eyes could review my proposed documentation changes here: https://github.com/cds-hooks/docs/pull/16

basicBrogrammer commented 7 years ago

Can we have a recommendedTarget key? Then, the cds tool can recommend opening the link in an iframe modal or a new tab.

isaacvetter commented 7 years ago

Hey @jcward10, I'm not sure that the cds tool always knows what the possible targets are for a given EHR. The EHR can be, at least, a desktop, web or mobile app. In two of these three scenario's, an iframe or new tab may not be possible.

olbrich commented 7 years ago

@isaacvetter I can see scenarios in which a hook might behave differently depending on the User agent. For example, a hook might not have enough space to work properly on a mobile browser and thus might suggest that the user should use a desktop web browser to complete their task instead. I suppose that's why @jcward10's suggestion is just a recommendation.

Penumbra69 commented 7 years ago

We have a similar card-generation capability in house (non-fhir), and in it we have a few fields for rendering the information to a client (we support mobile, web, and sms). One is Markdown compatible, One is 140 characters of text, one is HTML (we call it the 'div' field which always is a div - not an iframe, but you get it). The renderer can then pluck from the card the rendering it is capable of. More work to store, more data to pass - but solved the conundrum for us.

kpshek commented 7 years ago

Can we have a recommendedTarget key? Then, the cds tool can recommend opening the link in an iframe modal or a new tab.

@jcward10 and I had an offline conversation where I explained my thoughts on this but I want to also document them here. To cut to the chase, I agree with @isaacvetter in that the CDS service isn't the best entity to recommend a target. Even though this would be an optional recommendation, it has the high probability to be a crummy recommendation in which case I don't see a need to add this in the spec.

The target of the link is based upon the platform and capabilities of the EHR. Eg: mobile native app vs desktop native app vs pure browser. In the case of the Cerner EHR (of which I'm very familiar), we could launch a link (specifically a SMART app) in the following ways:

In each of these scenarios, the EHR knows best how to open the link and I would ignore any recommendation provided by the CDS service.

I've seen demos/prototypes of embedded SMART apps in other EHRs and each opened them in a variety of ways (new windows, new iframes opened in a slide over panel, etc).

For all of these reasons, I don't think we should add a new recommendedTarget field

kpshek commented 7 years ago

I just merged #16 which resolves this issue. Note - this is a new breaking change to the API.

If we want to continue the discussion on the sidebar conversation here around link targets we should open a new issue.

kpshek commented 7 years ago

Re-opening this issue. I realized that PR #16 addressed a few aspects of the questions I wanted to answer when I originally logged this issue.

Are there any expectations that links should always be TLS protected? (https)

I plan on updating the documentation to state that TLS protected endpoints are strongly recommended, but not required with absolute links. smart links must be TLS protected if they are HTTP links as the SMART project requires this.

Do links to SMART apps need any other metadata or identifying information?

Answered this with the new smart type field on links.

Are links absolute URLs?

Answered this with the new absolute type field on links.

Should EHRs open links in a new window? embedded frame? etc.

I plan on putting my thoughts above on this topic into the documentation.

Do we support custom URL schemes? (eg, native links like myapp://)

Right now I want to be silent on this until we actually prototype something out. I think this would work with what we have today (this would be an absolute link).

jmandel commented 7 years ago

The other requirement to highlight here, I think, is letting an app finish and return control back to the EHR — in our test harness for example we use this capability to link to a hypertension management tool where a user can select a drug, and then return to the EHR and have the UI updated with their choice.

Right now we do this using a redirect URL: the app's job is to redirect to that URL if/when the user has finished working with the app (e.g. made a final decision); the EHR gets to host whatever content it needs at this URL (e.g. execute a JS function to communicate back with the EHR host window, or execute a server-side process to mark the session as complete).

vadi2 commented 7 years ago

Another thing that's unclear in the documentation to me is about this redirect url - is the hookInstance supplied back to the EHR, so the EHR knows which decision is complete?

jmandel commented 7 years ago

Ah. Once control returns to the EHR, the EHR re-invokes the notification using the same hookId, and it's at that point that the external service returns a user's "decisions". So as currently specified, the external service needs to retain this state (id + decision) long enough to support the return loop.

vadi2 commented 7 years ago

Hm... but then if multiple hooks are started at once, the EHR wouldn't know which one is it that came back. I guess the EHR would have to re-try them all and see if decisions are available for them?

vadi2 commented 7 years ago

The other way is - I'm not finding anything saying otherwise in the spec - is that the redirect URL supplied by the EHR to begin with would have this hookInstance within it as a parameter, so when the CDS pings it, it would know which is completed.

I guess I'm looking to see what current implementations, if any, are doing in this regard?

jmandel commented 7 years ago

So far, I think the only implementation is our test harness. In the current implementation: the redirect URL is a static URL hosted by the test harness; it's just a page whose job is basically "find my window.opener and notify them that i'm done; then close me". But that's an implementation detail; the spec is just "EHR creates a redirect URL and app redirects there"; the details of how it works is up to the EHR.

It would be possible for the main EHR frame to keep track of which apps it has launched in which child windows (e.g. by generating unique redirect URLs, or tracking metadata about its window objects in JS, or some other mechanism), to prevent re-triggering all services. But that's technically an optimization; it should also be safe to re-trigger all hooks any time a user returns from an app.

It's certainly an "open design issue" to determine what's implementable within real-world EHRs, and this is one of the trickier workflow steps to pull off.

vadi2 commented 7 years ago

Thanks, I suppose I was just looking for someone to say that's an implementation detail as the spec is unclear on this part. Appreciated!

kpshek commented 7 years ago

@jmandel - Sorry for coming in late on this:

The other requirement to highlight here, I think, is letting an app finish and return control back to the EHR... ...Right now we do this using a redirect URL: the app's job is to redirect to that URL if/when the user has finished working with the app...

To be clear, the discussion on this issue prior to Feb 21 been focused around links -- both absolute and SMART app links. We weren't discussing the decision workflow. I mention this because I did not see the redirect field being used just anytime a user follows a link in a card and wants to come back to the EHR. I saw redirect existing purely for the decision workflow.

If redirect is to be called when the user closes any link they follow on a card, how is the EHR supposed to know not to re-trigger decision support (since no decisions were made)?

Also, redirect would only need to be called if the link were capable of even following the redirect. I'm guessing in nearly every absolute link case, we cannot follow the redirect (since those likely aren't apps). Additionally, my assumption here is that EHRs aren't going to want to change navigation to a link in a card. Rather, I'm assuming that EHRs will likely want to open the link in a new window or component/tab within the EHR itself.

@jmandel - Did you think we were using redirect when a user is done visiting any link on a card or did some wires get crossed here?

jmandel commented 7 years ago

If redirect is to be called when the user closes any link they follow on a card, how is the EHR supposed to know not to re-trigger decision support (since no decisions were made)?

Under the current model, the EHR always re-triggers any time a site or app explicitly "returns control" via the redirect. That's part of handling a "return" operation. Of course, we could add on a parameter (&refetch=yes-please, or whatever) to save the extra work.

I'm assuming that EHRs will likely want to open the link in a new window or component/tab within the EHR itself.

Sure, this is up to the EHR to decide. But in the event that the EHR opens a new window and doesn't receive "return", the EHR is basically saying "You don't get to make decisions". That's okay...

@jmandel - Did you think we were using redirect when a user is done visiting any link on a card or did some wires get crossed here?

I think redriect can apply any time a user clicks a link on a card. Now, there could always be additional (out of band, from the CDS Hooks perspective) ways to "finish with an app" (e.g. the EHR might also display a big "X" next to the app view, or might just open it in a new window that's totally disconnected from the user session). But as an in-band way of "returning", the redirect works equally well for SMART apps and other linkable services.

kpshek commented 7 years ago

So I went back through and re-read our documentation on redirect. I totally missed that it is intended to be used to return control back to the EHR after following any link. I always thought it was just for the decision workflow. 😛

I'll log a new issue around redirect and we can move the discussion there.