Review Request: Merge request for telepathy-kde-contactlist

[David Edmundson]

> On March 25, 2011, 1:30 a.m., David Edmundson wrote:
> > main-widget.cpp, line 377
> > <http://git.reviewboard.kde.org/r/100838/diff/4/?file=12746#file12746line377>
> >
> >     If you need to manually invalidate your filter you have something wrong with either your model or the filter code.
> 
> Martin Klapetek wrote:
>     This is the case I described above - this is a slot called when the account connection state changes. When it gets disconnected, the filtering needs to be invalidated to 'refilter' the items, therefore this is called here. See the docs for QSortFilterProxyModel::invalidateFilter() - "This function should be called if you are implementing custom filtering (e.g. filterAcceptsRow()), and your filter parameters have changed." - This is exactly the case.
> 
> David Edmundson wrote:
>     I'm not convinced. You haven't changed the filter - you're still filtering out offline accounts, therefore the filter parameters haven't changed.
>     
>     When the model emits dataChanged() it should automatically call filterAcceptsRow();
>     If you need something outside the model to monitor for changes and poke the filter, it seems to defeat the point of it. 
>     
>     I'll try taking a look. If I can make it work without it, then we remove it. Otherwise it clearly needs to stay.

Fixed it!

It's a bug in AccountsModel. 

AccountModelItem doesn't call onChange() when the connectionStatus changes. 
As soon as you make it do this everything starts working properly without your workaround.

Catch me on IRC sometime because I want to put this fix in our upstream branch too and update that bug report and such.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100838/#review2132
-----------------------------------------------------------


On March 23, 2011, 9:32 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100838/
> -----------------------------------------------------------
> 
> (Updated March 23, 2011, 9:32 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This is the work that begun in my clone repo and has matured enough now to be merged back to 'upstream', where the work should continue now.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 2c62ea10556cdba38d1bca0fe63603df97414336 
>   account-button.h PRE-CREATION 
>   account-button.cpp PRE-CREATION 
>   account-filter-model.h PRE-CREATION 
>   account-filter-model.cpp PRE-CREATION 
>   accounts-model-item.h PRE-CREATION 
>   accounts-model-item.cpp PRE-CREATION 
>   accounts-model.h PRE-CREATION 
>   accounts-model.cpp PRE-CREATION 
>   contact-delegate-overlay.h PRE-CREATION 
>   contact-delegate-overlay.cpp PRE-CREATION 
>   contact-delegate.h PRE-CREATION 
>   contact-delegate.cpp PRE-CREATION 
>   contact-model-item.h PRE-CREATION 
>   contact-model-item.cpp PRE-CREATION 
>   contact-overlays.h PRE-CREATION 
>   contact-overlays.cpp PRE-CREATION 
>   contact-view-hover-button.h PRE-CREATION 
>   contact-view-hover-button.cpp PRE-CREATION 
>   filter-bar.h PRE-CREATION 
>   filter-bar.cpp PRE-CREATION 
>   main-widget.h aed6f7c8336321bc8a6ffb3b9af6b1d493f85790 
>   main-widget.cpp 6146b62eec17b54be63b594200613931673ac5fe 
>   main-widget.ui 5b0d5aaaf3a4e2f49eb030b98b15fcae3a86170c 
>   main.cpp bba0e4175e8853afe603f26ea4707f4974192d0f 
>   ontologies/CMakeLists.txt 3e0bbe8c634908f689dcd360409aeddcc6fc0d23 
>   tree-node.h PRE-CREATION 
>   tree-node.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/100838/diff
> 
> 
> Testing
> -------
> 
> We did a lot of thourough testing on #kde-telepathy, also some people emailed their reports which all has been fixed.
> 
> 
> Thanks,
> 
> Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110325/30e90e70/attachment.htm