Refactor to make more manageable

converse.js

@@ -3365,6 +3365,38 @@
                 }
             },
 
+            getGroup: function (name) {
+                var groups, $groups, group_lower, index;
+                var $group = $el.find('.roster-group[data-group="'+name+'"]');
+                if ($group.length > 0) {
+                    return $group;
+                }
+                $groups = $el.find('.roster-group');
+                $group = $(converse.templates.group_header({
+                        label_group: name,
+                        desc_group_toggle: DESC_GROUP_TOGGLE,
+                        toggle_state: 'opened' // TODO: remember state...
+                    }));
+                if ($groups.length) {
+                    group_lower = name.toLowerCase();
+                    groups = $.map($groups, function(o) { return o.dataset.name.toLowerCase(); })
+                    groups.push(group_lower);
+                    index = groups.sort().indexOf(group_lower);
+                    if (index == 0) {
+                        $($groups.first()).before($group);
+                    } else if (index == groups.length) {
+                        $($groups.last()).after($group);
+                    } else {
+                        $($groups.eq(index)).before($group);
+                    }
+                } else {
+                    // This shouldn't actually happen, since
+                    // there's always the Ungrouped.
+                    this.getRosterElement().append($group);
+                }
+                return $group;
+            },
+
             addCurrentContact: function (view) {
                 var $el = this.getRosterElement(),
                     item = view.model;
@@ -3373,37 +3405,7 @@
                         $el.find('.roster-group[data-group="'+HEADER_UNGROUPED+'"]').after(view.el);
                     } else {
                         _.each(item.get('groups'), $.proxy(function (group) {
-                            var groups, $groups, group_lower, index, header;
-                            var $group = $el.find('.roster-group[data-group="'+group+'"]');
-                            if ($group.length > 0) {
-                                $group.after(view.el);
-                            } else {
-                                $groups = $el.find('.roster-group');
-
-                                header = $(converse.templates.group_header({
-                                        label_group: group,
-                                        desc_group_toggle: DESC_GROUP_TOGGLE,
-                                        toggle_state: 'opened' // TODO: remember state...
-                                    })).after(view.el)
-
-                                if ($groups.length) {
-                                    group_lower = group.toLowerCase();
-                                    groups = $.map($groups, function(o) { return o.dataset.group.toLowerCase(); })
-                                    groups.push(group_lower);
-                                    index = groups.sort().indexOf(group_lower);
-                                    if (index == 0) {
-                                        $($groups.first()).before(header);
-                                    } else if (index == groups.length) {
-                                        $($groups.last()).after(header);
-                                    } else {
-                                        $($groups.eq(index)).before(header);
-                                    }
-                                } else {
-                                    // This shouldn't actually happen, since
-                                    // there's always the Ungrouped.
-                                    this.getRosterElement().append(header);
-                                }
-                            }
+                            this.getGroup(group).after(view.el);
                         },this));
                     }
                 } else {

spec/controlbox.js

@@ -128,6 +128,47 @@
 
     describe("The Contacts Roster", $.proxy(function (mock, utils) {
 
+        describe("Roster Groups", $.proxy(function () {
+            function _clearContacts () {
+                utils.clearBrowserStorage();
+                converse.rosterview.model.reset();
+                converse.rosterview.initialize(); // Register new listeners and document fragment
+            };
+
+            it("can be used to organize existing contacts", $.proxy(function () {
+                _clearContacts();
+                var i=0, j=0, t;
+                spyOn(converse, 'emit');
+                spyOn(this.rosterview, 'updateCount').andCallThrough();
+                converse.roster_groups = true;
+                converse.rosterview.render();
+                var groups = {
+                    'colleagues': 3,
+                    'friends & acquaintences': 3,
+                    'Family': 4,
+                    'ænemies': 3,
+                    'Ungrouped': 2
+                };
+                _.each(_.keys(groups), $.proxy(function (name) {
+                    j = i;
+                    for (i=j; i<j+groups[name]; i++) {
+                        this.roster.create({
+                            jid: mock.cur_names[i].replace(/ /g,'.').toLowerCase() + '@localhost',
+                            subscription: 'both',
+                            ask: null,
+                            groups: name === 'ungrouped'? [] : [name],
+                            fullname: mock.cur_names[i],
+                            is_last: i===(mock.cur_names.length-1)
+                        });
+                    }
+                }, converse));
+                // Check that the groups appear alphabetically
+                t = this.rosterview.$el.find('dt.roster-group a.group-toggle').text();
+                expect(t).toEqual('colleaguesFamilyfriends & acquaintencesUngrouped');
+            }, converse));
+        }, converse));
+
+
         describe("Pending Contacts", $.proxy(function () {
             function _clearContacts () {
                 utils.clearBrowserStorage();
@@ -269,38 +310,6 @@
                 expect(t).toEqual(mock.cur_names.slice(0,i+1).sort().join(''));
             }, converse));
 
-            it("can be assigned to groups inside the roster", $.proxy(function () {
-                _clearContacts();
-                var i=0, j=0, t;
-                spyOn(converse, 'emit');
-                spyOn(this.rosterview, 'updateCount').andCallThrough();
-                converse.roster_groups = true;
-                converse.rosterview.render();
-                var groups = {
-                    'colleagues': 3,
-                    'friends & acquaintences': 3,
-                    'Family': 4,
-                    'ænemies': 3,
-                    'Ungrouped': 2
-                };
-                _.each(_.keys(groups), $.proxy(function (name) {
-                    j = i;
-                    for (i=j; i<j+groups[name]; i++) {
-                        this.roster.create({
-                            jid: mock.cur_names[i].replace(/ /g,'.').toLowerCase() + '@localhost',
-                            subscription: 'both',
-                            ask: null,
-                            groups: name === 'ungrouped'? [] : [name],
-                            fullname: mock.cur_names[i],
-                            is_last: i===(mock.cur_names.length-1)
-                        });
-                    }
-                }, converse));
-                // Check that the groups appear alphabetically
-                t = this.rosterview.$el.find('dt.roster-group a.group-toggle').text();
-                expect(t).toEqual('colleaguesFamilyfriends & acquaintencesUngrouped');
-            }, converse));
-
             it("can change their status to online and be sorted alphabetically", $.proxy(function () {
                 _addContacts();
                 var item, view, jid, t;