Skip to content

[ BB2-1275 ] FIX server & client issues preventing start up.#27

Merged
dtisza1 merged 5 commits intomasterfrom
dtisza1/BB2-1133-fix-docker-startup
Apr 26, 2022
Merged

[ BB2-1275 ] FIX server & client issues preventing start up.#27
dtisza1 merged 5 commits intomasterfrom
dtisza1/BB2-1133-fix-docker-startup

Conversation

@dtisza1
Copy link
Copy Markdown
Contributor

@dtisza1 dtisza1 commented Apr 19, 2022

JIRA Ticket:
BB2-1275

User Story or Bug Summary:

This fixes a few issues with being able to start up the server & client components of the sample app.

This is affecting the current "master" branch.

What Does This PR Do?

The following is a summary of the changes in this PR

  • Add missing res parameter to the getBenefitData() function in server/src/routes/Authorize.ts. TY James for fix info!

  • Fix missing return for getBenefitData() function in server/src/routes/Data.ts.

  • Change module setting from esnext to commonjs in server/tsconfig.json to resolve the following error (TY James for fix info!):

    server_1  | (node:29) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
    server_1  | (Use `node --trace-warnings ...` to show where the warning was created)
    server_1  | /server/src/index.ts:1
    server_1  | import './pre-start'; // Must be the first import
    server_1  | ^^^^^^
    server_1  |
    server_1  | SyntaxError: Cannot use import statement outside a module
    
  • Add resolutions setting to client/package.json to resolve the following error:

    client_1  | /client/src/components/patientData.tsx
    client_1  | TypeScript error in /client/src/components/patientData.tsx(42,18):
    client_1  | 'Button' cannot be used as a JSX component.
    client_1  |   Its instance type 'Button' is not a valid JSX element.
    client_1  |     The types returned by 'render()' are incompatible between these types.
    client_1  |       Type 'React.ReactNode' is not assignable to type 'import("/client/node_modules/@types/react-router/node_modules/@types/react/index").ReactNode'.
    client_1  |         Type '{}' is not assignable to type 'ReactNode'.  TS2786
    
  • Fix "Lint server source" issues from CI check:

    /home/runner/work/bluebutton-sample-client-nodejs-react/bluebutton-sample-client-nodejs-react/server/src/routes/Data.ts
    Warning:   23:52  warning  'res' is defined but never used        @typescript-eslint/no-unused-vars
    Error:   43:3   error    Unsafe return of an `any` typed value  @typescript-eslint/no-unsafe-return
    
  • Comment out failing test in server_test.ts that is preventing CI check completion. To be resolved later!

    $ jest --coverage
     FAIL  src/__tests__/server_test.ts (6.134 s)
      ✓ expect patient end point returns patient data. (41 ms)
      ✓ expect profile end point returns profile data. (5 ms)
      ✓ expect coverage end point returns coverage data. (4 ms)
      ✕ expect eob end point returns eob data. (5000 ms)
    
      ● expect eob end point returns eob data.
    
        thrown: "Exceeded timeout of 5000 ms for a test.
        Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."
    
          100 | });
          101 |
        > 102 | test("expect eob end point returns eob data.", async () => {
              | ^
          103 |     const loggedInUser = getLoggedInUser(db);
          104 |
          105 |     loggedInUser.authToken = MOCK_AUTH_TOKEN;
    
          at Object.<anonymous> (src/__tests__/server_test.ts:102:1)
          at TestScheduler.scheduleTests (node_modules/@jest/core/build/TestScheduler.js:333:13)
    

What Should Reviewers Watch For?

If you're reviewing this PR, please check these things, in particular:

  • Are there any unhandled and/or untested edge cases you can think of?
  • Does this make any backwards-incompatible changes that might break end user clients?
  • Can you find any bugs if you run the code locally and test it manually?
  • Naming in general? Do the namings of variables, methods, classes, and other items make sense? Are they too verbose, or not verbose enough? Any redundancies? Are the comments descriptive enough?
  • Are audit logging items properly handled? Is sensitive information either not included or hashed?

How to review:

  • Validate that both the server and client docker-compose containers function as expected.

  • Validate that the native OS server and client components work as expected.

NOTE: Reference the README.md for instructions on how to configure and run (under the Usage Examples section).

TEST SCRIPT:

Please follow this test script to validate that you are getting the same issue for the "master" branch and to test out this PR branch.

The best synthetic users for testing are: BBUser29998 and BBUser29999

  1. Validate that the "master" branch is currently not working in your local development setup:
    $ git fetch origin
    
    $ git checkout master
    
    $ # Remove containers:
    $ docker rmi bluebutton-sample-client-nodejs-react_client
    $ docker rmi bluebutton-sample-client-nodejs-react_server
      
    $ docker-compose up -d
    
    $ docker-compose logs -f # To see the logs
    

Go to this page in a web browser: http://localhost:3000/

NOTE: This should not work and you should see the errors in the client & server logs referenced in this PR.

  1. Validate that this PR branch is working for you:

    $ docker-compose down
    
    $ # Remove containers:
    $ docker rmi bluebutton-sample-client-nodejs-react_client
    $ docker rmi bluebutton-sample-client-nodejs-react_server
    
    $ git checkout dtisza1/BB2-1133-fix-docker-startup
    
    $ docker-compose up -d
    
    $ docker-compose logs -f # To see the logs
    

NOTE: This should work as expected with the authorize and claims data populated OK.

@dtisza1 dtisza1 self-assigned this Apr 19, 2022
@dtisza1 dtisza1 requested review from JFU-GIT and oragame April 19, 2022 20:05
@dtisza1 dtisza1 marked this pull request as draft April 21, 2022 14:05
@dtisza1
Copy link
Copy Markdown
Contributor Author

dtisza1 commented Apr 21, 2022

@JFU-GIT @oragame I've switched this PR to draft. I was able to get past the issues and get the sample client working. However, I'm having some trouble getting one of the tests to work. Since the code is changed in Nick's PR, I'm going to continue working on getting that running via this PR instead: #26

@dtisza1 dtisza1 requested a review from bfausett77 April 25, 2022 20:22
@dtisza1 dtisza1 marked this pull request as ready for review April 25, 2022 20:22
@dtisza1
Copy link
Copy Markdown
Contributor Author

dtisza1 commented Apr 25, 2022

Just took this out of draft mode and it is ready for review. TY!

Copy link
Copy Markdown
Contributor

@JFU-GIT JFU-GIT left a comment

Choose a reason for hiding this comment

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

check it out locally and the end to end flow works as expected.

there are un used vars in tests, and can be removed to pass linter.

Looks good to me and thanks for bringing main back to normal;-)

Copy link
Copy Markdown
Contributor

@oragame oragame left a comment

Choose a reason for hiding this comment

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

This looks good, error on master branch and was able to get to the authorize medicare.gov link on this branch. It failed on the auth call but that would be expected I think.

I am getting:

Error: invalid_request
Invalid client_id parameter value.

on the /v1/o/authorize endpoint (@ sandbox.bluebutton.cms.gov) - thought I would make sure that is expected first :)

@dtisza1
Copy link
Copy Markdown
Contributor Author

dtisza1 commented Apr 26, 2022

This looks good, error on master branch and was able to get to the authorize medicare.gov link on this branch. It failed on the auth call but that would be expected I think.

I am getting:

Error: invalid_request
Invalid client_id parameter value.

on the /v1/o/authorize endpoint (@ sandbox.bluebutton.cms.gov) - thought I would make sure that is expected first :)

@oragame For the master branch, you should see both server (preventing startup) and client (affecting the auth button) errors. On this PR branch, the full sample app flow should work.

From the error message, I'm wondering if there is an issue with the config? Review the client_id used in the config file at: server/src/configs/config.ts (example is sample.config.ts).

TY for helping to review and test this!

@oragame
Copy link
Copy Markdown
Contributor

oragame commented Apr 26, 2022

@oragame For the master branch, you should see both server (preventing startup) and client (affecting the auth button) errors. On this PR branch, the full sample app flow should work.

From the error message, I'm wondering if there is an issue with the config? Review the client_id used in the config file at: server/src/configs/config.ts (example is sample.config.ts).

TY for helping to review and test this!

@dtisza1 no problem, glad to help! I am seeing that the sample.config.ts and config.ts are identical on the server contianer. Is that ok? Maybe we can pair up on this if needed?

@dtisza1 dtisza1 changed the title [ BB2-1133 ] FIX server & client issues preventing start up. [ BB2-1275 ] FIX server & client issues preventing start up. Apr 26, 2022
Copy link
Copy Markdown
Contributor

@oragame oragame left a comment

Choose a reason for hiding this comment

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

Thanks for the help on the config @dtisza1 ... this looks good, nice job!

@dtisza1 dtisza1 merged commit a01d12b into master Apr 26, 2022
@dtisza1 dtisza1 deleted the dtisza1/BB2-1133-fix-docker-startup branch April 26, 2022 15:51
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.

3 participants