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

Generate OpenAPI spec during compile #1279

Open
wants to merge 73 commits into
base: master
Choose a base branch
from
Open

Conversation

adamw
Copy link
Member

@adamw adamw commented Aug 6, 2024

No description provided.

@adamw adamw marked this pull request as draft August 6, 2024 15:29
@katekozlowska katekozlowska marked this pull request as draft December 15, 2024 23:06
@katekozlowska katekozlowska marked this pull request as ready for review December 30, 2024 08:10
const login = mutation.mutateAsync;

useEffect(() => {
if (isSuccess) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could it be done still with onSuccess option, but passed to usePostUserLogin hook?

Choose a reason for hiding this comment

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

Ok, fixed as proposed

import { FormikInput, FeedbackButton } from "components";
import { useMutation } from "react-query";
import { usePostUserChangepassword } from "../../../api/apiComponents";
Copy link
Contributor

Choose a reason for hiding this comment

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

api can be also a named path like contexts or services

Choose a reason for hiding this comment

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

Already fixed

@adamw
Copy link
Member Author

adamw commented Dec 31, 2024

@katekozlowska the tests seem to be failing

ui/package.json Outdated
@@ -18,31 +19,34 @@
"@types/react-router-bootstrap": "^0.26.6",
"@types/react-router-dom": "^5.3.3",
"@types/yup": "^0.32.0",
"axios": "^1.6.5",
"axios": "0.27.2",
Copy link
Member Author

Choose a reason for hiding this comment

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

@katekozlowska so we're still using a downgraded axios?

Choose a reason for hiding this comment

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

axios has been removed

ui/package.json Outdated
"bootstrap": "^5.3.2",
"eslint-config-prettier": "^9.1.0",
"formik": "^2.4.5",
"immer": "^10.0.3",
"openapicmd": "^2.4.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

is this the openapi generator that we are using? the docs in stack.md mention openapi-codegen, is that still used?

Choose a reason for hiding this comment

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

not needed anymore. Removed it

ui/package.json Outdated
"react-router-bootstrap": "^0.26.2",
"react-router-dom": "^6.21.1",
"react-scripts": "^5.0.1",
"typescript": "5.1.6",
"typescript": "4.9.5",
Copy link
Member Author

Choose a reason for hiding this comment

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

is the TS downgrade necessary?

Choose a reason for hiding this comment

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

Upgraded to latest

@@ -0,0 +1,321 @@
/**
* Generated by @openapi-codegen
Copy link
Member Author

Choose a reason for hiding this comment

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

if this is generated, should it be checked in to VCS?

@@ -0,0 +1,4 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
Copy link
Member Author

Choose a reason for hiding this comment

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

Should this be here?

Copy link
Member Author

Choose a reason for hiding this comment

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

(the entire file)

Choose a reason for hiding this comment

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

The yarn.lock file is added to a frontend project to ensure consistent dependency versions across all environments where the project is installed. It locks the specific versions of dependencies and their sub-dependencies, preventing potential issues caused by version mismatches. Including it in the repository ensures that all team members and deployment environments use the exact same dependency tree, improving reliability and avoiding unexpected bugs.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, but this one is in the top-level directory, shouldn't we have a non-empty ui/yarn.lock?

@olafkrawczyk
Copy link

nice job @katekozlowska 👏
since services are no longer mocked I think that a slight change in tests is required. From the jest logs looks like we should wrap components with query provider https://tanstack.com/query/v4/docs/framework/react/guides/testing

@BartoszButrymSoftwareMill

Somewhat unrelated to the core task, but maybe https://softwaremill.github.io/bootzooka/stack.html would be good to extend with the main libraries used on the frontend - there are things like Formik or Yup (and possibly others) which I might guess as to why they are used, but it would be good to describe it too

This information has been added to the stack.md file. I'm guessing you can't see it at the given address because the changes haven't been deployed yet.

@BartoszButrymSoftwareMill

After starting I get a warning:

[0] Browserslist: caniuse-lite is outdated. Please run:
[0]   npx update-browserslist-db@latest
[0]   Why you should do it regularly: https://github.com/browserslist/update-db#readme
[0] (node:38772) [DEP_WEBPACK_DEV_SERVER_ON_AFTER_SETUP_MIDDLEWARE] DeprecationWarning: 'onAfterSetupMiddleware' option is deprecated. Please use the 'setupMiddlewares' option.
[0] (Use `node --trace-deprecation ...` to show where the warning was created)
[0] (node:38772) [DEP_WEBPACK_DEV_SERVER_ON_BEFORE_SETUP_MIDDLEWARE] DeprecationWarning: 'onBeforeSetupMiddleware' option is deprecated. Please use the 'setupMiddlewares' option.
[0] Starting the development server...
[0]
[0] Browserslist: caniuse-lite is outdated. Please run:
[0]   npx update-browserslist-db@latest
[0]   Why you should do it regularly: https://github.com/browserslist/update-db#readme
[0] Compiled with warnings.

Another thing to update probably?

Fixed, not showing anymore

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.

6 participants