GnuCash
Contact   Instructions
Bug 796987 - Import Bills & Invoices: fixing import items only works in special case
Summary: Import Bills & Invoices: fixing import items only works in special case
Status: RESOLVED FIXED
Alias: None
Product: GnuCash
Classification: Unclassified
Component: Import - CSV (show other bugs)
Version: git-maint
Hardware: PC Mac OS
: Normal minor
Target Milestone: ---
Assignee: Rob Laan
QA Contact: import
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-12-19 15:33 EST by Rob Laan
Modified: 2020-01-30 15:55 EST (History)
3 users (show)

See Also:


Attachments

Description Rob Laan 2018-12-19 15:33:13 EST
In the Import Bills & Invoices function, in file dialog-bi-import.c, form gnc_bi_import_fix_bis attempts to 'fix' some of the import fields, by using values of a previous line item of the same invoice id.

However, the code resets the values of the previous line item, when it encounters a valid invoice id. It would be more useful if the code would reset the values of the previous line item, when it encounters a NEW valid invoice id.

E.g. in a file like below, the code would set the it / owner_id of the second item to 10 / 1001, the values of the previous item.

id; date_opened; owner_id;...
10; 18/12/2018; 1001;...
; 18/12/2018;;...                  // no id, and no owner id provided

But in the file below, the code would reset the previous line item values for the second item, because it encounters a valid id (10). The value of the first item is not used for the second item, which is then deleted because of 'no owner id'.

id; date_opened; owner_id;...
10; 18/12/2018; 1001;...
10; 18/12/2018;;...                  // id provided, but no owner id provided

The first case seems to be a special case, as it would be difficult to organize an import file, if not every item  of an invoice is marked by its id.
The second case would be more common, but the current code does not fix the missing values.

See line 319 and next in dialog-bi-import.c; it says 'new invoice...', but it could very well be same id as the previous line, i.e. same invoice.

297             if (strlen (id) == 0)
298             {
299                 // no invoice id specified
300                 if (prev_id->len == 0)
301                 {
302                     // cannot fix -> delete row
303                     valid = gtk_list_store_remove (store, &iter);
304                     row_deleted = TRUE;
305                     g_string_append_printf (info,
306                                             _("ROW %d DELETED, ID_NOT_SET\n"), row);
307                 }
308                 else
309                 {
310                     // this is a fixable multi line invoice
311                     gtk_list_store_set (store, &iter, ID, prev_id->str, -1);
312                     row_fixed = TRUE;
313                 }
314             }
315             else
316             {
317                 // remember invoice id (to be able to fix multi line invoices)
318                 g_string_assign (prev_id, id);
319                 // new invoice => reset all other fixable entries
320                 g_string_assign (prev_date_opened, "");
321                 g_string_assign (prev_date_posted, "");
322                 g_string_assign (prev_owner_id, "");
323                 g_string_assign (prev_date, "");
324             }
Comment 1 Geert Janssens 2019-03-20 10:33:19 EDT
I'm currently reviewing your PR and in particular the fixes for this bug (which I had missed earlier).

For consistency with the generic csv importer I believe the "special case" as you name it should be supported. That is if an invoice is comprising of multiple lines (because of multiple entries), the invoice header fields should only be required for the first line. The others can be empty, that is they can contain only entry related info. In other words if a line is missing an invoice id, you should assume the line is part of the last previously found invoice (the last earlier line having an id).
Comment 2 Rob Laan 2020-01-30 15:55:10 EST
Pull request #457 merged into maint, https://github.com/Gnucash/gnucash/pull/457.

Note You need to log in before you can comment on or make changes to this bug.