Merge lp:~thisfred/ubuntuone-client/filenames-in-notifications into lp:ubuntuone-client

Proposed by Eric Casteleijn on 2011-03-07
Status: Merged
Approved by: Eric Casteleijn on 2011-03-09
Approved revision: 916
Merged at revision: 915
Proposed branch: lp:~thisfred/ubuntuone-client/filenames-in-notifications
Merge into: lp:ubuntuone-client
Diff against target: 505 lines (+183/-98)
2 files modified
tests/status/test_aggregator.py (+101/-59)
ubuntuone/status/aggregator.py (+82/-39)
To merge this branch: bzr merge lp:~thisfred/ubuntuone-client/filenames-in-notifications
Reviewer Review Type Date Requested Status
Alejandro J. Cura (community) Approve on 2011-03-09
Manuel de la Peña (community) 2011-03-07 Approve on 2011-03-09
Review via email: mp+52483@code.launchpad.net

Commit message

Added a filename in the downloading/downloaded/uploading/uploaded notifications to give end user more context.

Description of the change

Added a filename in the downloading/downloaded/uploading/uploaded notifications to give end user more context.

Looks like:

"'foo.bar' was downloaded to your computer."

or:

"'baz.qux' and 796 other files are uploading to your personal cloud."

To post a comment you must log in.
Manuel de la Peña (mandel) wrote :

AFAIK most of the code from the notification is platform anostic, which means that we can reuse the logic when doing the port (and use something else instead of libnotify, is this right? if that is the case, we might have to remove the /n with os.sep. Since the /n were already in the code, I'll approve and will fix it in the windows branch whenever needed.

review: Approve
Eric Casteleijn (thisfred) wrote :

yeah, everything that is not under platform should be agnostic, so let's fix that.

914. By Eric Casteleijn on 2011-03-09

keep state in the right place

915. By Eric Casteleijn on 2011-03-09

move property settings outside of __init__ and escape warnings

916. By Eric Casteleijn on 2011-03-09

added assertions for the filenames being set

Alejandro J. Cura (alecu) wrote :

Great branch!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/status/test_aggregator.py'
2--- tests/status/test_aggregator.py 2011-03-03 23:29:37 +0000
3+++ tests/status/test_aggregator.py 2011-03-09 20:40:33 +0000
4@@ -31,6 +31,9 @@
5 from ubuntuone.syncdaemon import status_listener
6 from ubuntuone.syncdaemon.volume_manager import Share, UDF
7
8+FILENAME = 'example.txt'
9+FILENAME2 = 'another_example.mp3'
10+
11
12 class PatchedClock(Clock):
13 """Patch the clock to fix twisted bug #4823."""
14@@ -746,6 +749,9 @@
15 class FakeCommand(object):
16 """A fake command."""
17
18+ def __init__(self, path=''):
19+ self.path = path
20+
21
22 class FakeVolumeManager(object):
23 """A fake vm."""
24@@ -767,6 +773,8 @@
25 self.queued_commands = set()
26 self.notification_switch = aggregator.NotificationSwitch()
27 self.connected = False
28+ self.files_uploading = []
29+ self.files_downloading = []
30
31 def queue_done(self):
32 """The queue completed all operations."""
33@@ -784,18 +792,35 @@
34 """Create a new toggleable notification object."""
35 return self.notification_switch.build_notification()
36
37- download_started = misc_command_queued
38- download_finished = misc_command_unqueued
39- upload_started = misc_command_queued
40- upload_finished = misc_command_unqueued
41+ def download_started(self, command):
42+ """A download just started."""
43+ self.files_downloading.append(command)
44+ self.misc_command_queued(command)
45+
46+ def download_finished(self, command):
47+ """A download just finished."""
48+ if command in self.files_downloading:
49+ self.files_downloading.remove(command)
50+ self.misc_command_unqueued(command)
51+
52+ def upload_started(self, command):
53+ """An upload just started."""
54+ self.files_uploading.append(command)
55+ self.misc_command_queued(command)
56+
57+ def upload_finished(self, command):
58+ """An upload just finished."""
59+ if command in self.files_uploading:
60+ self.files_uploading.remove(command)
61+ self.misc_command_unqueued(command)
62
63 def connection_made(self):
64- """The client made the connection to the server."""
65- self.connected = True
66+ """The client made the connection to the server."""
67+ self.connected = True
68
69 def connection_lost(self):
70- """The client lost the connection to the server."""
71- self.connected = False
72+ """The client lost the connection to the server."""
73+ self.connected = False
74
75
76 class StatusFrontendTestCase(BaseTwistedTestCase):
77@@ -1255,9 +1280,11 @@
78
79 def test_file_download_started(self):
80 """Test that a file has started download."""
81- fc = FakeCommand()
82+ fc = FakeCommand(path='testfile.txt')
83+ self.assertEqual('', self.aggregator.downloading_filename)
84 self.status_frontend.download_started(fc)
85- self.assertEqual(self.aggregator.download_total, 1)
86+ self.assertEqual(1, self.aggregator.download_total)
87+ self.assertEqual('testfile.txt', self.aggregator.downloading_filename)
88 self.assertMiscCommandQueued(fc)
89 self.assertEqual(1, self.fake_bubble.count)
90
91@@ -1271,9 +1298,11 @@
92
93 def test_file_upload_started(self):
94 """Test that a file has started upload."""
95- fc = FakeCommand()
96+ fc = FakeCommand(path='testfile.txt')
97+ self.assertEqual('', self.aggregator.uploading_filename)
98 self.status_frontend.upload_started(fc)
99- self.assertEqual(self.aggregator.upload_total, 1)
100+ self.assertEqual(1, self.aggregator.upload_total)
101+ self.assertEqual('testfile.txt', self.aggregator.uploading_filename)
102 self.assertMiscCommandQueued(fc)
103 self.assertEqual(1, self.fake_bubble.count)
104
105@@ -1289,10 +1318,18 @@
106 """Test the message that's shown on the discovery bubble."""
107 uploading = 10
108 downloading = 8
109- self.aggregator.files_uploading.extend(range(uploading))
110- self.aggregator.files_downloading.extend(range(downloading))
111- expected = (aggregator.files_being_uploaded(uploading) +
112- "\n" + aggregator.files_being_downloaded(downloading))
113+ filename = 'upfile0.ext'
114+ filename2 = 'downfile0.ext'
115+ self.aggregator.files_uploading.extend([
116+ FakeCommand(path='upfile%d.ext' % n) for n in range(uploading)])
117+ self.aggregator.uploading_filename = filename
118+ self.aggregator.files_downloading.extend([
119+ FakeCommand(path='downfile%d.ext' % n) for n in
120+ range(downloading)])
121+ self.aggregator.downloading_filename = filename2
122+ expected = (
123+ aggregator.files_being_uploaded(filename, uploading) + "\n" +
124+ aggregator.files_being_downloaded(filename2, downloading))
125 result = self.aggregator.get_discovery_message()
126 self.assertEqual(expected, result)
127
128@@ -1300,44 +1337,44 @@
129 """Test the message that's shown on the progress bubble."""
130 self.aggregator.upload_done = 5
131 self.aggregator.upload_total = 10
132+ self.aggregator.uploading_filename = FILENAME
133 self.aggregator.download_done = 3
134 self.aggregator.download_total = 8
135+ self.aggregator.downloading_filename = FILENAME2
136 self.aggregator.done_counter = 9
137 self.aggregator.total_counter = 20
138 percentage = int(100.0 * self.aggregator.done_counter /
139 self.aggregator.total_counter)
140- format_args = {
141- "percentage_completed": percentage,
142- }
143-
144- format_string = (
145- aggregator.files_being_uploaded(self.aggregator.upload_total) +
146- "\n" +
147- aggregator.files_being_downloaded(self.aggregator.download_total) +
148- "\n" + aggregator.PROGRESS_COMPLETED)
149-
150- expected = format_string % format_args
151+
152+ expected = (
153+ aggregator.files_being_uploaded(
154+ FILENAME, self.aggregator.upload_total) + "\n" +
155+ aggregator.files_being_downloaded(
156+ FILENAME2, self.aggregator.download_total) + "\n" +
157+ aggregator.PROGRESS_COMPLETED) % {
158+ 'percentage_completed': percentage}
159+
160 result = self.aggregator.get_progress_message()
161 self.assertEqual(expected, result)
162
163 def test_get_progress_message_no_uploads(self):
164 """The progress message when no uploads are going on."""
165+
166 self.aggregator.upload_done = 0
167 self.aggregator.upload_total = 0
168+ self.aggregator.downloading_filename = FILENAME
169 self.aggregator.download_done = 3
170 self.aggregator.download_total = 8
171 self.aggregator.done_counter = 9
172 self.aggregator.total_counter = 20
173 percentage = int(100.0 * self.aggregator.done_counter /
174 self.aggregator.total_counter)
175- format_args = {
176- "percentage_completed": percentage}
177-
178- format_string = (
179- aggregator.files_being_downloaded(self.aggregator.download_total) +
180- "\n" + aggregator.PROGRESS_COMPLETED)
181-
182- expected = format_string % format_args
183+ expected = (
184+ aggregator.files_being_downloaded(
185+ FILENAME, self.aggregator.download_total) + "\n" +
186+ aggregator.PROGRESS_COMPLETED) % {
187+ 'percentage_completed': percentage}
188+
189 result = self.aggregator.get_progress_message()
190 self.assertEqual(expected, result)
191
192@@ -1345,21 +1382,19 @@
193 """The progress message when no downloads are going on."""
194 self.aggregator.upload_done = 5
195 self.aggregator.upload_total = 10
196+ self.aggregator.uploading_filename = FILENAME
197 self.aggregator.download_done = 0
198 self.aggregator.download_total = 0
199 self.aggregator.done_counter = 9
200 self.aggregator.total_counter = 20
201 percentage = int(100.0 * self.aggregator.done_counter /
202 self.aggregator.total_counter)
203- format_args = {
204- "percentage_completed": percentage,
205- }
206-
207- format_string = (
208- aggregator.files_being_uploaded(self.aggregator.upload_total) +
209- "\n" + aggregator.PROGRESS_COMPLETED)
210-
211- expected = format_string % format_args
212+ expected = (
213+ aggregator.files_being_uploaded(
214+ FILENAME, self.aggregator.upload_total) +
215+ "\n" + aggregator.PROGRESS_COMPLETED) % {
216+ 'percentage_completed': percentage}
217+
218 result = self.aggregator.get_progress_message()
219 self.assertEqual(expected, result)
220
221@@ -1371,40 +1406,47 @@
222 def test_get_final_status_message(self):
223 """The final status message."""
224 done = (5, 10)
225+ self.aggregator.uploading_filename = FILENAME
226+ self.aggregator.downloading_filename = FILENAME2
227 self.aggregator.upload_done, self.aggregator.download_done = done
228
229- format_string = (
230+ expected = (
231 aggregator.FINAL_COMPLETED + "\n" +
232- aggregator.files_were_uploaded(self.aggregator.upload_done) +
233- "\n" +
234- aggregator.files_were_downloaded(self.aggregator.download_done))
235+ aggregator.files_were_uploaded(
236+ FILENAME, self.aggregator.upload_done) + "\n" +
237+ aggregator.files_were_downloaded(
238+ FILENAME2, self.aggregator.download_done))
239
240 result = self.aggregator.get_final_status_message()
241- self.assertEqual(format_string, result)
242+ self.assertEqual(expected, result)
243
244 def test_get_final_status_message_no_uploads(self):
245 """The final status message when there were no uploads."""
246 done = (0, 12)
247 self.aggregator.upload_done, self.aggregator.download_done = done
248+ self.aggregator.downloading_filename = FILENAME2
249
250- format_string = (aggregator.FINAL_COMPLETED + "\n" +
251- aggregator.files_were_downloaded(
252- self.aggregator.download_done))
253+ expected = (
254+ aggregator.FINAL_COMPLETED + "\n" +
255+ aggregator.files_were_downloaded(
256+ FILENAME2, self.aggregator.download_done))
257
258 result = self.aggregator.get_final_status_message()
259- self.assertEqual(format_string, result)
260+ self.assertEqual(expected, result)
261
262 def test_get_final_status_message_no_downloads(self):
263 """The final status message when there were no downloads."""
264 done = (8, 0)
265 self.aggregator.upload_done, self.aggregator.download_done = done
266+ self.aggregator.uploading_filename = FILENAME
267
268- format_string = (aggregator.FINAL_COMPLETED + "\n" +
269- aggregator.files_were_uploaded(
270- self.aggregator.upload_done))
271+ expected = (
272+ aggregator.FINAL_COMPLETED + "\n" +
273+ aggregator.files_were_uploaded(
274+ FILENAME, self.aggregator.upload_done))
275
276 result = self.aggregator.get_final_status_message()
277- self.assertEqual(format_string, result)
278+ self.assertEqual(expected, result)
279
280 def test_started_progress_bubble(self):
281 """The progress bubble is started."""
282@@ -1490,7 +1532,7 @@
283 self.patch(aggregator, "UbuntuOneLauncher", FakeLauncher)
284 self.patch(aggregator.session, "Inhibitor", FakeInhibitor)
285 clock = PatchedClock()
286- upload = FakeCommand()
287+ upload = FakeCommand(path='upload.foo')
288 sf = aggregator.StatusFrontend(clock=clock)
289 sf.set_show_all_notifications(True)
290
291@@ -1507,7 +1549,7 @@
292 clock.advance(aggregator.FileDiscoveryGatheringState.initial_delay)
293 # files found notification
294 self.assertEqual(1, len(notifications_shown))
295- download = FakeCommand()
296+ download = FakeCommand('download.bar')
297 sf.download_started(download)
298 self.assertEqual(1, len(notifications_shown))
299 # the progress still is zero
300
301=== modified file 'ubuntuone/status/aggregator.py'
302--- ubuntuone/status/aggregator.py 2011-03-03 23:29:37 +0000
303+++ ubuntuone/status/aggregator.py 2011-03-09 20:40:33 +0000
304@@ -53,50 +53,72 @@
305 FILE_SYNC_IN_PROGRESS = Q_("File synchronization in progress")
306
307
308-def files_being_uploaded(files_uploading):
309+def files_being_uploaded(filename, files_uploading):
310 """Get the i18n string for files being uploaded."""
311- format_args = {"total_uploading_files": files_uploading}
312+ other_files = files_uploading - 1
313+ if other_files < 1:
314+ return Q_(
315+ "'%(filename)s' is being uploaded to your personal cloud.") % {
316+ 'filename': filename}
317+ format_args = {
318+ "filename": filename, "other_files": other_files}
319 return gettext.dngettext(
320 GETTEXT_PACKAGE,
321- "%(total_uploading_files)d file is being uploaded to your personal "
322- "cloud.",
323- "%(total_uploading_files)d files are being uploaded to your personal "
324- "cloud.",
325- files_uploading) % format_args
326-
327-
328-def files_being_downloaded(files_downloading):
329+ "'%(filename)s' and %(other_files)d other file is being "
330+ "uploaded to your personal cloud.",
331+ "'%(filename)s' and %(other_files)d other files are being "
332+ "uploaded to your personal cloud.", other_files) % format_args
333+
334+
335+def files_being_downloaded(filename, files_downloading):
336 """Get the i18n string for files being downloaded."""
337- format_args = {"total_downloading_files": files_downloading}
338+ other_files = files_downloading - 1
339+ if other_files < 1:
340+ return Q_(
341+ "'%(filename)s' is being downloaded to your computer.") % {
342+ 'filename': filename}
343+ format_args = {
344+ "filename": filename, "other_files": other_files}
345 return gettext.dngettext(
346 GETTEXT_PACKAGE,
347- "%(total_downloading_files)d file is being downloaded to your "
348- "computer.",
349- "%(total_downloading_files)d files are being downloaded to "
350- "your computer.",
351- files_downloading) % format_args
352-
353-
354-def files_were_uploaded(upload_done):
355+ "'%(filename)s' and %(other_files)d other file is being "
356+ "downloaded to your computer.",
357+ "'%(filename)s' and %(other_files)d other files are being "
358+ "downloaded to your computer.", other_files) % format_args
359+
360+
361+def files_were_uploaded(filename, upload_done):
362 """Get the i18n string for files that were uploaded."""
363- format_args = {'total_uploaded_files': upload_done}
364+ other_files = upload_done - 1
365+ if other_files < 1:
366+ return Q_(
367+ "'%(filename)s' was uploaded to your personal cloud.") % {
368+ 'filename': filename}
369+ format_args = {
370+ 'filename': filename, 'other_files': other_files}
371 return gettext.dngettext(
372 GETTEXT_PACKAGE,
373- "%(total_uploaded_files)d file was uploaded to your personal "
374- "cloud.",
375- "%(total_uploaded_files)d files were uploaded to your "
376- "personal cloud.", upload_done) % format_args
377-
378-
379-def files_were_downloaded(download_done):
380+ "'%(filename)s' and %(other_files)d other file was uploaded to "
381+ "your personal cloud.",
382+ "'%(filename)s' and %(other_files)d other files were uploaded "
383+ "to your personal cloud.", other_files) % format_args
384+
385+
386+def files_were_downloaded(filename, download_done):
387 """Get the i18n string for files that were downloaded."""
388- format_args = {'total_downloaded_files': download_done}
389+ other_files = download_done - 1
390+ if other_files < 1:
391+ return Q_(
392+ "'%(filename)s' was downloaded to your computer.") % {
393+ 'filename': filename}
394+ format_args = {
395+ 'filename': filename, 'other_files': other_files}
396 return gettext.dngettext(
397 GETTEXT_PACKAGE,
398- "%(total_downloaded_files)d file was downloaded to your "
399- "computer.",
400- "%(total_downloaded_files)d files were downloaded to your "
401- "computer.", download_done) % format_args
402+ "'%(filename)s' and %(other_files)d other file was "
403+ "downloaded to your computer.",
404+ "'%(filename)s' and %(other_files)d other files were "
405+ "downloaded to your computer.", other_files) % format_args
406
407
408 class ToggleableNotification(object):
409@@ -592,6 +614,7 @@
410 """Create a new toggleable notification object."""
411 return self.notification_switch.build_notification()
412
413+ # pylint: disable=W0201
414 def reset(self):
415 """Reset all counters and notifications."""
416 self.total_counter = 0
417@@ -601,7 +624,9 @@
418 self.upload_total = 0
419 self.upload_done = 0
420 self.files_uploading = []
421+ self.uploading_filename = ''
422 self.files_downloading = []
423+ self.downloading_filename = ''
424
425 if self.file_discovery_bubble:
426 self.file_discovery_bubble.cleanup()
427@@ -615,17 +640,21 @@
428 if self.final_status_bubble:
429 self.final_status_bubble.cleanup()
430 self.final_status_bubble = FinalStatusBubble(self)
431+ # pylint: enable=W0201
432
433 def get_discovery_message(self):
434 """Get the text for the discovery bubble."""
435 lines = []
436 files_uploading = len(self.files_uploading)
437 if files_uploading > 0:
438- lines.append(files_being_uploaded(files_uploading))
439-
440+ lines.append(files_being_uploaded(
441+ self.uploading_filename, files_uploading))
442 files_downloading = len(self.files_downloading)
443 if files_downloading > 0:
444- lines.append(files_being_downloaded(files_downloading))
445+ self.downloading_filename = self.files_downloading[0].path.split(
446+ os.path.sep)[-1]
447+ lines.append(files_being_downloaded(
448+ self.downloading_filename, files_downloading))
449 return "\n".join(lines)
450
451 def get_progress_message(self):
452@@ -634,10 +663,12 @@
453 parts = []
454 upload_total = self.upload_total
455 if upload_total:
456- parts.append(files_being_uploaded(upload_total))
457+ parts.append(files_being_uploaded(
458+ self.uploading_filename, upload_total))
459 download_total = self.download_total
460 if download_total:
461- parts.append(files_being_downloaded(download_total))
462+ parts.append(files_being_downloaded(
463+ self.downloading_filename, download_total))
464 progress_percentage = 100.0 * self.done_counter / self.total_counter
465 format_args = {"percentage_completed": int(progress_percentage)}
466 parts.append(PROGRESS_COMPLETED % format_args)
467@@ -649,11 +680,13 @@
468 parts.append(FINAL_COMPLETED)
469 upload_done = self.upload_done
470 if upload_done:
471- parts.append(files_were_uploaded(upload_done))
472+ parts.append(files_were_uploaded(
473+ self.uploading_filename, upload_done))
474
475 download_done = self.download_done
476 if download_done:
477- parts.append(files_were_downloaded(download_done))
478+ parts.append(files_were_downloaded(
479+ self.downloading_filename, download_done))
480 return "\n".join(parts)
481
482 def restart_progress_bubble(self):
483@@ -689,6 +722,11 @@
484 """A download just started."""
485 self.files_downloading.append(command)
486 self.download_total += 1
487+ # pylint: disable=W0201
488+ if not self.downloading_filename:
489+ self.downloading_filename = self.files_downloading[0].path.split(
490+ os.path.sep)[-1]
491+ # pylint: enable=W0201
492 self.misc_command_queued(command)
493 self.file_discovery_bubble.new_file_found()
494
495@@ -703,6 +741,11 @@
496 """An upload just started."""
497 self.files_uploading.append(command)
498 self.upload_total += 1
499+ # pylint: disable=W0201
500+ if not self.uploading_filename:
501+ self.uploading_filename = self.files_uploading[0].path.split(
502+ os.path.sep)[-1]
503+ # pylint: enable=W0201
504 self.misc_command_queued(command)
505 self.file_discovery_bubble.new_file_found()
506

Subscribers

People subscribed via source and target branches