Skip to content

Commit

Permalink
Return mutable array in selector
Browse files Browse the repository at this point in the history
All usage in the UI already converts
the list into a mutable array, and
they do not modify the mutable array,
so it much more useful for the selector
to directly return the mutable array.
  • Loading branch information
Christopher-Chianelli committed Dec 1, 2020
1 parent 30494bd commit 8544140
Show file tree
Hide file tree
Showing 30 changed files with 93 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { alert } from 'store/alert';
import { createIdMapFromList } from 'util/ImmutableCollectionOperations';
import { onGet, onPost, onDelete } from 'store/rest/RestTestUtils';
import { Contract } from 'domain/Contract';
import { List } from 'immutable';
import { mockStore } from '../mockStore';
import { AppState } from '../types';
import * as actions from './actions';
Expand Down Expand Up @@ -270,12 +269,12 @@ describe('Contract selectors', () => {
...storeState,
contractList: { ...storeState.contractList, isLoading: true },
});
expect(contractList).toEqual(List());
expect(contractList).toEqual([]);
});

it('should return a list of all contracts', () => {
const contractList = contractSelectors.getContractList(storeState);
expect(contractList.toArray()).toEqual(expect.arrayContaining([
expect(contractList).toEqual(expect.arrayContaining([
{
tenantId: 0,
id: 0,
Expand Down Expand Up @@ -307,6 +306,6 @@ describe('Contract selectors', () => {
maximumMinutesPerYear: 100,
},
]));
expect(contractList.size).toEqual(3);
expect(contractList.length).toEqual(3);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/
import { Contract } from 'domain/Contract';
import { Map, List } from 'immutable';
import { Map } from 'immutable';
import DomainObjectView from 'domain/DomainObjectView';
import { AppState } from '../types';

Expand All @@ -26,11 +26,11 @@ export const getContractById = (state: AppState, id: number): Contract => {
};

let oldContractMapById: Map<number, DomainObjectView<Contract>> | null = null;
let contractListForOldContractMapById: List<Contract> | null = null;
let contractListForOldContractMapById: Contract[] | null = null;

export const getContractList = (state: AppState): List<Contract> => {
export const getContractList = (state: AppState): Contract[] => {
if (state.contractList.isLoading) {
return List();
return [];
}
if (oldContractMapById === state.contractList.contractMapById && contractListForOldContractMapById !== null) {
return contractListForOldContractMapById;
Expand All @@ -39,7 +39,7 @@ export const getContractList = (state: AppState): List<Contract> => {
.sortBy(contract => contract.name).toList();

oldContractMapById = state.contractList.contractMapById;
contractListForOldContractMapById = out;
contractListForOldContractMapById = out.toArray();

return out;
return contractListForOldContractMapById;
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { onGet, onPost, onDelete, onUploadFile } from 'store/rest/RestTestUtils'
import { Employee } from 'domain/Employee';
import * as skillActions from 'store/skill/actions';
import * as contractActions from 'store/contract/actions';
import { List } from 'immutable';
import { mockStore } from '../mockStore';
import { AppState } from '../types';
import * as actions from './actions';
Expand Down Expand Up @@ -351,22 +350,22 @@ describe('Employee selectors', () => {
...storeState,
skillList: { ...storeState.skillList, isLoading: true },
});
expect(employeeList).toEqual(List());
expect(employeeList).toEqual([]);
employeeList = employeeSelectors.getEmployeeList({
...storeState,
contractList: { ...storeState.contractList, isLoading: true },
});
expect(employeeList).toEqual(List());
expect(employeeList).toEqual([]);
employeeList = employeeSelectors.getEmployeeList({
...storeState,
employeeList: { ...storeState.employeeList, isLoading: true },
});
expect(employeeList).toEqual(List());
expect(employeeList).toEqual([]);
});

it('should return a list of all employee', () => {
const employeeList = employeeSelectors.getEmployeeList(storeState);
expect(employeeList.toArray()).toEqual(expect.arrayContaining([
expect(employeeList).toEqual(expect.arrayContaining([
{
tenantId: 0,
id: 1,
Expand Down Expand Up @@ -413,6 +412,6 @@ describe('Employee selectors', () => {
color: '#FFFFFF',
},
]));
expect(employeeList.size).toEqual(2);
expect(employeeList.length).toEqual(2);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { contractSelectors } from 'store/contract';
import { skillSelectors } from 'store/skill';
import { Employee } from 'domain/Employee';
import DomainObjectView from 'domain/DomainObjectView';
import { Map, List } from 'immutable';
import { Map } from 'immutable';
import { AppState } from '../types';

export const getEmployeeById = (state: AppState, id: number): Employee => {
Expand All @@ -33,11 +33,11 @@ export const getEmployeeById = (state: AppState, id: number): Employee => {
};

let oldEmployeeMapById: Map<number, DomainObjectView<Employee>> | null = null;
let employeeListForOldEmployeeMapById: List<Employee> | null = null;
let employeeListForOldEmployeeMapById: Employee[] | null = null;

export const getEmployeeList = (state: AppState): List<Employee> => {
export const getEmployeeList = (state: AppState): Employee[] => {
if (state.employeeList.isLoading || state.skillList.isLoading || state.contractList.isLoading) {
return List();
return [];
}
if (oldEmployeeMapById === state.employeeList.employeeMapById && employeeListForOldEmployeeMapById !== null) {
return employeeListForOldEmployeeMapById;
Expand All @@ -47,6 +47,6 @@ export const getEmployeeList = (state: AppState): List<Employee> => {
.sortBy(employee => employee.name).toList();

oldEmployeeMapById = state.employeeList.employeeMapById;
employeeListForOldEmployeeMapById = out;
return out;
employeeListForOldEmployeeMapById = out.toArray();
return employeeListForOldEmployeeMapById;
};
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import { RosterState } from 'domain/RosterState';
import { ShiftRosterView } from 'domain/ShiftRosterView';
import { PaginationData, ObjectNumberMap, mapObjectNumberMap, mapObjectStringMap, error } from 'types';
import { PaginationData, ObjectNumberMap, mapObjectNumberMap, mapObjectStringMap } from 'types';
import moment from 'moment';
import { Spot } from 'domain/Spot';
import { alert } from 'store/alert';
Expand Down Expand Up @@ -176,7 +176,7 @@ ThunkCommandFactory<void, ShiftRosterViewAction> = () => (dispatch, state) => {
const startDate = moment(rosterState.firstDraftDate).startOf('week').toDate();
const endDate = moment(rosterState.firstDraftDate).endOf('week').toDate();
const spotList = spotSelectors.getSpotList(state());
const shownSpots = (spotList.size > 0) ? [spotList.get(0) ?? error()] : [];
const shownSpots = (spotList.length > 0) ? [spotList[0]] : [];

if (shownSpots.length > 0) {
dispatch(getShiftRosterFor({
Expand All @@ -199,7 +199,7 @@ ThunkCommandFactory<void, AvailabilityRosterViewAction> = () => (dispatch, state
const startDate = moment(rosterState.firstDraftDate).startOf('week').toDate();
const endDate = moment(rosterState.firstDraftDate).endOf('week').toDate();
const employeeList = employeeSelectors.getEmployeeList(state());
const shownEmployees = (employeeList.size > 0) ? [employeeList.get(0) ?? error()] : [];
const shownEmployees = (employeeList.length > 0) ? [employeeList[0]] : [];

if (shownEmployees.length > 0) {
dispatch(getAvailabilityRosterFor({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { Employee } from 'domain/Employee';
import { RosterState } from 'domain/RosterState';
import { serializeLocalDate } from 'store/rest/DataSerialization';
import { flushPromises } from 'setupTests';
import { doNothing, error } from 'types';
import { doNothing } from 'types';
import { Map, List } from 'immutable';
import { createIdMapFromList } from 'util/ImmutableCollectionOperations';
import { availabilityRosterReducer } from './reducers';
Expand Down Expand Up @@ -669,7 +669,7 @@ describe('Roster operations', () => {
it('should dispatch actions and call client on getInitialShiftRoster', async () => {
const { store, client } = mockStore(state);
const tenantId = store.getState().tenantData.currentTenantId;
const spotList: Spot[] = [spotSelectors.getSpotList(store.getState()).get(0) ?? error()];
const spotList: Spot[] = [spotSelectors.getSpotList(store.getState())[0]];
const fromDate = moment((store.getState().rosterState.rosterState as RosterState).firstDraftDate)
.startOf('week').toDate();
const toDate = moment((store.getState().rosterState.rosterState as RosterState).firstDraftDate)
Expand Down Expand Up @@ -822,7 +822,7 @@ describe('Roster operations', () => {
it('should dispatch actions and call client on getInitialAvailabilityRoster', async () => {
const { store, client } = mockStore(state);
const tenantId = store.getState().tenantData.currentTenantId;
const employeeList: Employee[] = [employeeSelectors.getEmployeeList(store.getState()).get(0) ?? error()];
const employeeList: Employee[] = [employeeSelectors.getEmployeeList(store.getState())[0]];
const fromDate = moment((store.getState().rosterState.rosterState as RosterState).firstDraftDate)
.startOf('week').toDate();
const toDate = moment((store.getState().rosterState.rosterState as RosterState).firstDraftDate)
Expand Down
12 changes: 6 additions & 6 deletions optaweb-employee-rostering-frontend/src/store/skill/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/
import { Skill } from 'domain/Skill';
import { List, Map } from 'immutable';
import { Map } from 'immutable';
import DomainObjectView from 'domain/DomainObjectView';
import { AppState } from '../types';

Expand All @@ -27,11 +27,11 @@ export const getSkillById = (state: AppState, id: number): Skill => {


let oldSkillMapById: Map<number, DomainObjectView<Skill>>| null = null;
let skillListForOldSkillMapById: List<Skill> | null = null;
let skillListForOldSkillMapById: Skill[] | null = null;

export const getSkillList = (state: AppState): List<Skill> => {
export const getSkillList = (state: AppState): Skill[] => {
if (state.skillList.isLoading) {
return List();
return [];
}
if (oldSkillMapById === state.skillList.skillMapById && skillListForOldSkillMapById !== null) {
return skillListForOldSkillMapById;
Expand All @@ -41,6 +41,6 @@ export const getSkillList = (state: AppState): List<Skill> => {
.sortBy(skill => skill.name).toList();

oldSkillMapById = state.skillList.skillMapById;
skillListForOldSkillMapById = out;
return out;
skillListForOldSkillMapById = out.toArray();
return skillListForOldSkillMapById;
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { alert } from 'store/alert';
import { createIdMapFromList } from 'util/ImmutableCollectionOperations';
import { onGet, onPost, onDelete } from 'store/rest/RestTestUtils';
import { Skill } from 'domain/Skill';
import { List } from 'immutable';
import { mockStore } from '../mockStore';
import { AppState } from '../types';
import * as actions from './actions';
Expand Down Expand Up @@ -202,12 +201,12 @@ describe('Skill selectors', () => {
...storeState,
skillList: { ...storeState.skillList, isLoading: true },
});
expect(skillList).toEqual(List());
expect(skillList).toEqual([]);
});

it('should return a list of all skills', () => {
const skillList = skillSelectors.getSkillList(storeState);
expect(skillList.toArray()).toEqual(expect.arrayContaining([
expect(skillList).toEqual(expect.arrayContaining([
{
tenantId: 0,
id: 1234,
Expand All @@ -221,6 +220,6 @@ describe('Skill selectors', () => {
name: 'Skill 3',
},
]));
expect(skillList.size).toEqual(2);
expect(skillList.length).toEqual(2);
});
});
12 changes: 6 additions & 6 deletions optaweb-employee-rostering-frontend/src/store/spot/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import { skillSelectors } from 'store/skill';
import { Spot } from 'domain/Spot';
import DomainObjectView from 'domain/DomainObjectView';
import { Map, List } from 'immutable';
import { Map } from 'immutable';
import { AppState } from '../types';

export const getSpotById = (state: AppState, id: number): Spot => {
Expand All @@ -31,11 +31,11 @@ export const getSpotById = (state: AppState, id: number): Spot => {
};

let oldSpotMapById: Map<number, DomainObjectView<Spot>> | null = null;
let spotListForOldSpotMapById: List<Spot> | null = null;
let spotListForOldSpotMapById: Spot[] | null = null;

export const getSpotList = (state: AppState): List<Spot> => {
export const getSpotList = (state: AppState): Spot[] => {
if (state.spotList.isLoading || state.skillList.isLoading) {
return List();
return [];
}
if (oldSpotMapById === state.spotList.spotMapById && spotListForOldSpotMapById !== null) {
return spotListForOldSpotMapById;
Expand All @@ -45,6 +45,6 @@ export const getSpotList = (state: AppState): List<Spot> => {
.sortBy(spot => spot.name).toList();

oldSpotMapById = state.spotList.spotMapById;
spotListForOldSpotMapById = out;
return out;
spotListForOldSpotMapById = out.toArray();
return spotListForOldSpotMapById;
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { alert } from 'store/alert';
import { createIdMapFromList, mapDomainObjectToView } from 'util/ImmutableCollectionOperations';
import { onGet, onPost, onDelete } from 'store/rest/RestTestUtils';
import { Spot } from 'domain/Spot';
import { List } from 'immutable';
import { mockStore } from '../mockStore';
import { AppState } from '../types';
import * as actions from './actions';
Expand Down Expand Up @@ -263,17 +262,17 @@ describe('Spot selectors', () => {
...storeState,
skillList: { ...storeState.skillList, isLoading: true },
});
expect(spotList).toEqual(List());
expect(spotList).toEqual([]);
spotList = spotSelectors.getSpotList({
...storeState,
spotList: { ...storeState.spotList, isLoading: true },
});
expect(spotList).toEqual(List());
expect(spotList).toEqual([]);
});

it('should return a list of all spots', () => {
const spotList = spotSelectors.getSpotList(storeState);
expect(spotList.toArray()).toEqual(expect.arrayContaining([
expect(spotList).toEqual(expect.arrayContaining([
{
tenantId: 0,
id: 1234,
Expand All @@ -296,6 +295,6 @@ describe('Spot selectors', () => {
requiredSkillSet: [],
},
]));
expect(spotList.size).toEqual(2);
expect(spotList.length).toEqual(2);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { useTranslation, Trans } from 'react-i18next';
import Actions from 'ui/components/Actions';
import { getRouterProps } from 'util/BookmarkableTestUtils';
import Schedule from 'ui/components/calendar/Schedule';
import { List } from 'immutable';
import {
AvailabilityRosterPage, Props, ShiftOrAvailability, isShift, isAvailability,
isAllDayAvailability, isDay, AvailabilityRosterUrlProps,
Expand All @@ -49,8 +48,8 @@ describe('Availability Roster Page', () => {
const availabilityRosterPage = shallow(<AvailabilityRosterPage
{...baseProps}
isLoading
allEmployeeList={List()}
shownEmployeeList={List()}
allEmployeeList={[]}
shownEmployeeList={[]}
employeeIdToAvailabilityListMap={new Map()}
employeeIdToShiftListMap={new Map()}
/>);
Expand All @@ -60,8 +59,8 @@ describe('Availability Roster Page', () => {
it('should render correctly when there are no employees', () => {
const availabilityRosterPage = shallow(<AvailabilityRosterPage
{...baseProps}
allEmployeeList={List()}
shownEmployeeList={List()}
allEmployeeList={[]}
shownEmployeeList={[]}
employeeIdToAvailabilityListMap={new Map()}
employeeIdToShiftListMap={new Map()}
/>);
Expand Down Expand Up @@ -98,7 +97,7 @@ describe('Availability Roster Page', () => {
expect(baseProps.getAvailabilityRosterFor).toBeCalledWith({
fromDate: newDateStart,
toDate: newDateEnd,
employeeList: baseProps.shownEmployeeList.toArray(),
employeeList: baseProps.shownEmployeeList,
});
});

Expand All @@ -119,8 +118,8 @@ describe('Availability Roster Page', () => {
it('should go to the Employees page if the user click on the link', () => {
const availabilityRosterPage = shallow(<AvailabilityRosterPage
{...baseProps}
allEmployeeList={List()}
shownEmployeeList={List()}
allEmployeeList={[]}
shownEmployeeList={[]}
employeeIdToAvailabilityListMap={new Map()}
employeeIdToShiftListMap={new Map()}
/>);
Expand Down Expand Up @@ -640,8 +639,8 @@ const baseProps: Props = {
isLoading: false,
totalNumOfSpots: 1,
rosterState,
allEmployeeList: List([employee, newEmployee]),
shownEmployeeList: List([employee]),
allEmployeeList: [employee, newEmployee],
shownEmployeeList: [employee],
employeeIdToShiftListMap: new Map([
[4, [shift]],
]),
Expand Down
Loading

0 comments on commit 8544140

Please sign in to comment.