Warn user about errors in the upload

Antonio Terceiro
1 parent 87bc57b3
Showing 4 changed files with 38 additions and 3 deletions Show diff stats
app/controllers/my_profile/cms_controller.rb
public/javascripts/article.js
public/stylesheets/application.css
test/functional/cms_controller_test.rb
@@ -357,7 +357,8 @@ class CmsController &lt; MyProfileController
         'title' => item.title,
         'url' => item.image? ? item.public_filename(:uploaded) : url_for(item.url),
         :icon => icon_for_article(item),
-        :content_type => item.mime_type
+        :content_type => item.mime_type,
+        :error => item.errors.any? ? _('%s could not be uploaded') % item.title : nil,
       }
     end.to_json
   end
@@ -26,6 +26,10 @@ jQuery(function($) {
   function insert_items(items, selector) {
     var html_for_items = '';
     $.each(items, function(i, item) {
+      if (item.error) {
+        html_for_items += '<li class="media-upload-error">' + item.error + '</li>';
+        return;
+      }
       if (item.content_type && item.content_type.match(/^image/)) {
         html_for_items += '<li class="icon-photos"><img src="' + item.url + '"/><br/><a href="' + item.url + '">' + item.title + '</a></li>';
       } else {
@@ -3406,6 +3406,9 @@ div.with_media_panel .formfield input {
  max-width: 96px;
  border: 1px solid #d3d7cf;
 }
+.text-editor-sidebar .media-upload-error {
+  color: red;
+}
  
 /* ==> public/stylesheets/controller_contact.css <== */
 /*** SELECT CITY ***/
@@ -1486,7 +1486,7 @@ class CmsControllerTest &lt; Test::Unit::TestCase
     assert_match /test.txt/, @response.body
     assert_equal 'application/json', @response.content_type
  
-    data = eval(@response.body.gsub('":', '"=>'))
+    data = parse_json_response
     assert_equal 'test.txt', data.first['title']
     assert_match /\/testinguser\/test.txt$/, data.first['url']
     assert_match /text/, data.first['icon']
@@ -1498,17 +1498,19 @@ class CmsControllerTest &lt; Test::Unit::TestCase
     assert_match 'test.txt', @response.body
     assert_equal 'application/json', @response.content_type
  
-    data = eval(@response.body.gsub('":', '"=>'))
+    data = parse_json_response
  
     assert_equal 'test.txt', data[0]['title']
     assert_match /\/testinguser\/test.txt$/, data[0]['url']
     assert_match /text/, data[0]['icon']
     assert_match /text/, data[0]['content_type']
+    assert_nil data[0]['error']
  
     assert_equal 'rails.png', data[1]['title']
     assert_no_match /\/public\/articles\/.*\/rails.png$/, data[1]['url']
     assert_match /png$/, data[1]['icon']
     assert_match /image/, data[1]['content_type']
+    assert_nil data[1]['error']
  
   end
  
@@ -1516,4 +1518,29 @@ class CmsControllerTest &lt; Test::Unit::TestCase
     post :media_upload, :profile => @profile.identifier
   end
  
+  should 'mark unsuccessfull uploads' do
+    file = UploadedFile.create!(:profile => profile, :uploaded_data => fixture_file_upload('files/rails.png', 'image/png'))
+
+    post :media_upload, :profile => profile.identifier, :media_listing => true, :file1 => fixture_file_upload('files/rails.png', 'image/png'), :file2 => fixture_file_upload('/files/test.txt', 'text/plain')
+
+    assert_equal 'application/json', @response.content_type
+    data = parse_json_response
+
+    assert_equal 'rails.png', data[0]['title']
+    assert_not_nil data[0]['error']
+    assert_match /rails.png/, data[0]['error']
+
+    assert_equal 'test.txt', data[1]['title']
+    assert_nil data[1]['error']
+  end
+
+  protected
+
+  # FIXME this is to avoid adding an extra dependency for a proper JSON parser.
+  # For now we are assuming that the JSON is close enough to Ruby and just
+  # making some adjustments.
+  def parse_json_response
+    eval(@response.body.gsub('":', '"=>').gsub('null', 'nil'))
+  end
+
 end
...	...	@@ -357,7 +357,8 @@ class CmsController < MyProfileController
357	357	'title' => item.title,
358	358	'url' => item.image? ? item.public_filename(:uploaded) : url_for(item.url),
359	359	:icon => icon_for_article(item),
360		- :content_type => item.mime_type
	360	+ :content_type => item.mime_type,
	361	+ :error => item.errors.any? ? _('%s could not be uploaded') % item.title : nil,
361	362	}
362	363	end.to_json
363	364	end
...	...
...	...	@@ -26,6 +26,10 @@ jQuery(function($) {
26	26	function insert_items(items, selector) {
27	27	var html_for_items = '';
28	28	$.each(items, function(i, item) {
	29	+ if (item.error) {
	30	+ html_for_items += '<li class="media-upload-error">' + item.error + '</li>';
	31	+ return;
	32	+ }
29	33	if (item.content_type && item.content_type.match(/^image/)) {
30	34	html_for_items += '<li class="icon-photos"><img src="' + item.url + '"/><br/><a href="' + item.url + '">' + item.title + '</a></li>';
31	35	} else {
...	...
...	...	@@ -3406,6 +3406,9 @@ div.with_media_panel .formfield input {
3406	3406	max-width: 96px;
3407	3407	border: 1px solid #d3d7cf;
3408	3408	}
	3409	+.text-editor-sidebar .media-upload-error {
	3410	+ color: red;
	3411	+}
3409	3412
3410	3413	/* ==> public/stylesheets/controller_contact.css <== */
3411	3414	/* SELECT CITY */
...	...
...	...	@@ -1486,7 +1486,7 @@ class CmsControllerTest < Test::Unit::TestCase
1486	1486	assert_match /test.txt/, @response.body
1487	1487	assert_equal 'application/json', @response.content_type
1488	1488
1489		- data = eval(@response.body.gsub('":', '"=>'))
	1489	+ data = parse_json_response
1490	1490	assert_equal 'test.txt', data.first['title']
1491	1491	assert_match /\/testinguser\/test.txt$/, data.first['url']
1492	1492	assert_match /text/, data.first['icon']
...	...	@@ -1498,17 +1498,19 @@ class CmsControllerTest < Test::Unit::TestCase
1498	1498	assert_match 'test.txt', @response.body
1499	1499	assert_equal 'application/json', @response.content_type
1500	1500
1501		- data = eval(@response.body.gsub('":', '"=>'))
	1501	+ data = parse_json_response
1502	1502
1503	1503	assert_equal 'test.txt', data[0]['title']
1504	1504	assert_match /\/testinguser\/test.txt$/, data[0]['url']
1505	1505	assert_match /text/, data[0]['icon']
1506	1506	assert_match /text/, data[0]['content_type']
	1507	+ assert_nil data[0]['error']
1507	1508
1508	1509	assert_equal 'rails.png', data[1]['title']
1509	1510	assert_no_match /\/public\/articles\/.*\/rails.png$/, data[1]['url']
1510	1511	assert_match /png$/, data[1]['icon']
1511	1512	assert_match /image/, data[1]['content_type']
	1513	+ assert_nil data[1]['error']
1512	1514
1513	1515	end
1514	1516
...	...	@@ -1516,4 +1518,29 @@ class CmsControllerTest < Test::Unit::TestCase
1516	1518	post :media_upload, :profile => @profile.identifier
1517	1519	end
1518	1520
	1521	+ should 'mark unsuccessfull uploads' do
	1522	+ file = UploadedFile.create!(:profile => profile, :uploaded_data => fixture_file_upload('files/rails.png', 'image/png'))
	1523	+
	1524	+ post :media_upload, :profile => profile.identifier, :media_listing => true, :file1 => fixture_file_upload('files/rails.png', 'image/png'), :file2 => fixture_file_upload('/files/test.txt', 'text/plain')
	1525	+
	1526	+ assert_equal 'application/json', @response.content_type
	1527	+ data = parse_json_response
	1528	+
	1529	+ assert_equal 'rails.png', data[0]['title']
	1530	+ assert_not_nil data[0]['error']
	1531	+ assert_match /rails.png/, data[0]['error']
	1532	+
	1533	+ assert_equal 'test.txt', data[1]['title']
	1534	+ assert_nil data[1]['error']
	1535	+ end
	1536	+
	1537	+ protected
	1538	+
	1539	+ # FIXME this is to avoid adding an extra dependency for a proper JSON parser.
	1540	+ # For now we are assuming that the JSON is close enough to Ruby and just
	1541	+ # making some adjustments.
	1542	+ def parse_json_response
	1543	+ eval(@response.body.gsub('":', '"=>').gsub('null', 'nil'))
	1544	+ end
	1545	+
1519	1546	end
...	...