Skip to content

Commit

Permalink
[fix]: Remove redux-form dependency from Monitor page (#2562)
Browse files Browse the repository at this point in the history 
## Which problem is this PR solving?
- part of #2556 

## Description of the changes
- This PR removes dependency of Monitor page on `redux-form`.
- To keep this PR small and easy to review, I haven't removed other
instances of `redux-form` in this same PR. Neither have I removed
`redux-form` package as of now.
- `Antd` `SearchableSelect` requires loading type to be `boolean |
undefined` so I have changed the related types in other files to fix
ts-lint errors.

### Why this change was needed?
- Monitor page has a few selectors that doesn't need any global state
sync to other components. `redux-form` is anyway deprecating so in-built
react state management is a good choice.
- React uses `state` object for state management, and `props` for
injecting props to the component. `redux-form` use to inject those
Dropdown Selector values `selectedService`, `selectedSpanKind`, and
`selectedTimeFrame` from redux store.
- This PR moves that to in-built react state, so we won't have to inject
those from store anymore.

## How was this change tested?
- `npm run test`
- `npm run start` with monitor page view. everything seems to be working
fine.

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `npm run lint` and `npm run test`

---------

Signed-off-by: Hariom Gupta <guptahariom03082003@gmail.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
  • Loading branch information
@hari45678 @yurishkuro
hari45678 and yurishkuro authored Jan 8, 2025
1 parent 73a16c0 commit c28adbb
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 236 deletions.
158 changes: 34 additions & 124 deletions packages/jaeger-ui/src/components/Monitor/ServicesView/__snapshots__/index.test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,11 @@ exports[`<MonitorATMServicesView> ATM snapshot test 1`] = `
>
Service
</h2>
<Field
component={[Function]}
name="service"
<SearchableSelect
className="select-a-service-input"
onChange={[Function]}
placeholder="Select A Service"
props={
Object {
"className": "select-a-service-input",
"disabled": null,
"loading": null,
"value": "s1",
}
}
value="s1"
>
<Option
key="s1"
Expand All @@ -40,7 +32,7 @@ exports[`<MonitorATMServicesView> ATM snapshot test 1`] = `
>
s2
</Option>
</Field>
</SearchableSelect>
</Col>
<Col
span={6}
Expand All @@ -50,20 +42,11 @@ exports[`<MonitorATMServicesView> ATM snapshot test 1`] = `
>
Span Kind
</h2>
<Field
component={[Function]}
name="spanKind"
<SearchableSelect
className="span-kind-selector"
onChange={[Function]}
placeholder="Select A Span Kind"
props={
Object {
"className": "span-kind-selector",
"defaultValue": "server",
"disabled": null,
"loading": null,
"value": "server",
}
}
value="server"
>
<Option
key="client"
Expand Down Expand Up @@ -95,7 +78,7 @@ exports[`<MonitorATMServicesView> ATM snapshot test 1`] = `
>
Consumer
</Option>
</Field>
</SearchableSelect>
</Col>
</Row>
<Row
Expand Down Expand Up @@ -124,23 +107,11 @@ exports[`<MonitorATMServicesView> ATM snapshot test 1`] = `
className="timeframe-selector"
span={8}
>
<Field
component={[Function]}
name="timeframe"
<SearchableSelect
className="select-a-timeframe-input"
onChange={[Function]}
placeholder="Select A Timeframe"
props={
Object {
"className": "select-a-timeframe-input",
"defaultValue": Object {
"label": "Last Hour",
"value": 3600000,
},
"disabled": null,
"loading": null,
"value": 3600000,
}
}
value={3600000}
>
<Option
key="300000"
Expand Down Expand Up @@ -196,7 +167,7 @@ exports[`<MonitorATMServicesView> ATM snapshot test 1`] = `
>
Last 2 days
</Option>
</Field>
</SearchableSelect>
</Col>
</Row>
<Row>
Expand Down Expand Up @@ -443,7 +414,6 @@ exports[`<MonitorATMServicesView> ATM snapshot test 1`] = `
"opsLatencies": null,
}
}
loading={null}
lookback={3600000}
serviceName="s1"
/>
Expand Down Expand Up @@ -486,19 +456,11 @@ exports[`<MonitorATMServicesView> ATM snapshot test with no metrics 1`] = `
>
Service
</h2>
<Field
component={[Function]}
name="service"
<SearchableSelect
className="select-a-service-input"
onChange={[Function]}
placeholder="Select A Service"
props={
Object {
"className": "select-a-service-input",
"disabled": null,
"loading": null,
"value": "s1",
}
}
value="s1"
/>
</Col>
<Col
Expand All @@ -509,20 +471,11 @@ exports[`<MonitorATMServicesView> ATM snapshot test with no metrics 1`] = `
>
Span Kind
</h2>
<Field
component={[Function]}
name="spanKind"
<SearchableSelect
className="span-kind-selector"
onChange={[Function]}
placeholder="Select A Span Kind"
props={
Object {
"className": "span-kind-selector",
"defaultValue": "server",
"disabled": null,
"loading": null,
"value": "server",
}
}
value="server"
>
<Option
key="client"
Expand Down Expand Up @@ -554,7 +507,7 @@ exports[`<MonitorATMServicesView> ATM snapshot test with no metrics 1`] = `
>
Consumer
</Option>
</Field>
</SearchableSelect>
</Col>
</Row>
<Row
Expand Down Expand Up @@ -583,23 +536,11 @@ exports[`<MonitorATMServicesView> ATM snapshot test with no metrics 1`] = `
className="timeframe-selector"
span={8}
>
<Field
component={[Function]}
name="timeframe"
<SearchableSelect
className="select-a-timeframe-input"
onChange={[Function]}
placeholder="Select A Timeframe"
props={
Object {
"className": "select-a-timeframe-input",
"defaultValue": Object {
"label": "Last Hour",
"value": 3600000,
},
"disabled": null,
"loading": null,
"value": 3600000,
}
}
value={3600000}
>
<Option
key="300000"
Expand Down Expand Up @@ -655,7 +596,7 @@ exports[`<MonitorATMServicesView> ATM snapshot test with no metrics 1`] = `
>
Last 2 days
</Option>
</Field>
</SearchableSelect>
</Col>
</Row>
<Row>
Expand Down Expand Up @@ -821,7 +762,6 @@ exports[`<MonitorATMServicesView> ATM snapshot test with no metrics 1`] = `
"opsLatencies": null,
}
}
loading={null}
lookback={3600000}
serviceName="s1"
/>
Expand Down Expand Up @@ -864,19 +804,11 @@ exports[`<MonitorATMServicesView> render one service latency 1`] = `
>
Service
</h2>
<Field
component={[Function]}
name="service"
<SearchableSelect
className="select-a-service-input"
onChange={[Function]}
placeholder="Select A Service"
props={
Object {
"className": "select-a-service-input",
"disabled": null,
"loading": null,
"value": "s1",
}
}
value="s1"
/>
</Col>
<Col
Expand All @@ -887,20 +819,11 @@ exports[`<MonitorATMServicesView> render one service latency 1`] = `
>
Span Kind
</h2>
<Field
component={[Function]}
name="spanKind"
<SearchableSelect
className="span-kind-selector"
onChange={[Function]}
placeholder="Select A Span Kind"
props={
Object {
"className": "span-kind-selector",
"defaultValue": "server",
"disabled": null,
"loading": null,
"value": "server",
}
}
value="server"
>
<Option
key="client"
Expand Down Expand Up @@ -932,7 +855,7 @@ exports[`<MonitorATMServicesView> render one service latency 1`] = `
>
Consumer
</Option>
</Field>
</SearchableSelect>
</Col>
</Row>
<Row
Expand Down Expand Up @@ -961,23 +884,11 @@ exports[`<MonitorATMServicesView> render one service latency 1`] = `
className="timeframe-selector"
span={8}
>
<Field
component={[Function]}
name="timeframe"
<SearchableSelect
className="select-a-timeframe-input"
onChange={[Function]}
placeholder="Select A Timeframe"
props={
Object {
"className": "select-a-timeframe-input",
"defaultValue": Object {
"label": "Last Hour",
"value": 3600000,
},
"disabled": null,
"loading": null,
"value": 3600000,
}
}
value={3600000}
>
<Option
key="300000"
Expand Down Expand Up @@ -1033,7 +944,7 @@ exports[`<MonitorATMServicesView> render one service latency 1`] = `
>
Last 2 days
</Option>
</Field>
</SearchableSelect>
</Col>
</Row>
<Row>
Expand Down Expand Up @@ -1200,7 +1111,6 @@ exports[`<MonitorATMServicesView> render one service latency 1`] = `
"opsLatencies": null,
}
}
loading={null}
lookback={3600000}
serviceName="s1"
/>
Expand Down
13 changes: 4 additions & 9 deletions packages/jaeger-ui/src/components/Monitor/ServicesView/index.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,7 @@ describe('<MonitorATMServicesView>', () => {

it('should update state after choosing a new timeframe', () => {
const firstGraphXDomain = wrapper.state().graphXDomain;
wrapper.setProps({
selectedTimeFrame: 3600000 * 2,
});
wrapper.instance().handleTimeFrameChange(3600000 * 2);

expect(wrapper.state().graphXDomain).not.toBe(firstGraphXDomain);
});
Expand Down Expand Up @@ -308,13 +306,13 @@ describe('<MonitorATMServicesView>', () => {
wrapper.find('Search').simulate('change', { target: { value: newValue } });
expect(trackSearchOperationSpy).toHaveBeenCalledWith(newValue);

wrapper.find('Field').first().simulate('change', null, newValue);
wrapper.find('SearchableSelect').first().prop('onChange')(newValue);
expect(trackSelectServiceSpy).toHaveBeenCalledWith(newValue);

wrapper.find({ name: 'spanKind' }).simulate('change', null, spanKindOption.value);
wrapper.find('.span-kind-selector').prop('onChange')(spanKindOption.value);
expect(trackSelectSpanKindSpy).toHaveBeenCalledWith(spanKindOption.label);

wrapper.find('Field').last().simulate('change', null, timeFrameOption.value);
wrapper.find('SearchableSelect').last().prop('onChange')(timeFrameOption.value);
expect(trackSelectTimeframeSpy).toHaveBeenCalledWith(timeFrameOption.label);

wrapper.find({ children: 'View all traces' }).simulate('click');
Expand Down Expand Up @@ -367,9 +365,6 @@ describe('mapStateToProps()', () => {
expect(mapStateToProps(state)).toEqual({
metrics: originInitialState,
services: [],
selectedService: 's1',
selectedSpanKind: 'server',
selectedTimeFrame: 3600000,
});
});
});
Expand Down
Loading

0 comments on commit c28adbb

Please sign in to comment.