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

RN-601: Added ability to reference query parameters in viz-builder expressions #4067

Merged
merged 1 commit into from
Sep 13, 2022

Conversation

rohan-bes
Copy link
Collaborator

@rohan-bes rohan-bes commented Aug 2, 2022

Issue RN-601:

I've wanted this feature for a while, as there are occasional cases where it'd be really handy to reference the original request parameters of the viz.

I see this as also paving the way for being able to have vizes take additional parameters (eg. A control to filter by Drug Type) which would be really cool.

It's also a pre-requisite for moving 'fetch' into the transform steps, which we want to have in place prior to moving on with the Data Tables epic

@rohan-bes rohan-bes force-pushed the rn-601-request-params-in-vb branch from f3aced7 to 234da67 Compare September 6, 2022 05:03
Copy link
Contributor

@biaoli0 biaoli0 left a comment

Choose a reason for hiding this comment

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

Very handy function! Thanks for adding it. Just got a question about the name of @params.
I hope we can present and even modify request params in viz builder somedays.

sumWhereMatchingOrgUnit:
'=sum(where(f(@otherRow) = eq($organisationUnit, @otherRow.organisationUnit)).BCD1)',
tableLength: '=length(@table)',
requestParam: '= @params.animal',
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel @params could not explain it is from request , which could be a bit confusing when we have @params.organisationUnitCodes and $organisationUnitCode at the same time. Perhaps we use @request or @requestParams? What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, initially I did have this as @request but then decided to change it to @params. Basically that decision was motivated by the data-tables re-architecture. We're planning on having each data-table define it's 'Parameters' (eg. entity, startDate, endDate, etc.) so that's why I went with this term, as eventually all reports are going to be converted into data-tables.

Also, for what it's worth I feel the fact that the it's a 'request' is some of an implementation detail that we should hide from the user. They don't know that we use web-requests to communicate, nor should they

@rohan-bes rohan-bes merged commit a122f65 into dev Sep 13, 2022
@rohan-bes rohan-bes deleted the rn-601-request-params-in-vb branch September 13, 2022 06:43
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

Successfully merging this pull request may close these issues.

2 participants