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

Integrate facility/processing type taxonomy into facility claims #1639

Merged
merged 4 commits into from Feb 16, 2022

Conversation

caseycesari
Copy link
Contributor

@caseycesari caseycesari commented Feb 11, 2022

Overview

Makes various changes to the facility claims form and back-end to integrate the new facility/processing type taxonomy.

  • Update the extended field creation for claims to create fields for processing type, facility type, and parent company. The logic of determining the values for these fields was refactored out so that it could be reused for claims and facilities.
  • Choice tuples are generated for the processing and facility types so that they can be used on the claims form and for the claims model. A test was updated to accommodate these changes.
  • FacilityClaims.FacilityType is lengthened to support multiple values in the facility type taxonomy.
  • Change "production type" to "processing type" on the claims form. "Production type" is the old name for "processing type". No variables names were changed at this time for the sake of simplicity.
  • FacilityClaims.FacilityType was pluralized on the front-end since users can now select multiple values for this field.

Connects #1590

Demo

Feb-11-2022 11-09-21

Notes

Follow-up work to be done:

  • Generate data migrations for facility claims on production that have a processing and/or facility type already to move them to a value within the taxonomy. A special case to be aware of is that "other" type for facilities.
  • Consider renaming FacilityClaim.ProductionType to ProcessingType, either just related variables, and/or the back-end model field. This will require a migration.

Testing Instructions

  • Run ./scripts/update to apply the included migration.
  • Create or a visit an existing facility claim. Example: http://localhost:6543/claimed/1/
  • Edit/add facility type, processing type, and parent company and save the changes.
  • Visit the facility on the main map, and verify the added/edited values for the extended fields are displayed.
  • Make edits to the initial values you set (for example, remove some processing types), and verify those edits are displayed on the main map.

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@@ -16,23 +16,54 @@ def extract_range_value(value):
MAX_PRODUCT_TYPE_COUNT = 50


def get_facility_and_processing_type_extendfield_value(field, field_value):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No logic was changed in this function and the one below. They were just pulled out of the create_extendedfield function so they could be reused by create_extendedfields_for_claim

@@ -61,3 +62,23 @@ def get_list_contributor_field_values(item, fields):
index = list_fields.index(f['column_name'])
f['value'] = data_values[index]
return fields


Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is mentioned in the commit message, but in case that is missed, this was moved here to avoid a circular import in when importing api.facility_type_processing_type variables into api.models. api.models is imported into api.matching (where clean was originally defined). Then api.facility_type_processing_type (which uses the clean function) was imported into api.models to get access to the facility/processing type taxonomy. This caused a circular import error.

Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

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

Nice implementation. I confirmed this is working as described.

I created #1648 to add a follow up migration.

I found 2 omissions as I was testing. If the my suggested addition works for you and you integrate it, consider this approved.

The first is product_type handling, which was not listed on the original issue but I was able to add quickly but following the guide of the existing code.

diff --git a/src/django/api/extended_fields.py b/src/django/api/extended_fields.py
index d61002a3..27f8ea1e 100644
--- a/src/django/api/extended_fields.py
+++ b/src/django/api/extended_fields.py
@@ -42,6 +42,17 @@ def get_facility_and_processing_type_extendfield_value(field, field_value):
     }
 
 
+def get_product_type_extendfield_value(field_value):
+    values = field_value
+
+    if isinstance(field_value, str):
+        values = (field_value.split('|') if '|' in field_value
+                  else [field_value])
+    return {
+        'raw_values': values
+    }
+
+
 def get_parent_company_extendedfield_value(field_value):
     matches = Contributor.objects.filter_by_name(field_value)
 
@@ -160,6 +171,7 @@ CLAIM_FIELDS = (
     ('facility_production_types', ExtendedField.PROCESSING_TYPE),
     ('facility_type', ExtendedField.FACILITY_TYPE),
     ('parent_company', ExtendedField.PARENT_COMPANY),
+    ('facility_product_types', ExtendedField.PRODUCT_TYPE),
 )
 
 
@@ -185,6 +197,8 @@ def create_extendedfields_for_claim(claim):
                         extended_field, field_value
                     )
                 )
+            elif extended_field == ExtendedField.PRODUCT_TYPE:
+                field_value = get_product_type_extendfield_value(field_value)
 
             try:
                 field = ExtendedField.objects.get(facility_claim=claim,

The second is the fact that processing_type and facility_type can get out of sync. If you submit processing_types via list or API, we are automatically creating the appropriate corresponding facility_type based on the taxonomy. The claim processing does not have this behavior, and adding it is not straightforward because saving a claim does not currently refresh the form on the client. Values for facility_type set on the server would not appear in the select box.

What we do about this is depended on our conversation with OAR about how to handle these two fields. It is possible that we end up remove facility_type as a separate editable fields. Because of that, we should hold off doing anything now and add. I created #1649 as a follow up issue.

@jwalgran jwalgran assigned caseycesari and unassigned jwalgran Feb 15, 2022
@caseycesari
Copy link
Contributor Author

@jwalgran Thanks for the heads up on product_type. I made a slight adjustment to your code suggestion by factoring out the code here to use as the contents for get_product_type_extendfield_value function. You can see that in f0fcac1.

@caseycesari caseycesari mentioned this pull request Feb 16, 2022
3 tasks
Avoids a circular import when importing api.facility_type_processing_type
variables into api.models.

Refs #1590
This is no longer an option. Facility type must be one of the options
for the taxonomy. Existing claims in production that have "other" set
for facility type will be migrated at a later date.

Refs #1590
Makes various changes to the facility claims form and back-end
to integrate the new facility/processing type taxonomy.

- Update the extended field creation for claims to create fields for
  processing type, facility type, and parent company. The logic of determining
  the values for these fields was refactored out so that it could be
  reused for claims and facilities.
- Choice tuples are generated for the processing and facility types so
  that they can be used on the claims form and for the claims model. A
  test was updated to accommodate these changes.
- FacilityClaims.FacilityType is lengthened to support all of the values
  in the facility type taxonomy.
- Change "production type" to "processing type" on the claims form.
  "Production type" is the old name for "processing type". No variables
  names were changed at this time for the sake of simplicity.
- FacilityClaims.FacilityType was pluralized on the front-end since
  users can now select multiple values for this field.

Refs #1590
@caseycesari caseycesari force-pushed the cpc/facility-processing-type-claims branch from f0fcac1 to ecf7d76 Compare February 16, 2022 14:18
@caseycesari caseycesari merged commit 048522b into develop Feb 16, 2022
@caseycesari caseycesari deleted the cpc/facility-processing-type-claims branch February 16, 2022 14:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants