[Bug 31195] Implement models

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=31195

Gustavo Boiko <gustavo.boiko at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gustavo.boiko at collabora.co.
                   |                            |uk

--- Comment #15 from Gustavo Boiko <gustavo.boiko at collabora.co.uk> 2011-03-17 07:56:31 PDT ---
(In reply to comment #13)
(...)
> 
> 1) Don't add accounts that are disabled or offline. Imagine this in the view -
> you add all available accounts, that creates one tree node for every account
> and those accounts that are disabled would be in the view for no reason. Same
> with the offline accounts, because the group would be empty anyway (there are
> no childs in this tree node until you actually connect), therefore it has no
> use in the view.

Keep in mind that this model is not used just by KDE, so there might be more
use cases than just the KDE ones. For instance, the accounts model can be used
(the toplevel items) in the kcm account for example, and there you really want
to display accounts that are offline and/or disabled.

Also, such accounts can be easily filtered out by adding a simple
sortfilterproxymodel on top of the existing model. I see really no point in
changing the original models to achieve that.

> 2) When an account goes offline in the current implementation, all the contacts
> of the account node are deleted, which leaves the account node with no
> childrens, ie. empty group. So we remove the account node as well (in fact, we
> remove only this node, all the children are deleted in the process). With this
> comes also one fix in tree-node.cpp, when you go offline and online several
> times, sooner or later it will crash because it tries to access already deleted
> pointer. We moved the "mPriv->mParent->mPriv->mChildren.removeOne(this);" line
> from destructor to the remove() method, which seems to fix the crashing.

Well, same point as the first one, I don't think removing this from the model
is a good idea.

> 3) There is one new method, AccountsModel::accountForContactIndex(const
> QModelIndex& index). This returns the account where the contact at index index
> belongs. This is needed when requesting text/audio/video channel, as you need
> to call this on an account object with the contact as a parameter.

This looks fine (the idea, haven't checked the implementation though).

> We'll continue to improve the model (implement groupping) and share all the
> changes at github (we'll try to separate the added functionality by commits
> with some descriptive commit message) so it can be eventuelly merged back.
> 
> Here's the repo: https://github.com/mck182/telepathy-qt4-yell
> 
> Feel free to ask about/comment whatever pops up there.

In any case, it is really nice that you are using/contributing back to the
models in tp-qt4-yell, thanks a lot.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.