Bug 309105

Same for me, too.

Only need to create new institution and add an Investment account to it.  Then, select a security, open to edit, but make no changes.  Just Next, Next, Next - > Crash.

I must be doing something wrong here or have a different option setting. I am not able to duplicate this. Not in 4.6.3 nor in master.

(In reply to comment #3)
> I must be doing something wrong here or have a different option setting. I
> am not able to duplicate this. Not in 4.6.3 nor in master.

I just tried a slight variation and it worked. (more than once).

If the new institution is created and an existing Investment is edited to belong to that institution, then a new security is created and then edited, all is well.

If an existing stock is used, or a new one is created, and then the institution is created and then that  existing investment is added to it, and then the existing security is edited (even without changing anything) then a crash ensues, every time.

It looks as though the editing of the investment to add the institution does not get applied correctly to an existing stock.  A newly created one is set up correctly.

So
1 Create new investment account
2 Add a stock to that account
3 Create new institution
4 Add the investment to the institution
5 Edit the stock Next>Next>Next>Crash

That seems to do it.

Yes, that does it. The problem is caused during step 4. Within void InstitutionsModel::slotObjectModified() the sequence

    slotObjectRemoved(MyMoneyFile::notifyAccount, oldAccount.id());
    slotObjectAdded(MyMoneyFile::notifyAccount, obj);

close to the end of the method is executed. oldAccount and obj reference the investment account created in 1. Step 5 operates on the stock created in step 2 which is somehow not part of the model anymore. Looks like the subordinate equity account gets lost during the call to slotObjectRemoved() in the model as it is not found anymore during step 5.

(In reply to comment #5)
> Yes, that does it. The problem is caused during step 4. Within void
> InstitutionsModel::slotObjectModified() the sequence
> 
>     slotObjectRemoved(MyMoneyFile::notifyAccount, oldAccount.id());
>     slotObjectAdded(MyMoneyFile::notifyAccount, obj);
> 
> close to the end of the method is executed. oldAccount and obj reference the
> investment account created in 1. Step 5 operates on the stock created in
> step 2 which is somehow not part of the model anymore. Looks like the
> subordinate equity account gets lost during the call to slotObjectRemoved()
> in the model as it is not found anymore during step 5.

I though I'd have a Iook at this although I have only a very slight idea of what I am doing here, not having encountered Model/View before.  However, via a circuitous route, I've come to the conclusion that the reason why the stock account is not in the model is that it wasn't loaded into it.  When the institution is first created, the stock account does get loaded, but later, void InstitutionsModel::slotObjectModified() doesn't reload it.

So, I've copied the code from the end of void InstitutionsModel::slotObjectAdded() and added that to the end of  void InstitutionsModel::slotObjectModified(), at line 879.

///  static_cast<InstitutionsPrivate *>(d)->loadInstitution(this, *account);
// create items for all the accounts
  QList<MyMoneyAccount> list;
  d->m_file->accountList(list);
  for (QList<MyMoneyAccount>::ConstIterator it_l = list.constBegin(); it_l != list.constEnd(); ++it_l) {
    static_cast<InstitutionsPrivate *>(d)->loadInstitution(this, *it_l);
  }
}

This seems to work, there is no crash, and Instittutions View shows the investment account and the stock.

Any thoughts?

Two points.
Firstly, the sentence above the code change should read -
 So, I've copied the code from the end of void InstitutionsModel::load() and added that to the end of void InstitutionsModel::slotObjectAdded at line 879.
Apologies for that.
Secondly, while the change I made appeared to do what was needed, it appears to do more than wanted.  As stated, the crash does not now happen and the stock does get added to the institution, but a subsequent and unconnected creation of an investment account results in a duplication of the entry for the first investment account just in Institution View,
More investigation needed.

It wasn't helpful to load all accounts from file into the model every time the
void InstitutionsModel::slotObjectAdded() routine was entered, the line d->m_file->accountList(list);
being the cause.

I've now revised the patch and the end of void InstitutionsModel::slotObjectAdded() now is -

  if (!account || account->parentAccountId().isEmpty() || account->isIncomeExpense())
    return;

  static_cast<InstitutionsPrivate *>(d)->loadInstitution(this, *account);
  QStringList list = account->accountList();
  for (QStringList::ConstIterator it_l = list.constBegin(); it_l != list.constEnd(); ++it_l) {
    const MyMoneyAccount& accnt = d->m_file->account(*it_l);
    static_cast<InstitutionsPrivate *>(d)->loadInstitution(this, accnt);
  }
}
The crash no longer occurs and varying the order of creation of the various institution, investment and stock accounts has not so far produced any problems.  Neither has testing with my live file.  I'll continue testing, and would appreciate a 'looks OK', or otherwise, before I commit.

Git commit a5371f1e0ad7b42c7cfd1a2790108abbac96a732 by Cristian Oneț.
Committed on 15/11/2012 at 20:51.
Pushed by conet into branch 'master'.

Fix the investment accounts handling in the institutions view.
Make the investment accounts the only accounts that are added
to their parent in the institutions view. The rest of the
accounts are added at the first level because they don't inherit
the institution from their parent. Thanks to Allen for working on
this.

M  +15   -1    kmymoney/models/accountsmodel.cpp

http://commits.kde.org/kmymoney/a5371f1e0ad7b42c7cfd1a2790108abbac96a732

Git commit f6659597570e3281cbd23449b39782b25c77963d by Cristian Oneț.
Committed on 15/11/2012 at 20:51.
Pushed by conet into branch '4.6'.

Fix the investment accounts handling in the institutions view.
Make the investment accounts the only accounts that are added
to their parent in the institutions view. The rest of the
accounts are added at the first level because they don't inherit
the institution from their parent. Thanks to Allen for working on
this.
(cherry picked from commit a5371f1e0ad7b42c7cfd1a2790108abbac96a732)

M  +15   -1    kmymoney/models/accountsmodel.cpp

http://commits.kde.org/kmymoney/f6659597570e3281cbd23449b39782b25c77963d

(In reply to comment #9)
> Git commit a5371f1e0ad7b42c7cfd1a2790108abbac96a732 by Cristian Oneț.
> Committed on 15/11/2012 at 20:51.
> Pushed by conet into branch 'master'.
> 
> Fix the investment accounts handling in the institutions view.
> Make the investment accounts the only accounts that are added
> to their parent in the institutions view. The rest of the
> accounts are added at the first level because they don't inherit
> the institution from their parent. Thanks to Allen for working on
> this.
> 
> M  +15   -1    kmymoney/models/accountsmodel.cpp
> 
> http://commits.kde.org/kmymoney/a5371f1e0ad7b42c7cfd1a2790108abbac96a732

I'm not sure this is right, I'm afraid.

I have a test file with no institutions.  There is an existing investment with five stocks below it.  All looks correct in Institutions view.  If I create a new investment account, isolated, the existing five stocks in the other investment account are all duplicated in Institutions view.  The new account looks OK.  If I add a stock to it, Institutions view shows it as having two identical stocks.  The other investment account, with the duplicated stocks, now has them in triplicate.

If I now create a new institution, and add the new investment account, with its apparent duplicate stocks, now only one stock is showing.

If I now create another investment account, all existing accounts get another stock., and when I add a stock to it, more get added.

I'll take a look at it.

Git commit e492b20918f569e218b65925b7f758aeef01d865 by Cristian Oneț.
Committed on 16/11/2012 at 07:49.
Pushed by conet into branch 'master'.

Only load the child accounts if there are any, otherwise MyMoneyFile::accountList will return all accounts.

M  +7    -5    kmymoney/models/accountsmodel.cpp

http://commits.kde.org/kmymoney/e492b20918f569e218b65925b7f758aeef01d865

Git commit ff5ec06b2eee8b367a2bbd2a50f5c04de7fa45ee by Cristian Oneț.
Committed on 16/11/2012 at 07:49.
Pushed by conet into branch '4.6'.

Only load the child accounts if there are any, otherwise MyMoneyFile::accountList will return all accounts.
(cherry picked from commit e492b20918f569e218b65925b7f758aeef01d865)

M  +7    -5    kmymoney/models/accountsmodel.cpp

http://commits.kde.org/kmymoney/ff5ec06b2eee8b367a2bbd2a50f5c04de7fa45ee

*** Bug 296518 has been marked as a duplicate of this bug. ***

*** Bug 283327 has been marked as a duplicate of this bug. ***

*** Bug 305787 has been marked as a duplicate of this bug. ***