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

PIMS-400 Install TypeORM #1971

Merged
merged 28 commits into from
Dec 18, 2023
Merged

PIMS-400 Install TypeORM #1971

merged 28 commits into from
Dec 18, 2023

Conversation

TaylorFries
Copy link
Collaborator

@TaylorFries TaylorFries commented Dec 11, 2023

🎯 Summary

PIMS-400:

Adding the foundation for TypeORM to be used in express-api

Changes include:

  • creating app-data-source.ts - used to create connection to database
  • creating typeorm folder - used to hold all entities and migrations
  • initializing connection in server.ts
  • adding settings to typescript config file

To test:

  • have docker containers for PIMS running
  • stop api container
  • navigate to PIMS/express-api
  • run "npm run dev"
    • output should be similar to:
{"level":"info","message":"Server started on port 5000.","timestamp":"2023-12-18 08:27:02.221 AM"}
{"level":"info","message":"query: SELECT * FROM current_schema()","timestamp":"2023-12-18 08:27:02.244 AM"}
{"level":"info","message":"query: SELECT version();","timestamp":"2023-12-18 08:27:02.256 AM"}
{"level":"info","message":"query: START TRANSACTION","timestamp":"2023-12-18 08:27:02.260 AM"}
{"level":"info","message":"query: SELECT * FROM \"information_schema\".\"tables\" WHERE \"table_schema\" = 'public' AND \"table_name\" = 'typeorm_metadata'","timestamp":"2023-12-18 08:27:02.262 AM"}
{"level":"info","message":"query: COMMIT","timestamp":"2023-12-18 08:27:02.279 AM"}
{"level":"info","message":"Database connection has been initialized","timestamp":"2023-12-18 08:27:02.280 AM"}

🔰 Checklist

  • I have read and agree with the following checklist and am following the guidelines in our Code of Conduct document.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation where required.
  • I have tested my changes to the best of my ability.
  • My changes generate no new warnings.

@TaylorFries TaylorFries changed the title Pims 400 Install TypeORM PIMS-400 Install TypeORM Dec 11, 2023
@TaylorFries TaylorFries self-assigned this Dec 11, 2023
Copy link

codeclimate bot commented Dec 12, 2023

Code Climate has analyzed commit fd4fd16 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 100.0%.

View more on Code Climate.

Copy link
Collaborator

@dbarkowsky dbarkowsky left a comment

Choose a reason for hiding this comment

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

To be consistent, does this file work as appDataSource? Camel case vs kebab case.
I realize that heath-router exists, but that should have been healthRouter probably too.

express-api/app-data-source.ts Outdated Show resolved Hide resolved
express-api/app-data-source.ts Outdated Show resolved Hide resolved
express-api/app-data-source.ts Outdated Show resolved Hide resolved
express-api/server.ts Outdated Show resolved Hide resolved
express-api/server.ts Outdated Show resolved Hide resolved
synchronize: true,
migrationsRun: false,
logging: true,
logger: new CustomWinstonLogger(true),
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does true argument do? I can't find documentation on this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was trying to find the link again but couldn't - there are cases (in stackoverflow) where not passing in 'true' means that the custom logger never initializes and then throws an error when it is used.

@dbarkowsky dbarkowsky self-requested a review December 14, 2023 21:40
dbarkowsky
dbarkowsky previously approved these changes Dec 14, 2023
Copy link
Collaborator

@dbarkowsky dbarkowsky left a comment

Choose a reason for hiding this comment

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

Works well. Happy to see the custom logger.
Made a small change so the message always prints. It looked like it was only printing the type before.
image

@TaylorFries TaylorFries marked this pull request as draft December 15, 2023 17:39
@dbarkowsky dbarkowsky self-requested a review December 15, 2023 20:26
@dbarkowsky dbarkowsky dismissed their stale review December 15, 2023 20:27

Contents have changed. Will re-review when ready.

@TaylorFries TaylorFries marked this pull request as ready for review December 18, 2023 16:52
Copy link
Collaborator

@dbarkowsky dbarkowsky left a comment

Choose a reason for hiding this comment

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

Ran in local with npm run dev and in container. Database connects successfully. Approved again.

@TaylorFries TaylorFries merged commit 12cd656 into main Dec 18, 2023
3 checks passed
@TaylorFries TaylorFries deleted the PIMS-400-install-typeorm branch December 18, 2023 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants