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

feat: get delete api and cleanup #18

Merged
merged 1 commit into from
Dec 5, 2023
Merged

feat: get delete api and cleanup #18

merged 1 commit into from
Dec 5, 2023

Conversation

oshinongit
Copy link
Contributor

No description provided.

@oshinongit oshinongit requested review from birme and removed request for birme December 5, 2023 11:26
@oshinongit
Copy link
Contributor Author

Problem found with returning objects from api. Investigating and fixing before re requesting review.

Copy link
Contributor

@birme birme left a comment

Choose a reason for hiding this comment

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

Some questions and suggestions to check

src/api_productions.ts Outdated Show resolved Hide resolved
@@ -284,7 +333,7 @@ const apiProductions: FastifyPluginCallback<ApiProductionsOptions> = (
schema: {
// description: 'Retrieves lines for a Production.',
response: {
200: Type.Object({ Line })
200: Type.Array(Type.Any())
Copy link
Contributor

Choose a reason for hiding this comment

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

Why Any() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An overarching problem is the fastify typecheck which does not seem to correctly identify returning types. Using Type.Object({ Line }) here will cause an internal error 500 when making this request.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should work, been using types all the time :)

@@ -302,22 +351,22 @@ const apiProductions: FastifyPluginCallback<ApiProductionsOptions> = (

fastify.get<{
Params: { name: string; lineName: string };
Reply: Line | string;
Reply: { line: Line } | string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason why it cannot return the Line object directly? Why the { line: Line } wrapping ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason it does not work when unwrapped. Removing the schema all together from the fastify request definition also fixes the problem. The overarching issue for the fastify requests are all related to the response schema object type. It does not seem to work seemlessly for some reason.

src/api_productions.ts Show resolved Hide resolved
@oshinongit oshinongit force-pushed the get-delete-api branch 3 times, most recently from f4227e5 to 3a1f613 Compare December 5, 2023 15:50
Copy link
Contributor

@birme birme left a comment

Choose a reason for hiding this comment

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

Looks good now :)

@oshinongit oshinongit merged commit 4d84fc0 into main Dec 5, 2023
4 checks passed
@oshinongit oshinongit deleted the get-delete-api branch December 5, 2023 15:55
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.

2 participants