fix!: Add Support for multiValueQueryStringParameters #181
Conversation
test/lambda-event.test.ts
Outdated
| multiValueQueryStringParameters: { | ||
| _tag: [ | ||
| 'http://lifeomic.com/fhir/questionnaire-type|survey-form', | ||
| 'http://lifeomic.com/fhir/dataset|0bb18fef-4e2d-4b91-a623-09527265a8b3', | ||
| 'http://lifeomic.com/fhir/primary|0343bfcf-4e2d-4b91-a623-095272783bf3', | ||
| ], | ||
| }, |
There was a problem hiding this comment.
I left the array values in queryStringParameters because I assume some service or workflow relied on this. In my testing, having duplicate parameters across queryStringParameters and multiValueQueryStringParameters made it so the values were parsed as an array when included in multiValueQueryStringParameters and were parsed as an empty string when only included in queryStringParameters
There was a problem hiding this comment.
Looking at how serverless-http parses the values, I think this is a mistake. I will switch to make sure they aren't duplicated.
sternetj
force-pushed
the
handle-multiValueQueryStringParameters
branch
from
9059383 to
44b021b
Compare
sternetj
changed the title
| @@ -31,12 +31,14 @@ test('Can parse URLs with duplicate parameters', () => { | |||
| httpMethod: 'GET', | |||
| path: '/lifeomic/dstu3/Questionnaire', | |||
| queryStringParameters: { | |||
| pageSize: '25', | |||
| }, | |||
| multiValueQueryStringParameters: { | |||
There was a problem hiding this comment.
Would this not be a breaking change in alpha since queryStringParameters will now never include any array query params?
There was a problem hiding this comment.
Yeah that does seem breaking. I'll update it.
BREAKING CHANGE: multi-value query params are now sent using the multiValueQueryStringParameters request property.
|
Merging this PR will result in a major version bump. Created by lifeomic-probot (Enforce Semantic Commits) |
sternetj
changed the title
No description provided.