CollectionTreeItemModelBase handling of various artists element and setRequiresUpdate()

[Nikhil Marathe]

Hi,

This is a query 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 (around line 900
of latest master)
    //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 following patch should clarify.
---------------------------------------------------8<------------------------------
diff --git a/src/browsers/CollectionTreeItemModelBase.cpp
b/src/browsers/CollectionTreeItemModelBase.cpp
index b651418..dc28727 100644
--- a/src/browsers/CollectionTreeItemModelBase.cpp
+++ b/src/browsers/CollectionTreeItemModelBase.cpp
@@ -853,6 +853,7 @@
CollectionTreeItemModelBase::handleSpecialQueryResult(
CollectionTreeItem::Type
                 }
             }
             //we have removed the special node if it existed
+            parent->setRequiresUpdate( false );
             return;
         }

---------------------------------8<---------------------------

I think this should happen, mainly because it triggers a pathological
response otherwise. If the QM gives empty results for Various Artists,
hundreds of new QMs get created for the same parent just because it
can't find Various Artists. But I might be missing some reason that
this setRequiresUpdate() call is not being done. So before I call it a
'bug' I'd rather Max and others have a look at it.

Regards,
Nikhil