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

implement revert logic by using originalSelector #603

Merged
merged 1 commit into from
Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions web/src/components/AssertionCard/AssertionCard.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {Tooltip} from 'antd';
import {useCallback} from 'react';
import {useStore} from 'react-flow-renderer';
import {Button, Tooltip} from 'antd';

import AssertionCheckRow from 'components/AssertionCheckRow';
import {useTestDefinition} from 'providers/TestDefinition/TestDefinition.provider';
import {useAppSelector} from 'redux/hooks';
import {useCallback} from 'react';
import {useStore} from 'react-flow-renderer';
import TestDefinitionSelectors from 'selectors/TestDefinition.selectors';
import {TAssertionResultEntry} from 'types/Assertion.types';
import {useAppSelector} from '../../redux/hooks';
import {TAssertionResultEntry} from '../../types/Assertion.types';
import * as S from './AssertionCard.styled';
import AssertionCardSelectorList from './AssertionCardSelectorList';

Expand All @@ -24,13 +24,16 @@ const AssertionCard: React.FC<TAssertionCardProps> = ({
onDelete,
onEdit,
}) => {
const {setSelectedAssertion} = useTestDefinition();
const {setSelectedAssertion, revert} = useTestDefinition();
const store = useStore();
const {selectedElements} = store.getState();

const selectedAssertion = useAppSelector(TestDefinitionSelectors.selectSelectedAssertion);
const {isDraft = false, isDeleted = false} =
useAppSelector(state => TestDefinitionSelectors.selectDefinitionBySelector(state, selector)) || {};
const {
isDraft = false,
isDeleted = false,
originalSelector = '',
} = useAppSelector(state => TestDefinitionSelectors.selectDefinitionBySelector(state, selector)) || {};
const spanCountText = `${spanIds.length} ${spanIds.length > 1 ? 'spans' : 'span'}`;

const getIsSelectedSpan = useCallback(
Expand All @@ -49,6 +52,13 @@ const AssertionCard: React.FC<TAssertionCardProps> = ({
setSelectedAssertion(selector);
};

const resetDefinition: React.MouseEventHandler = useCallback(
e => {
e.stopPropagation();
revert(originalSelector);
},
[revert, originalSelector]
);
return (
<S.AssertionCard
$isSelected={selectedAssertion === selector}
Expand All @@ -60,7 +70,14 @@ const AssertionCard: React.FC<TAssertionCardProps> = ({
<AssertionCardSelectorList selectorList={selectorList} pseudoSelector={pseudoSelector} />
</div>
<S.ActionsContainer>
{isDraft && <S.StatusTag>draft</S.StatusTag>}
{isDraft && (
<>
<S.StatusTag>draft</S.StatusTag>
<Button type="link" data-cy="assertion-action-revert" onClick={resetDefinition}>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if this onClick also needs the e.stopPropagation(); similar to what we have in the Edit/Delete icons.

Revert Assertion
</Button>
</>
)}
{isDeleted && <S.StatusTag color="#61175E">deleted</S.StatusTag>}
<S.SpanCountText>{spanCountText}</S.SpanCountText>
<Tooltip color="white" title="Edit Assertion">
Expand Down
1 change: 1 addition & 0 deletions web/src/models/AssertionResults.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const AssertionResults = ({allPassed = false, results = []}: TRawAssertionResult
id: uniqueId(),
selector,
spanIds: AssertionService.getSpanIds(resultList),
originalSelector: selector,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

creating the field on AsserionResult

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this in the model? I see we also have a assertionResultsToDefinitionList in web/src/redux/slices/TestDefinition.slice.ts

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is good here for the moment, but I get your point

pseudoSelector: SelectorService.getPseudoSelector(selector),
selectorList: SelectorService.getSpanSelectorList(selector),
resultList: resultList.map(assertionResult => AssertionResult(assertionResult)),
Expand Down
10 changes: 7 additions & 3 deletions web/src/providers/TestDefinition/TestDefinition.provider.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {noop} from 'lodash';
import {createContext, useContext, useMemo, useEffect, useCallback} from 'react';

import {useTestRun} from 'providers/TestRun/TestRun.provider';
import {createContext, useCallback, useContext, useEffect, useMemo} from 'react';
import {useGetTestByIdQuery} from 'redux/apis/TraceTest.api';
import {useAppDispatch, useAppSelector} from 'redux/hooks';
import {
Expand All @@ -15,6 +15,7 @@ import {TTestDefinitionEntry} from 'types/TestDefinition.types';
import useTestDefinitionCrud from './hooks/useTestDefinitionCrud';

interface IContext {
revert: (originalSelector: string) => void;
add(testDefinition: TTestDefinitionEntry): void;
update(selector: string, testDefinition: TTestDefinitionEntry): void;
remove(selector: string): void;
Expand All @@ -33,6 +34,7 @@ interface IContext {

export const Context = createContext<IContext>({
add: noop,
revert: () => noop,
update: noop,
remove: noop,
publish: noop,
Expand Down Expand Up @@ -62,7 +64,7 @@ const TestDefinitionProvider: React.FC<IProps> = ({children, testId, runId}) =>
const isInitialized = useAppSelector(state => TestDefinitionSelectors.selectIsInitialized(state));
const {data: test} = useGetTestByIdQuery({testId});

const {add, cancel, publish, remove, dryRun, update, isDraftMode, init, reset} = useTestDefinitionCrud({
const {add, cancel, publish, remove, dryRun, update, isDraftMode, init, reset, revert} = useTestDefinitionCrud({
testId,
runId,
});
Expand All @@ -75,7 +77,7 @@ const TestDefinitionProvider: React.FC<IProps> = ({children, testId, runId}) =>
return () => {
reset();
};
}, []);
}, [reset]);

useEffect(() => {
if (isInitialized && run.state === 'FINISHED') dryRun(definitionList);
Expand Down Expand Up @@ -111,8 +113,10 @@ const TestDefinitionProvider: React.FC<IProps> = ({children, testId, runId}) =>
test,
setAffectedSpans,
setSelectedAssertion,
revert,
}),
[
revert,
add,
assertionResults,
cancel,
Expand Down
10 changes: 9 additions & 1 deletion web/src/providers/TestDefinition/hooks/useTestDefinitionCrud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
updateDefinition,
reset as resetAction,
clearAffectedSpans,
revertDefinition,
} from '../../../redux/slices/TestDefinition.slice';
import {TAssertionResults} from '../../../types/Assertion.types';
import {TTestDefinitionEntry} from '../../../types/TestDefinition.types';
Expand All @@ -25,6 +26,13 @@ const useTestDefinitionCrud = ({runId, testId}: IProps) => {
const dispatch = useAppDispatch();
const navigate = useNavigate();

const revert = useCallback(
(originalSelector: string) => {
return dispatch(revertDefinition({originalSelector}));
},
[dispatch]
);

const dryRun = useCallback(
(definitionList: TTestDefinitionEntry[]) => {
return dispatch(TestDefinitionActions.dryRun({testId, runId, definitionList})).unwrap();
Expand Down Expand Up @@ -80,7 +88,7 @@ const useTestDefinitionCrud = ({runId, testId}: IProps) => {
dispatch(resetAction());
}, [dispatch]);

return {init, reset, add, remove, update, publish, cancel, dryRun, isDraftMode};
return {revert, init, reset, add, remove, update, publish, cancel, dryRun, isDraftMode};
};

export default useTestDefinitionCrud;
65 changes: 49 additions & 16 deletions web/src/redux/slices/TestDefinition.slice.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {createSlice, PayloadAction} from '@reduxjs/toolkit';
import {CaseReducer, createSlice, PayloadAction} from '@reduxjs/toolkit';

import TestDefinitionActions, {TChange} from 'redux/actions/TestDefinition.actions';
import {TAssertionResults} from 'types/Assertion.types';
import {TTestDefinitionEntry} from 'types/TestDefinition.types';
import {TAssertionResults} from '../../types/Assertion.types';
import {TTestDefinitionEntry} from '../../types/TestDefinition.types';
import TestDefinitionActions, {TChange} from '../actions/TestDefinition.actions';

interface ITestDefinitionState {
initialDefinitionList: TTestDefinitionEntry[];
Expand Down Expand Up @@ -30,38 +30,70 @@ export const assertionResultsToDefinitionList = (assertionResults: TAssertionRes
isDraft: false,
isDeleted: false,
selector,
originalSelector: selector,
assertionList: resultList.flatMap(({assertion}) => [assertion]),
}));
};

const testDefinitionSlice = createSlice({
const testDefinitionSlice = createSlice<
ITestDefinitionState,
{
reset: CaseReducer<ITestDefinitionState>;
initDefinitionList: CaseReducer<ITestDefinitionState, PayloadAction<{assertionResults: TAssertionResults}>>;
addDefinition: CaseReducer<ITestDefinitionState, PayloadAction<{definition: TTestDefinitionEntry}>>;
updateDefinition: CaseReducer<
ITestDefinitionState,
PayloadAction<{definition: TTestDefinitionEntry; selector: string}>
>;
removeDefinition: CaseReducer<ITestDefinitionState, PayloadAction<{selector: string}>>;
revertDefinition: CaseReducer<ITestDefinitionState, PayloadAction<{originalSelector: string}>>;
resetDefinitionList: CaseReducer<ITestDefinitionState>;
setAssertionResults: CaseReducer<ITestDefinitionState, PayloadAction<TAssertionResults>>;
clearAffectedSpans: CaseReducer<ITestDefinitionState>;
setAffectedSpans: CaseReducer<ITestDefinitionState, PayloadAction<string[]>>;
setSelectedAssertion: CaseReducer<ITestDefinitionState, PayloadAction<string>>;
},
'testDefinition'
>({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add better typescript typings

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm always up for this. Should this mean that we can drop the types from each of the reducer functions as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm 🤔 I'm not sure about this. IMO we should let typescript infer it when possible. It's doing it already.

Screen Shot 2022-06-01 at 10 23 59

name: 'testDefinition',
initialState,
reducers: {
reset() {
return initialState;
},
initDefinitionList(state, {payload: {assertionResults}}: PayloadAction<{assertionResults: TAssertionResults}>) {
initDefinitionList(state, {payload: {assertionResults}}) {
const definitionList = assertionResultsToDefinitionList(assertionResults);

state.initialDefinitionList = definitionList;
state.definitionList = definitionList;
state.isInitialized = true;
},
addDefinition(state, {payload: {definition}}: PayloadAction<{definition: TTestDefinitionEntry}>) {
addDefinition(state, {payload: {definition}}) {
state.definitionList = [...state.definitionList, definition];
},
updateDefinition(
state,
{payload: {definition, selector}}: PayloadAction<{definition: TTestDefinitionEntry; selector: string}>
) {
revertDefinition(state, {payload: {originalSelector}}) {
const initialDefinition = state.initialDefinitionList.find(
definition => definition.originalSelector === originalSelector
);

state.definitionList = initialDefinition
? state.definitionList.map(definition =>
definition.originalSelector === originalSelector ? initialDefinition : definition
)
: state.definitionList.filter(definition => definition.originalSelector === originalSelector);
},
updateDefinition(state, {payload: {definition, selector}}) {
state.definitionList = state.definitionList.map(def => {
if (def.selector === selector) return definition;
if (def.originalSelector === selector)
return {
...definition,
originalSelector: def.originalSelector,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need to ensure originalDefinition is carried over

};

return def;
});
},
removeDefinition(state, {payload: {selector}}: PayloadAction<{selector: string}>) {
removeDefinition(state, {payload: {selector}}) {
state.definitionList = state.definitionList.map(def => {
if (def.selector === selector)
return {
Expand All @@ -76,16 +108,16 @@ const testDefinitionSlice = createSlice({
resetDefinitionList(state) {
state.definitionList = state.initialDefinitionList;
},
setAssertionResults(state, {payload}: PayloadAction<TAssertionResults>) {
setAssertionResults(state, {payload}) {
state.assertionResults = payload;
},
clearAffectedSpans(state) {
state.affectedSpans = [];
},
setAffectedSpans(state, {payload: spanIds}: PayloadAction<string[]>) {
setAffectedSpans(state, {payload: spanIds}) {
state.affectedSpans = spanIds;
},
setSelectedAssertion(state, {payload: selectorId}: PayloadAction<string>) {
setSelectedAssertion(state, {payload: selectorId}) {
const assertionResult = state?.assertionResults?.resultList?.find(assertion => assertion.selector === selectorId);
const spanIds = assertionResult?.spanIds ?? [];
state.selectedAssertion = selectorId;
Expand Down Expand Up @@ -125,6 +157,7 @@ export const {
setAssertionResults,
initDefinitionList,
resetDefinitionList,
revertDefinition,
reset,
clearAffectedSpans,
setAffectedSpans,
Expand Down
32 changes: 28 additions & 4 deletions web/src/redux/slices/__tests__/TestDefinition.slice.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,26 @@ import TestRunMock from '../../../models/__mocks__/TestRun.mock';
import {TTestDefinitionEntry} from '../../../types/TestDefinition.types';
import Reducer, {
addDefinition,
assertionResultsToDefinitionList,
initDefinitionList,
resetDefinitionList,
initialState,
removeDefinition,
reset,
resetDefinitionList,
revertDefinition,
setAssertionResults,
initialState,
assertionResultsToDefinitionList,
updateDefinition,
} from '../TestDefinition.slice';

const {definitionList} = TestDefinitionMock.model();

const definitionSelector = `span[http.status_code] = "304"]`;

const definition: TTestDefinitionEntry = {
selector: `span[http.status_code] = "304"]`,
selector: definitionSelector,
isDraft: true,
assertionList: new Array(faker.datatype.number({min: 2, max: 10})).fill(null).map(() => AssertionMock.model()),
originalSelector: definitionSelector,
};

const state = {
Expand Down Expand Up @@ -99,6 +104,25 @@ describe('TestDefinitionReducer', () => {
definitionList: [definition, ...definitionList.slice(1, definitionList.length)],
});
});
it('should handle the revert definition action', () => {
const initialSelector = 'span[http.status_code] = "204"]';
const result = Reducer(
{
...state,
definitionList: [
{
...definition,
originalSelector: initialSelector,
},
],
},
revertDefinition({
originalSelector: definitionSelector,
})
);

expect(result.definitionList[0].originalSelector).toEqual(initialSelector);
});

it('should handle the remove definition action', () => {
const result = Reducer(
Expand Down
1 change: 1 addition & 0 deletions web/src/types/Assertion.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type TRawAssertionResults = TTestSchemas['AssertionResults'];
export type TAssertionResultEntry = {
id: string;
selector: string;
originalSelector: string;
spanIds: string[];
pseudoSelector?: TPseudoSelector;
selectorList: TSpanSelector[];
Expand Down
1 change: 1 addition & 0 deletions web/src/types/TestDefinition.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {Model, TTestSchemas} from './Common.types';
export type TRawTestDefinition = TTestSchemas['TestDefinition'];

export type TTestDefinitionEntry = {
originalSelector?: string;
selector: string;
assertionList: TAssertion[];
isDraft: boolean;
Expand Down