-
Notifications
You must be signed in to change notification settings - Fork 61
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.userId containing ResourceType and ResourceId causes problems #520
Comments
Hi Matt, I seem to recall that the reason userId was specified this way was to support different user types.
Since we use this syntax we have not had any problems with it. |
@cfeltner - that only works if all you want to do is a FHIR READ. If you want to do a SEARCH, for example because you want to also Also, wanting to do SEARCH for cases like described above feels like a fairly reasonable / frequent use-case. |
@mattvarghese Perhaps you could give a prefetch example that is causing problems. |
Sure - I can give the specific example which led me to write this up. This example is in the light of PR #515 and so I am using PractitionerRole type resource, but the problem itself is generic. Since Context.UserId can be PractitionerRole after the above PR, I was trying to write a prefetch query that will also include the Practitioner resource referenced in the PractitionerRole resource. For this I could write a query like:
if Context.userId was just the ID, this would work. And similar queries using Context.patientId, and Context.encounterId work fine. However, the above query fails because Context.userId contains both type and id. |
Another example, in a case where Context.userId is a Patient resource:
|
Hey @mattvarghese , @cfeltner, The CDS wg talked through this issue among related others -- will you guys check out this jira ticket: https://jira.hl7.org/browse/FHIR-25761 ? We're proposing a potentially breaking change to address this deficiency in pre-fetch. Isaac |
@mattvarghese I am trying to understand the use case where there is a need to get to Practitioner from PractitionerRole or is it more about having userId to be in sync with patientId and encounterId. Also wondering if we can leave the PR #515 as it and introduce new context like below if need be "context" : { |
We, and I imagine many others, are actively using userId in our current CDS Interactions. Therefore, I suggest leaving it's meaning as is and introduce a new ID and type as @isaacvetter has suggested. For where we are currently using it, it still seems like the CDS Service would be interested in the Practitioner ID and not the PractitionerRole ID. The CDS Service is using the Practitioner ID to correlate practitioners between the EHR and the CDS Service. Returning the PractitionerRole ID instead of the Practitioner ID would cause them to perform a separate lookup or return another resource in the prefetch. By keeping userId as is and adding new context parameters (e.g., FhirUserId, FhirUserType) this would allow for a smoother transition as EHRs and CDS Services deploy updates to support these. |
Hey @yashaskram , @cfeltner , @mattvarghese , All - Overall, CDS Hooks' Of course, this proposed change would also affect essentially every hook. We definitely don't want to break existing implementations. Below is one possible, brute-force solution --
|
Metadata | Value |
---|---|
specificationVersion | 1.0 |
hookVersion | 1.0 |
Hook maturity | 4 - Documented |
Workflow
The user has just opened a patient's record.
Context
The patient whose record was opened, including their encounter, if applicable.
Field | Optionality | Prefetch Token | Type | Description |
---|---|---|---|---|
userId |
REQUIRED | Yes | string | The id of the current user. The resource type MUST be one of Practitioner, PractitionerRole, Patient, or RelatedPerson. For example, if the user was a Practitioner, this value would be Practitioner/123 |
userPractitionerId |
OPTIONAL | Yes | string | The id of the current user, if current user is represented by the FHIR Practitioner resource. |
userPractitionerRoleId |
OPTIONAL | Yes | string | The id of the current user, if current user is represented by the FHIR PractitionerRole resource. |
userPatientId |
OPTIONAL | Yes | string | The id of the current user, if current user is represented by the FHIR Patient resource. |
userRelatedPersonId |
OPTIONAL | Yes | string | The id of the current user, if current user is represented by the FHIR Patient resource. |
patientId |
REQUIRED | Yes | string | The FHIR Patient.id of the current patient in context |
encounterId |
OPTIONAL | Yes | string | The FHIR Encounter.id of the current encounter in context |
Examples
"context":{
"userId" : "PractitionerRole/abc",
"userPractitionerRoleId" : "abc",
"userPractitionerId" : "123",
"patientId" : "1288992"
}
"context":{
"userId" : "Practitioner/123",
"patientId" : "1288992",
"encounterId" : "456"
}
Change Log
Version | Description |
---|---|
1.0 | Initial Release |
1.1 | Add PractitionerRole to resources for representing the current user. Add optional user*Id context fields to better enable prefetch. |
Catching up here and missed a CDS call where this was discussed, so forgive me if I'm rehashing. @mattvarghese, do you need the Practitioner id as well for other prefetch queries? I'm trying to understand if the discrete ids were all added because we expect multiple of them to be used simultaneously (e.g. Practitioner and PractitionerRole ids) or if it's just to get to the raw values for each type. Do your other FHIR queries support PractitionerRole id consistently or will they use a mixture of PractitionerRole and Practitioner ids from the context? I’m wondering if for provider-facing use-cases, is it a unique need to have both ids vs just a single id for a consumer-facing scenario? (Edit: removed invalid prefetch alternative; simplified comment to get more info about the need) |
@raj-wk : Apparently, the spec says context may not contain objects. Needs to be a flat dictionary; with top level properties only. @cfeltner : I'm open to a solution that keeps userId and adds new fields. @dennispatterson : I am a client. So I don't define prefetch template. I want to be able to support all types of prefetch templates, and I can imagine services which need practitioner role and also practitioner simultaneously in the prefetch. The examples I gave about I think are legitimate prefetch template queries that I should support as a client. @isaacvetter : all the new IDs are optional, and the original userId which we want to maybe deprecate, is the required one in what you're proposing. I don't know if that is the best way to address this. We should keep userId for backward compatibility, but we should plan to deprecate it in favor of resource type specific IDs? Maybe a "Conditional" requirement (like @jmandel suggested elsewhere) is more appropriate? |
@mattvarghese I was thinking along of the lines of someObject in the example -> https://cds-hooks.hl7.org/1.0/#examples and #377 |
Hey @yashaskram , @cfeltner , @mattvarghese , All - Overall, CDS Hooks' Of course, this proposed change would also affect essentially every hook. We definitely don't want to break existing implementations. Below is a possible, more elegant solution that places a bit more work on the CDS Client and doesn't break backward compatibility, keeps our over-the-wire message structure clean and still enables prefetch searches on the user. This is a modification to the base spec which leaves the patient-view (and every other hook) as is, by essentially creating a few special "prefetch variables" which aren't exactly explicitly provided in context. Prefetch tokensA prefetch token is a placeholder in a prefetch template that is replaced by information from the hook's context to construct the FHIR URL used to request the prefetch data. Prefetch tokens MUST be delimited by Individual hooks specify which of their *_ Prefetch tokens identifying the userA prefetch template enables a CDS Service to learn more about the current user through a FHIR read, like so:
or though a FHIR search:
A prefetch template may include any of the following prefetch tokens:
No single FHIR resource represents a user, rather Practitioner and PractitionerRole may be jointly used to represent a provider or other, and Patient or Person are used to represent a patient or their proxy. Hook definitions typically define a ... |
What about adding something like a "modifier" that allowed the prefetch token to specify which part of the Id to use?
|
just wondering with above if below is how prefetch look like?
Basically the idea is to avoid second level context references like context.userInfo.resourceType |
Hey @yashaskram, For Bryn's suggestion, yes, you've got the basic idea right. The problem is that there's no value provided by If
Of course, if
Both the |
Recurring problem. This prefetch query template doesn't work, because
Possible solutions are well identified, above:
For the above query, #1 is appealing simple on the surface, e.g.:
but quickly fails for realistical pre-fetch queries, e.g.:
We should add #2. |
Seems like specifying the _include would be difficult with trying to keep it generic with {{userResourceType}}. For the 2nd sample pre-fetch query you specified it seems to be implying {{userResourceType}}=PractitionerRole. If the EHR supports both Practitioner and PractitionerRole, how would EHR know which to put in for {{userResourceType}}? Seems like using the separate {{userPractitionerId}} and {{userPractitionerRoleId}} would be more straight forward. Then 2 pre-fetch queries could be specified:
If the EHR did not support PractitionerRole, it would just not return that pre-fetch data. |
Thanks, @cfeltner. Agree with you. @dennispatterson points out that there could be a third option:
So, for read we wouldn't be changing anything, it would look like this:
and generate For search, it would look like this:
or
... This pattern does exist in FHIR as a search parameter modifier, perhaps created to better enable chaining. This approach has the benefit of being more general than the simple enumeration of user resource types, in that it would apply to any contextual prefetch token, e.g. The advantage of .2 is that it actually tightly targets the specific problem of Practitioner and PractitionerRole being tightly related and sometimes necessary at the same time; whereas, a CDS Service won't need both Practitioner and Patient as user as part of the same query. |
On the use of the accessor, wouldn't it be:
This syntax would seem to provide an easier pattern for the EHR to match against then specifying each resource type (e.g., context.userId:Practitioner, context.userId:PractitionerRole, context.userId:Patient, context.userId:RelatedPerson). Since only the Practitioner and PractitionerRole are likely to be needed at the same time perhaps another alternative is just to introduce a new context parameter userRoleId. This would seem to minimize the amount of changes since the meaning of userId would remain the same. |
I don't completely follow. The ":id" idea looks like Bryn's :resourceId proposal which we've already shown doesn't work when the raw id should only be used in queries where the appropriate type matches. e.g. "PractitionerRole?_id={{context.userId:id}}" but the userId was a Practitioner so there's a mismatch
Both of these quotes hone in on the fact that this isn't just a context field flexing between two distinct types. It's two separate values needed at the same time because PractitionerRole includes the Practitioner type data. When the spec walks through an example of a context with a full Patient resource, it says if there's a need to access a field on that resource, it should be added to the context as a new, separate attribute. Given that spec example, @cfeltner 's proposal may make sense for userRoleId as a new context parameter for the specific problem of accessing both ids.
|
@dennispatterson, I just want to confirm the proposal here. Are you suggesting that we just add a new context parameter "userRoleId"? This could help fetching both PractitionerRole and Practitioner (using _include) as you proposed. However, this still limits the pre-fetch ability to only get a Practitioner resource along with a PractitionerRole resource but not independently. While I don't see a use case yet where there is only a need of Practitioner but not PractitionerRole, we should still try to allow for more flexibility. Also, it appears that the pre-fetch should have both the "userId" and the _include query you have above for the CDS Hooks request to generate the "Practitioner" resource if the user doesn't happen to have a userRoleId. One additional thing I was confused with is the example with "context.userId:RelatedPerson". Are you proposing that we add this accessor capability as well on top of new context parameter "userRoleId"? |
@chandra-bala I think it was being recognized that when the user is a practitioner, two ids are needed in context in order to support the optional presence of the role. The userId would hold the practitioner id and the new field would hold the practitioner role id. Both available for independent prefetch queries that need them. The modifier option could be used to distinguish between personas, but since the practitioner persona can use two ids, the separate field assists with that use-case. |
Guys,
I don't think the above is true.
Note that the 80%+ use-case of provider-facing, remote CDS means that there will virtually always be a Practitioner/PractitionerRole id, (regardless of the hook and it's other context fields). I really think that the proposed PR is the simplest, least invasive and non-breaking addition. Could you please re-review? |
@isaacvetter The Doesn't introducing 1 new prefetch token seem simpler than introducing 4 new prefetch tokens? |
This is not really accurate as the userId is not a simple ID but rather is of the form
This will return whatever the resource it was pointing to originally (either Practitioner or PractitionerRole) which is decided by the CDS client. The CDS Service can't control which resource it wants to get with this form of This is one of the things that Isaac's proposal would address with resource specific pre-fetch tokens.
The one thing that I'm not sure about this approach is, what if the user doesn't have a roleId, how would the last query above would be handled? I'm imagining it won't return anything, however, should the CDS Service still have the existing pre-fetch query |
@chandra-bala
A possible prefetch template could be:
Since the context.userId can represent these different types, it would seem that the type of the logged in user would would dictate which would be used or if the hook implementation guide specified the type. If a provider is logged in, then the userId would be Practitioner/123. If a patient is logged in, the userId would be Patient/123. The userRoleId would be an optional prefetch token and would only be needed if the logged in user is a provider. |
Re-reviewing this thread and the proposed solutions, I agree with @isaacvetter 's proposed solution on the grounds that it's really the only solution that has addressed the real issue, which is that the resource type must be statically known in order for pre-fetch templates to be written effectively. In general, you have to know what kind of id you have in order to use it correctly in a pre-fetch template, and so Isaac's proposed solution of "hook-independent pre-fetch tokens" addresses that problem generally for all hooks, doesn't require modification of any existing hook definitions, and allows the solution to be solved without changing any existing behavior. |
Why is Chuck's example not "correct"? Paraphrasing the example:
If |
Again, this seems to suggest that we change the meaning of "userId" to only be of type
As explained above, Chuck's proposal solves the issue to send both Practitioner and PractitionerRole resources, only if we can definitively make userId a Even if we go with this solution and we do make that expectation from now on, this type of
The above query just can't use the
Based on all these reasons, I'm strongly leaning towards the hook-independent, resource specific pre-fetch tokens for the Ids and completely avoiding any new Ids of the form |
Apologies for misrepresenting @cfeltner's proposal as updated context rather than a prefetch token. I also agree with the statements made by @isaacvetter and @chandra-bala that we can't force existing implementations to stop using PractitionerRole within userId. My counter-proposal that Isaac shared was a modifier, only applicable for multi-type context fields (of which this is the first), that would allow using the id if only if it's a particular type, kind of like the prefetch templates. Example:
1 and 2 seem interchangeable to me, but 1 defines a pattern for multi-type fields instead of a list of specific tokens. However, the other goal I'm hoping to clarify is the desire to get the raw Practitioner id when the user is a PractitionerRole. It sounds like we're trying to say when the context.userId is PractitionerRole/123, we also want to support this prefetch:
Is that a must-have? That seems to beg for a separate context parameter, which is what I've been having a hard time with. That ship may have sailed when we made a design choice to add PractitionerRole into userId instead of starting out with allowing for both ids. If so, the token approach seems like the only way here without context redesign. |
I can't understand what you're saying here. If we say "context.xyz" is a string of the form
this context would:
... and of course different contexts could be created to meet other needs. The pattern feels pretty flexible. Does this make sense, or have I already said something confusing? Edit: I think I'm starting to understand the problem is th other direction, when you're going to practitioner role and you want to look up the associated practitioner. You can't quite do it without string manipulation. |
Yes, we have a business use case where this is exactly what we need. To avoid duplication of data in resources, we implemented the resources such that some details are only available in PractitionerRole (like specialty) and some details are only available in Pratitioner (like NPI). The only way for a CDS Service at this point to get both resources is:
And this is information that the service wants on every call. So, doing above impacts performance and not to forget any networking setup the service has to do with each CDS client to make such call backs to CDS Client's FHIR server. |
Hi Folks,
With the
patient-view
hook, there are three context variables:userId
,patientId
,encounterId
.Of these
patientId
andencounterId
are just IDs.However,
userId
is unique - it has the ResourceType and the ID separated by "/" in it.This causes problems.
For example, with
patientId
I can formulate a query like:patient?_id={{context.patientId}}
This is perfectly legitimate, and is sometimes necessary to convert a read into a search to add additional search parameters such as
_include
etc.However, if I try the same with
userId
, then say the query is as below and the ID is 123practitioner?_id={{context.userId}}
This leads to two problems:
practitioner?_id=Practitioner/123
and this just fails because of the repetition of the resource type!
Can we do something to address this?
/matt
The text was updated successfully, but these errors were encountered: