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

Filter by conditions in eCR Library (frontend) #2981

Merged
merged 43 commits into from
Dec 12, 2024

Conversation

angelathe
Copy link
Collaborator

@angelathe angelathe commented Dec 3, 2024

PULL REQUEST

Summary

Main story:

  • Create Filters and FilterReportableConditions component

  • Clicking the Filter button should open the Filter by Reportable Conditions combo box, and vice versa.

    • Combo box should be populated with the available conditions, populated by the /conditions endpoint query
  • Scrollable section should be available for the condition checkboxes, while the Select All checkbox and Apply filter button should remain static.

  • When a user filters for conditions and clicks Apply filter:

    • URL query should populate with the =condition param that reflects the conditions selected to filter on.
    • The eCRs displayed should update to reflect the new filters. They should have at least one of the reportable conditions selected to filter on.
    • The tag displaying number of conditions to filter on updates.
  • When a user checks boxes for conditions but closes button before clicking 'Apply fliter', the filter conditions and the # conditions tag should reset to their most recent "Applied" state.

  • Opening and closing the filter combo box when filters are applied should not change the conditions filtered on, and should maintain the state of which conditions are checkmarked.

  • Select all/Deselect all - By default, all conditions are selected, the button displays "Deselect all," and the URL search query does not have the condition param. When no conditions are selected, the button changes to "Select all," and only eCRs with no conditions are displayed (though none should exist).

  • Query persists over a page reload

Misc:

  • CSS styling to reflect the Figma Designs
  • Add snapshot and unit tests
  • Update backend listEcrDataService & tests so that if no conditions are given, then only eCRs with no reportable conditions are returned. If filterConditions is undefined, it defaults to including all conditions.

Related Issue

Fixes #2751

Design Review

Link to Figma design in Dev mode
Link to Figma prototype

Acceptance Criteria

  • Condition filter is implemented according to designs
  • Condition list is populated by a custom backend query (see FILTER: Query for all unique conditions #2749)
  • When user checks boxes for conditions and searches, only eCRs with those conditions in the RR are shown
  • Reset button appears when filtering is applied Wrote a separate ticket for this

Additional Information

This ticket was getting big, so I'm breaking out a few other related tickets from this issue:

- The filter by conditions combo box should default to all conditions being selected. If no conditions are selected, only eCRs with no reportable conditions should appear. Added in this ticket
- Modify button styling when in regular/hover/applied filter states Added in this ticket

  • Adding a No Conditions Reported option
  • Adding a filter reset button
  • Clicking anywhere outside of the button should close the button
  • Pre-existing bug: Empty space below the eCR Library?

Checklist

  1. Pull down branch and open the eCR Library
  2. Test that the interactions with the filter conditions button and combo box follow the scenarios above (and any others you may think of)

@angelathe angelathe self-assigned this Dec 3, 2024
@angelathe angelathe changed the title [DRAFT] Filter by conditions in eCR Library (front-end) Filter by conditions in eCR Library (frontend) Dec 4, 2024
@angelathe angelathe marked this pull request as ready for review December 4, 2024 04:59
@angelathe
Copy link
Collaborator Author

@ashton-skylight Just thought of another question -- should the filter conditions combo box default to all conditions selected? Right now, it defaults to having selected no conditions (which acts the same as all conditions being selected), which may not make as much sense to users.

@ashton-skylight
Copy link
Collaborator

@angelathe Yes, that's correct. If that's the interaction, I'm making revised designs for the initial state and how users continue through the dropdown. This will probably be a new ticket.

@angelathe
Copy link
Collaborator Author

angelathe commented Dec 4, 2024

@angelathe Yes, that's correct. If that's the interaction, I'm making revised designs for the initial state and how users continue through the dropdown. This will probably be a new ticket.

Great, thanks so much @ashton-skylight! I'll add this to the list of tickets that I'll write 👌 BTW, I ended up adding this change to the ticket.

Copy link

codecov bot commented Dec 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.67%. Comparing base (2bd0e03) to head (3f9637a).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2981      +/-   ##
==========================================
- Coverage   86.00%   85.67%   -0.34%     
==========================================
  Files         162       25     -137     
  Lines       10855     1424    -9431     
==========================================
- Hits         9336     1220    -8116     
+ Misses       1519      204    -1315     
Flag Coverage Δ
fhir-converter ?
ingestion ?
message-parser ?
message-refiner ?
orchestration 85.67% <ø> (ø)
record-linkage ?
trigger-code-reference ?
validation ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 137 files with indirect coverage changes

@mcmcgrath13
Copy link
Collaborator

mcmcgrath13 commented Dec 11, 2024

@angelathe it looks like the extra space the filters take up is offsetting the footer placement

@angelathe
Copy link
Collaborator Author

@angelathe it looks like the extra space the filters take up is offsetting the footer placement

Good call, just updated height-ecrLibrary

Copy link
Collaborator

@mcmcgrath13 mcmcgrath13 left a comment

Choose a reason for hiding this comment

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

looking good from the technical side to me!

@ashton-skylight
Copy link
Collaborator

ashton-skylight commented Dec 12, 2024

Reviewed w/ Angela! Noting for future tickets - we'll add a ticket for the "Apply Filter" to be inactive if no condition is selected

@angelathe angelathe enabled auto-merge December 12, 2024 21:33
@angelathe angelathe added this pull request to the merge queue Dec 12, 2024
Merged via the queue into main with commit d5f9948 Dec 12, 2024
16 checks passed
@angelathe angelathe deleted the angela/2751-condition-frontend branch December 12, 2024 21:39
BobanL added a commit that referenced this pull request Dec 13, 2024
* First pass, filter conditions functionality

* add select/deselect all functionality

* update some styling, maintain checkbox state when toggling filter button

* styling updates, wip

* checkbox color, add icon, add uswds sprite.svg to assets

* adjust padding to fix checkbox focus ring cut off

* fix icon not displaying by adding static file route

* fix unintentional scrolling bug

* update filter row top border

* wip, add comments, decompose conditions filter to separate const

* fix scrolling bug by adding position-relative

* add snapshot and unit tests

* add JSDocs

* remove css classes and use utilities instead

* update snapshot test

* update select all/deselect all functionality s.t. default is all conditions checked, update tests

* update so that filters reset if clicking off filter before clicking the Apply button, add tests

* update basepath so it works in prod

* update tests

* update styles in diff button states, update icon size, make capitalization consistent

* Remove log

Co-authored-by: Mary McGrath <[email protected]>

* use as form/fieldset, update sync state bug, update tests

* remove manual checkboxing for select all, lets react handle the render

* rework state management, update tests

* code review changes, minor

* query should persist over a reload

* update backend so default (all conditions) would leave out condition param from URL query, add/update tests

* use import for icon

* Update base_path env var name

Co-authored-by: Boban <[email protected]>

* update snapshot test

* re-use resetFilterConditions

* one more nit

* update ecr library height to accommodate fiter bar

* update env var name for base path

---------

Co-authored-by: Mary McGrath <[email protected]>
Co-authored-by: Boban <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Dec 19, 2024
* permanently set base path in ecr-viewer

* Rename middleware to be a .ts file

* replace a tags with Next's Link tag

* remove title from Header link

* retype base_path to be a string

* fix metrics test

* update header snapshot test

* address middleware tests

* add back button to retrieval failed

* remove unused rewrite in next config

* update matcher

* Prevent access to /api/fhir-data

* [pre-commit.ci] auto fixes from pre-commit hooks

* fix url in readme

* remove accident commited next-runtime-env

* [pre-commit.ci] auto fixes from pre-commit hooks

* Filter by conditions in eCR Library (frontend) (#2981)

* First pass, filter conditions functionality

* add select/deselect all functionality

* update some styling, maintain checkbox state when toggling filter button

* styling updates, wip

* checkbox color, add icon, add uswds sprite.svg to assets

* adjust padding to fix checkbox focus ring cut off

* fix icon not displaying by adding static file route

* fix unintentional scrolling bug

* update filter row top border

* wip, add comments, decompose conditions filter to separate const

* fix scrolling bug by adding position-relative

* add snapshot and unit tests

* add JSDocs

* remove css classes and use utilities instead

* update snapshot test

* update select all/deselect all functionality s.t. default is all conditions checked, update tests

* update so that filters reset if clicking off filter before clicking the Apply button, add tests

* update basepath so it works in prod

* update tests

* update styles in diff button states, update icon size, make capitalization consistent

* Remove log

Co-authored-by: Mary McGrath <[email protected]>

* use as form/fieldset, update sync state bug, update tests

* remove manual checkboxing for select all, lets react handle the render

* rework state management, update tests

* code review changes, minor

* query should persist over a reload

* update backend so default (all conditions) would leave out condition param from URL query, add/update tests

* use import for icon

* Update base_path env var name

Co-authored-by: Boban <[email protected]>

* update snapshot test

* re-use resetFilterConditions

* one more nit

* update ecr library height to accommodate fiter bar

* update env var name for base path

---------

Co-authored-by: Mary McGrath <[email protected]>
Co-authored-by: Boban <[email protected]>

* fix base path for filters

* [pre-commit.ci] auto fixes from pre-commit hooks

* update filter test

* Update middleware.ts

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Angela The <[email protected]>
Co-authored-by: Mary McGrath <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jan 2, 2025
* ignore .next in jest

* mute console error from jest results

* rename app_env to nbs_auth

docker compose --env-file .env.local --profile "*" up to test with different configs

* Remove app env check around s3 client

* add collection variables for env groups

* fix typos in .env files

* create single azure-storage image that contains both azurite and azure cli

* add additional chaining to run specific collections

* fix local-dev command to use collection

* update create-seed-data to use different configs depending on metadata

* rename schemas

* update .env.sample to include collection

* remove azure-storage-init from orchestration

* update orchestration docker compose to add collection

* add metadata db schema in the env variable

* remove undefined values

* remove nbs_auth value

* Remove redundant variables

* change to aws non integrated and add timeout

* add docker ignore

* add more to docker ignore

* Comment all from .env

* fix tests that were relying on .env file

* make metadata variables optional

* pull out saveFhirMetadata into it's own function

* delete .env

* Use PersistenceResponse instead of Nextjs response

* console log error messages

* Do not close pool when done with mssql

https://www.npmjs.com/package/mssql#advanced-pool-management:~:text=Also%20notice%20that,of%20the%20script.

* increase sqlserver timeout to 30 s

* don't return anything from save ecr data query

* be more specific with env variable names

* remove return for ecr data

* update .env.sample variables

* remove .env

* Remove app_env from dockerfile

* make metadata env variable optional

* fix tests without .env

* move getDb from top level to be inside methods

* add dockerignore

* Remove NEXT_PUBLIC_BASEPATH since it is determined at build time anyways

* fix first batch of seed script conflicts

* use rewrite instead of redirect to keep url

* set pool min to 1

* fix middleware test after changes to middleware rewrite to error page

* replace dotenvx with next runtime env and instrumentation

* remove console logs

* update collection in orchestration

* change docker compose collection to default to sql server non integrated

* Rename PersistenceResponse to SaveResponse

* Remove build from convert-seed-data

* remove useless console.error logs

* update instrumentation tests

* fix middleware

* permanently set base path in ecr-viewer

* Rename middleware to be a .ts file

* replace a tags with Next's Link tag

* remove title from Header link

* retype base_path to be a string

* fix metrics test

* update header snapshot test

* address middleware tests

* add back button to retrieval failed

* remove unused rewrite in next config

* update matcher

* Prevent access to /api/fhir-data

* [pre-commit.ci] auto fixes from pre-commit hooks

* fix url in readme

* remove accident commited next-runtime-env

* [pre-commit.ci] auto fixes from pre-commit hooks

* use next-runtime-env to access NEXT_PUBLIC_NON_INTEGRATED

* [pre-commit.ci] auto fixes from pre-commit hooks

* use proper variable name for middleware test

* simplify clear-local command

* rename collection to CONFIG_NAME

* update design script to handle config variable

* fix package.json

* update lighthouse startup

* add dummy keys to .env.sample

* Filter by conditions in eCR Library (frontend) (#2981)

* First pass, filter conditions functionality

* add select/deselect all functionality

* update some styling, maintain checkbox state when toggling filter button

* styling updates, wip

* checkbox color, add icon, add uswds sprite.svg to assets

* adjust padding to fix checkbox focus ring cut off

* fix icon not displaying by adding static file route

* fix unintentional scrolling bug

* update filter row top border

* wip, add comments, decompose conditions filter to separate const

* fix scrolling bug by adding position-relative

* add snapshot and unit tests

* add JSDocs

* remove css classes and use utilities instead

* update snapshot test

* update select all/deselect all functionality s.t. default is all conditions checked, update tests

* update so that filters reset if clicking off filter before clicking the Apply button, add tests

* update basepath so it works in prod

* update tests

* update styles in diff button states, update icon size, make capitalization consistent

* Remove log

Co-authored-by: Mary McGrath <[email protected]>

* use as form/fieldset, update sync state bug, update tests

* remove manual checkboxing for select all, lets react handle the render

* rework state management, update tests

* code review changes, minor

* query should persist over a reload

* update backend so default (all conditions) would leave out condition param from URL query, add/update tests

* use import for icon

* Update base_path env var name

Co-authored-by: Boban <[email protected]>

* update snapshot test

* re-use resetFilterConditions

* one more nit

* update ecr library height to accommodate fiter bar

* update env var name for base path

---------

Co-authored-by: Mary McGrath <[email protected]>
Co-authored-by: Boban <[email protected]>

* fix base path for filters

* update config name in create-seed-data

* move start transaction in the if statement for saveMetadataToSqlServer

* remove random transaction from conditons api sql server

* move start transaction in the if statement for saveMetadataToSqlServer

* remove random transaction from conditons api sql server

* [pre-commit.ci] auto fixes from pre-commit hooks

* refactor service for condition to simplify logic

* [pre-commit.ci] auto fixes from pre-commit hooks

* update filter test

* update jsdocs

* add nbs pub key to environment variables

* remove NBS_AUTH variable from local environment setup

* fix broken merge

* fix where clause fpor date statement sql server

* Update .env.sample to replace mssql server_server with server_host

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Angela The <[email protected]>
Co-authored-by: Mary McGrath <[email protected]>
gordonfarrell pushed a commit that referenced this pull request Jan 8, 2025
* ignore .next in jest

* mute console error from jest results

* rename app_env to nbs_auth

docker compose --env-file .env.local --profile "*" up to test with different configs

* Remove app env check around s3 client

* add collection variables for env groups

* fix typos in .env files

* create single azure-storage image that contains both azurite and azure cli

* add additional chaining to run specific collections

* fix local-dev command to use collection

* update create-seed-data to use different configs depending on metadata

* rename schemas

* update .env.sample to include collection

* remove azure-storage-init from orchestration

* update orchestration docker compose to add collection

* add metadata db schema in the env variable

* remove undefined values

* remove nbs_auth value

* Remove redundant variables

* change to aws non integrated and add timeout

* add docker ignore

* add more to docker ignore

* Comment all from .env

* fix tests that were relying on .env file

* make metadata variables optional

* pull out saveFhirMetadata into it's own function

* delete .env

* Use PersistenceResponse instead of Nextjs response

* console log error messages

* Do not close pool when done with mssql

https://www.npmjs.com/package/mssql#advanced-pool-management:~:text=Also%20notice%20that,of%20the%20script.

* increase sqlserver timeout to 30 s

* don't return anything from save ecr data query

* be more specific with env variable names

* remove return for ecr data

* update .env.sample variables

* remove .env

* Remove app_env from dockerfile

* make metadata env variable optional

* fix tests without .env

* move getDb from top level to be inside methods

* add dockerignore

* Remove NEXT_PUBLIC_BASEPATH since it is determined at build time anyways

* fix first batch of seed script conflicts

* use rewrite instead of redirect to keep url

* set pool min to 1

* fix middleware test after changes to middleware rewrite to error page

* replace dotenvx with next runtime env and instrumentation

* remove console logs

* update collection in orchestration

* change docker compose collection to default to sql server non integrated

* Rename PersistenceResponse to SaveResponse

* Remove build from convert-seed-data

* remove useless console.error logs

* update instrumentation tests

* fix middleware

* permanently set base path in ecr-viewer

* Rename middleware to be a .ts file

* replace a tags with Next's Link tag

* remove title from Header link

* retype base_path to be a string

* fix metrics test

* update header snapshot test

* address middleware tests

* add back button to retrieval failed

* remove unused rewrite in next config

* update matcher

* Prevent access to /api/fhir-data

* [pre-commit.ci] auto fixes from pre-commit hooks

* fix url in readme

* remove accident commited next-runtime-env

* [pre-commit.ci] auto fixes from pre-commit hooks

* use next-runtime-env to access NEXT_PUBLIC_NON_INTEGRATED

* [pre-commit.ci] auto fixes from pre-commit hooks

* use proper variable name for middleware test

* simplify clear-local command

* rename collection to CONFIG_NAME

* update design script to handle config variable

* fix package.json

* update lighthouse startup

* add dummy keys to .env.sample

* Filter by conditions in eCR Library (frontend) (#2981)

* First pass, filter conditions functionality

* add select/deselect all functionality

* update some styling, maintain checkbox state when toggling filter button

* styling updates, wip

* checkbox color, add icon, add uswds sprite.svg to assets

* adjust padding to fix checkbox focus ring cut off

* fix icon not displaying by adding static file route

* fix unintentional scrolling bug

* update filter row top border

* wip, add comments, decompose conditions filter to separate const

* fix scrolling bug by adding position-relative

* add snapshot and unit tests

* add JSDocs

* remove css classes and use utilities instead

* update snapshot test

* update select all/deselect all functionality s.t. default is all conditions checked, update tests

* update so that filters reset if clicking off filter before clicking the Apply button, add tests

* update basepath so it works in prod

* update tests

* update styles in diff button states, update icon size, make capitalization consistent

* Remove log

Co-authored-by: Mary McGrath <[email protected]>

* use as form/fieldset, update sync state bug, update tests

* remove manual checkboxing for select all, lets react handle the render

* rework state management, update tests

* code review changes, minor

* query should persist over a reload

* update backend so default (all conditions) would leave out condition param from URL query, add/update tests

* use import for icon

* Update base_path env var name

Co-authored-by: Boban <[email protected]>

* update snapshot test

* re-use resetFilterConditions

* one more nit

* update ecr library height to accommodate fiter bar

* update env var name for base path

---------

Co-authored-by: Mary McGrath <[email protected]>
Co-authored-by: Boban <[email protected]>

* fix base path for filters

* update config name in create-seed-data

* move start transaction in the if statement for saveMetadataToSqlServer

* remove random transaction from conditons api sql server

* move start transaction in the if statement for saveMetadataToSqlServer

* remove random transaction from conditons api sql server

* [pre-commit.ci] auto fixes from pre-commit hooks

* refactor service for condition to simplify logic

* [pre-commit.ci] auto fixes from pre-commit hooks

* update filter test

* update jsdocs

* add nbs pub key to environment variables

* add playwright init and tests

* ignore e2e for jest

* don't skip e2e test

* set working directory for playwright tests

* convert data and start server for e2e tests

* fix test command

* set profile for logs

* debug s3 client performance

* update images

* remove snapshots

* replace snapshots with accessibility tests

* opt into fully parallel tests

* remove test:e2e:ci and snapshot related

* increase timeout for for first test

* delete cypress

* Remove cypress config

* Update .dockerignore to replace 'cypress' with 'e2e'

* Remove Cypress-related scripts and dependencies from package.json

* Revert "debug s3 client performance"

This reverts commit bd080ec.

* Remove unnecessary dependency in e2e-tests workflow

* change base URL in playwright config

* remove cypress from gitignore

* remove makefile

* update server_host to be localhost

* update test:e2e command name

* fix broken tests and reorder them

* remove cypress commands

* rename .yml to .yaml

* remove comments from playwright config

* add testing section for readme

* update playwright config to include webserver

* update the tests to be more clear

* remove extra playwright browser install

* update playwright test command

* actually don't reuse, but remove starting service

* Revert "update server_host to be localhost"

This reverts commit 294a090.

* Delete another cypress folder

* update read me to include assumption around stored data

* Update containers/ecr-viewer/README.md

Co-authored-by: Mary McGrath <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit hooks

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Angela The <[email protected]>
Co-authored-by: Mary McGrath <[email protected]>
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.

FILTER: Frontend condition filtering
5 participants