GNOME Bugzilla – Bug 503166
QIF Import Druid Flow incorrect
Last modified: 2018-06-29 21:56:16 UTC
Please describe the problem: When going through the QIF import druid hitting next seems to take me to the correct pages but if I hit the back button especially towards the end it takes me to pages that I never saw previously. Hitting back should take me backwards through the same flow in which I came. The flow might be something like, Select File -> Select Account Name -> Add other files -> Select QIF Accounts -> Select Category -> Select the accounts the payments go to -> Apply screen <- Empty duplicates screen <- Select currency. But I never had the duplicates or currency form come up to begin with so how could I go back to them? Steps to reproduce: 1. Open Gnucash 2. File->Import->QIF Import 3. Run through the QIF Import to the final form. Then hit back several times to go backwards though the import druid. Actual results: Forms that were not displayed going forwards are displayed when going backwards. Expected results: The forms that were displayed going forward should be displayed in reverse order when going backwards. Does this happen every time? Hard to say. It happens often. Other information:
Here are the steps to reliably reproduce the bug for the duplicates page. Steps to reproduce: 1. Start GnuCash with an existing data file containing at least one transaction. 2. Begin importing a QIF containing no duplicates of existing transactions. 3. Keep clicking Forward until the final "Apply" page is reached. 4. Notice that the "duplicates" page was not shown. 5. Click Back. 6. The duplicates page is shown.
Here are the steps to reliably reproduce the bug for the "Tradable commodities" page. Steps to reproduce: 1. Begin importing a QIF containing a transaction using a never-before-seen QIF security. 2. Keep clicking Forward until the final "Apply" page is reached. 3. Click Back until the "Tradable commodities" page is shown.
I looked into the appearance of the currency selection page, but because the currency selection page is now ALWAYS part of the druid steps (since bug 504007 was patched), it is no longer relevant to this bug. So I think the only page appearances that need to be fixed are those described in comment 1 and comment 2.
Created attachment 104152 [details] [review] Proposed patch This patch does the following: -Check for presence of duplicates before showing duplicate pages. -Register the callback for the "back" button on commodity pages.
If you go forward to the commodities page, then back to the currency page, then forward again it skips the commodities page the second time around. Steps to Reproduce, 1.) Load this QIF: http://bugzilla.gnome.org/attachment.cgi?id=103399 2.) Click Next until you get to the Commodities page. 3.) Click Back to go back until you get to the currency page (1 or 2 backs). 4.) Click Next to go forward. Notice that this time the commodities page is skipped.
Ian, regarding comment 5, I took a look at that one and it's not a simple fix. It requires substantial and coordinated changes to the Scheme and C sides. That could take a fair bit of time... Could we split that one off into its own bug? That way the patch already completed for comments 1 and 2 can be applied without waiting.
Ok, I'll filed it as bug 514027. Let me get some time to test out the patch you submitted. It is supposed to fix comment 1 and 2 right?
That's right, the patch is supposed to fix comment 1 and comment 2 (the duplicates page and the "tradable commodities" page, respectively).
Charles, Do you have an example QIF file for testing comment 2?
Created attachment 104342 [details] Test QIF file for comment 2 Actually, any QIF that makes the druid show a commodity page will reproduce comment 2. That would have been a better way to say it.
I tested this patch but when I get to the last page where I can hit "Apply" and I go back it indeed skips the duplicates page but it doesn't seem to go back to the right page. In the case I tested it went back to the commodities doc page, which wasn't shown on the way though. It should have gone back to the currency page for me. This kind of stuff about which page to go back and forward to gets complicated, is prone to error and introducing bugs, so I kind of want to take a think about re-structuring it. I think that determining the next page to go to back to being handled in the next and back handlers respectively is the wrong approach. The right approach might be a function for each page that determines "show me or not show me" and an algorithm that just goes though the pages one by one asking the respective function if it should be shown or not. But given that we still would like to rewrite parts of the importer to use the generic import code I'm not sure how far I'll go with that idea.
Charles, BTW, in connection with what I said above, be sure to test QIF druid flow problems both with the doc pages turned on an off. It can be revealing sometimes.
Regarding comment 11: Hmm... that's weird! I tested that and it definitely skipped the commodities doc page both ways for me, so I'm scratching my head trying to figure out how your testing was different than mine. Ah, I bet you were testing with doc pages on. I must admit that I never tried that... good catch! When going forward, the druid makes sure there are new commodities before showing the commodity doc page. But it doesn't check going backwards. OK, that should be easy to add. I'll post a revision to the patch tomorrow. It works correctly with doc pages turned off though, right? I like the idea of asking pages whether they should be shown. At the moment the logic for that is being added to the generic handler directly rather than in separate functions. It's not so bad right now since there are only a few special cases to worry about, but it could get pretty messy if there were more.
Created attachment 105453 [details] [review] Revised patch Revised patch to take care of comment 11.
Committed as r16962. Awaiting backport for 2.2.4.
Applied to branches/2.2 as r16975 for GnuCash 2.2.4. Thanks a lot!
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=503166. Please update any external references or bookmarks.