Skip to content

Commit 82957b7

Browse files
committed
fix: makeRequest never dispatch error even if api res status is error
1 parent 649a0f5 commit 82957b7

6 files changed

Lines changed: 61 additions & 34 deletions

File tree

src/app/containers/StarsPage.test.tsx

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ import {mapStateToProps, UnconnectedStars} from "./StarsPage";
66

77
describe("<Stars />", () => {
88
it("matches snapshot when rendering stars", () => {
9-
const component = shallow(<UnconnectedStars isFetching={false} stargazers_count={61} dispatch={jest.fn()} />);
9+
const component = shallow(<UnconnectedStars isFetching={false} stargazersCount={61} dispatch={jest.fn()} />);
1010
expect(component).toMatchSnapshot();
1111
});
1212

1313
it("matches snapshot when rendering fetching text", () => {
14-
const component = shallow(<UnconnectedStars isFetching={true} stargazers_count={-1} dispatch={jest.fn()} />);
14+
const component = shallow(<UnconnectedStars isFetching={true} stargazersCount={-1} dispatch={jest.fn()} />);
1515
expect(component).toMatchSnapshot();
1616
});
1717

@@ -23,22 +23,22 @@ describe("<Stars />", () => {
2323
}
2424
};
2525
const props = mapStateToProps({stars});
26-
expect(props).toEqual({isFetching: false, stargazers_count: 100});
26+
expect(props).toEqual({errorMessage: undefined, isFetching: false, stargazersCount: 100});
2727
});
2828

29-
it("dispatches LOAD_STARS action before rendering if stargazers_count === -1", () => {
29+
it("dispatches LOAD_STARS action before rendering if stargazersCount === -1", () => {
3030
const dispatch = jest.fn();
3131
const expectedValue: IAction<IStars> = {
3232
type: LOAD_STARS
3333
};
3434
expect(dispatch).not.toHaveBeenCalled();
35-
shallow(<UnconnectedStars dispatch={dispatch} isFetching={false} stargazers_count={-1}/>);
35+
shallow(<UnconnectedStars dispatch={dispatch} isFetching={false} stargazersCount={-1}/>);
3636
expect(dispatch).toHaveBeenCalledWith(expectedValue);
3737
});
3838

39-
it("does not dispatch LOAD_STARS action before rendering if stargazers_count !== -1", () => {
39+
it("does not dispatch LOAD_STARS action before rendering if stargazersCount !== -1", () => {
4040
const dispatch = jest.fn();
41-
shallow(<UnconnectedStars dispatch={dispatch} isFetching={false} stargazers_count={10}/>);
41+
shallow(<UnconnectedStars dispatch={dispatch} isFetching={false} stargazersCount={10}/>);
4242
expect(dispatch).not.toHaveBeenCalled();
4343
});
4444
});

src/app/containers/StarsPage.tsx

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,29 +7,39 @@ import {loadStars} from "../redux/modules/starsModule";
77

88
class StarsPage extends React.Component<IStateToProps & IDispatchToProps> {
99
public componentWillMount(): void {
10-
if (this.props.stargazers_count === -1) {
10+
if (this.props.stargazersCount === -1) {
1111
this.props.dispatch(loadStars());
1212
}
1313
}
1414

1515
public render(): JSX.Element {
16-
const {isFetching, stargazers_count} = this.props;
1716
return (
1817
<div>
19-
{isFetching ? <FormattedMessage id="stars.fetching" defaultMessage="Fetching Stars.." /> : stargazers_count}
18+
{this.renderStars()}
2019
</div>
2120
);
2221
}
22+
23+
private renderStars(): JSX.Element | string | number {
24+
const {errorMessage, isFetching, stargazersCount} = this.props;
25+
if (isFetching) {
26+
return <FormattedMessage id="stars.fetching" defaultMessage="Fetching Stars.." />;
27+
} else {
28+
return errorMessage ? errorMessage : stargazersCount;
29+
}
30+
}
2331
}
2432

2533
interface IStateToProps {
2634
isFetching: boolean;
27-
stargazers_count: number;
35+
stargazersCount: number;
36+
errorMessage?: string;
2837
}
2938

3039
const mapStateToProps = (state: Pick<IStore, "stars">) => ({
40+
errorMessage: state.stars.message,
3141
isFetching: state.stars.isFetching,
32-
stargazers_count: state.stars.payload.stargazers_count
42+
stargazersCount: state.stars.payload.stargazers_count
3343
});
3444
const connectedStars = connect<IStateToProps, IDispatchToProps>(mapStateToProps)(StarsPage);
3545
export {StarsPage as UnconnectedStars, connectedStars as StarsPage, mapStateToProps};

src/app/sagas/dummyApi.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,13 @@ import {IStars} from "../redux/modules/starsModule";
55
// tslint:disable:no-http-string
66

77
export const dummyApi = {
8-
getStars: (): Promise<IStars> => {
9-
return fetch("https://api.github.com/repos/barbar/vortigern").then((res) => res.json());
8+
getStars: (): Promise<IStars | {error: string}> => {
9+
return fetch("https://api.github.com/repos/barbar/vortigern").then((res) => {
10+
if (res.ok) {
11+
return res.json();
12+
}
13+
return res.json().then((json) => ({error: json.message}));
14+
});
1015
},
1116
getTranslations: (payload: string): Promise<ISettings> => {
1217
return fetch(`http://localhost:8889/translation/${payload}`).then((res) => res.json());

src/app/sagas/makeRequest.test.ts

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,46 @@ import makeRequest from "./makeRequest";
33

44
describe("makeRequest", () => {
55
const promiseFunction = () => Promise.resolve("success!");
6-
const gen = makeRequest(
7-
{
8-
FULFILLED: "FULFILLED_ACTION",
9-
PENDING: "PENDING_ACTION",
10-
REJECTED: "REJECTED_ACTION"
11-
},
12-
promiseFunction,
13-
"arg1",
14-
"arg2"
15-
);
6+
let gen;
7+
8+
beforeEach(() => {
9+
gen = makeRequest(
10+
{
11+
FULFILLED: "FULFILLED_ACTION",
12+
PENDING: "PENDING_ACTION",
13+
REJECTED: "REJECTED_ACTION"
14+
},
15+
promiseFunction,
16+
"arg1",
17+
"arg2"
18+
);
19+
});
1620

1721
it("must dispatch actionPending", () => {
1822
expect(gen.next().value).toEqual(put({type: "PENDING_ACTION"}));
1923
});
2024

2125
it("must call apiMethod with correct arguments", () => {
26+
gen.next();
2227
expect(gen.next().value).toEqual(call(promiseFunction, "arg1", "arg2", undefined, undefined, undefined, undefined));
2328
});
2429

25-
it("must dispatch actionSuccess if promise is resolved", () => {
30+
it("must dispatch actionSuccess", () => {
31+
gen.next();
32+
gen.next();
2633
expect(gen.next("data").value).toEqual(put({type: "FULFILLED_ACTION", payload: "data"}));
2734
});
2835

29-
it("must dispatch actionFailure if promise is rejected", () => {
30-
expect(gen.throw({message: "error!"}).value).toEqual(put({type: "REJECTED_ACTION", message: "error!"}));
36+
it("must dispatch actionFailure ", () => {
37+
gen.next();
38+
gen.next();
39+
expect(gen.next({error: "error!"}).value).toEqual(put({type: "REJECTED_ACTION", message: "error!"}));
3140
});
3241

3342
it("must be done", () => {
43+
gen.next();
44+
gen.next();
45+
gen.next("data");
3446
expect(gen.next()).toEqual({done: true, value: undefined});
3547
});
3648
});

src/app/sagas/makeRequest.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@ import {IPromiseActions} from "../helpers/promiseReducer";
44

55
// tslint:disable:max-line-length
66
export default function* makeRequest(requestType: IPromiseActions, apiMethod: (payload: any) => Promise<any>, ...args: any[]): IterableIterator<PutEffect<Action> | CallEffect> {
7-
try {
8-
yield put({type: requestType.PENDING});
9-
const [arg0, arg1, arg2, arg3, arg4, arg5, ...rest] = args; // we do this because of redux saga typing limitation
10-
const payload = yield call(apiMethod, arg0, arg1, arg2, arg3, arg4, arg5, ...rest);
7+
yield put({type: requestType.PENDING});
8+
const [arg0, arg1, arg2, arg3, arg4, arg5, ...rest] = args; // we do this because of redux saga typing limitation
9+
const payload = yield call(apiMethod, arg0, arg1, arg2, arg3, arg4, arg5, ...rest);
10+
if (payload.error) {
11+
yield put({type: requestType.REJECTED, message: payload.error});
12+
} else {
1113
yield put({type: requestType.FULFILLED, payload});
12-
} catch (e) {
13-
yield put({type: requestType.REJECTED, message: e.message});
1414
}
1515
}

src/server.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ app.get("*", (req, res) => {
6666
return;
6767
}
6868

69-
const languageHelper = new LanguageHelper(req.headers["accept-settings"]);
69+
const languageHelper = new LanguageHelper(req.headers["accept-language"]);
7070
const store = configureStore(router, {
7171
router: {
7272
previousRoute: null,

0 commit comments

Comments
 (0)