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

Define a custom search parameter? #109

Closed
bryanvanhoorn opened this issue Dec 5, 2018 · 8 comments
Closed

Define a custom search parameter? #109

bryanvanhoorn opened this issue Dec 5, 2018 · 8 comments

Comments

@bryanvanhoorn
Copy link

Is it possible to define a custom search parameter for a particular profile? In my case, I would like to add an endDateTime as a parameter when searching for Slots. However, this parameter is getting removed by the arg sanitizer because it is not listed in the FHIR spec. Is there a way to specify that I would like to allow this custom parameter through? Thank you!

@jonterrylee
Copy link
Contributor

jonterrylee commented Dec 5, 2018

@bryanvanhoorn

I've added my initially support for custom arguments. We're still thinking this through so this may not be the final implementation.

Check out this branch

Specifically this file slot.arguments.js

Let me know what you think.

@Robert-W
Copy link
Contributor

@jonterrylee Where are we on this? If we can support this and we want to merge this in, we should add this to #119 and get some documentation on this.

@dominathan
Copy link
Collaborator

I will need this to not be working off my fork as I have custom params.

@elementechemlyn
Copy link
Contributor

I've been looking at custom arguments myself recently - and thinking that it would be useful to remove them as well as add additional ones.

It doesn't quite seem right to "sanitise" the arguments to a list that may not be supported by the specific service implementation so I was wondering whether adding an optional SearchParameters object to the service may be the way to go? The configureResourceRoutes function in route_setter could then use those if they exist or fallback to the builtin set if not? It would also keep local customisation out of the main code tree.

@jonterrylee
Copy link
Contributor

Hi @elementechemlyn, the original intention of the sanitize params were to help with security and conform to the standards. It turns out that more people are planning to customize this for a custom implementation so this is actually making things harder.

We were thinking of only sanitizing the params and not removing them but adding them to a optional list sounds better and would let the implementor know what is part of the standard and what is not.

I'll post your solution to the team and see what they think.

Thanks!!

@elementechemlyn
Copy link
Contributor

I've implemented this in my fork. If the team agree it's sensible I can submit a PR (although I can't see a develop branch at the moment?).

@Robert-W
Copy link
Contributor

Robert-W commented Jan 2, 2019

Hey @elementechemlyn, sorry about that. We are a little behind because of the holidays on updating documentation. We have removed develop recently since we are now publishing this to npm and it was just causing things to be out of sync (patches pushed directly to master and getting overriden when develop was merged back in). This means installing and versioning is much easier and safer.

You can submit your PR to master, we are going to change the documentation soon to encourage people to install from npm and not master where we can more easily control releases.

We have a priority task today to go through the Wiki and Contributors guide and get all these things updated.

@Robert-W
Copy link
Contributor

Robert-W commented May 8, 2019

Closing for now, feel free to reopen or post a question on StackOverflow with the tags node-fhir-server-core or javascript-fhir.

@Robert-W Robert-W closed this as completed May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants