Skip to content

Add reselect and redux actions to organizations #269

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Aug 21, 2017
Merged

Add reselect and redux actions to organizations #269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8031126
Adding Immutable - Reselect - Redux Actions
Aug 8, 2017
0627705
Merge branch 'master' of github.com:gitpoint/git-point into immutable
alejandronanez Aug 14, 2017
da0d703
Initial refactor for reducer / actions / constants
alejandronanez Aug 14, 2017
8ecc214
Merge branch 'master' of github.com:gitpoint/git-point into immutable
alejandronanez Aug 19, 2017
ee8f821
Add reselect into organizations
alejandronanez Aug 19, 2017
9c7970f
More refactor´
alejandronanez Aug 19, 2017
800fb1e
Fixing base selector errors
alejandronanez Aug 19, 2017
37f1d83
Completed refactor for Organization's code
alejandronanez Aug 19, 2017
3e0e038
Fix eslint error
alejandronanez Aug 19, 2017
a08f0b4
Removing immutable
alejandronanez Aug 19, 2017
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
4 changes: 3 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@
"react-redux": "^5.0.2",
"react-syntax-highlighter": "^5.6.2",
"redux": "^3.6.0",
"redux-actions": "^2.2.1",
"redux-logger": "^2.7.4",
"redux-persist": "^4.3.1",
"redux-persist-transform-encrypt": "^1.0.2",
"redux-thunk": "^2.2.0"
"redux-thunk": "^2.2.0",
"reselect": "^3.0.1"
},
"devDependencies": {
"@commitlint/cli": "^3.1.0",
Expand Down
8 changes: 8 additions & 0 deletions src/auth/auth.selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { createSelector } from 'reselect';

export const getAuthFromStore = state => state.auth;

export const getAuthLanguage = createSelector(
getAuthFromStore,
auth => auth.getAuthLanguage
);
1 change: 1 addition & 0 deletions src/auth/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './auth.action';
export * from './auth.reducer';
export * from './auth.type';
export * from './auth.selectors';

export * from './screens';
3 changes: 2 additions & 1 deletion src/organization/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './organization.action';
export * from './organization.reducer';
export * from './organization.type';
export * from './organization.constants';
export * from './organization.selectors';

export * from './screens';
124 changes: 68 additions & 56 deletions src/organization/organization.action.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,68 +1,80 @@
import { fetchOrg, fetchOrgMembers, fetchUrl } from 'api';
import { GET_ORG, GET_ORG_REPOS, GET_ORG_MEMBERS } from './organization.type';
import { createAction } from 'redux-actions';

export const getOrg = orgName => {
return (dispatch, getState) => {
const accessToken = getState().auth.accessToken;
import {
fetchOrg,
fetchOrgMembers,
fetchUrl,
} from 'api';
import {
GET_ORG,
GET_ORG_LOADING,
GET_ORG_ERROR,
GET_ORG_REPOS,
GET_ORG_REPOS_LOADING,
GET_ORG_REPOS_ERROR,
GET_ORG_MEMBERS,
GET_ORG_MEMBERS_LOADING,
GET_ORG_MEMBERS_ERROR,
} from './organization.constants';

dispatch({ type: GET_ORG.PENDING });
export const getOrg = createAction(GET_ORG);
Copy link
Member Author

Choose a reason for hiding this comment

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

Creating actions by using redux-actions I think this is more organized than have a manual dispatch with some boilerplate like {type: ... , payload: ...}

Copy link
Member

Choose a reason for hiding this comment

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

Nice agreed, makes things look so much more oganized 👍

export const getOrgLoading = createAction(GET_ORG_LOADING);
export const getOrgError = createAction(GET_ORG_ERROR);
export const getOrgRepos = createAction(GET_ORG_REPOS);
export const getOrgReposLoading = createAction(GET_ORG_REPOS_LOADING);
export const getOrgReposError = createAction(GET_ORG_REPOS_ERROR);
export const getOrgMembers = createAction(GET_ORG_MEMBERS);
export const getOrgMembersLoading = createAction(GET_ORG_MEMBERS_LOADING);
export const getOrgMembersError = createAction(GET_ORG_MEMBERS_ERROR);

return fetchOrg(orgName, accessToken)
.then(data => {
dispatch({
type: GET_ORG.SUCCESS,
payload: data,
});
})
.catch(error => {
dispatch({
type: GET_ORG.ERROR,
payload: error,
});
});
};
export const fetchOrganizations = orgName => (dispatch, getState) => {
// use a selector here
const accessToken = getState().auth.accessToken;

dispatch(getOrgLoading(true));
dispatch(getOrgError(''));

return fetchOrg(orgName, accessToken)
.then(data => {
dispatch(getOrgLoading(false));
dispatch(getOrg(data));
})
.catch(error => {
dispatch(getOrgLoading(false));
dispatch(getOrgError(error));
});
};

export const getOrgRepos = url => {
return (dispatch, getState) => {
const accessToken = getState().auth.accessToken;
export const fetchOrganizationRepos = url => (dispatch, getState) => {
const accessToken = getState().auth.accessToken;

dispatch({ type: GET_ORG_REPOS.PENDING });
dispatch(getOrgReposLoading(true));
dispatch(getOrgReposError(''));

fetchUrl(url, accessToken)
.then(data => {
dispatch({
type: GET_ORG_REPOS.SUCCESS,
payload: data,
});
})
.catch(error => {
dispatch({
type: GET_ORG_REPOS.ERROR,
payload: error,
});
});
};
fetchUrl(url, accessToken)
.then(data => {
dispatch(getOrgReposLoading(false));
dispatch(getOrgRepos(data));
})
.catch(error => {
dispatch(getOrgReposLoading(false));
dispatch(getOrgReposError(error));
});
};

export const getOrgMembers = orgName => {
return (dispatch, getState) => {
const accessToken = getState().auth.accessToken;
export const fetchOrganizationMembers = orgName => (dispatch, getState) => {
const accessToken = getState().auth.accessToken;

dispatch({ type: GET_ORG_MEMBERS.PENDING });
dispatch(getOrgMembersLoading(true));
dispatch(getOrgMembersError(''));

return fetchOrgMembers(orgName, accessToken)
.then(data => {
dispatch({
type: GET_ORG_MEMBERS.SUCCESS,
payload: data,
});
})
.catch(error => {
dispatch({
type: GET_ORG_MEMBERS.ERROR,
payload: error,
});
});
};
return fetchOrgMembers(orgName, accessToken)
.then(data => {
dispatch(getOrgMembersLoading(false));
dispatch(getOrgMembers(data));
})
.catch(error => {
dispatch(getOrgMembersLoading(false));
dispatch(getOrgMembersError(error));
});
};
9 changes: 9 additions & 0 deletions src/organization/organization.constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export const GET_ORG = 'GET_ORG';
export const GET_ORG_LOADING = 'GET_ORG_LOADING';
export const GET_ORG_ERROR = 'GET_ORG_ERROR';
export const GET_ORG_REPOS = 'GET_ORG_REPOS';
export const GET_ORG_REPOS_LOADING = 'GET_ORG_REPOS_LOADING';
export const GET_ORG_REPOS_ERROR = 'GET_ORG_REPOS_ERROR';
export const GET_ORG_MEMBERS = 'GET_ORG_MEMBERS';
export const GET_ORG_MEMBERS_LOADING = 'GET_ORG_MEMBERS_LOADING';
export const GET_ORG_MEMBERS_ERROR = 'GET_ORG_MEMBERS_ERROR';
132 changes: 73 additions & 59 deletions src/organization/organization.reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
import { GET_ORG, GET_ORG_REPOS, GET_ORG_MEMBERS } from './organization.type';
import { handleActions } from 'redux-actions';

import {
GET_ORG,
GET_ORG_LOADING,
GET_ORG_ERROR,
GET_ORG_REPOS,
GET_ORG_REPOS_LOADING,
GET_ORG_REPOS_ERROR,
GET_ORG_MEMBERS,
GET_ORG_MEMBERS_LOADING,
GET_ORG_MEMBERS_ERROR,
} from './organization.constants';

const initialState = {
organization: {},
Expand All @@ -7,63 +19,65 @@ const initialState = {
isPendingOrg: false,
isPendingRepos: false,
isPendingMembers: false,
error: '',
organizationError: '',
Copy link
Member Author

Choose a reason for hiding this comment

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

Created one error for organization, another for repositories and another for members.

organizationRepositoriesError: '',
organizationMembersError: '',
};

export const organizationReducer = (state = initialState, action = {}) => {
switch (action.type) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Got rid of this ugly switch statement.

Copy link
Member

Choose a reason for hiding this comment

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

OMG how have I not used redux-actions before, it looks so much cleaner 😍

case GET_ORG.PENDING:
return {
...state,
isPendingOrg: true,
};
case GET_ORG.SUCCESS:
return {
...state,
organization: action.payload,
isPendingOrg: false,
};
case GET_ORG.ERROR:
return {
...state,
error: action.payload,
isPendingOrg: false,
};
case GET_ORG_REPOS.PENDING:
return {
...state,
isPendingRepos: true,
};
case GET_ORG_REPOS.SUCCESS:
return {
...state,
repositories: action.payload,
isPendingRepos: false,
};
case GET_ORG_REPOS.ERROR:
return {
...state,
error: action.payload,
isPendingRepos: false,
};
case GET_ORG_MEMBERS.PENDING:
return {
...state,
isPendingMembers: true,
};
case GET_ORG_MEMBERS.SUCCESS:
return {
...state,
members: action.payload,
isPendingMembers: false,
};
case GET_ORG_MEMBERS.ERROR:
return {
...state,
error: action.payload,
isPendingMembers: false,
};
default:
return state;
}
};
export const organizationReducer = handleActions({
[GET_ORG]: (state, { payload }) => {
return {
...state,
organization: payload,
};
},
[GET_ORG_LOADING]: (state, { payload }) => {
return {
...state,
isPendingOrg: payload,
};
},
[GET_ORG_ERROR]: (state, { payload }) => {
return {
...state,
organizationError: payload,
};
},
[GET_ORG_REPOS]: (state, { payload }) => {
return {
...state,
repositories: payload,
};
},
[GET_ORG_REPOS_LOADING]: (state, { payload }) => {
return {
...state,
isPendingRepos: payload,
};
},
[GET_ORG_REPOS_ERROR]: (state, { payload }) => {
return {
...state,
organizationRepositoriesError: payload,
};
},
[GET_ORG_MEMBERS]: (state, { payload }) => {
return {
...state,
members: payload,
};
},
[GET_ORG_MEMBERS_LOADING]: (state, { payload }) => {
return {
...state,
isPendingMembers: payload,
};
},
[GET_ORG_MEMBERS_ERROR]: (state, { payload }) => {
return {
...state,
organizationMembersError: payload,
};
},
}, initialState);

48 changes: 48 additions & 0 deletions src/organization/organization.selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { createSelector } from 'reselect';

const getOrganizationFromStore = state => state.organization;

export const getOrganization = createSelector(
getOrganizationFromStore,
organization => organization.organization || {}
Copy link
Member

Choose a reason for hiding this comment

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

I was having a disussion with colleagues from work this past week and a few were under the impression that when simply getting object parameters with dot notation doesn't necessarily need createSelector. For example, this would be sufficient:

export const getOrganization = state => state.organization.organization;

Not necessarily sure if I agree however. Majority of all our selectors are simple getters and not involve manipulation of data in anyway so it seems if we went with this approach we wouldn't really be using createSelector for the majority of our state.

Just wanted to get your input on this mate. Do you usually use createSelector for all data or only if you're retrieving data and performing manipulation to it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey dude! I usually use it to query the store and do some computations based on that data when needed. The big plus I see is that we can compose any selectors we want, for example combine one selector from organizations, one from repos and two from auth, then produce a new prop to pass to any component we want. I truly believe this is a good adition to the stack.

Here's a copy paste from reselect's site

Selectors can compute derived data, allowing Redux to store the minimal possible state.

Selectors are efficient. A selector is not recomputed unless one of its arguments change.

Selectors are composable. They can be used as input to other selectors.

);

export const getOrganizationRepositories = createSelector(
getOrganizationFromStore,
organization => organization.repositories || []
);

export const getOrganizationMembers = createSelector(
getOrganizationFromStore,
organization => organization.members || []
);

export const getOrganizationIsPendingOrg = createSelector(
getOrganizationFromStore,
organization => organization.isPendingOrg || false
);

export const getOrganizationIsPendingRepos = createSelector(
getOrganizationFromStore,
organization => organization.isPendingRepos || false
);

export const getOrganizationIsPendingMembers = createSelector(
getOrganizationFromStore,
organization => organization.isPendingMembers || false
);

export const getOrganizationError = createSelector(
getOrganizationFromStore,
organization => organization.organizationError || ''
);

export const getOrganizationRepositoriesError = createSelector(
getOrganizationFromStore,
organization => organization.organizationRepositoriesError || ''
);

export const getOrganizationMembersError = createSelector(
getOrganizationFromStore,
organization => organization.organizationMembersError || ''
);
5 changes: 0 additions & 5 deletions src/organization/organization.type.js
View file
Open in desktop

This file was deleted.

Loading