"Send to" upload (sender) frame palette can get into a state with no way to dismiss

[garycmartin]

Testing the "send to" Journal feature (over Gabble) can lead to an upload frame icon palette getting into a state where it can't be cancelled or dismissed. This happens when the friend you are sending to declines your file download. The uploader's upload icon palette now only shows a palette with some (inaccurate) "Transfer to ..." text and no way to dismiss (see attached screen shot).

Ideally the palette should say "Transfer declined by ...", and have a dismiss option.

Steps to reproduce:
1) connect 2 Sugar users to the same Jabber server
2) make sure both can see each other in the Neighbourhood view
3) make each a friend of the other (buddy icon will appear in Group view)
4) on machine A use the Journal "send to --> machine B"
5) on machine B note that you now have a download request in the Frame
6) on machine B, decline the download with the icons palette
7) on machine A note that the upload request icon now has no way of closing

[garycmartin]

Oh my... I might actually have a patch!

diff --git a/src/jarabe/frame/activitiestray.py b/src/jarabe/frame/activitiestray.py
index 1e2b8e8..8dd75e7 100644
--- a/src/jarabe/frame/activitiestray.py
+++ b/src/jarabe/frame/activitiestray.py
@@ -877,6 +877,21 @@ class OutgoingTransferPalette(BaseTransferPalette):
 
             self.update_progress()
 
+        elif self.file_transfer.props.state == filetransfer.FT_STATE_CANCELLED:
+
+            for item in self.menu.get_children():
+                self.menu.remove(item)
+
+            nick = self.file_transfer.buddy.props.nick
+            self.props.secondary_text = _('Transfer declined by %r') % nick
+
+            menu_item = MenuItem(_('Dismiss'), icon_name='dialog-cancel')
+            menu_item.connect('activate', self.__dismiss_activate_cb)
+            self.menu.append(menu_item)
+            menu_item.show()
+
+            self.update_progress()
+
     def __cancel_activate_cb(self, menu_item):
         self.file_transfer.cancel()

[tomeu]

Sounds good, though we should avoid having the same string in several places because each will appear as one more string to translate in pootle.

We can also remove some code duplication in the same shot if we move dismiss_activate_cb to BaseTransferPalette and add there a method add_dismiss_menu_item().

Does it make sense?

[garycmartin]

OK, well let me take another look. I was hoping to keep any changes to a bare minimum (due to the pain of testing this, and the time it burns for me). Would you consider an even more minimal patch to fix the current bug? Unfortunately it provides no user acknowledgement that the remote user declined the transfer :-(

diff --git a/src/jarabe/frame/activitiestray.py b/src/jarabe/frame/activitiestray.py
index 1e2b8e8..65b349d 100644
--- a/src/jarabe/frame/activitiestray.py
+++ b/src/jarabe/frame/activitiestray.py
@@ -865,7 +865,8 @@ class OutgoingTransferPalette(BaseTransferPalette):
 
             self.update_progress()
 
-        elif self.file_transfer.props.state == filetransfer.FT_STATE_COMPLETED:
+        elif self.file_transfer.props.state == filetransfer.FT_STATE_COMPLETED\
+             or self.file_transfer.props.state == filetransfer.FT_STATE_CANCELLED:
 
             for item in self.menu.get_children():
                 self.menu.remove(item)

[tomeu]

Pushed, thanks!

Do you want to also land the most complete patch?

[garycmartin]

Replying to tomeu:

Pushed, thanks!

Fab :-)

Do you want to also land the most complete patch?

I'm going for your BaseTransferPalette suggestion, to reduce code footprint for you. Hopefully have another patch for 0.85.3, with the aim of getting some extra strings in there to aid usability – like the above 'Transfer declined by %r'. Sound OK? Should this go in a new ticket (it's really a small re-factor for better user feedback)?

[tomeu]

Replying to garycmartin:

Replying to tomeu:

Pushed, thanks!

Fab :-)

Do you want to also land the most complete patch?

I'm going for your BaseTransferPalette suggestion, to reduce code footprint for you. Hopefully have another patch for 0.85.3, with the aim of getting some extra strings in there to aid usability – like the above 'Transfer declined by %r'. Sound OK?

Yup, sounds great.

Should this go in a new ticket (it's really a small re-factor for better user feedback)?

As you prefer, as long as it appears in the review queue, that's fine.