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

Add "Fill Fields" button to query page, add FHIR Server drop down, & collapse demo and test user journeys #2549

Merged
merged 23 commits into from
Sep 17, 2024

Conversation

m-goggins
Copy link
Collaborator

@m-goggins m-goggins commented Sep 12, 2024

PULL REQUEST

Summary

This PR removes the autofill functionality on the demo user journey (users now have to click on "Fill Fields") and removes the concept of a userJourney. I also had to add the Advanced -> FHIR Server drop down because there was no way for users to specify it otherwise.

I decided to keep the /query/test page so that folks can navigate to it, but it will look exactly the same as the demo journey. I figured that since we have sent the dibbs.cloud/tefca-viewer/query/test link to folks, we might not want to create a dead link.

Related Issue

Fixes #2527 & #2526

Acceptance Criteria

Right now, when a user goes to the query page, the fields are auto populated based on what they chose from the modal. We are removing the modal and would like to instead add a "fill fields" button, removing the autopopulation.

Additional Information

Anything else the review team should know?

Checklist

  • If this code affects the other scrum team, have they been notified? (In Slack, as reviewers, etc.)

@m-goggins m-goggins changed the title Add "Fill Fields" button to query page & collapse demo and test user journeys Add "Fill Fields" button to query page, add FHIR Server drop down, & collapse demo and test user journeys Sep 12, 2024
@m-goggins m-goggins marked this pull request as ready for review September 13, 2024 21:47
Copy link
Collaborator

@fzhao99 fzhao99 left a comment

Choose a reason for hiding this comment

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

Code overall looks good / works well so going to give an approval!

I have a bit of a concern that folks will miss the "fill fields" button after selecting the patient example case and move straight to filling out the patient info. A better pattern might be to keep the auto-fill but just move it into the listener function for when the patient example dropdown changes. Don't think it's too much of a blocker since that if it's actually an issue, it'll come up at the connectathon / is a pretty easy readd, but flagging for observation.
cc: @lg-king

Another larger UX pattern to consider here is that commonly, "advanced" or "show more" options are put at the end of the form rather than at the beginning. The reasoning is that since advanced options are usually "in addition" to the basic search params, they're only relevant when everything is filled out and there are still additional needs. Therefore, they're put at the end to extend the existing config options. Example off the top of my head is the below Wikipedia language selection option (it's a bit simplified since it's just a two-field form, but hopefully still illustrates the idea)

Screen.Recording.2024-09-16.at.9.42.34.AM.mov

There might be a higher level UX treatment we should do here to 1) move this / any future advanced search fields to the end of the form rather than the beginning and 2) have sort of border styling / other UI that would denote "this section are all the advanced search params" to differentiate the new options against the existing ones.

@m-goggins
Copy link
Collaborator Author

@fzhao99 Definitely agree that we should do a comprehensive review and move toward better UX practices in the future

Copy link
Collaborator

@robertandremitchell robertandremitchell left a comment

Choose a reason for hiding this comment

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

Screenshot 2024-09-16 at 11 22 42 AM

broadly looks good, but a funky thing I noticed: when I click from home page to the query page, the cancer patient dropdown did not auto-populate, nor was it available for me to select as a drop down.

@m-goggins
Copy link
Collaborator Author

@robertandremitchell should be fixed now that I added back Brandon's 'Select an Option' code 😅

Copy link
Collaborator

@bamader bamader left a comment

Choose a reason for hiding this comment

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

Looks mostly good, just some questions/concerns around data updating in the SearchForm re: queryType and a function invocation vs a useEffect.

@bamader
Copy link
Collaborator

bamader commented Sep 16, 2024

Hmm when I check out the branch and get the app running, I encounter failures when trying to fetch query valuesets. The posts all return empty.

@bamader
Copy link
Collaborator

bamader commented Sep 16, 2024

Update: no actual DB issues, I just forgot to put the connectionString back in 🙄

Copy link
Collaborator

@bamader bamader left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for making those changes!

@bamader
Copy link
Collaborator

bamader commented Sep 16, 2024

Oh did we want to investigate port vs connectionString now or just punt to later? If later, could we add that back in?

Copy link
Collaborator

@robertandremitchell robertandremitchell left a comment

Choose a reason for hiding this comment

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

currently getting this error when I click customize query button:

tefca-viewer-1  |   ▲ Next.js 14.2.4
tefca-viewer-1  |   - Local:        http://localhost:3000
tefca-viewer-1  |   - Network:      http://0.0.0.0:3000
tefca-viewer-1  | 
tefca-viewer-1  |  ✓ Starting...
tefca-viewer-1  |  ✓ Ready in 325ms
tefca-viewer-1  | Error retrieving query: Error: connect ECONNREFUSED ::1:5432
tefca-viewer-1  |     at /app/node_modules/pg-pool/index.js:45:11
tefca-viewer-1  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
tefca-viewer-1  |     at async p (/app/containers/tefca-viewer/.next/server/app/query/page.js:9:396)
tefca-viewer-1  |     at async /app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:16:418
tefca-viewer-1  |     at async rP (/app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:15:7978)
tefca-viewer-1  |     at async r9 (/app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:18:1139)
tefca-viewer-1  |     at async doRender (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1438:30)
tefca-viewer-1  |     at async cacheEntry.responseCache.get.routeKind (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1599:28)
tefca-viewer-1  |     at async NextNodeServer.renderToResponseWithComponentsImpl (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1507:28)
tefca-viewer-1  |     at async NextNodeServer.renderPageComponent (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1931:24) {
tefca-viewer-1  |   errno: -111,
tefca-viewer-1  |   code: 'ECONNREFUSED',
tefca-viewer-1  |   syscall: 'connect',
tefca-viewer-1  |   address: '::1',
tefca-viewer-1  |   port: 5432
tefca-viewer-1  | }
tefca-viewer-1  | Error: connect ECONNREFUSED ::1:5432
tefca-viewer-1  |     at /app/node_modules/pg-pool/index.js:45:11
tefca-viewer-1  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
tefca-viewer-1  |     at async p (/app/containers/tefca-viewer/.next/server/app/query/page.js:9:396)
tefca-viewer-1  |     at async /app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:16:418
tefca-viewer-1  |     at async rP (/app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:15:7978)
tefca-viewer-1  |     at async r9 (/app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:18:1139)
tefca-viewer-1  |     at async doRender (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1438:30)
tefca-viewer-1  |     at async cacheEntry.responseCache.get.routeKind (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1599:28)
tefca-viewer-1  |     at async NextNodeServer.renderToResponseWithComponentsImpl (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1507:28)
tefca-viewer-1  |     at async NextNodeServer.renderPageComponent (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1931:24) {
tefca-viewer-1  |   errno: -111,
tefca-viewer-1  |   code: 'ECONNREFUSED',
tefca-viewer-1  |   syscall: 'connect',
tefca-viewer-1  |   address: '::1',
tefca-viewer-1  |   port: 5432,
tefca-viewer-1  |   digest: '281846365'
tefca-viewer-1  | }
tefca-viewer-1  | Error retrieving query: Error: connect ECONNREFUSED ::1:5432
tefca-viewer-1  |     at /app/node_modules/pg-pool/index.js:45:11
tefca-viewer-1  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
tefca-viewer-1  |     at async p (/app/containers/tefca-viewer/.next/server/app/query/page.js:9:396)
tefca-viewer-1  |     at async /app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:16:418
tefca-viewer-1  |     at async rP (/app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:15:7978)
tefca-viewer-1  |     at async r9 (/app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:18:1139)
tefca-viewer-1  |     at async doRender (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1438:30)
tefca-viewer-1  |     at async cacheEntry.responseCache.get.routeKind (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1599:28)
tefca-viewer-1  |     at async NextNodeServer.renderToResponseWithComponentsImpl (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1507:28)
tefca-viewer-1  |     at async NextNodeServer.renderPageComponent (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1931:24) {
tefca-viewer-1  |   errno: -111,
tefca-viewer-1  |   code: 'ECONNREFUSED',
tefca-viewer-1  |   syscall: 'connect',
tefca-viewer-1  |   address: '::1',
tefca-viewer-1  |   port: 5432
tefca-viewer-1  | }
tefca-viewer-1  | Error: connect ECONNREFUSED ::1:5432
tefca-viewer-1  |     at /app/node_modules/pg-pool/index.js:45:11
tefca-viewer-1  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
tefca-viewer-1  |     at async p (/app/containers/tefca-viewer/.next/server/app/query/page.js:9:396)
tefca-viewer-1  |     at async /app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:16:418
tefca-viewer-1  |     at async rP (/app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:15:7978)
tefca-viewer-1  |     at async r9 (/app/containers/tefca-viewer/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:18:1139)
tefca-viewer-1  |     at async doRender (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1438:30)
tefca-viewer-1  |     at async cacheEntry.responseCache.get.routeKind (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1599:28)
tefca-viewer-1  |     at async NextNodeServer.renderToResponseWithComponentsImpl (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1507:28)
tefca-viewer-1  |     at async NextNodeServer.renderPageComponent (/app/containers/tefca-viewer/node_modules/next/dist/server/base-server.js:1931:24) {
tefca-viewer-1  |   errno: -111,
tefca-viewer-1  |   code: 'ECONNREFUSED',
tefca-viewer-1  |   syscall: 'connect',
tefca-viewer-1  |   address: '::1',
tefca-viewer-1  |   port: 5432,
tefca-viewer-1  |   digest: '281846365'
tefca-viewer-1  | }

I think this has to do with the fields not getting filled. I fixed this in a prior merge last week that seems to have fallen out, but the principle of it is this bit of code: https:/CDCgov/phdi/blob/239e4486200bc9af867f89df7fc2432681892d80/containers/tefca-viewer/src/app/query/components/SearchForm.tsx#L138C2-L167C8

 async function HandleSubmit(
    event?: React.FormEvent<HTMLFormElement>,
    customizeQuery = false,
  ) {
    if (!localUseCase || !fhirServer) {
      console.error("Use case and FHIR server are required.");
      return;
    }
    if (event) {
      event.preventDefault();
    }
    setLoading(true);

    const originalRequest = {
      first_name: firstName,
      last_name: lastName,
      dob: dob,
      mrn: mrn,
      fhir_server: fhirServer,
      use_case: localUseCase,
      phone: FormatPhoneAsDigits(phone),
    };
    setOriginalRequest(originalRequest);
    const queryResponse = await UseCaseQuery(originalRequest);
    setUseCaseQueryResponse(queryResponse);
    // Check if it's a customize query or a standard search
    if (customizeQuery) {
      if (queryResponse) {
        setMode("customize-queries");
      }
    } else {
      // Normal flow: switch modes based on the query response
      if (!queryResponse.Patient || queryResponse.Patient.length === 0) {
        setMode("no-patients");
      } else if (queryResponse.Patient.length === 1) {
        setMode("results");
      } else {
        setMode("multiple-patients");
      }
    }
    setLoading(false);
  }

  useEffect(() => {
    window.scrollTo(0, 0);
  }, []);

@bamader
Copy link
Collaborator

bamader commented Sep 17, 2024

@robertandremitchell I don't think that's what the error is. I believe it's actually your machine being unable to connect to the DB where the valuesets are stored. This PR deletes the ConnectionString parameter from the DBConfig, which is how my machine connects to the DB. If you add that missing line back in, rebuild, and then re-test, I'd bet anything this error disappears. I don't have the useEffect code present and I was seeing the same error, but I put the ConnectionString back in and everything works fine now. So the takeaway might be that we actually just need to leave that parameter in DBConfig.

Copy link
Collaborator

@robertandremitchell robertandremitchell left a comment

Choose a reason for hiding this comment

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

per convo in slack, this works for me:

// Load environment variables from tefca.env and establish a Pool configuration
dotenv.config({ path: "tefca.env" });
const dbConfig: PoolConfig = {
  // user: process.env.POSTGRES_USER,
  // password: process.env.POSTGRES_PASSWORD,
  // host: process.env.POSTGRES_HOST,
  // port: Number(process.env.POSTGRES_PORT),
  database: process.env.POSTGRES_DB,
  connectionString: process.env.DATABASE_URL,
  max: 10, // Maximum # of connections in the pool
  idleTimeoutMillis: 30000, // A client must sit idle this long before being released
  connectionTimeoutMillis: 2000, // Wait this long before timing out when connecting new client
};
const dbClient = new Pool(dbConfig);

@m-goggins m-goggins added this pull request to the merge queue Sep 17, 2024
Merged via the queue into main with commit e2b6520 Sep 17, 2024
10 checks passed
@m-goggins m-goggins deleted the marcelle/2527/remove-autopop branch September 17, 2024 15:19
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.

Remove autopopulation of query fields
4 participants