[Kmymoney-devel] Review Request 114767: Add ability to modify loan institution attribute.

[Thomas Baumgart]

> On Jan. 1, 2014, 4:15 p.m., Thomas Baumgart wrote:
> > kmymoney/wizards/newloanwizard/keditloanwizard.cpp, line 470
> > <https://git.reviewboard.kde.org/r/114767/diff/1/?file=228526#file228526line470>
> >
> >     Think about using "void institutionList(QList<MyMoneyInstitution>& list) const" here. Avoids another copy operation.
> 
> Jeremy Whiting wrote:
>     I copied this bit of code directly from KNewAccountDlg.cpp, I'll update my patch, but that code I copied from ought to be fixed also probably.

Yes. It's not wrong per se, but new code should use the method I mentioned.


> On Jan. 1, 2014, 4:15 p.m., Thomas Baumgart wrote:
> > kmymoney/wizards/newloanwizard/loanattributeswizardpage.cpp, line 84
> > <https://git.reviewboard.kde.org/r/114767/diff/1/?file=228530#file228530line84>
> >
> >     Better use
> >     
> >      if(dlg->exec() && dlg != 0)
> >     
> >     here. See http://blogs.kde.org/node/3919 for details.
> 
> Jeremy Whiting wrote:
>     Noted.
>     
>     I do have one other question about this task. I can't seem to get the institutions to be sorted in the combobox alphabetically.  I set the insertPolicy of the combobox to be alphabetical, yet the entries appear in the order in which they appear in the xml file I guess. The institution list in the KNewAccountDlg is sorted alphabetically, but I don't see what's sorting it. Any ideas?

Check NewAccountWizard::InstitutionPage::slotLoadWidgets() which contains a qsort() statement. KNewAccountDlg::slotLoadInstitutions(const QString& name) should probably also receive the sorting feature.


- Thomas


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


On Dec. 31, 2013, 6:37 p.m., Jeremy Whiting wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114767/
> -----------------------------------------------------------
> 
> (Updated Dec. 31, 2013, 6:37 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 257619
>     http://bugs.kde.org/show_bug.cgi?id=257619
> 
> 
> Repository: kmymoney
> 
> 
> Description
> -------
> 
> Add ability to modify loan institution attribute.
> 
> Note this is definitely a work in progress, the QWizardPage title "Attributes" is not best probably, and the page order doesn't make much sense either imo. It probably should go after the radio buttons to ask what you want to change. I'd like to add a fourth option to just modify attributes if that makes sense to everyone, since it's a bit cumbersome to recalculate interest, payoff, etc. just to change the institution the loan belongs to.
> 
> 
> Diffs
> -----
> 
>   kmymoney/wizards/newloanwizard/CMakeLists.txt 939d569c2bba56ccfd3c9d6b28f8b90d414b9332 
>   kmymoney/wizards/newloanwizard/keditloanwizard.cpp 5590a359ffc16c92fc3dc24b6484aac1180b1507 
>   kmymoney/wizards/newloanwizard/knewloanwizard.h 6b3f049912605f4f9cb0e78ee83b14e2767586b8 
>   kmymoney/wizards/newloanwizard/knewloanwizarddecl.ui 831d85ce388b51a904c76cd998f54ca6a099b347 
>   kmymoney/wizards/newloanwizard/loanattributeswizardpage.h PRE-CREATION 
>   kmymoney/wizards/newloanwizard/loanattributeswizardpage.cpp PRE-CREATION 
>   kmymoney/wizards/newloanwizard/loanattributeswizardpagedecl.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/114767/diff/
> 
> 
> Testing
> -------
> 
> It works here to set and to show the current institution.
> 
> 
> Thanks,
> 
> Jeremy Whiting
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20140103/10191303/attachment-0001.html>