[blam: 3/5] ChannelCollection: only remove a channel's item on explicit removal

[Carlos Martín Nieto <cmartin src gnome org>]

commit 3bdbe3c5ee81b2470d4bd0a422406f05dc039d5a
Author: Carlos Martín Nieto <cmn dwim me>
Date:   Tue Jun 18 23:49:02 2013 +0200

    ChannelCollection: only remove a channel's item on explicit removal
    
    Moving a channel into a group used to cause us to remove all the items
    for that channel, causing an NRE when we then try to access the items
    we assume are there.
    
    Make ChannelCollection.Remove() delete these items instead of the
    ChannelList, as that's not its job.

 src/Application.cs       |    9 ++---
 src/ChannelCollection.cs |   76 ++++++++++++++++++++++++++++-----------------
 src/ChannelList.cs       |   18 +++++------
 3 files changed, 59 insertions(+), 44 deletions(-)
---
diff --git a/src/Application.cs b/src/Application.cs
index 82123d7..f24367f 100644
--- a/src/Application.cs
+++ b/src/Application.cs
@@ -243,7 +243,7 @@ namespace Imendio.Blam {
             addGroupDialog = new AddGroupDialog (this);
             preferencesDialog = new PreferencesDialog (this.Window);
             opmlDialog = new OpmlDialog (this.Window);
-            opmlDialog.ChannelAdded += mCollection.Add;
+            opmlDialog.ChannelAdded += channel => mCollection.Add(channel, true);
             opmlDialog.ImportFinished += OpmlImportFinished;
 
         }
@@ -568,18 +568,17 @@ namespace Imendio.Blam {
             using (var diag = new AddChannelDialog(TheApp.Window)) {
                 var response = diag.Run();
 
+                diag.Hide();
                 switch (response) {
                 case (int)ResponseType.Ok:
                     var chan = new Channel();
                     chan.Url = diag.Url;
                     chan.http_username = diag.Username;
                     chan.http_password = diag.Password;
-                    TheApp.CCollection.Add(chan);
+                    TheApp.CCollection.Add(chan, true);
                     break;
                 }
-
-                diag.Hide();
-            }
+                       }
         }
 
         public void AddGroupActivated (object obj, EventArgs args)
diff --git a/src/ChannelCollection.cs b/src/ChannelCollection.cs
index f581127..1bcd581 100644
--- a/src/ChannelCollection.cs
+++ b/src/ChannelCollection.cs
@@ -190,20 +190,22 @@ namespace Imendio.Blam {
                }
        }
 
-        public async void Add (IChannel channel)
+        public async Task Add (IChannel channel, bool update = false)
         {
             // If we already have this feed, simply return
-            if (mChannels.Cast<Channel>().Any(channel.Url.Equals))
+                       if (mChannels.Cast<Channel>().Any(channel.Url.Equals))
                 return;
 
-        if(channel.Name == null){
-            channel.Name = channel.Url;
-        }
+                       if(channel.Name == null){
+                               channel.Name = channel.Url;
+                       }
 
-        mChannels.Add (channel);
-                       bool res = await channel.RefreshAsync();
-                       if (res)
-                               MarkAsDirty();
+                       mChannels.Add (channel);
+                       if (update) {
+                               bool res = await channel.RefreshAsync();
+                               if (res)
+                                       MarkAsDirty();
+                       }
 
                        NotifyAdd(channel);
                }
@@ -211,7 +213,6 @@ namespace Imendio.Blam {
         public async void Add(ChannelGroup group, IChannel channel)
         {
             group.Add(channel);
-                       bool updated = await channel.RefreshAsync();
                        channel.PropertyChanged += ChannelChanged;
 
                        NotifyAdd(new { Group = group, Channel = channel });
@@ -228,26 +229,43 @@ namespace Imendio.Blam {
                                PropertyChanged(this, args);
                }
 
-       public void Remove (IChannel channel)
-       {
-        /* Try to find out from which list we need to remove. */
-        if(mChannels.Contains(channel)){
-            mChannels.Remove (channel);
-        } else if(Groups.Contains(channel)){
-            Groups.Remove(channel);
-        } else {
-            /* It's not a first-level channel or group. Dig deeper. */
-            foreach(ChannelGroup group in Groups){
-                if(group.Channels.Contains(channel)){
-                    group.Channels.Remove(channel);
-                    break;
-                }
-            }
-        }
+               void DoRemove(IChannel channel)
+               {
+                       /* Try to find out from which list we need to remove. */
+                       if(mChannels.Contains(channel)){
+                               mChannels.Remove (channel);
+                       } else if(Groups.Contains(channel)){
+                               Groups.Remove(channel);
+                       } else {
+                               /* It's not a first-level channel or group. Dig deeper. */
+                               foreach(ChannelGroup group in Groups){
+                                       if(group.Channels.Contains(channel)){
+                                               group.Channels.Remove(channel);
+                                               break;
+                                       }
+                               }
+                       }
 
-        NotifyRemove(channel);
-        MarkAsDirty();
-       }
+                       NotifyRemove(channel);
+               }
+
+               public void Remove (IChannel channel)
+               {
+                       DoRemove(channel);
+                       channel.RemoveItems();
+                       MarkAsDirty();
+               }
+
+               public void Move(IChannel src, IChannel dst)
+               {
+                       DoRemove(src);
+                       if (dst == null)
+                               Add(src);
+                       else
+                               Add((ChannelGroup)dst, src);
+
+                       MarkAsDirty();
+               }
 
                public async void RefreshAll()
                {
diff --git a/src/ChannelList.cs b/src/ChannelList.cs
index 1851991..0a8a13d 100644
--- a/src/ChannelList.cs
+++ b/src/ChannelList.cs
@@ -143,7 +143,7 @@ namespace Imendio.Blam {
 
                }
 
-               void DragDataReceivedHandler(object o, DragDataReceivedArgs args)
+               async void DragDataReceivedHandler(object o, DragDataReceivedArgs args)
                {
                        TreePath path;
                        TreeViewDropPosition pos;
@@ -168,16 +168,15 @@ namespace Imendio.Blam {
                                if (tmp_path.Equals(tmp_path_src))
                                        return;
 
-                               Collection.Remove(src);
-
                                Model.GetIter(out tmp_iter, tmp_path);
                                IChannel drop_parent = Model.GetValue(tmp_iter, 0) as IChannel;
                                //System.Console.WriteLine("dp: {0} depth:{1}", drop_parent.GetType(), 
path.Depth);
                                if(path.Depth == 2 && drop_parent is ChannelGroup){ // Dropped inside a group.
-                                       Collection.Add((ChannelGroup)drop_parent, src);
+                                       Collection.Move(src, drop_parent);
                                } else{
-                                       Collection.Add(src);
+                                       Collection.Move(src, null);
                                }
+
                        /* Directly over a channel. */
                        } else {
                                TreePath tmp_path_src = Model.GetPath(src.Iter).Copy();
@@ -187,8 +186,8 @@ namespace Imendio.Blam {
 
                                if(dst is Channel && path.Depth == 1 && // dst is top-level
                                   src is Channel && Model.GetPath(src.Iter).Depth == 2){ // src is in a group
-                                       Collection.Remove(src);
-                                       Collection.Add(src);
+                                       Console.WriteLine("think you dropped on a group add -> remove");
+                                       Collection.Move(src, null);
                                }
 
                                /*
@@ -199,10 +198,10 @@ namespace Imendio.Blam {
                                 */
                                if((dst is Channel && path.Depth == 1) || src is ChannelGroup ||
                                   path == tmp_path_src){ // Move to the same group
+                                       Console.WriteLine("not doing anything as you dropped on a top-level 
channel");
                                        return;
                                }
 
-                               Collection.Remove(src);
                                /*
                                 * If we drop on a channel which is inside a group, we add that channel to
                                 * the group.
@@ -213,7 +212,7 @@ namespace Imendio.Blam {
                                        dst = Model.GetValue(tmp_iter, 0) as ChannelGroup;
                                }
 
-                               Collection.Add(dst as ChannelGroup, src);
+                               Collection.Move(src, dst);
                        }
                }
 
@@ -277,7 +276,6 @@ namespace Imendio.Blam {
                 this.Selection.Changed -= selectionChangedHandler;
 
                 valid = (this.Model as TreeStore).Remove(ref iter);
-                channel.RemoveItems();
 
                 this.Selection.Changed += selectionChangedHandler;