Refactor to eventually allow buddies to belong to multiple groups.

In order to make group values non-distinct, (i.e. buddies can be in multiple
groups), we can't assume there's one view per roster item.

This means we cannot manage the rosterview as an overview (since it makes this
assumption).

converse.js

@@ -2828,6 +2828,22 @@
                 "click .remove-xmpp-contact": "removeContact"
             },
 
+            initialize: function () {
+                this.model.on("change", this.onChange, this);
+            },
+
+            onChange: function () {
+                if (converse.show_only_online_users) {
+                    if (this.model.get('chat_status') !== 'online') {
+                        this.$el.hide();
+                    } else {
+                        this.$el.show();
+                    }
+                } else {
+                    this.render();
+                }
+            },
+
             openChat: function (ev) {
                 ev.preventDefault();
                 return converse.chatboxviews.showChat({
@@ -2846,10 +2862,11 @@
                 var result = confirm(__("Are you sure you want to remove this contact?"));
                 if (result === true) {
                     var bare_jid = this.model.get('jid');
-                    converse.connection.roster.remove(bare_jid, function (iq) {
+                    converse.connection.roster.remove(bare_jid, $.proxy(function (iq) {
                         converse.connection.roster.unauthorize(bare_jid);
                         converse.rosterview.model.remove(bare_jid);
-                    });
+                        this.remove();
+                    }, this));
                 }
             },
 
@@ -3248,9 +3265,9 @@
                 this.model.off(); 
                 this.model.on("add", this.onAdd, this);
                 this.model.on('change', this.onChange, this); 
-                this.model.on("remove", this.removeRosterItemView, this);
-                this.model.on("destroy", this.removeRosterItemView, this);
-                this.model.on("reset", this.removeAllRosterItemViews, this);
+                this.model.on("remove", this.update, this);
+                this.model.on("destroy", this.update, this);
+                this.model.on("reset", this.reset, this);
                 this.render();
                 this.model.fetch({add: true}); // Get the cached roster items from localstorage
             },
@@ -3279,6 +3296,16 @@
                 this.$fragment.append($(roster_markup));
             },
 
+            update: function (item) {
+                this.updateCount().toggleHeaders();
+            },
+
+            reset: function () {
+                this.removeAll();
+                this.update();
+                return this;
+            },
+
             insertRosterFragment: function () {
                 if (this.$fragment) {
                     this.$el.html(this.$fragment)
@@ -3303,7 +3330,7 @@
                 if ((_.size(item.changed) === 1) && _.contains(_.keys(item.changed), 'sorted')) {
                     return;
                 }
-                this.updateChatBox(item).renderRosterItem(item).toggleHeaders().updateCount();
+                this.updateChatBox(item).toggleHeaders().updateCount();
                 if (item.changed.chat_status) { // A changed chat status implies a new sort order
                     this.sortRoster();
                 }
@@ -3325,35 +3352,6 @@
                 return this;
             },
 
-            removeAllRosterItemViews: function () {
-                this.removeAll();
-                this.updateCount().toggleHeaders();
-                return this;
-            },
-
-            removeRosterItemView: function (item) {
-                if (this.get(item.id)) {
-                    this.get(item.id).remove();
-                    this.updateCount().toggleHeaders();
-                }
-                return this;
-            },
-
-            renderRosterItem: function (item) {
-                var view = this.get(item.id);
-                if (!view) {
-                    converse.log("renderRosterItem called with item that doesn't have a view", "error");
-                    return this;
-                }
-                if ((converse.show_only_online_users) && (item.get('chat_status') !== 'online')) {
-                    view.$el.remove();
-                    view.delegateEvents();
-                } else {
-                    view.render()
-                }
-                return this;
-            },
-
             getRoster: function () {
                 // TODO: see is _ensureElement can be used.
                 // Return the document fragment if it exists.
@@ -3431,6 +3429,7 @@
             },
 
             updateCount: function () {
+                // XXX: Is this still being used/valid?
                 var $count = $('#online-count');
                 $count.text('('+this.model.getNumOnlineContacts()+')');
                 if (!$count.is(':visible')) {
@@ -3441,27 +3440,35 @@
 
             toggleHeaders: function () {
                 var $el = this.getRoster();
-                var $contact_requests = $el.find('#xmpp-contact-requests'),
-                    $pending_contacts = $el.find('#pending-xmpp-contacts');
                 var $groups = $el.find('.roster-group');
-                // Hide the headers if there are no contacts under them
-                _.each([$groups, $contact_requests, $pending_contacts], function (h) {
-                    var show_or_hide = function (h) {
-                        if (h.nextUntil('dt').length) {
-                            if (!h.is(':visible')) {
-                                h.show();
+                if (_.contains(this.model.pluck('ask'), 'subscribe')) {
+                    $el.find('#pending-xmpp-contacts').show();
+                } else {
+                    $el.find('#pending-xmpp-contacts').hide();
+                }
+
+                if (_.contains(this.model.pluck('requesting'), true)) {
+                    $el.find('#xmpp-contact-requests').show();
+                } else {
+                    $el.find('#xmpp-contact-requests').hide();
+                }
+
+                // Hide the group headers if there are no contacts under them
+                var show_or_hide = function ($groups) {
+                    if ($groups.nextUntil('dt').length) {
+                        if (!$groups.is(':visible')) {
+                            $groups.show();
                         }
                     }
-                        else if (h.is(':visible')) { h.hide(); }
+                    else if ($groups.is(':visible')) { $groups.hide(); }
                 };
-                    if (h.length > 1) {
+                if ($groups.length > 1) {
                     $groups.each(function (idx, group) {
                         show_or_hide($(group));
                     });
                 } else {
-                        show_or_hide(h);
+                    show_or_hide($groups);
                 }
-                });
                 return this;
             },
 

spec/controlbox.js

@@ -182,9 +182,27 @@
                     var names = $.map($contacts, function (o) { return $(o).text().trim(); });
                     expect(names).toEqual(_.clone(names).sort());
                 }, converse));
+            }, converse));
 
-                // Check that pending and requesting contacts appear after
-                // current contacts.
+            it("can share contacts among them (values aren't distinct)", $.proxy(function () {
+                // TODO: this test is not finished yet and the thing that it's
+                // testing not yet implemented.
+                _clearContacts();
+                var i=0, j=0, t;
+                spyOn(converse, 'emit');
+                spyOn(this.rosterview, 'updateCount').andCallThrough();
+                converse.roster_groups = true;
+                converse.rosterview.render();
+                for (i=0; i<mock.cur_names.length; i++) {
+                    this.roster.create({
+                        jid: mock.cur_names[i].replace(/ /g,'.').toLowerCase() + '@localhost',
+                        subscription: 'both',
+                        ask: null,
+                        groups: ['colleagues', 'friends'],
+                        fullname: mock.cur_names[i],
+                        is_last: i===(mock.cur_names.length-1)
+                    });
+                }
             }, converse));
         }, converse));
 
@@ -242,12 +260,17 @@
                 spyOn(this.connection.roster, 'remove').andCallThrough();
                 spyOn(this.connection.roster, 'unauthorize');
                 spyOn(this.rosterview.model, 'remove').andCallThrough();
+                spyOn(view, 'removeContact').andCallThrough();
+                spyOn(view, 'remove').andCallThrough();
+                view.delegateEvents();
 
                 view.$el.find('.remove-xmpp-contact').click();
                 expect(window.confirm).toHaveBeenCalled();
                 expect(this.connection.roster.remove).toHaveBeenCalled();
                 expect(this.connection.roster.unauthorize).toHaveBeenCalled();
                 expect(this.rosterview.model.remove).toHaveBeenCalled();
+                expect(view.removeContact).toHaveBeenCalled();
+                expect(view.remove).toHaveBeenCalled();
                 // The element must now be detached from the DOM.
                 expect(view.$el.closest('html').length).toBeFalsy();
             }, converse));
@@ -552,7 +575,7 @@
                 this.rosterview.model.reset();
                 spyOn(converse, 'emit');
                 spyOn(this.connection.roster, 'unauthorize');
-                spyOn(this.rosterview, 'removeRosterItemView').andCallThrough();
+                spyOn(this.rosterview, 'update').andCallThrough();
                 spyOn(window, 'confirm').andReturn(true);
                 this.rosterview.initialize(); // Must be initialized only after the spy has been called
                 utils.createContacts('requesting').openControlBox();
@@ -566,7 +589,7 @@
                 accept_button.click();
                 expect(view.declineRequest).toHaveBeenCalled();
                 expect(window.confirm).toHaveBeenCalled();
-                expect(this.rosterview.removeRosterItemView).toHaveBeenCalled();
+                expect(this.rosterview.update).toHaveBeenCalled();
                 expect(this.connection.roster.unauthorize).toHaveBeenCalled();
                 // There should now be one less contact
                 expect(this.roster.length).toEqual(mock.req_names.length-1);