Review Request: Possible error in the CollectionTreeItemModelBase fetch cycle

[Nikhil Shantanu Marathe]

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

Review request for amarok.


Summary
-------

Just another heads up of something I sent earlier but didn't get replies to. Markey suggested this might be a good place.

This is a fix regarding the exact conditions under which a
setRequiresUpdate(false) is done on a CollectionTreeItem, the parent
of a "Various Artists" item.

Consider the case when there are no entries tagged "Various Artists"
returned by a QueryMaker. In that case, the
CollectionTreeItemModelBase::handleSpecialQueryResult() function
receives a dataList of size == 0. If dataList is empty() we remove the
special node from the tree (or it may not exist previously). This
means we are no longer looking for a various artists type result.

Now consider the middle of :handleSpecialQueryResult

   //populate children will call setRequiresUpdate on vaNode
   //but as the special query is based on specialNode's parent,
   //we have to call setRequiresUpdate on the parent too
   //yes, this will mean we will call setRequiresUpdate twice
   parent->setRequiresUpdate( false );

where we have actually FOUND entries, and so we stop trying to update
the parent node again.

My question is, shouldn't this be the case when dataList is empty too?
Which means shouldn't we call parent->setRequiresUpdate( false ); even
when we get empty results. The attached patch fixes what I think is a bug.


Diffs
-----

  src/browsers/CollectionTreeItemModelBase.cpp c25549b 

Diff: http://git.reviewboard.kde.org/r/100003/diff


Testing
-------


Thanks,

Nikhil Shantanu

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20100927/312b847f/attachment.htm