Review Request 128817: Restructure pages in CSV Importer

[Thomas Baumgart]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128817/#review98964
-----------------------------------------------------------



I have not compiled nor run it. Simply visual inspection.


kmymoney/plugins/csvimport/csvdialog.cpp (line 196)
<https://git.reviewboard.kde.org/r/128817/#comment66627>

    amount = MyMoneyMoney();
    
    does exactly the same and is much easier.



kmymoney/plugins/csvimport/csvwizard.cpp (line 573)
<https://git.reviewboard.kde.org/r/128817/#comment66628>

    Don't we know about a lot more currency symbols in the engine (MyMoneyFile::instance()->currencyList())? The method would be tradingSymbol(). See KMyMoneyView::loadDefaultCurrencies() for the list of known currencies. Why do some of them are hard coded here?



kmymoney/plugins/csvimport/csvwizard.cpp (line 576)
<https://git.reviewboard.kde.org/r/128817/#comment66629>

    Doesn't that match an empty txt as well?



kmymoney/plugins/csvimport/csvwizard.cpp (line 1890)
<https://git.reviewboard.kde.org/r/128817/#comment66631>

    Can't this be written as
    
    bool FormatsPage::isComplete() const
    {
      const bool enable = m_isDecimalSymbolOK && m_isDateFormatOK;
      wizard()->button(QWizard::CustomButton2)->setEnabled(enable);
      wizard()->button(QWizard::CustomButton3)->setEnabled(enable);
      return enable;
    }


- Thomas Baumgart


On Sept. 4, 2016, 8:29 nachm., Łukasz Wojniłowicz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128817/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2016, 8:29 nachm.)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> -------
> 
> I actually decided to get rid of completion page and restructure LinesDate page. In my opinion it's more logical now and better structured for new feature I introduced ie. calculate fee column on fly.
> 
> 1) shortened updateDecimalSymbol and renamed it to
> validateDecimalSymbol,
> 2) introduced detectDecimalSymbol for autodetecting decimal symbol,
> 3) simplified createStatement,
> 4) simplified and deduplicated slotImportClicked,
> 5) new decimal symbol option 'auto' for columns with various decimal
> symbols,
> 6) decimal symbol and date format get checked every time formats page is
> being opened,
> 7) removed completion page,
> 8) introduced rows page after separator page which is usefull for
> calculate fee
> 9) introduced formats page which groups all validity checks in one
> place,
> 10) checking securities existence is now part of
> InvestmentPage::validatePage(),
> 11) validateDateFormat operates on tableWidget and not on m_lines so it
> can be faster,
> 12) renamed page labels so they represent the content better,
> 13) Rows and Formats page have layout equal to Separator page, so look is more consistent.
> 
> 
> Diffs
> -----
> 
>   kmymoney/plugins/csvimport/CMakeLists.txt 216c79c 
>   kmymoney/plugins/csvimport/completionwizardpage.ui 616e0c3 
>   kmymoney/plugins/csvimport/csvdialog.h 59e6898 
>   kmymoney/plugins/csvimport/csvdialog.cpp ed9e1d8 
>   kmymoney/plugins/csvimport/csvwizard.h ecec5b0 
>   kmymoney/plugins/csvimport/csvwizard.cpp b576dea 
>   kmymoney/plugins/csvimport/csvwizard.ui 723e128 
>   kmymoney/plugins/csvimport/investprocessing.h 902f9bb 
>   kmymoney/plugins/csvimport/investprocessing.cpp f050441 
>   kmymoney/plugins/csvimport/separatorwizardpage.ui 2ad5abd 
> 
> Diff: https://git.reviewboard.kde.org/r/128817/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20160907/5af77d1b/attachment.html>