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

UIPFU-77 - Add 'User assignment status' filter group #249

Merged
merged 11 commits into from
Jan 16, 2024
25 changes: 24 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
@@ -1,4 +1,27 @@
{
"parser": "@babel/eslint-parser",
"extends": "@folio/eslint-config-stripes"
"extends": "@folio/eslint-config-stripes",
"overrides": [
{
"files": [ "src/**/tests/*", "*.test.js", "test/**/*" ],
"rules": {
"react/prop-types": "off",
"import/prefer-default-export": "off"
}
}
],
"env": {
"jest": true
},
"settings": {
"import/resolver": {
"alias": {
"map": [
["__mock__", "./test/jest/__mock__"],
["fixtures", "./test/jest/fixtures"],
["helpers", "./test/jest/helpers"]
]
}
}
}
}
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
node_modules
artifacts
artifacts
junit.xml
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## 7.1.0 IN PROGRESS

* Add "User assignment status" filter group. Refs UIPFU-77.

## [7.0.0](https://github.com/folio-org/ui-plugin-find-user/tree/v7.0.0) (2023-10-12)
[Full Changelog](https://github.com/folio-org/ui-plugin-find-user/compare/v6.3.0...v7.0.0)

Expand Down
10 changes: 10 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const path = require('path');
const config = require('@folio/jest-config-stripes');

module.exports = {
...config,
setupFiles: [
...config.setupFiles,
path.join(__dirname, './test/jest/setupFiles.js'),
],
};
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
"lint": "eslint .",
"build-mod-descriptor": "stripes mod descriptor --full --strict | jq '.[]' > module-descriptor.json ",
"formatjs-compile": "formatjs compile-folder --ast --format simple ./translations/ui-plugin-find-user ./translations/ui-plugin-find-user/compiled",
"test": "stripes test karma"
"test": "stripes test karma",
"test:jest": "jest --ci --coverage --colors"
},
"okapiInterfaces": {
"users": "16.0",
Expand All @@ -37,6 +38,7 @@
"@bigtest/interactor": "^0.9.1",
"@bigtest/mocha": "^0.5.2",
"@folio/eslint-config-stripes": "^7.0.0",
"@folio/jest-config-stripes": "^2.0.0",
"@folio/stripes": "^9.0.0",
"@folio/stripes-cli": "^3.0.0",
"@folio/stripes-core": "^10.0.0",
Expand Down
92 changes: 72 additions & 20 deletions src/UserSearchContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ import {
StripesConnectedSource,
} from '@folio/stripes/smart-components';

import filterConfig from './filterConfig';
import filterConfig, { filterConfigWithUserAssignedStatus } from './filterConfig';
import { updateResourceData } from './utils';
import {
ASSIGNED_FILTER_KEY,
UNASSIGNED_FILTER_KEY,
UAS,
} from './constants';

const INITIAL_RESULT_COUNT = 30;
const RESULT_COUNT_INCREMENT = 30;
Expand All @@ -32,6 +38,28 @@ const compileQuery = template(
{ interpolate: /%{([\s\S]+?)}/g }
);

export function buildQuery(queryParams, pathComponents, resourceData, logger, props) {
const filters = props.initialSelectedUsers ? filterConfigWithUserAssignedStatus : filterConfig;
const updatedResourceData = props.initialSelectedUsers && resourceData?.query?.filters?.includes(`${UAS}`) ? updateResourceData(resourceData) : resourceData;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const updatedResourceData = props.initialSelectedUsers && resourceData?.query?.filters?.includes(`${UAS}`) ? updateResourceData(resourceData) : resourceData;
const updatedResourceData = props.initialSelectedUsers && resourceData?.query?.filters?.includes(UAS) ? updateResourceData(resourceData) : resourceData;


return makeQueryFunction(
'cql.allRecords=1',
(parsedQuery, _, localProps) => localProps.query.query.trim().split(/\s+/).map(query => compileQuery({ query })).join(' and '),
{
// the keys in this object must match those passed to
// SearchAndSort's columnMapping prop
'active': 'active',
'name': 'personal.lastName personal.firstName',
'patronGroup': 'patronGroup.group',
'username': 'username',
'barcode': 'barcode',
'email': 'personal.email',
},
filters,
2,
)(queryParams, pathComponents, updatedResourceData, logger, props);
}

class UserSearchContainer extends React.Component {
static manifest = Object.freeze({
initializedFilterConfig: { initialValue: false },
Expand All @@ -46,24 +74,7 @@ class UserSearchContainer extends React.Component {
perRequest: 100,
path: 'users',
GET: {
params: {
query: makeQueryFunction(
'cql.allRecords=1',
(parsedQuery, props, localProps) => localProps.query.query.trim().split(/\s+/).map(query => compileQuery({ query })).join(' and '),
{
// the keys in this object must match those passed to
// SearchAndSort's columnMapping prop
'active': 'active',
'name': 'personal.lastName personal.firstName',
'patronGroup': 'patronGroup.group',
'username': 'username',
'barcode': 'barcode',
'email': 'personal.email',
},
filterConfig,
2,
),
},
params: { query: buildQuery },
staticFallback: { params: {} },
},
},
Expand Down Expand Up @@ -113,6 +124,7 @@ class UserSearchContainer extends React.Component {
*/
// eslint-disable-next-line react/no-unused-prop-types
tenantId: PropTypes.string.isRequired,
initialSelectedUsers: PropTypes.object,
}

constructor(props) {
Expand Down Expand Up @@ -168,6 +180,46 @@ class UserSearchContainer extends React.Component {
return get(this.props.resources, 'query', {});
}

getUsers = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest to move this function outside the component. I will much easier to test all if...else conditions.
But it is up to you.

const {
resources,
initialSelectedUsers,
} = this.props;
const fetchedUsers = get(resources, 'records.records', []);
const activeFilters = get(resources, 'query.filters', '');

if (activeFilters.includes(`${UAS}`)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (activeFilters.includes(`${UAS}`)) {
if (activeFilters.includes(UAS)) {

const assignedUsers = Object.values(initialSelectedUsers);
const assignedUserIds = Object.keys(initialSelectedUsers);
const hasBothUASFilters = activeFilters.includes(`${ASSIGNED_FILTER_KEY}`) && activeFilters.includes(`${UNASSIGNED_FILTER_KEY}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const hasBothUASFilters = activeFilters.includes(`${ASSIGNED_FILTER_KEY}`) && activeFilters.includes(`${UNASSIGNED_FILTER_KEY}`);
const hasBothUASFilters = activeFilters.includes(ASSIGNED_FILTER_KEY) && activeFilters.includes(UNASSIGNED_FILTER_KEY);

const hasNoneOfUASFilters = !activeFilters.includes(`${ASSIGNED_FILTER_KEY}`) && !activeFilters.includes(`${UNASSIGNED_FILTER_KEY}`);

if (hasBothUASFilters || hasNoneOfUASFilters) {
return fetchedUsers;
}
const uasFilterValue = activeFilters.split(',').filter(f => f.includes(`${UAS}`))[0].split('.')[1];

let otherFilterGroups = activeFilters.split(',').filter(f => !f.includes(`${UAS}`)).map(f => f.split('.')[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need check other filter values because this result list is alredy returned from back-end for a search with these filters. We just need to select from fetched users assigned or not assigned based on uas filter value

if (activeFilters.includes(`${UAS}`)) {
      const assignedUserIds = Object.keys(initialSelectedUsers);
      
      const uasFilterValues = activeFilters.split(',').filter(f => f.find(`${UAS}`));
      
      // both or neither Assignment status values are selected, so we can return result from BE
      if (uasFilterValues.length === 2 || uasFilterValues.length === 0) {
        return fetchedUsers;
      }
      
      if (uasFilterValues.includes(ASSIGNED_FILTER_KEY)) {
        return fetchedUsers.filter(user => assignedUserIds.includes(user.id));
      } else {
        return fetchedUsers.filter(user => !assignedUserIds.includes(user.id));
      }
}

return fetchedUsers;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you again for your review!
I agree that it is not needed for Unassigned filter as we are propagating the filters to make cql. But I think it is needed in case of Assigned filter as we are not propagating the filter string to make the cql(as we don't actually need to fetch)

if (otherFilterGroups.indexOf('pg') !== -1) {
otherFilterGroups = otherFilterGroups.with(otherFilterGroups.indexOf('pg'), 'patronGroup');
}

let otherFilterValues = activeFilters.split(',').filter(f => !f.includes(`${UAS}`)).map(f => f.split('.')[1]);
if (otherFilterValues.indexOf('active') !== -1) {
otherFilterValues = otherFilterValues.with(otherFilterValues.indexOf('active'), true);
}

if (uasFilterValue === 'Assigned') {
if (!otherFilterGroups.length) return assignedUsers;
return assignedUsers.filter(u => otherFilterGroups.every((g, i) => u[g] === otherFilterValues[i]));
}
const unAssignedUsers = fetchedUsers.filter(u => !assignedUserIds.includes(u.id));
if (!otherFilterGroups.length) return unAssignedUsers;
return unAssignedUsers.filter(u => otherFilterGroups.every((g, i) => u[g] === otherFilterValues[i]));
}
return fetchedUsers;
}

render() {
const {
resources,
Expand All @@ -188,7 +240,7 @@ class UserSearchContainer extends React.Component {
resultOffset,
data: {
patronGroups: (resources.patronGroups || {}).records || [],
users: get(resources, 'records.records', []),
users: this.getUsers(),
},
});
}
Expand Down
1 change: 1 addition & 0 deletions src/UserSearchModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ class UserSearchModal extends Component {
{...this.props}
onComponentWillUnmount={onCloseModal}
tenantId={tenantId || stripes.okapi.tenant}
initialSelectedUsers={initialSelectedUsers}
>
{(viewProps) => <UserSearchView
{...viewProps}
Expand Down
12 changes: 10 additions & 2 deletions src/UserSearchView.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
SearchAndSortSearchButton as FilterPaneToggle,
} from '@folio/stripes/smart-components';

import filterConfig from './filterConfig';
import filterConfig, { filterConfigWithUserAssignedStatus } from './filterConfig';
import Filters from './Filters';

import css from './UserSearch.css';
Expand Down Expand Up @@ -173,6 +173,13 @@ class UserSearchView extends React.Component {

isSelected = ({ item }) => Boolean(this.state.checkedMap[item.id]);

getFilterConfig = () => {
if (this.props.initialSelectedUsers) {
return filterConfigWithUserAssignedStatus;
}
return filterConfig;
}

render() {
const {
onSelectRow,
Expand Down Expand Up @@ -339,7 +346,7 @@ class UserSearchView extends React.Component {
<Filters
onChangeHandlers={getFilterHandlers()}
activeFilters={activeFilters}
config={filterConfig}
config={this.getFilterConfig()}
resultOffset={resultOffset}
/>
</form>
Expand All @@ -355,6 +362,7 @@ class UserSearchView extends React.Component {
<MultiColumnList
visibleColumns={builtVisibleColumns}
isSelected={this.isSelected}
// contentData={getContentData()}
contentData={users}
totalCount={count}
id="list-plugin-find-user"
Expand Down
8 changes: 8 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/* eslint-disable import/prefer-default-export */
export const UAS = 'uas';
export const ASSIGNED_FILTER_KEY = 'uas.Assigned';
export const UNASSIGNED_FILTER_KEY = 'uas.Unassigned';
// export const ASSIGNED = 'Assigned';
// export const UNASSIGNED = 'Unassigned';
// export const ACTIVE = 'active';
// export const INACTIVE = 'inactive';
21 changes: 21 additions & 0 deletions src/filterConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,25 @@ const filterConfig = [
},
];

export const filterConfigWithUserAssignedStatus = [
...filterConfig,
{
label: <FormattedMessage id="ui-plugin-find-user.userAssignmentStatus" />,
name: 'uas',
cql: 'uas',
values: [
{
name: 'Assigned',
cql: 'true',
displayName: <FormattedMessage id="ui-plugin-find-user.assigned" />,
},
{
name: 'Unassigned',
cql: 'false',
displayName: <FormattedMessage id="ui-plugin-find-user.unassigned" />,
},
],
},
];

export default filterConfig;
38 changes: 38 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import cloneDeep from 'lodash/cloneDeep';
import {
ASSIGNED_FILTER_KEY,
UNASSIGNED_FILTER_KEY,
UAS,
} from './constants';

// eslint-disable-next-line import/prefer-default-export
export const updateResourceData = (rData) => {
const filterString = rData?.query?.filters;
const newRData = cloneDeep(rData);
if (filterString === `${UNASSIGNED_FILTER_KEY}` || filterString === `${ASSIGNED_FILTER_KEY},${UNASSIGNED_FILTER_KEY}` || filterString === `${UNASSIGNED_FILTER_KEY},${ASSIGNED_FILTER_KEY}`) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (filterString === `${UNASSIGNED_FILTER_KEY}` || filterString === `${ASSIGNED_FILTER_KEY},${UNASSIGNED_FILTER_KEY}` || filterString === `${UNASSIGNED_FILTER_KEY},${ASSIGNED_FILTER_KEY}`) {
if (filterString === UNASSIGNED_FILTER_KEY || filterString === `${ASSIGNED_FILTER_KEY},${UNASSIGNED_FILTER_KEY}` || filterString === `${UNASSIGNED_FILTER_KEY},${ASSIGNED_FILTER_KEY}`) {

/*
* When Unassigned filter is selected on 'User assignment Status' filter group, with no other filter from other groups,
* fetch all the user records. The filter string is adjusted to include both active and inactive status filters. This will result in (cql.allRecords=1)
*
* The same applies when both Assigned and Unassigned are selected in any sequential order.
*/
const alteredfilters = 'active.active,active.inactive';
newRData.query.filters = alteredfilters;
} else if (filterString.includes(`${UNASSIGNED_FILTER_KEY}`)) {
/*
* When UnAssigned filter is selected in combination with any other filters,
* filter a string for Unassigned is removed and the rest of the filter string is propagated to makeQueryFunction.
*/
const alteredfilters = newRData.query.filters.split(',').filter(str => !str.startsWith(`${UAS}`)).join(',');
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't startsWith(`${UAS}`) remove uas.Assigned too? In case when there are filters selected from other groups and both Assigned and Unassigned are checked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uas.Assigned should be removed in any case, as it will never participate in cql formation. So here the logic is appropriate. I see a need to update the comment above this code of line. Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const alteredfilters = newRData.query.filters.split(',').filter(str => !str.startsWith(`${UAS}`)).join(',');
const alteredfilters = newRData.query.filters.split(',').filter(str => !str.startsWith(UAS)).join(',');

newRData.query.filters = alteredfilters;
} else if (filterString.includes(`${ASSIGNED_FILTER_KEY}`)) {
/*
* When Assigned filter is selected on 'User assignment Status' filter group, in any combination of filters in other filter groups,
* cql formation is not needed.
* hence remove uas filter before propagating it further to makeQueryFunction
*/
const alteredfilters = '';
newRData.query.filters = alteredfilters;
}
return newRData;
};
57 changes: 57 additions & 0 deletions src/utils.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { updateResourceData } from './utils';
import { UNASSIGNED_FILTER_KEY, ASSIGNED_FILTER_KEY } from './constants';

describe('updatedResourceData', () => {
describe('when only UnAssigned filter is selected', () => {
[`${UNASSIGNED_FILTER_KEY}`, `${ASSIGNED_FILTER_KEY},${UNASSIGNED_FILTER_KEY}`, `${UNASSIGNED_FILTER_KEY},${ASSIGNED_FILTER_KEY}`].forEach(filterStr => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[`${UNASSIGNED_FILTER_KEY}`, `${ASSIGNED_FILTER_KEY},${UNASSIGNED_FILTER_KEY}`, `${UNASSIGNED_FILTER_KEY},${ASSIGNED_FILTER_KEY}`].forEach(filterStr => (
[UNASSIGNED_FILTER_KEY, `${ASSIGNED_FILTER_KEY},${UNASSIGNED_FILTER_KEY}`, `${UNASSIGNED_FILTER_KEY},${ASSIGNED_FILTER_KEY}`].forEach(filterStr => (

it(`should remove ${filterStr} from filter string and add active and inactive filter strings`, () => {
const resourceData = {
query: {
filters: filterStr,
}
};
const expectedResourceData = {
query: {
...resourceData.query,
filters: 'active.active,active.inactive',
},
};
expect(updateResourceData(resourceData)).toMatchObject(expectedResourceData);
})
));
});

describe('when Unassigned filter is selected along with filters from other filter groups', () => {
it('should remove Unassigned filter and return the rest', () => {
const resourceData = {
query: {
filters: `${UNASSIGNED_FILTER_KEY},active.active`,
}
};
const expectedResourceData = {
query: {
...resourceData.query,
filters: 'active.active',
},
};
expect(updateResourceData(resourceData)).toMatchObject(expectedResourceData);
});
});

describe('when Assigned filter is selected with or without combination of filters from other filter groups', () => {
it('should remove filter string', () => {
const resourceData = {
query: {
filters: `${ASSIGNED_FILTER_KEY},active.active`,
}
};
const expectedResourceData = {
query: {
...resourceData.query,
filters: '',
},
};
expect(updateResourceData(resourceData)).toMatchObject(expectedResourceData);
});
});
});
4 changes: 4 additions & 0 deletions test/jest/setupFiles.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// See https://github.com/facebook/jest/issues/335#issuecomment-703691592
// import './__mock__';

import 'regenerator-runtime/runtime';
Loading
Loading