CRITFC / Helpdesk

Parent repository for tribal CDMS documentation and issues
1 stars 0 forks source link

CDMS 2.2. Importer Issues #32

Open nowinski opened 4 years ago

nowinski commented 4 years ago

From Clark Watry RE: Data import Fri 8/21

Related issue… so I have an .xlsx file that successfully loads into the importer. I successfully deleted records from the file, saved then reloaded it w/out problems w/ as few as 2 records; however, when I remove all but one record I suddenly get the following error…

Capture

On another yet similar note, we imported several records that were flagged as errors during the import process but were still imported into CDMS_PROD even though the dataset is configured to prevent saving w/ errors. Meanwhile, when I try to import the same file into my CDMS_DEV, I receive an error (as expected) indicating that errors exist in the data and the data is not saved. As you know, I recently updated my DEV and PROD to the same version and haven’t changed any configuration settings, so I’m at a loss to explain this inconsistent behavior.

nowinski commented 4 years ago

Issue 1:

I finally had time to test the CSV file you sent on Thursday and got this error message in dev tools:

1

Checked the CSV file and sure enough there are duplicate X and Y columns. Removed one set, tried again, and the file imported without issue. Please try importing the attached file and let me know if that resolves the CSV errors. If so, we can improve the error messaging for this situation.

Issue 2:

CDMS 2.2 has a new resource for Excel file imports (ExcelReader2) that appears to use the first two data rows to help determine column data types. So, there’s a faulty assumption that import files will always have more than one data row. We just need to provide an alternate path for cases when there’s only one row. My proposed fix is commented out in this screen shot. I’d like to discuss with Ken on Tuesday.

2

Issue 3:

Confirmed—the importer flags errors in the import multiple activities table but the Save button is still enabled. It should be disabled until all the errors are cleared.

Separate but related—the data entry page save button IS disabled when grid cell errors are detected; however, the grid cell validators only fire if the cursor focus is placed on each cell. Maybe the save button should trigger a validation routine for the entire grid instead?

nowinski commented 4 years ago

Follow up on issue 3:

CDMS 2.0 definitely used to count and display the number of errors detected in the import gridt, but it looks like I deleted that code when I worked on the duplicate checker. See lines 29-44 in this old commit:

https://github.com/CRITFC/cdms-fe-public/commit/2b72122c87ea5dee4b6839446895f697cd7ba8fb 3

I’m usually pretty cautious about deleting code and think in this case the row error count was only being displayed on screen and was not used the enable/disable the Save button. Otherwise, I would have needed to edit the Save button in modal-activities-grid.html too . . .

Anyway, I went ahead and added the original row error count code and wired it up to the Save button and pushed that commit to GitHub. Basically, when allow saving with errors is false, the Save button is disabled until the user corrects errors in the import grid. Once errors are corrected in the, the button is enabled.

https://github.com/CRITFC/cdms-fe-public/commit/24c92f7af04548c4734278ba4c7d708d4039e218

One loose end is the default dataset configuration—which I finally confirmed is just an empty set of brackets {}. It should be this instead:

{"SpecifyActivityListFields":false,"EnableDuplicateChecking":false,"AllowSaveWithErrors":false}

So, as users create new datasets, the default config in the database will actually prevent saving with errors.

nowinski commented 4 years ago

Following up on the time cell validator issue discussed yesterday:

The validator code (cell-control-types.js) tries to append ActivityDate to Time values in both the data entry grid and the modal activities grid (importer). The problem is that the params object is structured differently depending on the grid. Header fields aren’t included in the data entry grid. so the code has to look outside the grid for ActivityDate

var the_new_date = moment(params.api._headerrow.Activity.ActivityDate);

But params.api._headerrow is undefined on the modal activities grid because header fields are inside the grid. So, do this instead:

var the_new_date = moment(params.data.Activity.ActivityDate);

And then add an to get ActivityDate from the right place depending on what’s available:

4

This seems to work fine for me but want to make sure I’m seeing the full picture. Maybe I should try this with a CSV import too since Excel and CSV data seem to behave a little differently. Other thoughts?

nowinski commented 4 years ago

From Ken Burcham RE: Data import - time validator 10:22 AM

The test for undefined should be more like this: if (typeof myVar !== 'undefined')

That makes total sense…

It is probably a good idea to test with both CSV and Excel, though the services should be returning the same structure for either…

gcCtuir commented 2 years ago

Incorporated fix.