--- a/sat_frontends/quick_frontend/quick_contact_list.py	Thu Jun 21 01:21:44 2018 +0200
+++ b/sat_frontends/quick_frontend/quick_contact_list.py	Sun Jun 24 21:59:29 2018 +0200
@@ -17,7 +17,8 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Contact List handling multi profiles at once, should replace quick_contact_list module in the future"""
+"""Contact List handling multi profiles at once,
+    should replace quick_contact_list module in the future"""
 
 from sat.core.i18n import _
 from sat.core.log import getLogger
@@ -32,7 +33,8 @@
 try:
     # FIXME: to be removed when an acceptable solution is here
     unicode('')  # XXX: unicode doesn't exist in pyjamas
-except (TypeError, AttributeError):  # Error raised is not the same depending on pyjsbuild options
+except (TypeError, AttributeError):  # Error raised is not the same depending on
+                                     # pyjsbuild options
     # XXX: pyjamas' max doesn't support key argument, so we implement it ourself
     pyjamas_max = max
 
@@ -88,11 +90,24 @@
         self.show_empty_groups = True
         self.show_resources = False
         self.show_status = False
+        # do we show entities with notifications?
+        # if True, entities will be show even if they normally would not
+        # (e.g. not in contact list) if they have notifications attached
+        self.show_entities_with_notifs = True
 
-        self.host.bridge.asyncGetParamA(C.SHOW_EMPTY_GROUPS, "General", profile_key=profile, callback=self._showEmptyGroups)
-        self.host.bridge.asyncGetParamA(C.SHOW_OFFLINE_CONTACTS, "General", profile_key=profile, callback=self._showOfflineContacts)
+        self.host.bridge.asyncGetParamA(C.SHOW_EMPTY_GROUPS,
+                                        "General",
+                                        profile_key=profile,
+                                        callback=self._showEmptyGroups)
 
-        # FIXME: workaround for a pyjamas issue: calling hash on a class method always return a different value if that method is defined directly within the class (with the "def" keyword)
+        self.host.bridge.asyncGetParamA(C.SHOW_OFFLINE_CONTACTS,
+                                        "General",
+                                        profile_key=profile,
+                                        callback=self._showOfflineContacts)
+
+        # FIXME: workaround for a pyjamas issue: calling hash on a class method always
+        #        return a different value if that method is defined directly within the
+        #        class (with the "def" keyword)
         self.presenceListener = self.onPresenceUpdate
         self.host.addListener('presence', self.presenceListener, [self.profile])
         self.nickListener = self.onNickUpdate
@@ -116,12 +131,13 @@
         """Check if entity is in contact list
 
         An entity can be in contact list even if not in roster
-        @param entity (jid.JID): jid of the entity (resource is not ignored, use bare jid if needed)
+        @param entity (jid.JID): jid of the entity (resource is not ignored,
+            use bare jid if needed)
         """
         if entity.resource:
             try:
                 return entity.resource in self.getCache(entity.bare, C.CONTACT_RESOURCES)
-            except KeyError:
+            except exceptions.NotFound:
                 return False
         return entity in self._cache
 
@@ -139,7 +155,8 @@
 
         @return (set[jid.JID])
         """
-        return set([entity for entity in self._roster if self.getCache(entity, C.PRESENCE_SHOW) is not None])
+        return set([entity for entity in self._roster
+            if self.getCache(entity, C.PRESENCE_SHOW) is not None])
 
     @property
     def roster_entities_by_group(self):
@@ -186,7 +203,8 @@
         entities are not sorted
         key: bare jid, value: data
         """
-        return {jid_:cache for jid_, cache in self._cache.iteritems() if self.entityToShow(jid_)}
+        return {jid_:cache for jid_, cache in self._cache.iteritems()
+                if self.entityVisible(jid_)}
 
 
     def getItem(self, entity):
@@ -198,7 +216,11 @@
         return self._cache[entity]
 
     def _gotContacts(self, contacts):
-        """Called during filling, add contacts and notice parent that contacts are filled"""
+        """Add contacts and notice parent that contacts are filled
+
+        Called during initial contact list filling
+        @param contacts(tuple): all contacts
+        """
         for contact in contacts:
             self.host.newContactHandler(*contact, profile=self.profile)
         handler._contactsFilled(self.profile)
@@ -214,31 +236,43 @@
     def fill(self):
         handler.fill(self.profile)
 
-    def getCache(self, entity, name=None, bare_default=True):
+    def getCache(self, entity, name=None, bare_default=True, create_if_not_found=False):
         """Return a cache value for a contact
 
-        @param entity(jid.JID): entity of the contact from who we want data (resource is used if given)
+        @param entity(jid.JID): entity of the contact from who we want data
+            (resource is used if given)
             if a resource specific information is requested:
-                - if no resource is given (bare jid), the main resource is used, according to priority
+                - if no resource is given (bare jid), the main resource is used,
+                    according to priority
                 - if resource is given, it is used
         @param name(unicode): name the data to get, or None to get everything
-        @param bare_default(bool, None): if True and entity is a full jid, the value of bare jid
-            will be returned if not value is found for the requested resource.
+        @param bare_default(bool, None): if True and entity is a full jid,
+            the value of bare jid will be returned if not value is found for
+            the requested resource.
             If False, None is returned if no value is found for the requested resource.
             If None, bare_default will be set to False if entity is in a room, True else
+        @param create_if_not_found(bool): if True, create contact if it's not found
+            in cache
         @return: full cache if no name is given, or value of "name", or None
+        @raise NotFound: entity not found in cache
         """
         # FIXME: resource handling need to be reworked
-        # FIXME: bare_default work for requesting full jid to get bare jid, but not the other way
-        #        e.g.: if we have set an avatar for user@server.tld/resource and we request user@server.tld
+        # FIXME: bare_default work for requesting full jid to get bare jid,
+        #        but not the other way
+        #        e.g.: if we have set an avatar for user@server.tld/resource
+        #        and we request user@server.tld
         #        we won't get the avatar set in the resource
         try:
             cache = self._cache[entity.bare]
         except KeyError:
-            self.setContact(entity)
-            cache = self._cache[entity.bare]
+            if create_if_not_found:
+                self.setContact(entity)
+                cache = self._cache[entity.bare]
+            else:
+                raise exceptions.NotFound
 
         if name is None:
+            # full cache is requested
             return cache
 
         if name in ('status', C.PRESENCE_STATUSES, C.PRESENCE_PRIORITY, C.PRESENCE_SHOW):
@@ -337,7 +371,8 @@
 
         @param entity(jid.JID): jid of the special entity
             if the jid is full, will be added to special extras
-        @param special_type: one of special type (e.g. C.CONTACT_SPECIAL_GROUP) or None to remove special flag
+        @param special_type: one of special type (e.g. C.CONTACT_SPECIAL_GROUP)
+            or None to remove special flag
         """
         assert special_type in C.CONTACT_SPECIAL_ALLOWED + (None,)
         self.setCache(entity, C.CONTACT_SPECIAL, special_type)
@@ -345,14 +380,16 @@
     def getSpecials(self, special_type=None, bare=False):
         """Return all the bare JIDs of the special roster entities of with given type.
 
-        @param special_type(unicode, None): if not None, filter by special type (e.g. C.CONTACT_SPECIAL_GROUP)
+        @param special_type(unicode, None): if not None, filter by special type
+            (e.g. C.CONTACT_SPECIAL_GROUP)
         @param bare(bool): return only bare jids if True
         @return (iter[jid.JID]): found special entities
         """
         for entity in self._specials:
             if bare and entity.resource:
                 continue
-            if special_type is not None and self.getCache(entity, C.CONTACT_SPECIAL) != special_type:
+            if (special_type is not None
+                and self.getCache(entity, C.CONTACT_SPECIAL) != special_type):
                 continue
             yield entity
 
@@ -381,7 +418,8 @@
         attribute must not be set to the default group but ignored. If not,
         you may move your contact from its actual group(s) to the default one.
 
-        None value for 'groups' has a different meaning than [None] which is for the default group.
+        None value for 'groups' has a different meaning than [None]
+        which is for the default group.
 
         @param entity (jid.JID): entity to add or replace
             if entity is a full jid, attributes will be cached in for the full jid only
@@ -394,7 +432,10 @@
             attributes = {}
 
         entity_bare = entity.bare
-        update_type = C.UPDATE_MODIFY if entity_bare in self._cache else C.UPDATE_ADD
+        # we check if the entity is visible before changing anything
+        # this way we know if we need to do an UPDATE_ADD, UPDATE_MODIFY
+        # or an UPDATE_DELETE
+        was_visible = self.entityVisible(entity_bare)
 
         if in_roster:
             self._roster.add(entity_bare)
@@ -403,19 +444,23 @@
                                                      C.CONTACT_MAIN_RESOURCE: None,
                                                      C.CONTACT_SELECTED: set()})
 
-        assert not C.CONTACT_DATA_FORBIDDEN.intersection(attributes) # we don't want forbidden data in attributes
+        # we don't want forbidden data in attributes
+        assert not C.CONTACT_DATA_FORBIDDEN.intersection(attributes)
 
         # we set groups and fill self._groups accordingly
         if groups is not None:
             if not groups:
                 groups = [None]  # [None] is the default group
             if C.CONTACT_GROUPS in cache:
-                # XXX: don't use set(cache[C.CONTACT_GROUPS]).difference(groups) because it won't work in Pyjamas if None is in cache[C.CONTACT_GROUPS]
-                for group in [group for group in cache[C.CONTACT_GROUPS] if group not in groups]:
+                # XXX: don't use set(cache[C.CONTACT_GROUPS]).difference(groups) because
+                #      it won't work in Pyjamas if None is in cache[C.CONTACT_GROUPS]
+                for group in [group for group in cache[C.CONTACT_GROUPS]
+                              if group not in groups]:
                     self._groups[group]['jids'].remove(entity_bare)
             cache[C.CONTACT_GROUPS] = groups
             for group in groups:
-                self._groups.setdefault(group, {}).setdefault('jids', set()).add(entity_bare)
+                self._groups.setdefault(group, {}).setdefault('jids', set()).add(
+                    entity_bare)
 
         # special entities management
         if C.CONTACT_SPECIAL in attributes:
@@ -428,7 +473,8 @@
 
         # now the attributes we keep in cache
         # XXX: if entity is a full jid, we store the value for the resource only
-        cache_attr = cache[C.CONTACT_RESOURCES].setdefault(entity.resource, {}) if entity.resource else cache
+        cache_attr = (cache[C.CONTACT_RESOURCES].setdefault(entity.resource, {})
+                      if entity.resource else cache)
         for attribute, value in attributes.iteritems():
             if value is None:
                 # XXX: pyjamas hack: we need to use pop instead of del
@@ -439,23 +485,34 @@
             else:
                 if attribute == 'nick' and self.isSpecial(entity, C.CONTACT_SPECIAL_GROUP):
                     # we don't want to keep nick for MUC rooms
-                    # FIXME: this is here as plugin XEP-0054 can link resource's nick with bare jid
-                    #        which in the case of MUC set the nick for the whole MUC
+                    # FIXME: this is here as plugin XEP-0054 can link resource's nick
+                    #        with bare jid which in the case of MUC
+                    #        set the nick for the whole MUC
                     #        resulting in bad name displayed in some frontends
                     continue
                 cache_attr[attribute] = value
 
-        # we can update the display
-        self.update([entity], update_type, self.profile)
+        # we can update the display if needed
+        if self.entityVisible(entity_bare):
+            # if the contact was not visible, we need to add a widget
+            # else we just update id
+            update_type = C.UPDATE_MODIFY if was_visible else C.UPDATE_ADD
+            self.update([entity], update_type, self.profile)
+        elif was_visible:
+            # the entity was visible and is not anymore, we remove it
+            self.update([entity], C.UPDATE_DELETE, self.profile)
 
-    def entityToShow(self, entity, check_resource=False):
+    def entityVisible(self, entity, check_resource=False):
         """Tell if the contact should be showed or hidden.
 
         @param entity (jid.JID): jid of the contact
         @param check_resource (bool): True if resource must be significant
         @return (bool): True if that contact should be showed in the list
         """
-        show = self.getCache(entity, C.PRESENCE_SHOW)
+        try:
+            show = self.getCache(entity, C.PRESENCE_SHOW)
+        except exceptions.NotFound:
+            return False
 
         if check_resource:
             selected = self._selected
@@ -464,10 +521,11 @@
         return ((show is not None and show != C.PRESENCE_UNAVAILABLE)
                 or self.show_disconnected
                 or entity in selected
-                or next(self.host.getNotifs(entity.bare, profile=self.profile), None)
+                or (self.show_entities_with_notifs
+                and next(self.host.getNotifs(entity.bare, profile=self.profile), None))
                 )
 
-    def anyEntityToShow(self, entities, check_resources=False):
+    def anyEntityVisible(self, entities, check_resources=False):
         """Tell if in a list of entities, at least one should be shown
 
         @param entities (list[jid.JID]): list of jids
@@ -476,7 +534,7 @@
         """
         # FIXME: looks inefficient, really needed?
         for entity in entities:
-            if self.entityToShow(entity, check_resources):
+            if self.entityVisible(entity, check_resources):
                 return True
         return False
 
@@ -495,6 +553,7 @@
         @param entity(jid.JID): jid of the entity to remove (bare jid is used)
         """
         entity_bare = entity.bare
+        was_visible = self.entityVisible(entity_bare)
         try:
             groups = self._cache[entity_bare].get(C.CONTACT_GROUPS, set())
         except KeyError:
@@ -507,14 +566,17 @@
         for group in groups:
             self._groups[group]['jids'].remove(entity_bare)
             if not self._groups[group]['jids']:
-                self._groups.pop(group) # FIXME: we use pop because of pyjamas: http://wiki.goffi.org/wiki/Issues_with_Pyjamas/en
+                # FIXME: we use pop because of pyjamas:
+                #        http://wiki.goffi.org/wiki/Issues_with_Pyjamas/en
+                self._groups.pop(group)
         for iterable in (self._selected, self._specials):
             to_remove = set()
             for set_entity in iterable:
                 if set_entity.bare == entity.bare:
                     to_remove.add(set_entity)
             iterable.difference_update(to_remove)
-        self.update([entity], C.UPDATE_DELETE, self.profile)
+        if was_visible:
+            self.update([entity], C.UPDATE_DELETE, self.profile)
 
     def onPresenceUpdate(self, entity, show, priority, statuses, profile):
         """Update entity's presence status
@@ -525,7 +587,10 @@
         @param statuses: dict of statuses
         @param profile: %(doc_profile)s
         """
-        cache = self.getCache(entity)
+        # FIXME: cache modification should be done with setContact
+        #        the resources/presence handling logic should be moved there
+        was_visible = self.entityVisible(entity.bare)
+        cache = self.getCache(entity, create_if_not_found=True)
         if show == C.PRESENCE_UNAVAILABLE:
             if not entity.resource:
                 cache[C.CONTACT_RESOURCES].clear()
@@ -534,12 +599,14 @@
                 try:
                     del cache[C.CONTACT_RESOURCES][entity.resource]
                 except KeyError:
-                    log.error(u"Presence unavailable received for an unknown resource [{}]".format(entity))
+                    log.error(u"Presence unavailable received "
+                              u"for an unknown resource [{}]".format(entity))
                 if not cache[C.CONTACT_RESOURCES]:
                     cache[C.CONTACT_MAIN_RESOURCE] = None
         else:
             if not entity.resource:
-                log.warning(_(u"received presence from entity without resource: {}".format(entity)))
+                log.warning(_(u"received presence from entity "
+                              u"without resource: {}".format(entity)))
             resources_data = cache[C.CONTACT_RESOURCES]
             resource_data = resources_data.setdefault(entity.resource, {})
             resource_data[C.PRESENCE_SHOW] = show
@@ -549,9 +616,15 @@
             if entity.bare not in self._specials:
                 # we may have resources with no priority
                 # (when a cached value is added for a not connected resource)
-                priority_resource = max(resources_data, key=lambda res: resources_data[res].get(C.PRESENCE_PRIORITY, -2**32))
+                priority_resource = max(resources_data,
+                                        key=lambda res: resources_data[res].get(
+                                            C.PRESENCE_PRIORITY, -2**32))
                 cache[C.CONTACT_MAIN_RESOURCE] = priority_resource
-        self.update([entity], C.UPDATE_MODIFY, self.profile)
+        if self.entityVisible(entity.bare):
+            update_type = C.UPDATE_MODIFY if was_visible else C.UPDATE_ADD
+            self.update([entity], update_type, self.profile)
+        elif was_visible:
+            self.update([entity], C.UPDATE_DELETE, self.profile)
 
     def onNickUpdate(self, entity, new_nick, profile):
         """Update entity's nick
@@ -562,7 +635,6 @@
         """
         assert profile == self.profile
         self.setCache(entity, 'nick', new_nick)
-        self.update([entity], C.UPDATE_MODIFY, profile)
 
     def onNotification(self, entity, notif, profile):
         """Update entity with notification
@@ -572,7 +644,7 @@
         @param profile: %(doc_profile)s
         """
         assert profile == self.profile
-        if entity is not None:
+        if entity is not None and self.entityVisible(entity):
             self.update([entity], C.UPDATE_MODIFY, profile)
 
     def unselect(self, entity):
@@ -616,7 +688,7 @@
                 self.update([entity], C.UPDATE_SELECTION, profile=self.profile)
 
     def showOfflineContacts(self, show):
-        """Tell if offline contacts should shown
+        """Tell if offline contacts should be shown
 
         @param show(bool): True if offline contacts should be shown
         """
@@ -638,7 +710,7 @@
         if self.show_resources == show:
             return
         self.show_resources = show
-        self.update(profile=self.profile)
+        self.update(type_=C.UPDATE_STRUCTURE, profile=self.profile)
 
     def plug(self):
         handler.addProfile(self.profile)
@@ -657,7 +729,8 @@
         self.host = host
         global handler
         if handler is not None:
-            raise exceptions.InternalError(u"QuickContactListHandler must be instanciated only once")
+            raise exceptions.InternalError(u"QuickContactListHandler must be "
+                                           u"instanciated only once")
         handler = self
         self._clist = {} # key: profile, value: ProfileContactList
         self._widgets = set()
@@ -670,7 +743,8 @@
     def __contains__(self, entity):
         """Check if entity is in contact list
 
-        @param entity (jid.JID): jid of the entity (resource is not ignored, use bare jid if needed)
+        @param entity (jid.JID): jid of the entity (resource is not ignored,
+            use bare jid if needed)
         """
         for contact_list in self._clist.itervalues():
             if entity in contact_list:
@@ -825,7 +899,8 @@
 
         If special_type is None, return all special extras.
 
-        @param special_type(unicode, None): one of special type (e.g. C.CONTACT_SPECIAL_GROUP)
+        @param special_type(unicode, None): one of special type
+            (e.g. C.CONTACT_SPECIAL_GROUP)
             None to return all special extras.
         @return (set[jid.JID])
         """
@@ -838,6 +913,7 @@
         self._to_fill.remove(profile)
         if not self._to_fill:
             del self._to_fill
+            # we need a full update when all contacts are filled
             self.update()
 
     def fill(self, profile=None):
@@ -864,7 +940,8 @@
 
         remaining = to_fill.difference(filled)
         if remaining != to_fill:
-            log.debug(u"Not re-filling already filled contact list(s) for {}".format(u', '.join(to_fill.intersection(filled))))
+            log.debug(u"Not re-filling already filled contact list(s) for {}".format(
+                u', '.join(to_fill.intersection(filled))))
         for profile in remaining:
             self._clist[profile]._fill()
 
@@ -875,6 +952,7 @@
         """
         for contact_list in self._clist.itervalues():
             contact_list.clearContacts(keep_cache)
+        # we need a full update
         self.update()
 
     def select(self, entity):
@@ -895,7 +973,8 @@
             if set to False, widget state can be inconsistent, be sure to know
             what youa re doing!
         """
-        log.debug(u"Contact lists updates are now {}".format(u"LOCKED" if locked else u"UNLOCKED"))
+        log.debug(u"Contact lists updates are now {}".format(
+            u"LOCKED" if locked else u"UNLOCKED"))
         self._update_locked = locked
         if not locked and do_update:
             self.update()
@@ -910,7 +989,8 @@
     """This class manage the visual representation of contacts"""
     SINGLE=False
     PROFILES_MULTIPLE=True
-    PROFILES_ALLOW_NONE=True # Can be linked to no profile (e.g. at the early forntend start)
+    # Can be linked to no profile (e.g. at the early frontend start)
+    PROFILES_ALLOW_NONE=True
 
     def __init__(self, host, profiles):
         super(QuickContactList, self).__init__(host, None, profiles)
@@ -937,7 +1017,7 @@
 
     @property
     def items_sorted(self):
-        return handler.items
+        return handler.items_sorted
 
     def update(self, entities=None, type_=None, profile=None):
         """Update the display when something changed
@@ -950,8 +1030,11 @@
             - C.UPDATE_ADD: entity added
             - C.UPDATE_SELECTION: selection modified
             or None for undefined update
+            Note that events correspond to addition, modification and deletion
+            of items on the whole contact list. If the contact is visible or not
+            has no influence on the type_.
         @param profile(unicode, None): profile concerned with the update
-            None if unknown
+            None if all profiles need to be updated
         """
         raise NotImplementedError