Bug 230277 - Suppress display of unused institutions
Summary: Suppress display of unused institutions
Status: RESOLVED FIXED
Alias: None
Product: kmymoney
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: KMyMoney Devel Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-11 00:02 UTC by David Houlden
Modified: 2011-03-13 20:07 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Houlden 2010-03-11 00:02:50 UTC
Version:           SVN 1101783 (using KDE 4.3.5)
OS:                Linux
Installed from:    Compiled From Sources

After accounts are closed, the institutions view can be left displaying institutions which no longer have any open accounts. My suggestion is that if closed accounts are not displayed and all accounts for an institution are closed then the institution should not be displayed.
Comment 1 Alvaro Soliverez 2010-05-03 02:10:04 UTC
SVN commit 1122029 by asoliverez:

Fixed whether closed accounts would be displayed in the institutions view so that the general setting would be taken into account
Hide an institution if it has no accounts and the "Do not show accounts" setting is activated or the "Show all accounts" action is disabled.
BUG:230277

 M  +9 -2      kinstitutionsview.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1122029
Comment 2 David Houlden 2010-07-22 00:23:24 UTC
I'm sure this was fixed but I just noticed that it is no longer working. Using SVN 1152395 I am seeing all institutions even those where all accounts are closed.
Comment 3 Cristian Oneț 2010-07-22 22:12:14 UTC
The fix must have been lost while removing qt3support.
Comment 4 Cristian Oneț 2010-07-23 21:19:59 UTC
After further looking into this I've decided that the current behavior is a correct one. I say this considering the fact the adding a new institution will display the empty institution in the institutions view (which seems OK to me since the institution was just added). If this is so I don't see a reason to hide institutions with closed accounts only (which are hidden). That would mean that there is an inconsistency.
So I'm closing this as invalid, of course, any other suggestions, points of view are welcome.
Comment 5 Alvaro Soliverez 2010-07-23 21:48:31 UTC
The behaviour was to hide empty institutions only if "hide closed accounts" was checked. ie, it follows the logic that you hide unnecessary information if you desire to do so.
I find that logic sensible for users who only want to see the most useful info.
Comment 6 Cristian Oneț 2010-07-23 21:54:45 UTC
(In reply to comment #5)
> The behaviour was to hide empty institutions only if "hide closed accounts" was
> checked. ie, it follows the logic that you hide unnecessary information if you
> desire to do so.
> I find that logic sensible for users who only want to see the most useful info.

But 'hide closed accounts' does not mean hide institutions and as I said empty institutions are not hidden, which is OK I think. If the institution is not needed it can be safely deleted.
Comment 7 David Houlden 2010-07-23 22:02:55 UTC
Cristian,
Kmymoney does not allow an institution to be deleted even if all accounts for that institution are closed. The fix as implemented by Alvaro still seems logical to me. It removed a lot of old unused institutions from the institution view which I did not need to see but still retained the option of getting them back by showing all accounts.
Comment 8 Alvaro Soliverez 2010-07-29 15:10:47 UTC
To me, this is a valid bug. I've added the previous fix and I agree with the logic behind it.
Comment 9 Cristian Oneț 2010-07-29 15:39:51 UTC
I'm sorry to disagree but to me hiding institutions on the 'Hide closed accounts' options does not seem right. Why don't we hide the liability group when there are no accounts, or only closed accounts, isn't that the same? Why do we have a separate 'Hide unused categories' and don't use the 'Hide closed accounts' option? Because we don't want to overload the semantics of the option.

I would agree to have a 'Hide unused institutions' with the semantics that if the institution has no visible accounts (closed-hidden or no accounts) then it would become hidden.
Comment 10 Alvaro Soliverez 2010-07-29 16:04:02 UTC
Accounts and categories are central to the application, so they deserve their own separate options.
Institutions are not, and adding another option just for this would only add bloat, IMHO. 
However, if you feel really strong about this, let's change it to a wishlist to add the option to hide unused institutions when release is done.

The bug should not be closed because it was a feature we already had, and added recently enough to know it's current.
Comment 11 Cristian Oneț 2010-07-29 16:09:27 UTC
I agree. If it's just me and this seems fine to everyone else than we can go ahead and do it as requested. I mean I also wanted to fix this immediately after reported but I got the second thoughts after thinking about it.

We also had the 'feature' that and empty institution (with no accounts) was not displayed which I think was more of a bug and it is fixed with the current behavior.
Comment 12 Cristian Oneț 2011-03-12 12:03:46 UTC
SVN commit 1224575 by conet:

Instead of synchronizing the whole account tree on each data changed take advantage of the new, more granular, signals and perform the appropriate modifications in the model's data after it has been loaded once. 
Also create a separate instance for the institutions model. This change has the following rationale. Mixing the two hierarchies in the same model (by account type and institution) would make it difficult to implement changes since depending on the changes it would imply structural modifications in only one instance of the same account. So for example changing the institution means a structural change in the institution hierarchy but no change in the accounts by type hierarchy. Separating the two hierarchies means that one account can be found only once in the model and it also simplifies the implementation of the modifications slots.

REVIEW: 6589
BUG: 230277

 M  +394 -205  models/accountsmodel.cpp  
 M  +60 -7     models/accountsmodel.h  
 M  +12 -11    models/models.cpp  
 M  +5 -3      models/models.h  
 M  +2 -2      views/kinstitutionsview.cpp  
 M  +39 -3     views/kmymoneyview.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1224575
Comment 13 David Houlden 2011-03-12 21:15:19 UTC
Cristian has closed this as fixed but I still see all of my unused institutions. I can't find a way to suppress them even though all accounts for the institutions are closed.
Comment 14 Cristian Oneț 2011-03-12 22:09:45 UTC
Did you try checking 'hide closed accounts' and unchecking 'show all accounts'? It works here.
Comment 15 David Houlden 2011-03-12 23:10:32 UTC
Yes, those settings are what I always use. I do not see any closed accounts but I see institutions with no accounts under them in the institutions view.
Comment 16 Cristian Oneț 2011-03-13 12:28:46 UTC
Since I don't have any other ideas... Are you sure that you are running the latest revision from SVN?
Comment 17 David Houlden 2011-03-13 14:27:34 UTC
New checkout from svn. Help -> About KMyMoney shows Version 4.5.90-svn1224684. Running on KDE 4.5.5. I even tried starting with a new kmymoneyrc. I still see the same problem. No closed accounts are shown but the institutions are still shown.
Comment 18 Cristian Oneț 2011-03-13 19:37:49 UTC
SVN commit 1224707 by conet:

BUG: 230277

The 'hide unused institutions' implementation was not working correctly if one institution had child accounts. Fix that by handling only empty institutions (with no accounts) with the flag (hideClosedAccounts()) and all the other institutions with the visibility of it's children.

 M  +5 -3      accountsmodel.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1224707
Comment 19 Cristian Oneț 2011-03-13 19:39:42 UTC
Now it should really be fixed. Thanks for your perseverance :).
Comment 20 David Houlden 2011-03-13 20:07:17 UTC
Yes, that has fixed it. Thanks Cristian.