From 64d1fd632547cbb960e536a53fbc172a35ffbdc7 Mon Sep 17 00:00:00 2001 From: Tai Wilkin-Corraggio Date: Wed, 8 Sep 2021 16:02:12 -0400 Subject: [PATCH] Allow users to transfer matches between facilities In some cases, list items have been incorrectly matched to a facility. Currently, users will 'split' those list items away from the incorrect facility. As a second step, they sometimes merge the match to a different facility. However, when the list items are not able to be geocoded, it's impossible to use the 'split' functionality to create a new facility from the match. This adds the functionality to move or transfer a list item directly from one existing facility to another existing facility, avoiding the need to create an new, intermediate facility. --- CHANGELOG.md | 2 + src/app/src/actions/adjustFacilityMatches.js | 69 +++++++ .../components/DashboardAdjustMatchCard.jsx | 38 +++- .../src/components/DashboardFacilityCard.jsx | 94 +-------- .../DashboardFacilityCardDetails.jsx | 147 ++++++++++++++ src/app/src/components/MoveMatchDialog.jsx | 183 ++++++++++++++++++ .../reducers/AdjustFacilityMatchesReducer.js | 34 ++++ src/app/src/util/util.js | 2 + src/django/api/constants.py | 2 + src/django/api/facility_history.py | 53 ++++- src/django/api/tests.py | 39 ++++ src/django/api/views.py | 59 ++++++ 12 files changed, 626 insertions(+), 96 deletions(-) create mode 100644 src/app/src/components/DashboardFacilityCardDetails.jsx create mode 100644 src/app/src/components/MoveMatchDialog.jsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c608195..af2fad74f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added +- Allow users to transfer matches between facilities [#1464](https://github.com/open-apparel-registry/open-apparel-registry/pull/1464) + ### Changed ### Deprecated diff --git a/src/app/src/actions/adjustFacilityMatches.js b/src/app/src/actions/adjustFacilityMatches.js index 54674010c..e4c2a5e82 100644 --- a/src/app/src/actions/adjustFacilityMatches.js +++ b/src/app/src/actions/adjustFacilityMatches.js @@ -6,6 +6,7 @@ import { logErrorAndDispatchFailure, makeSplitFacilityAPIURL, makePromoteFacilityMatchAPIURL, + makeTransferFacilityAPIURL, } from '../util/util'; export const startFetchFacilityToAdjust = createAction( @@ -54,6 +55,37 @@ export function fetchFacilityToAdjust() { }; } +export const startFetchFacilityToTransfer = createAction( + 'START_FETCH_FACILITY_TO_TRANSFER', +); +export const failFetchFacilityToTransfer = createAction( + 'FAIL_FETCH_FACILITY_TO_TRANSFER', +); +export const completeFetchFacilityToTransfer = createAction( + 'COMPLETE_FETCH_FACILITY_TO_TRANSFER', +); +export const clearFacilityToTransfer = createAction( + 'CLEAR_FACILITY_TO_TRANSFER', +); +export function fetchFacilityToTransfer(oarID) { + return dispatch => { + dispatch(startFetchFacilityToTransfer()); + + return apiRequest + .get(makeSplitFacilityAPIURL(oarID)) + .then(({ data }) => dispatch(completeFetchFacilityToTransfer(data))) + .catch(err => + dispatch( + logErrorAndDispatchFailure( + err, + 'An error prevented fetching that facility', + failFetchFacilityToTransfer, + ), + ), + ); + }; +} + export const startSplitFacilityMatch = createAction( 'START_SPLIT_FACILITY_MATCH', ); @@ -129,3 +161,40 @@ export function promoteFacilityMatch(matchID) { ); }; } + +export const startTransferFacilityMatch = createAction( + 'START_TRANSFER_FACILITY_MATCH', +); +export const failTransferFacilityMatch = createAction( + 'FAIL_TRANSFER_FACILITY_MATCH', +); +export const completeTransferFacilityMatch = createAction( + 'COMPLETE_TRANSFER_FACILITY_MATCH', +); + +export function transferFacilityMatch({ matchID, oarID }) { + return dispatch => { + if (!oarID || !matchID) { + return null; + } + + dispatch(startTransferFacilityMatch()); + + return apiRequest + .post(makeTransferFacilityAPIURL(oarID), { match_id: matchID }) + .then(({ data }) => { + dispatch(completeTransferFacilityMatch(data)); + // Refresh facility to show match is no longer present + dispatch(fetchFacilityToAdjust()); + }) + .catch(err => + dispatch( + logErrorAndDispatchFailure( + err, + 'An error prevented moving that facility match', + failTransferFacilityMatch, + ), + ), + ); + }; +} diff --git a/src/app/src/components/DashboardAdjustMatchCard.jsx b/src/app/src/components/DashboardAdjustMatchCard.jsx index 1b405954a..e4f391b21 100644 --- a/src/app/src/components/DashboardAdjustMatchCard.jsx +++ b/src/app/src/components/DashboardAdjustMatchCard.jsx @@ -15,6 +15,8 @@ import find from 'lodash/find'; import { Link } from 'react-router-dom'; import { toast } from 'react-toastify'; +import MoveMatchDialog from './MoveMatchDialog'; + import { makeFacilityDetailLink } from '../util/util'; import { facilityDetailsPropType } from '../util/propTypes'; @@ -107,6 +109,7 @@ export default function DashboardAdjustMatchCard({ const [matchToSplit, setMatchToSplit] = useState(null); const [matchToPromote, setMatchToPromote] = useState(null); const [loading, setLoading] = useState(false); + const [matchToMove, setMatchToMove] = useState(null); const closeDialog = () => { setMatchToSplit(null); @@ -183,17 +186,36 @@ export default function DashboardAdjustMatchCard({ } return ( -
+
+ {(match.facility_created_by_item || match.is_geocoded) && ( + + )}
+
{createButtonControls(match)}
{dialogContent} + setMatchToMove(null)} + /> ); } diff --git a/src/app/src/components/DashboardFacilityCard.jsx b/src/app/src/components/DashboardFacilityCard.jsx index 424930a89..caf425491 100644 --- a/src/app/src/components/DashboardFacilityCard.jsx +++ b/src/app/src/components/DashboardFacilityCard.jsx @@ -2,16 +2,13 @@ import React from 'react'; import { arrayOf, bool, func, string } from 'prop-types'; import Card from '@material-ui/core/Card'; import CardActions from '@material-ui/core/CardActions'; -import CardContent from '@material-ui/core/CardContent'; -import CircularProgress from '@material-ui/core/CircularProgress'; import TextField from '@material-ui/core/TextField'; import Typography from '@material-ui/core/Typography'; import Button from '@material-ui/core/Button'; -import get from 'lodash/get'; import { facilityDetailsPropType } from '../util/propTypes'; -import FacilityDetailsStaticMap from './FacilityDetailsStaticMap'; +import DashboardFacilityCardDetails from './DashboardFacilityCardDetails'; const dashboardFacilityCardStyles = Object.freeze({ cardStyles: Object.freeze({ @@ -107,87 +104,6 @@ export default function DashboardFacilityCard({ ); - const cardContent = (() => { - if (fetching) { - return ; - } - - if (error && error.length) { - return ( -
    - {error.map(err => ( -
  • - - {err} - -
    • - ))} -
- ); - } - - if (!data) { - return null; - } - - const contributorContent = get(data, 'properties.contributors', []) - .length && ( -
    - {data.properties.contributors.map(({ name }) => ( -
  • - - {name} - -
    • - ))} -
- ); - - return ( - <> -
- -
-
- - Name - - - {get(data, 'properties.name', null)} - - - Address - - - {get(data, 'properties.address', null)} - - - Country - - - {get(data, 'properties.country_name', null)} - - - Location - - - {get(data, 'geometry.coordinates', []).join(', ')} - - - Contributors - -
- {contributorContent} -
-
- - ); - })(); - return ( {cardActions} - - {cardContent} - + ); } diff --git a/src/app/src/components/DashboardFacilityCardDetails.jsx b/src/app/src/components/DashboardFacilityCardDetails.jsx new file mode 100644 index 000000000..59f7270f4 --- /dev/null +++ b/src/app/src/components/DashboardFacilityCardDetails.jsx @@ -0,0 +1,147 @@ +import React from 'react'; +import { arrayOf, bool, string } from 'prop-types'; +import CardContent from '@material-ui/core/CardContent'; +import CircularProgress from '@material-ui/core/CircularProgress'; +import Typography from '@material-ui/core/Typography'; +import get from 'lodash/get'; + +import { facilityDetailsPropType } from '../util/propTypes'; + +import FacilityDetailsStaticMap from './FacilityDetailsStaticMap'; + +const dashboardFacilityCardStyles = Object.freeze({ + cardContentStyles: Object.freeze({ + display: 'flex', + flexDirection: 'column', + padding: '0 20px', + }), + labelStyles: Object.freeze({ + fontSize: '16px', + fontWeight: '700', + padding: '5px 0 0', + }), + fieldStyles: Object.freeze({ + fontSize: '16px', + padding: '0 0 5px', + }), + listStyles: Object.freeze({ + margin: '0 5px', + }), + mapStyles: Object.freeze({ + padding: '0', + display: 'flex', + justifyContent: 'center', + alignItems: 'center', + width: '100%', + }), + errorStyles: Object.freeze({ + color: 'red', + }), + infoContainerStyles: Object.freeze({ + marging: '10px 0', + }), +}); + +export default function DashboardFacilityCardDetails({ + data, + fetching, + error, +}) { + const cardContent = (() => { + if (fetching) { + return ; + } + + if (error && error.length) { + return ( +
    + {error.map(err => ( +
  • + + {err} + +
    • + ))} +
+ ); + } + + if (!data) { + return null; + } + + const contributorContent = get(data, 'properties.contributors', []) + .length && ( +
    + {data.properties.contributors.map(({ name }) => ( +
  • + + {name} + +
    • + ))} +
+ ); + + return ( + <> +
+ +
+
+ + Name + + + {get(data, 'properties.name', null)} + + + Address + + + {get(data, 'properties.address', null)} + + + Country + + + {get(data, 'properties.country_name', null)} + + + Location + + + {get(data, 'geometry.coordinates', []).join(', ')} + + + Contributors + +
+ {contributorContent} +
+
+ + ); + })(); + + return ( + + {cardContent} + + ); +} + +DashboardFacilityCardDetails.defaultProps = { + data: null, + error: null, +}; + +DashboardFacilityCardDetails.propTypes = { + data: facilityDetailsPropType, + fetching: bool.isRequired, + error: arrayOf(string), +}; diff --git a/src/app/src/components/MoveMatchDialog.jsx b/src/app/src/components/MoveMatchDialog.jsx new file mode 100644 index 000000000..405bf3fac --- /dev/null +++ b/src/app/src/components/MoveMatchDialog.jsx @@ -0,0 +1,183 @@ +import React, { useState } from 'react'; +import { connect } from 'react-redux'; +import Typography from '@material-ui/core/Typography'; +import TextField from '@material-ui/core/TextField'; +import Button from '@material-ui/core/Button'; +import Dialog from '@material-ui/core/Dialog'; +import Divider from '@material-ui/core/Divider'; +import CardActions from '@material-ui/core/CardActions'; + +import DashboardFacilityCardDetails from './DashboardFacilityCardDetails'; +import { + fetchFacilityToTransfer, + clearFacilityToTransfer, + transferFacilityMatch, +} from '../actions/adjustFacilityMatches'; +import { getValueFromEvent } from '../util/util'; + +const styles = Object.freeze({ + cardStyles: Object.freeze({ + width: '45%', + margin: '0 20px', + padding: '10px', + }), + textSectionStyles: Object.freeze({ + padding: '10px 20px', + }), + cardActionsStyles: Object.freeze({ + display: 'flex', + justifyContent: 'space-between', + alignItems: 'center', + padding: '10px 20px', + }), + cardContentStyles: Object.freeze({ + display: 'flex', + flexDirection: 'column', + padding: '0 20px', + }), + labelStyles: Object.freeze({ + fontSize: '16px', + fontWeight: '700', + padding: '5px 0 0', + }), + fieldStyles: Object.freeze({ + fontSize: '16px', + padding: '0 0 5px', + }), + listStyles: Object.freeze({ + margin: '0 5px', + }), + mapStyles: Object.freeze({ + padding: '0', + display: 'flex', + justifyContent: 'center', + alignItems: 'center', + width: '100%', + }), + oarIDStyles: Object.freeze({ + fontSize: '18px', + }), + errorStyles: Object.freeze({ + color: 'red', + }), + infoContainerStyles: Object.freeze({ + margin: '10px 0', + }), + dialogStyle: Object.freeze({ + margin: '25px', + }), + container: Object.freeze({ + margin: '20px', + }), + contentsContainer: Object.freeze({ + maxHeight: '300px', + overflow: 'scroll', + }), +}); + +function MoveMatchDialog({ + matchToMove, + handleClose, + fetchFacility, + data, + fetching, + error, + clearFacility, + moveMatch, +}) { + const [oarID, updateOARID] = useState(''); + const onClose = () => { + updateOARID(''); + clearFacility(); + handleClose(); + }; + + const handleFetchFacility = () => fetchFacility(oarID); + const handleMoveMatch = () => { + moveMatch({ oarID, matchID: matchToMove.match_id }); + onClose(); + }; + const fetchOnEnter = ({ key }) => { + if (key === 'Enter') { + handleFetchFacility(); + } + }; + + return ( + +
+ + Transfer match to alternate facility + + + updateOARID(getValueFromEvent(e))} + value={oarID} + placeholder="Enter an OAR ID" + onKeyPress={fetchOnEnter} + /> + + + {data && } +
+ +
+ {data && } + {data && ( + + + + + )} +
+ + ); +} + +function mapStateToProps({ + adjustFacilityMatches: { + transferFacility: { data, fetching, error }, + }, +}) { + return { + data, + fetching, + error, + }; +} + +function mapDispatchToProps(dispatch) { + return { + fetchFacility: oarID => dispatch(fetchFacilityToTransfer(oarID)), + clearFacility: () => dispatch(clearFacilityToTransfer()), + moveMatch: ({ oarID, matchID }) => + dispatch(transferFacilityMatch({ oarID, matchID })), + }; +} + +export default connect(mapStateToProps, mapDispatchToProps)(MoveMatchDialog); diff --git a/src/app/src/reducers/AdjustFacilityMatchesReducer.js b/src/app/src/reducers/AdjustFacilityMatchesReducer.js index a2491b690..ebe6ceb22 100644 --- a/src/app/src/reducers/AdjustFacilityMatchesReducer.js +++ b/src/app/src/reducers/AdjustFacilityMatchesReducer.js @@ -18,6 +18,10 @@ import { startPromoteFacilityMatch, failPromoteFacilityMatch, completePromoteFacilityMatch, + startFetchFacilityToTransfer, + failFetchFacilityToTransfer, + completeFetchFacilityToTransfer, + clearFacilityToTransfer, } from '../actions/adjustFacilityMatches'; const initialState = Object.freeze({ @@ -32,6 +36,11 @@ const initialState = Object.freeze({ fetching: false, error: null, }), + transferFacility: Object.freeze({ + data: null, + fetching: false, + error: null, + }), }); const handleCompleteSplitFacilityMatch = (state, data) => { @@ -138,6 +147,31 @@ export default createReducer( data: { $set: data }, }, }), + [startFetchFacilityToTransfer]: state => + update(state, { + transferFacility: { + fetching: { $set: true }, + error: { $set: initialState.transferFacility.error }, + }, + }), + [failFetchFacilityToTransfer]: (state, error) => + update(state, { + transferFacility: { + fetching: { $set: initialState.transferFacility.fetching }, + error: { $set: error }, + }, + }), + [completeFetchFacilityToTransfer]: (state, data) => + update(state, { + transferFacility: { + data: { $set: data }, + fetching: { $set: initialState.transferFacility.fetching }, + }, + }), + [clearFacilityToTransfer]: state => + update(state, { + transferFacility: { $set: initialState.transferFacility }, + }), [resetAdjustFacilityState]: constant(initialState), }, initialState, diff --git a/src/app/src/util/util.js b/src/app/src/util/util.js index 27001fedd..281fb2a12 100644 --- a/src/app/src/util/util.js +++ b/src/app/src/util/util.js @@ -135,6 +135,8 @@ export const makeClaimFacilityAPIURL = oarId => `/api/facilities/${oarId}/claim/`; export const makeSplitFacilityAPIURL = oarID => `/api/facilities/${oarID}/split/`; +export const makeTransferFacilityAPIURL = oarID => + `/api/facilities/${oarID}/move/`; export const makePromoteFacilityMatchAPIURL = oarID => `/api/facilities/${oarID}/promote/`; diff --git a/src/django/api/constants.py b/src/django/api/constants.py index f6ff226e0..1fc7ffb17 100644 --- a/src/django/api/constants.py +++ b/src/django/api/constants.py @@ -20,6 +20,7 @@ class ProcessingAction: PROMOTE_MATCH = 'promote_match' MERGE_FACILITY = 'merge_facility' SPLIT_FACILITY = 'split_facility' + MOVE_FACILITY = 'move_facility' NOTIFY_COMPLETE = 'notify_complete' @@ -68,6 +69,7 @@ class FacilityHistoryActions: DELETE = 'DELETE' MERGE = 'MERGE' SPLIT = 'SPLIT' + MOVE = 'MOVE' OTHER = 'OTHER' ASSOCIATE = 'ASSOCIATE' DISSOCIATE = 'DISSOCIATE' diff --git a/src/django/api/facility_history.py b/src/django/api/facility_history.py index f37e846da..a78479dcf 100644 --- a/src/django/api/facility_history.py +++ b/src/django/api/facility_history.py @@ -225,6 +225,17 @@ def maybe_get_split_action_time_from_processing_results(item): return next(iter(split_processing_times), None) +def maybe_get_move_action_time_from_processing_results(item): + move_processing_times = [ + r.get('finished_at', None) + for r + in item.processing_results + if r.get('action', None) == ProcessingAction.MOVE_FACILITY + ] + + return next(iter(move_processing_times), None) + + def processing_results_has_split_action_for_oar_id(list_item, facility_id): return facility_id in [ r.get('previous_facility_oar_id', None) @@ -234,6 +245,15 @@ def processing_results_has_split_action_for_oar_id(list_item, facility_id): ] +def processing_results_has_move_action_for_oar_id(list_item, facility_id): + return facility_id in [ + r.get('previous_facility_oar_id', None) + for r + in list_item.processing_results + if r.get('action', None) == ProcessingAction.MOVE_FACILITY + ] + + def create_facility_claim_entry(claim): if claim.status == FacilityClaim.REVOKED: return { @@ -330,6 +350,35 @@ def create_facility_history_list(entries, facility_id, user=None): ) ] + facility_move_entries = [ + { + 'updated_at': maybe_get_move_action_time_from_processing_results( + m.facility_list_item + ), + 'action': FacilityHistoryActions.MOVE, + 'detail': 'Match {} was moved from {}'.format( + m.id, + facility_id, + ) + } + for m + in FacilityMatch + .objects + .filter(status__in=[ + FacilityMatch.CONFIRMED, + FacilityMatch.AUTOMATIC, + FacilityMatch.MERGED, + ]) + .annotate( + processing_results=F('facility_list_item__processing_results')) + .extra( + where=['processing_results @> \'[{"action": "move_facility"}]\'']) + if processing_results_has_move_action_for_oar_id( + m.facility_list_item, + facility_id, + ) + ] + facility_match_entries = [ { 'updated_at': str(m.history_date), @@ -395,6 +444,6 @@ def create_facility_history_list(entries, facility_id, user=None): ] return sorted(history_entries + facility_split_entries + - facility_match_entries + facility_claim_entries + - replaced_entries, + facility_move_entries + facility_match_entries + + facility_claim_entries + replaced_entries, key=lambda entry: entry['updated_at'], reverse=True) diff --git a/src/django/api/tests.py b/src/django/api/tests.py index bfc326b8d..aefa9a65b 100644 --- a/src/django/api/tests.py +++ b/src/django/api/tests.py @@ -4798,6 +4798,45 @@ def test_serializes_facility_match_split(self): 4, ) + @override_flag('can_get_facility_history', active=True) + def test_serializes_facility_match_move(self): + move_facility_url = '/api/facilities/{}/move/'.format( + self.facility.id) + + move_facility_data = { + 'match_id': self.match_two.id, + } + + move_facility_response = self.client.post( + move_facility_url, + move_facility_data, + ) + + self.assertEqual(move_facility_response.status_code, 200) + + self.match_two.refresh_from_db() + + response = self.client.get(self.facility_two_history_url) + data = json.loads(response.content) + + self.assertEqual( + data[0]['action'], + 'MOVE', + ) + + self.assertEqual( + data[0]['detail'], + 'Match {} was moved from {}'.format( + self.match_two.id, + self.facility_two.id, + ) + ) + + self.assertEqual( + len(data), + 3, + ) + @override_flag('can_get_facility_history', active=True) def test_handles_request_for_invalid_facility_id(self): invalid_history_url = '/api/facilities/hello/history/' diff --git a/src/django/api/views.py b/src/django/api/views.py index 10201db2a..8c15e9505 100644 --- a/src/django/api/views.py +++ b/src/django/api/views.py @@ -699,6 +699,9 @@ def get_link(self, path, method, base_url): if 'split' in path: return None + if 'move' in path: + return None + if 'promote' in path: return None @@ -1793,6 +1796,8 @@ def split(self, request, pk=None): m.facility_list_item.source.contributor.id if m.facility_list_item.source.contributor else None, 'match_id': m.id, + 'is_geocoded': + m.facility_list_item.geocoded_point is not None, 'facility_created_by_item': Facility.objects.filter( created_from=m.facility_list_item.id)[0].id @@ -1884,6 +1889,60 @@ def split(self, request, pk=None): except Facility.DoesNotExist: raise NotFound() + @action(detail=True, methods=['POST'], + permission_classes=(IsRegisteredAndConfirmed,)) + @transaction.atomic + def move(self, request, pk=None): + if not request.user.is_superuser: + raise PermissionDenied() + + try: + match_id = request.data.get('match_id') + + if match_id is None: + raise BadRequestException('Missing required param match_id') + + match = FacilityMatch.objects.get(pk=match_id) + old_facility = match.facility + list_item_for_match = match.facility_list_item + + new_facility = Facility.objects.get(pk=pk) + + match.facility = new_facility + match.confidence = 1.0 + match.status = FacilityMatch.CONFIRMED + match.results = { + 'match_type': 'moved_by_administator', + 'move_to_oar_id': match.facility.id, + } + + match.save() + + now = str(datetime.utcnow()) + + list_item_for_match.facility = new_facility + list_item_for_match.processing_results.append({ + 'action': ProcessingAction.MOVE_FACILITY, + 'started_at': now, + 'error': False, + 'finished_at': now, + 'previous_facility_oar_id': old_facility.id, + }) + + list_item_for_match.save() + + return Response({ + 'match_id': match.id, + 'new_oar_id': new_facility.id, + }) + + except FacilityListItem.DoesNotExist: + raise NotFound() + except FacilityMatch.DoesNotExist: + raise NotFound() + except Facility.DoesNotExist: + raise NotFound() + @action(detail=True, methods=['POST'], permission_classes=(IsRegisteredAndConfirmed,)) @transaction.atomic