Data Import appears Broken in v9 as of Nov-02-2017 --Production

Frappe Framework Bug
1

Up until version 9, the import function would process the CSV file you feed it and report a list of the the line numbers with errors.

Now it seems it has been modified to only report a single error message:

IMPORT FAILED
Please correct the format of the file and import again.

Problem #1:
First, the data entry screens in ERPNext are not consistent with the “Required” fields in the import function. For example, it is possible to save a “Suppliers” record in the regular user interface without filling in the “Default Payable Account” but if you leave that field blank in the import function it will error without any details.

You can prove this for yourself buy using the Supplier user interface screen to enter a dummy supplier record without the Default Payable Account field. Then export the Supplier template with data and you will see the dummy record comes through without the field populated. If you then delete all other records but the line with the dummy record, delete the information in column B, and then change the name and other information to be yet another dummy Supplier., make sure only the “Do not update but insert rows” box and the “Do not send emails” box are checked and all other boxes left unchecked, then try to import that file back and it errors. If a field is required in the import function, should it also be required in the user interface?

Problem #2:
In previous versions of the import function, required linked fields in one file must already exist in the system for them to import without errors. For example, if you import “Item.csv” then every item must also have a valid default warehouse that is already part of the system BEFORE you attempt to execute the import. In previous versions, if the default warehouse column contained a value that was not already in the system, an error would be generated that listed the line number of the offending record. In the current version, if you list a default warehouse that does NOT already exist in the Warehouse data of ERPNext, the whole file will import EXCEPT the offending record and no error is generated to indicate the record was skipped. Remember, the “Ignore encoding errors” and the “Skip rows with errors” boxes are NOT checked! This should not happen.

Yes, I know that line 17 of the file indicates that default_warehouse is not a required field. Yet, it still causes errors if populated with bad data, but those errors are not listed to the user running the import. This means that records are LOST during the import and no notification is presented to the user.

I discovered this by accident when looking for an important Item record after the import. When I went back to the original CSV file The only thing I could find wrong was the default_warehouse was misspelled. I recreated the file with only the line that I had corrected and it imported fine after the spelling fix. However, when it was part of the larger import session, I was not notified of the problem and all other records were imported just fine.

Apparent Conclusion:

The single error message with no details of the errors, is a step backwards in this new version. The error trapping and reporting part of the import function has been changed and is no longer working properly. It allows errors to pass even if the check boxes for skipping error rows are unchecked.

BKM

1 Like
2

Thank you for your analysis BKM, we owe you a debt of gratitude for this -

Indeed if this is the case, I request that, whoever identifies and fixes this ‘high profile’ defect, please also write some select tests to reproduce such glaring cases to help avoid them in the future.

For ERPNext to adopt test driven design principles, or at a minimum in the case of when flaws like this occur, makes good sense to keep technical debt in check.

Otherwise, without such ‘change detection’ and ‘smoke’ tests, a developer has no idea what they have broken until after a release where the court of public opinion may not be understanding.

3

Can you replicate issue on demo erp?
We got many times such issue when excel formatting was wrong.
So you can download blank template and add some data and check it. Many times it will resolve by re-uploading new excel. I think we can improve formatting error log in details.

For your information, today we imported some master data without any issue and with correct error message. Version of system.

ERPNext: v9.2.2 (master)

Frappe Framework: v9.2.3 (master)

4

Thank you for the followup @kolate_sambhaji

I am the first to admit I am woefully ignorant here of test coverage and results. , so please accept my apologies as I don’t mean to be disresptful - of course I admit if I spoke too soon

./env/lib/python2.7/site-packages/IPython/core/tests/test_imports.py
./apps/frappe/frappe/tests/test_data_import.py

and I am only vaguely aware of test results say here https://next.travis-ci.org/frappe/erpnext/builds/253157940

5

I do not know about “demo erp” because I never use it. It is far to limited to do much. So I spin up a fresh install every time on Debian 8 VM servers. For this instance, I spun up a fresh server and used the install.py to load the latest production version on November 2 at 6pm US eastern time. So whatever was in the production sources at that time is what I ma testing.

I used a fresh server, logged in with the Administrator account and added only a single user. After this I imported the following files without any errors:

  • Item_Group
  • UOM
  • User
  • Warehouse
  • Territory
  • Employee
  • Sales_Person

It was when I started to import the Supplier file and the Item file that the problems started to show up. So this was a fresh system and the first 7 files went just fine.

The Excel formatting was not wrong. The blank templates were downloaded and populated from the same version.

So, YES… if you follow the examples I set out for you in the original post, the problem is reproducible in the production version after a fresh install. For me, the demo version is a waste of time because I cannot be familiar with the default data as easily as I can with my own data and a fresh install.

Here is the data from the installed version:

Installed Apps

ERPNext: v9.2.3 (master)

Frappe Framework: v9.2.4 (master)

BKM

6

So, it appears that testing may have been done using automated scripts. I encountered the errors using the same Graphical User Interface that any regular user would have to encounter. I try NOT to do testing with automated methods because sometimes the problem is not the underlying database but the user interface itself.

That is probably not the case here. This appears to be a problem in how errors are handled.

  • The system seems to think the “Ignore error rows” box is checked. This might be a flag set wrong somewhere.
  • The system also is not consistent with how “required” data is handled. In the user interface you can get away with missing data, but in the import function it will fail.

Note: I really miss the detailed errors that I used to get with previous versions. They were exceptionally helpful for fixing data problems in the CSV files before resubmitting them to the import.

BKM

7

I do have one positive observation…

When using the templates for importing data, it is best to pay close attention to the columns with the tilde character " ~ " as they seem to define data groups. The data columns between the tilde characters are all related and if you start populating data, the system likes to see the rest of the data in that group to be populated as well.

The way I managed to get data into the system was to delete entire groups of columns (between the tilde characters) when there were columns with “Yes” in the required field (usually row 17 of the files) and if I had no data to put in them. I found that I could delete the entire group and get the data to import. That is not the preferred way to do it, but at least I got something to import.

The lost records was the frustrating part. I have no way of detecting them now that the line numbered errors do not show up.

BKM

8

Likely the problem is the tests do not exercise from the gui widgets and their range of values (for eg use selenium web browser test framework?) but instead run a a function layer below and removed - a ‘form’ or ‘view’ layer - so a disconnect exists between the browser and it that invites the problems you are experiencing here…automated testing is most worthwhile but an uphill battle if the habit and motivation is not there. However once you reach the ‘test infected’ tipping point the results are most rewarding as bugs becomes childs play to identify and fix!

9

Agreed !!

When possible I use automation to exercise my RPi designs, but they are much simpler than an ERP system, so the reward is not as big as using it on a larger scale. However, I would never be able to do burn-in testing of the underlying framework of my designs without it. When the automated testing is finished and the bugs are fixed, I then spend another week exercising the user controls.

BKM

10

@bkm @clarkej I will test it in latest version.
As of now, we observer two issue which you also have, some time data is inserted but success message not shown. sometime it give excel formatting error without detail log.

1 Like
11

Thank you @kolate_sambhaji !

One question please - who are the ERPNext ‘Test Champions’ - which one or two developers are most hands-on keen on test results, monitoring, coverage and identifying gaps and how to leverage and improve the testing regime?

Asked another way, who worries the most about that issue!?

Is that a fair question for you or best for Rushabh, Nabin or even Revant to respond?

thanks

12

@clarkej as per github I think, @utkarsh_goswami and @vishdha written most of UI Test cases.

I think end user most worries about that issue, as for developer it might be small bug, but for end user small bug can stop them using system.

13

Thank you @kolate_sambhaji

“I think end user most worries about that issue, as for developer it might be small bug, but for end user small bug can stop them using system.”

Yes - regardless of test coverage and density (that can be a double-edged ie a curse or a blessing) - with user increases at a healthy rate, momentum and motivation will mean ‘squeaky wheel’ bugs or issues are shortlived and stability will reign so users will not worry.