Skip to content

Commit

Permalink
remove redux-form dependency from sort form
Browse files Browse the repository at this point in the history
Signed-off-by: Hariom Gupta <[email protected]>
  • Loading branch information
hari45678 committed Jan 9, 2025
1 parent c28adbb commit 6904941
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
jest.mock('../../utils/tracking');

import {
middlewareHooks,
trackFormInput,
CATEGORY_LIMIT,
CATEGORY_LOOKBACK,
Expand All @@ -26,16 +25,15 @@ import {
CATEGORY_SORTBY,
CATEGORY_TAGS,
CATEGORY_SERVICE,
trackSortByChange,
} from './SearchForm.track';
import { FORM_CHANGE_ACTION_TYPE } from '../../constants/search-form';
import { trackEvent } from '../../utils/tracking';

describe('GA tracking', () => {
it('tracks changing sort criteria', () => {
const action = { meta: { form: 'sortBy' }, payload: 'MOST_RECENT' };
middlewareHooks[FORM_CHANGE_ACTION_TYPE]({}, action);
expect(trackEvent.mock.calls.length).toBe(1);
expect(trackEvent.mock.calls[0]).toEqual([CATEGORY_SORTBY, expect.any(String)]);
trackEvent.mockClear();
trackSortByChange('MOST_RECENT');
expect(trackEvent).toHaveBeenCalledWith(CATEGORY_SORTBY, 'MOST_RECENT');
});

it('sends form input to GA', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import { Store } from 'redux';

import * as constants from '../../constants/search-form';
import { trackEvent } from '../../utils/tracking';
import { ReduxState } from '../../types';

export const ACTION_SET = 'set';
export const ACTION_CLEAR = 'clear';
Expand Down Expand Up @@ -49,10 +46,6 @@ export function trackFormInput(
trackEvent(CATEGORY_SERVICE, serviceName);
}

export const middlewareHooks = {
[constants.FORM_CHANGE_ACTION_TYPE]: (_: Store<ReduxState>, action: any) => {
if (action.meta.form === 'sortBy') {
trackEvent(CATEGORY_SORTBY, action.payload);
}
},
};
export function trackSortByChange(sortBy: string) {
trackEvent(CATEGORY_SORTBY, sortBy);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import * as React from 'react';
import { Select } from 'antd';
import { History as RouterHistory, Location } from 'history';
import { Link } from 'react-router-dom';
import { Field, formValueSelector, reduxForm } from 'redux-form';
import queryString from 'query-string';

import AltViewOptions from './AltViewOptions';
Expand All @@ -36,7 +35,6 @@ import { getLocation } from '../../TracePage/url';
import * as orderBy from '../../../model/order-by';
import { getPercentageOfDuration } from '../../../utils/date';
import { stripEmbeddedState } from '../../../utils/embedded-url';
import reduxFormFieldAdapter from '../../../utils/redux-form-field-adapter';

import { FetchedTrace } from '../../../types';
import { SearchQuery } from '../../../types/search';
Expand Down Expand Up @@ -64,41 +62,35 @@ type SearchResultsProps = {
spanLinks?: Record<string, string> | undefined;
traces: Trace[];
rawTraces: TraceData[];
sortBy: string;
handleSortChange: (sortBy: string) => void;
};

type SelectSortProps = {
sortBy: string;
handleSortChange: (sortBy: string) => void;
};

const Option = Select.Option;

/**
* Contains the dropdown to sort and filter trace search results
*/
function SelectSortImpl() {
function SelectSort({ sortBy, handleSortChange }: SelectSortProps) {

Check warning on line 79 in packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx

View check run for this annotation

Codecov / codecov/patch

packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx#L79

Added line #L79 was not covered by tests
return (
<label>
Sort:{' '}
<Field
name="sortBy"
component={reduxFormFieldAdapter({ AntInputComponent: SearchableSelect })}
props={{}}
>
<SearchableSelect value={sortBy} onChange={(value: string) => handleSortChange(value)}>

Check warning on line 83 in packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx

View check run for this annotation

Codecov / codecov/patch

packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx#L83

Added line #L83 was not covered by tests
<Option value={orderBy.MOST_RECENT}>Most Recent</Option>
<Option value={orderBy.LONGEST_FIRST}>Longest First</Option>
<Option value={orderBy.SHORTEST_FIRST}>Shortest First</Option>
<Option value={orderBy.MOST_SPANS}>Most Spans</Option>
<Option value={orderBy.LEAST_SPANS}>Least Spans</Option>
</Field>
</SearchableSelect>
</label>
);
}

const SelectSort = reduxForm({
form: 'traceResultsSort',
initialValues: {
sortBy: orderBy.MOST_RECENT,
},
})(SelectSortImpl);

export const sortFormSelector = formValueSelector('traceResultsSort');

// export for tests
export function createBlob(rawTraces: TraceData[]) {
return new Blob([`{"data":${JSON.stringify(rawTraces)}}`], { type: 'application/json' });
Expand Down Expand Up @@ -150,6 +142,8 @@ export class UnconnectedSearchResults extends React.PureComponent<SearchResultsP
skipMessage,
spanLinks,
traces,
sortBy,
handleSortChange,
} = this.props;

const traceResultsView = queryString.parse(location.search).view !== 'ddg';
Expand Down Expand Up @@ -205,7 +199,7 @@ export class UnconnectedSearchResults extends React.PureComponent<SearchResultsP
<h2 className="ub-m0 u-flex-1">
{traces.length} Trace{traces.length > 1 && 's'}
</h2>
{traceResultsView && <SelectSort />}
{traceResultsView && <SelectSort sortBy={sortBy} handleSortChange={handleSortChange} />}
{traceResultsView && <DownloadResults onDownloadResultsClicked={this.onDownloadResultsClicked} />}
<AltViewOptions traceResultsView={traceResultsView} onDdgViewClicked={this.onDdgViewClicked} />
{showStandaloneLink && (
Expand Down
28 changes: 20 additions & 8 deletions packages/jaeger-ui/src/components/SearchTracePage/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ import store from 'store';
import memoizeOne from 'memoize-one';

import SearchForm from './SearchForm';
import SearchResults, { sortFormSelector } from './SearchResults';
import SearchResults from './SearchResults';
import { isSameQuery, getUrl, getUrlState } from './url';
import * as jaegerApiActions from '../../actions/jaeger-api';
import * as fileReaderActions from '../../actions/file-reader-api';
import * as orderBy from '../../model/order-by';
import ErrorMessage from '../common/ErrorMessage';
import LoadingIndicator from '../common/LoadingIndicator';
import { getLocation as getTraceLocation } from '../TracePage/url';
Expand All @@ -39,9 +40,14 @@ import FileLoader from './FileLoader';
import './index.css';
import JaegerLogo from '../../img/jaeger-logo.svg';
import withRouteProps from '../../utils/withRouteProps';
import { trackSortByChange } from './SearchForm.track';

// export for tests
export class SearchTracePageImpl extends Component {
state = {
sortBy: orderBy.MOST_RECENT,
};

componentDidMount() {
const {
diffCohort,
Expand Down Expand Up @@ -70,6 +76,11 @@ export class SearchTracePageImpl extends Component {
}
}

handleSortChange = sortBy => {
this.setState({ sortBy });
trackSortByChange(sortBy);
};

goToTrace = traceID => {
const { queryOfResults } = this.props;
const searchUrl = queryOfResults ? getUrl(stripEmbeddedState(queryOfResults)) : getUrl();
Expand All @@ -89,12 +100,15 @@ export class SearchTracePageImpl extends Component {
loadingTraces,
maxTraceDuration,
services,
traceResults,
traceResultsToDownload,
queryOfResults,
loadJsonTraces,
urlQueryParams,
sortedTracesXformer,
traces,
} = this.props;
const { sortBy } = this.state;
const traceResults = sortedTracesXformer(traces, sortBy);
const hasTraceResults = traceResults && traceResults.length > 0;
const showErrors = errors && !loadingTraces;
const showLogo = isHomepage && !hasTraceResults && !loadingTraces && !errors;
Expand Down Expand Up @@ -145,6 +159,8 @@ export class SearchTracePageImpl extends Component {
spanLinks={urlQueryParams && urlQueryParams.spanLinks}
traces={traceResults}
rawTraces={traceResultsToDownload}
sortBy={this.state.sortBy}
handleSortChange={this.handleSortChange}
/>
)}
{showLogo && (
Expand All @@ -163,8 +179,6 @@ export class SearchTracePageImpl extends Component {
SearchTracePageImpl.propTypes = {
isHomepage: PropTypes.bool,
// eslint-disable-next-line react/forbid-prop-types
traceResults: PropTypes.array,
// eslint-disable-next-line react/forbid-prop-types
traceResultsToDownload: PropTypes.array,
// eslint-disable-next-line react/forbid-prop-types
diffCohort: PropTypes.array,
Expand Down Expand Up @@ -270,8 +284,6 @@ export function mapStateToProps(state) {
if (serviceError) {
errors.push(serviceError);
}
const sortBy = sortFormSelector(state, 'sortBy');
const traceResults = sortedTracesXformer(traces, sortBy);
return {
queryOfResults,
diffCohort,
Expand All @@ -281,11 +293,11 @@ export function mapStateToProps(state) {
disableFileUploadControl,
loadingTraces,
services,
traceResults,
traces,
traceResultsToDownload: rawTraces,
errors: errors.length ? errors : null,
maxTraceDuration: maxDuration,
sortTracesBy: sortBy,
sortedTracesXformer,
urlQueryParams: Object.keys(query).length > 0 ? query : null,
};
}
Expand Down
34 changes: 22 additions & 12 deletions packages/jaeger-ui/src/components/SearchTracePage/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import SearchForm from './SearchForm';
import LoadingIndicator from '../common/LoadingIndicator';
import { fetchedState } from '../../constants';
import traceGenerator from '../../demo/trace-generators';
import { MOST_RECENT } from '../../model/order-by';
import { MOST_RECENT, MOST_SPANS } from '../../model/order-by';
import transformTraceData from '../../model/transform-trace-data';
import { store as globalStore } from '../../utils/configure-store';

Expand All @@ -53,12 +53,12 @@ const AllProvider = ({ children }) => (
describe('<SearchTracePage>', () => {
const queryOfResults = {};
let wrapper;
let traceResults;
let traces;
let traceResultsToDownload;
let props;

beforeEach(() => {
traceResults = [
traces = [
{ traceID: 'a', spans: [], processes: {} },
{ traceID: 'b', spans: [], processes: {} },
];
Expand All @@ -68,17 +68,17 @@ describe('<SearchTracePage>', () => {
];
props = {
queryOfResults,
traceResults,
traces,
traceResultsToDownload,
diffCohort: [],
isHomepage: false,
loadingServices: false,
loadingTraces: false,
disableFileUploadControl: false,
maxTraceDuration: 100,
numberOfTraceResults: traceResults.length,
numberOfTraceResults: traces.length,
services: null,
sortTracesBy: MOST_RECENT,
sortedTracesXformer: jest.fn(),
urlQueryParams: { service: 'svc-a' },
// actions
fetchServiceOperations: jest.fn(),
Expand Down Expand Up @@ -124,6 +124,18 @@ describe('<SearchTracePage>', () => {
store.get = oldFn;
});

it('calls sortedTracesXformer with correct arguments', () => {
const sortBy = MOST_RECENT;
wrapper.setState({ sortBy });
expect(props.sortedTracesXformer).toHaveBeenCalledWith(traces, sortBy);
});

it('handles sort change correctly', () => {
const sortBy = MOST_SPANS;
wrapper.instance().handleSortChange(sortBy);
expect(wrapper.state('sortBy')).toBe(sortBy);
});

it('goToTrace pushes the trace URL with {fromSearch: true} to history', () => {
const traceID = '15810714d6a27450';
const query = 'some-query';
Expand Down Expand Up @@ -215,10 +227,9 @@ describe('mapStateToProps()', () => {
},
};

const { maxTraceDuration, traceResults, traceResultsToDownload, diffCohort, ...rest } =
mapStateToProps(state);
expect(traceResults).toHaveLength(stateTrace.search.results.length);
expect(traceResults[0].traceID).toBe(trace.traceID);
const { maxTraceDuration, traceResultsToDownload, diffCohort, traces, ...rest } = mapStateToProps(state);
expect(traces).toHaveLength(stateTrace.search.results.length);
expect(traces[0].traceID).toBe(trace.traceID);
expect(traceResultsToDownload[0].traceID).toBe(trace.traceID);
expect(maxTraceDuration).toBe(trace.duration);
expect(diffCohort).toHaveLength(state.traceDiff.cohort.length);
Expand All @@ -230,8 +241,7 @@ describe('mapStateToProps()', () => {
embedded: undefined,
queryOfResults: undefined,
isHomepage: true,
// the redux-form `formValueSelector` mock returns `null` for "sortBy"
sortTracesBy: null,
sortedTracesXformer: expect.any(Function),
urlQueryParams: null,
services: [
{
Expand Down
3 changes: 1 addition & 2 deletions packages/jaeger-ui/src/middlewares/track.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@
import { Action } from 'redux-actions';
import { Dispatch, Store } from 'redux';

import { middlewareHooks as searchHooks } from '../components/SearchTracePage/SearchForm.track';
import { middlewareHooks as timelineHooks } from '../components/TracePage/TraceTimelineViewer/duck.track';
import { isWaEnabled } from '../utils/tracking';
import { ReduxState } from '../types';

type TMiddlewareFn = (store: Store<ReduxState>, action: Action<any>) => void;

const middlewareHooks: { [actionType: string]: TMiddlewareFn } = { ...timelineHooks, ...searchHooks };
const middlewareHooks: { [actionType: string]: TMiddlewareFn } = { ...timelineHooks };

function trackingMiddleware(store: Store<ReduxState>) {
return function inner(next: Dispatch<ReduxState>) {
Expand Down

0 comments on commit 6904941

Please sign in to comment.