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

refactor(26237): Refactor the request infrastructure to the OpenAPI specs #566

Open
wants to merge 29 commits into
base: epic/23777/bi-directional-frontend
Choose a base branch
from

Conversation

vanch3d
Copy link
Contributor

@vanch3d vanch3d commented Sep 18, 2024

See https://hivemq.kanbanize.com/ctrl_board/57/cards/26237/details/

This PR is part of the bi-directional epic and refactors the query infrastructure to abide to the OpenAPI specs proposed in #563

Design

The current implementation of the bi-directional features in the frontend is still based on a series of mocks (both data and process) to bypass the work-in-progress nature of the backend.

To drive the alignment of the two parts of the engineering process, a proposal was made for the expected API endpoints.

The PR refactors the bespoke mocks into a proper micro-service API mocking

  • The OpenAPI specs are used to generate the stubs for the services and models
  • All the expected endpoints are covered by their relevant React Query hooks
  • All the still-unsupported endpoints are covered by mocks, using MSW to intercept the requests and populate the returned body
  • The internal MQTT Client, used for sampling topics and schema, has been "demoted" to dev-only and integrated with the relevant MSW handlers

Note that MSW needs to be activated for the mocks to be introduced. Change the following environment variable in the env.local files:

VITE_FLAG_MOCK_SERVER=true

@vanch3d vanch3d self-assigned this Sep 18, 2024
@cla-bot cla-bot bot added the cla-signed label Sep 18, 2024
@vanch3d vanch3d marked this pull request as ready for review September 23, 2024 13:37
@vanch3d vanch3d force-pushed the epic/23777/bi-directional-frontend branch from 6a4233f to 27b5978 Compare September 23, 2024 14:41
@vanch3d vanch3d force-pushed the refactor/26237/update-openapi-bi-direct branch 2 times, most recently from a20eb7f to 00135a9 Compare September 23, 2024 15:21
@vanch3d vanch3d force-pushed the epic/23777/bi-directional-frontend branch from fa1dd1b to 17ec687 Compare September 27, 2024 13:19
@vanch3d vanch3d force-pushed the refactor/26237/update-openapi-bi-direct branch from 00135a9 to 4f049d1 Compare September 27, 2024 13:23
Copy link
Contributor

@h2xd h2xd left a comment

Choose a reason for hiding this comment

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

LGTM! 🐊 Just found some small nits. Generally a really nice yet big PR :D

export const deviceHandlers = [
http.get('*/protocol-adapters/adapters/:adapterId/tags', ({ params }) => {
const { adapterId } = params
console.log('>', { adapterId })
Copy link
Contributor

Choose a reason for hiding this comment

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

watch out a console that you might not want

it('should load the data', async () => {
const { result } = renderHook(() => usePrivateMqttClient(), { wrapper })

expect(result.current).toStrictEqual<PrivateMqttClientType>({ state: undefined, actions: undefined })
Copy link
Contributor

Choose a reason for hiding this comment

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

question: is it right that the data is empty? Does not match the title of the test

Comment on lines +21 to +23
<ConditionalWrapper
condition={import.meta.env.VITE_FLAG_MOCK_SERVER === 'true'}
wrapper={(children) => <PrivateMqttClientProvider>{children}</PrivateMqttClientProvider>}
Copy link
Contributor

Choose a reason for hiding this comment

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

Neat!

@@ -23,6 +24,8 @@ vi.mock('@chakra-ui/react', async () => {
return { ...actual, useTheme: vi.fn<[], Partial<WithCSSVar<Dict>>>(() => MOCK_THEME) }
})

//// 44-48,53-63,67-81,86-93
Copy link
Contributor

Choose a reason for hiding this comment

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

question: what does that mean? seems like a debug comment to me

@h2xd
Copy link
Contributor

h2xd commented Sep 27, 2024

Some unit tests seem to fail due to not matching objects, I've added my approval so you can move forward after they are turned green.

@h2xd h2xd closed this Sep 27, 2024
@h2xd h2xd reopened this Sep 27, 2024
@h2xd
Copy link
Contributor

h2xd commented Sep 27, 2024

Sorry miss clicked on the wrong button.

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