Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Commit

Permalink
Conditionally change split operation to revert
Browse files Browse the repository at this point in the history
In production we encountered a situation where a list item was used to create a
facility but then through some sequence of moderation activities was matched to
a different facility. Attempts to use split function in the dashboard failed
because attempting to create a new facility from the list item violated the
uniqueness constraint on `Facility.created_from`.

To work around this problem we now include `facility_created_by_item` in the
data used to populate the dashboard split tool and conditionally offer a
"revert" option in place of split and mirror that change in the API view to
connect the match to the originally created facility rather than attempting to
make a new facility record.
  • Loading branch information
jwalgran committed Jun 10, 2021
1 parent 7f91024 commit e8b6d68
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 21 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

### Added

- Conditionally change split operation to revert [#1386](https://github.com/open-apparel-registry/open-apparel-registry/pull/1386)

### Changed

### Deprecated
Expand Down
29 changes: 20 additions & 9 deletions src/app/src/components/DashboardAdjustMatchCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,15 @@ export default function DashboardAdjustMatchCard({
promoteMatch(matchToPromote.match_id);
};

const successfulSplitMessage = match =>
match.facility_created_by_item
? 'Reverted match to original facility'
: 'New facility was created';

useEffect(() => {
if (!adjusting && loading) {
const toastMessage = matchToSplit
? 'New facility was created'
? successfulSplitMessage(matchToSplit)
: 'Match was promoted';

setMatchToSplit(null);
Expand Down Expand Up @@ -188,7 +193,7 @@ export default function DashboardAdjustMatchCard({
}
style={adjustMatchCardStyles.buttonStyles}
>
Split
{match.facility_created_by_item ? 'Revert' : 'Split'}
</Button>
<Button
variant="contained"
Expand Down Expand Up @@ -222,15 +227,21 @@ export default function DashboardAdjustMatchCard({
action,
} = matchToSplit
? {
title: `Create a new facility from Match ${get(
matchToSplit,
'match_id',
'',
)}?`,
subtitle: 'This will create a new facility from:',
title: matchToSplit.facility_created_by_item
? `Revert match to original facility ${matchToSplit.facility_created_by_item}`
: `Create a new facility from Match ${get(
matchToSplit,
'match_id',
'',
)}?`,
subtitle: matchToSplit.facility_created_by_item
? 'This will connect the item to the original facility'
: 'This will create a new facility from:',
name: get(matchToSplit, 'name', ''),
address: get(matchToSplit, 'address', ''),
actionLabel: 'Create facility',
actionLabel: matchToSplit.facility_created_by_item
? 'Revert match'
: 'Create facility',
action: handleSplitMatch,
}
: {
Expand Down
36 changes: 36 additions & 0 deletions src/django/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -3334,6 +3334,42 @@ def test_post_returns_bad_request_without_match_id(self):
post_response = self.client.post(self.split_url, {})
self.assertEqual(post_response.status_code, 400)

def test_post_reverts_to_created_facility(self):
self.facility_two = Facility \
.objects \
.create(name='Name',
address='Address',
country_code='US',
location=Point(0, 0),
ppe_product_types=['two_a', 'two_b'],
ppe_contact_phone='222-222-2222',
ppe_contact_email='ppe_two@example.com',
ppe_website='https://example.com/ppe_two',
created_from=self.list_item_two)

initial_facility_count = Facility \
.objects \
.all() \
.count()
self.assertEqual(initial_facility_count, 2)
self.assertEqual(self.match_two.facility, self.facility_one)

self.client.login(email=self.superuser_email,
password=self.superuser_password)
post_response = self.client.post(self.split_url,
{'match_id': self.match_two.id})
self.assertEqual(post_response.status_code, 200)

updated_facility_count = Facility \
.objects \
.all() \
.count()

self.assertEqual(updated_facility_count, initial_facility_count)

self.match_two.refresh_from_db()
self.assertEqual(self.match_two.facility, self.facility_two)

def test_post_creates_a_new_facility(self):
initial_facility_count = Facility \
.objects \
Expand Down
41 changes: 29 additions & 12 deletions src/django/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1747,6 +1747,12 @@ 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,
'facility_created_by_item':
Facility.objects.filter(
created_from=m.facility_list_item.id)[0].id
if Facility.objects.filter(
created_from=m.facility_list_item.id).exists()
else None,
}
for m
in facility.get_other_matches()
Expand All @@ -1767,18 +1773,29 @@ def split(self, request, pk=None):

list_item_for_match = match_for_new_facility.facility_list_item

new_facility = Facility \
.objects \
.create(
name=list_item_for_match.name,
address=list_item_for_match.address,
country_code=list_item_for_match.country_code,
location=list_item_for_match.geocoded_point,
ppe_product_types=list_item_for_match.ppe_product_types,
ppe_contact_phone=list_item_for_match.ppe_contact_phone,
ppe_contact_email=list_item_for_match.ppe_contact_email,
ppe_website=list_item_for_match.ppe_website,
created_from=list_item_for_match)
facility_qs = Facility.objects.filter(
created_from=list_item_for_match)
if facility_qs.exists():
# `Facility.created_by` must be unique. If the item was
# previously used to create a facility, we must related it to
# that existing facility rather than creating a new facility
new_facility = facility_qs[0]
else:
new_facility = Facility \
.objects \
.create(
name=list_item_for_match.name,
address=list_item_for_match.address,
country_code=list_item_for_match.country_code,
location=list_item_for_match.geocoded_point,
ppe_product_types=(
list_item_for_match.ppe_product_types),
ppe_contact_phone=(
list_item_for_match.ppe_contact_phone),
ppe_contact_email=(
list_item_for_match.ppe_contact_email),
ppe_website=list_item_for_match.ppe_website,
created_from=list_item_for_match)

match_for_new_facility.facility = new_facility
match_for_new_facility.confidence = 1.0
Expand Down

0 comments on commit e8b6d68

Please sign in to comment.