2226 lost data on import #2235
2226 lost data on import #2235
Conversation
adrian-lara
force-pushed
the
2226_lost_data_on_import
branch
from
30c529b
to
1b1f907
Compare
|
Leaving a note here - we should discuss if we should patch this to prod. |
seed/models/columns.py
Outdated
| all_from_cols = Column.objects.filter( | ||
| organization=organization, | ||
| table_name__in=[None, ''], | ||
| column_name=field['from_field'], | ||
| is_extra_data=False | ||
| ) | ||
|
|
||
| ColumnMapping.objects.filter(column_raw__id__in=models.Subquery(all_from_cols.values('id'))).delete() | ||
| all_from_cols.delete() | ||
|
|
||
| from_org_col, _ = Column.objects.get_or_create( | ||
| organization=organization, | ||
| table_name__in=[None, ''], | ||
| column_name=field['from_field'], | ||
| is_extra_data=False # data from header rows in the files are NEVER extra data | ||
| ) |
There was a problem hiding this comment.
I think some comments in this area would be very helpful at a higher level. e.g. "Since multiple were found, we're deleting these for such and such reason"
There was a problem hiding this comment.
Agree - I'll do that.
seed/models/columns.py
Outdated
| organization=organization, | ||
| table_name__in=[None, ''], | ||
| column_name=field['from_field'], | ||
| is_extra_data=False # data from header rows in the files are NEVER extra data |
There was a problem hiding this comment.
Is it true that data from header rows are never extra data? I thought I could make up an arbitrary column in my csv/excel file and map it as extra data
There was a problem hiding this comment.
The type of Column object this is referring to is a representation of the "raw" header row we use for the ColumnMapping model. The ColumnMapping model takes a "PropertyState" or "TaxLotState" Column object (see table_name attribute), and associates that to one of these "raw" Column objects to create the mapping.
The comment could be improved, so I'll do that for this line and the original line in the try block above here.
seed/models/columns.py
Outdated
| ColumnMapping.objects.filter(column_raw__id__in=models.Subquery(all_from_cols.values('id'))).delete() | ||
| all_from_cols.delete() | ||
|
|
||
| from_org_col, _ = Column.objects.get_or_create( |
There was a problem hiding this comment.
For clarity's sake, should this just be Column.objects.create since we deleted above?
There was a problem hiding this comment.
Yup! Nice catch!
Short version: The |
|
Per our conversation, I'll be building a test to add more confirmation that this is working. |
Any background context you want to provide?
Data could potentially be lost on import (see attached issue).
What's this PR do?
Fixes a bug where multiple Column objects were being created for the same raw import file header.
This caused multiple ColumnMapping objects to be created when only one was expected in order to initially create incoming import records.
How should this be manually tested?
Import a file twice. The first time, don't map incoming columns to unit-aware SEED columns. The second time, map to unit-aware columns. See that the data persists throughout import.
Import a file once, but map twice. The first time mapping, don't map incoming columns to unit-aware SEED columns. The second time (by clicking "Back to Mapping"), map to unit-aware columns. See that the data persists throughout import.
What are the relevant tickets?
#2226
Screenshots (if appropriate)