Add GET /canonicalFields endpoint#55
Conversation
slifty
force-pushed
the
30-add-get-canonical-fields
branch
from
299374d to
a24cf2c
Compare
slifty
force-pushed
the
30-add-get-canonical-fields
branch
from
a24cf2c to
e9f5be7
Compare
|
Overall this is tremendous. |
|
I am able to run it with |
slifty
force-pushed
the
30-add-get-canonical-fields
branch
from
e9f5be7 to
36f75a3
Compare
Codecov Report
@@ Coverage Diff @@
## main #55 +/- ##
==========================================
- Coverage 71.60% 71.55% -0.06%
==========================================
Files 11 16 +5
Lines 81 116 +35
Branches 5 8 +3
==========================================
+ Hits 58 83 +25
- Misses 22 32 +10
Partials 1 1
Continue to review full report at Codecov.
|
|
I think it can be merged as-is, but it would be nice to rebase onto main. I see you used |
|
Yes -- in fact, I'm going to update our main branch protections to require PRs to be up to date before allowing merge. |
|
@bickelj the local tests may be giving you trouble if you're running them against a database that has an existing schema where migrations have been run ( this is due to ThomWright/postgres-migrations#93 not being merged yet ) If you set up |
slifty
force-pushed
the
30-add-get-canonical-fields
branch
from
e4c36fd to
43033ed
Compare
|
@bickelj another round! (sorry I know you keep approving and then I keep making all the optional changes -- please feel free to flag more improvements!) |
bickelj
left a comment
bickelj
left a comment
There was a problem hiding this comment.
I assumed the .keep files were for the purpose of keeping directories present in git (no such thing as a directory in git), but I also see "build:assets": "copyfiles -u 1 \"src/**/*.sql\" \"src/**/.keep\" dist",, but I think it is OK because those will not be copied over if they do not exist.
Thanks for iterating on this one with me a few times, I think this is huge. 👌
This adds support for retrieving all canonical fields contained in the system. As part of this change we: 1. Introduce `ajv` which is a runtime validator that uses JSONSchema in order to ensure that data conforms to a given TypeScript type. 2. Introduce migrations and basic queries for the `canonical_field` table. 3. Introduces a new pattern for integration tests that rely on fixture data. We want to be sure the data result conforms to our API spec / expected type output so we actually validate it before passing it along to the response middleware. Note that Express route handlers cannot be asynchronous which is why we have to use the promise `then` and `catch` syntax. Issue #30 Implement GET /canonicalFields API endpoint
We want to make it easy for files to create loggers that point back to the file generating a given log. By exposing ONLY `getLogger` method we support a cleaner for the logger syntax for the logger (rather than importing `logger` and having to use `childLogger` in our code, we can just import `getChildLogger` and assign the child logger to a local `logger` value). This also ensures that all loggers are generated with a name (I suppose someone could choose to name with an empty string).
slifty
force-pushed
the
30-add-get-canonical-fields
branch
from
43033ed to
bc0b98c
Compare
It served both purposes! We needed the
Same to you! thank you for all the great feedback; I like where this has landed |
That works. |
3 participants
This PR adds a real implementation for the
GET /canonicalFieldsendpoint.In doing so it also introduces the concept of data validation as well as test fixtures for integration tests.
Resolves #30
(This includes the code in #54 so it should be rebased against main once that is merged)