diff --git a/ckan/lib/uploader.py b/ckan/lib/uploader.py index a805270e5ae..eaebc207d15 100644 --- a/ckan/lib/uploader.py +++ b/ckan/lib/uploader.py @@ -214,7 +214,8 @@ def __init__(self, resource): if url and config_mimetype_guess == 'file_ext': self.mimetype = mimetypes.guess_type(url)[0] - if isinstance(upload_field_storage, ALLOWED_UPLOAD_TYPES): + if bool(upload_field_storage) and \ + isinstance(upload_field_storage, ALLOWED_UPLOAD_TYPES): self.filesize = 0 # bytes self.filename = upload_field_storage.filename diff --git a/ckan/public/base/css/fuchsia.css b/ckan/public/base/css/fuchsia.css index 9353e542365..c0e2f7fd3c3 100644 --- a/ckan/public/base/css/fuchsia.css +++ b/ckan/public/base/css/fuchsia.css @@ -8150,6 +8150,11 @@ fieldset[disabled] .control-custom.disabled .checkbox.btn.focus { .js .image-upload .btn-remove-url .icon-remove { margin-right: 0; } +.js .image-upload .error-inline { + margin-top: 5px; + margin-left: 2px; + font-weight: bold; +} .add-member-form .control-label { display: block; } @@ -8481,6 +8486,29 @@ fieldset[disabled] .control-custom.disabled .checkbox.btn.focus { #activity-archive-notice { clear: both; } +/* diff styles */ +table.diff { + font-family: Courier; + border: medium; +} +.diff_header { + background-color: #e0e0e0; +} +td.diff_header { + text-align: right; +} +.diff_next { + background-color: #c0c0c0; +} +.diff_add { + background-color: #aaffaa; +} +.diff_chg { + background-color: #ffff77; +} +.diff_sub { + background-color: #ffaaaa; +} .search-form { margin-bottom: 20px; padding-bottom: 25px; @@ -10366,6 +10394,13 @@ h4 small { .activity .item.follow-group .icon { background-color: #8ba669; } +.select-time { + width: 250px; + display: inline; +} +br.line-height2 { + line-height: 2; +} .dropdown:hover .dropdown-menu { display: block; } @@ -10760,25 +10795,3 @@ iframe { .reduced-padding { padding: 3px 5px; } -table.diff { - font-family:Courier; - border:medium; -} -.diff_header { - background-color:#e0e0e0; -} -td.diff_header { - text-align:right; -} -.diff_next { - background-color:#c0c0c0; -} -.diff_add { - background-color:#aaffaa; -} -.diff_chg { - background-color:#ffff77; -} -.diff_sub { - background-color:#ffaaaa; -} diff --git a/ckan/public/base/css/green.css b/ckan/public/base/css/green.css index 13a38aacf8b..6042ab9eeaa 100644 --- a/ckan/public/base/css/green.css +++ b/ckan/public/base/css/green.css @@ -8150,6 +8150,11 @@ fieldset[disabled] .control-custom.disabled .checkbox.btn.focus { .js .image-upload .btn-remove-url .icon-remove { margin-right: 0; } +.js .image-upload .error-inline { + margin-top: 5px; + margin-left: 2px; + font-weight: bold; +} .add-member-form .control-label { display: block; } @@ -8481,6 +8486,29 @@ fieldset[disabled] .control-custom.disabled .checkbox.btn.focus { #activity-archive-notice { clear: both; } +/* diff styles */ +table.diff { + font-family: Courier; + border: medium; +} +.diff_header { + background-color: #e0e0e0; +} +td.diff_header { + text-align: right; +} +.diff_next { + background-color: #c0c0c0; +} +.diff_add { + background-color: #aaffaa; +} +.diff_chg { + background-color: #ffff77; +} +.diff_sub { + background-color: #ffaaaa; +} .search-form { margin-bottom: 20px; padding-bottom: 25px; @@ -10366,6 +10394,13 @@ h4 small { .activity .item.follow-group .icon { background-color: #8ba669; } +.select-time { + width: 250px; + display: inline; +} +br.line-height2 { + line-height: 2; +} .dropdown:hover .dropdown-menu { display: block; } @@ -10760,25 +10795,3 @@ iframe { .reduced-padding { padding: 3px 5px; } -table.diff { - font-family:Courier; - border:medium; -} -.diff_header { - background-color:#e0e0e0; -} -td.diff_header { - text-align:right; -} -.diff_next { - background-color:#c0c0c0; -} -.diff_add { - background-color:#aaffaa; -} -.diff_chg { - background-color:#ffff77; -} -.diff_sub { - background-color:#ffaaaa; -} diff --git a/ckan/public/base/css/main.css b/ckan/public/base/css/main.css index 2c670a61371..eb978223865 100644 --- a/ckan/public/base/css/main.css +++ b/ckan/public/base/css/main.css @@ -8150,6 +8150,11 @@ fieldset[disabled] .control-custom.disabled .checkbox.btn.focus { .js .image-upload .btn-remove-url .icon-remove { margin-right: 0; } +.js .image-upload .error-inline { + margin-top: 5px; + margin-left: 2px; + font-weight: bold; +} .add-member-form .control-label { display: block; } @@ -8481,6 +8486,29 @@ fieldset[disabled] .control-custom.disabled .checkbox.btn.focus { #activity-archive-notice { clear: both; } +/* diff styles */ +table.diff { + font-family: Courier; + border: medium; +} +.diff_header { + background-color: #e0e0e0; +} +td.diff_header { + text-align: right; +} +.diff_next { + background-color: #c0c0c0; +} +.diff_add { + background-color: #aaffaa; +} +.diff_chg { + background-color: #ffff77; +} +.diff_sub { + background-color: #ffaaaa; +} .search-form { margin-bottom: 20px; padding-bottom: 25px; @@ -10767,25 +10795,3 @@ iframe { .reduced-padding { padding: 3px 5px; } -table.diff { - font-family:Courier; - border:medium; -} -.diff_header { - background-color:#e0e0e0; -} -td.diff_header { - text-align:right; -} -.diff_next { - background-color:#c0c0c0; -} -.diff_add { - background-color:#aaffaa; -} -.diff_chg { - background-color:#ffff77; -} -.diff_sub { - background-color:#ffaaaa; -} diff --git a/ckan/public/base/css/maroon.css b/ckan/public/base/css/maroon.css index 49d29a3d18f..a4ac0d0b4d2 100644 --- a/ckan/public/base/css/maroon.css +++ b/ckan/public/base/css/maroon.css @@ -8150,6 +8150,11 @@ fieldset[disabled] .control-custom.disabled .checkbox.btn.focus { .js .image-upload .btn-remove-url .icon-remove { margin-right: 0; } +.js .image-upload .error-inline { + margin-top: 5px; + margin-left: 2px; + font-weight: bold; +} .add-member-form .control-label { display: block; } @@ -8481,6 +8486,29 @@ fieldset[disabled] .control-custom.disabled .checkbox.btn.focus { #activity-archive-notice { clear: both; } +/* diff styles */ +table.diff { + font-family: Courier; + border: medium; +} +.diff_header { + background-color: #e0e0e0; +} +td.diff_header { + text-align: right; +} +.diff_next { + background-color: #c0c0c0; +} +.diff_add { + background-color: #aaffaa; +} +.diff_chg { + background-color: #ffff77; +} +.diff_sub { + background-color: #ffaaaa; +} .search-form { margin-bottom: 20px; padding-bottom: 25px; @@ -10366,6 +10394,13 @@ h4 small { .activity .item.follow-group .icon { background-color: #8ba669; } +.select-time { + width: 250px; + display: inline; +} +br.line-height2 { + line-height: 2; +} .dropdown:hover .dropdown-menu { display: block; } @@ -10760,25 +10795,3 @@ iframe { .reduced-padding { padding: 3px 5px; } -table.diff { - font-family:Courier; - border:medium; -} -.diff_header { - background-color:#e0e0e0; -} -td.diff_header { - text-align:right; -} -.diff_next { - background-color:#c0c0c0; -} -.diff_add { - background-color:#aaffaa; -} -.diff_chg { - background-color:#ffff77; -} -.diff_sub { - background-color:#ffaaaa; -} diff --git a/ckan/public/base/css/red.css b/ckan/public/base/css/red.css index 3c0a73f413e..e4aeeba39d4 100644 --- a/ckan/public/base/css/red.css +++ b/ckan/public/base/css/red.css @@ -8150,6 +8150,11 @@ fieldset[disabled] .control-custom.disabled .checkbox.btn.focus { .js .image-upload .btn-remove-url .icon-remove { margin-right: 0; } +.js .image-upload .error-inline { + margin-top: 5px; + margin-left: 2px; + font-weight: bold; +} .add-member-form .control-label { display: block; } @@ -8481,6 +8486,29 @@ fieldset[disabled] .control-custom.disabled .checkbox.btn.focus { #activity-archive-notice { clear: both; } +/* diff styles */ +table.diff { + font-family: Courier; + border: medium; +} +.diff_header { + background-color: #e0e0e0; +} +td.diff_header { + text-align: right; +} +.diff_next { + background-color: #c0c0c0; +} +.diff_add { + background-color: #aaffaa; +} +.diff_chg { + background-color: #ffff77; +} +.diff_sub { + background-color: #ffaaaa; +} .search-form { margin-bottom: 20px; padding-bottom: 25px; @@ -10366,6 +10394,13 @@ h4 small { .activity .item.follow-group .icon { background-color: #8ba669; } +.select-time { + width: 250px; + display: inline; +} +br.line-height2 { + line-height: 2; +} .dropdown:hover .dropdown-menu { display: block; } @@ -10760,25 +10795,3 @@ iframe { .reduced-padding { padding: 3px 5px; } -table.diff { - font-family:Courier; - border:medium; -} -.diff_header { - background-color:#e0e0e0; -} -td.diff_header { - text-align:right; -} -.diff_next { - background-color:#c0c0c0; -} -.diff_add { - background-color:#aaffaa; -} -.diff_chg { - background-color:#ffff77; -} -.diff_sub { - background-color:#ffaaaa; -} diff --git a/ckan/public/base/javascript/modules/image-upload.js b/ckan/public/base/javascript/modules/image-upload.js index 49255b711d5..9437305632b 100644 --- a/ckan/public/base/javascript/modules/image-upload.js +++ b/ckan/public/base/javascript/modules/image-upload.js @@ -11,7 +11,8 @@ this.ckan.module('image-upload', function($) { field_url: 'image_url', field_clear: 'clear_upload', field_name: 'name', - upload_label: '' + upload_label: '', + previous_upload: false }, /* Should be changed to true if user modifies resource's name @@ -43,6 +44,7 @@ this.ckan.module('image-upload', function($) { this.label_location = $('label[for="field-image-url"]'); // determines if the resource is a data resource this.is_data_resource = (this.options.field_url === 'url') && (this.options.field_upload === 'upload'); + this.previousUpload = this.options.previous_upload; // Is there a clear checkbox on the form already? var checkbox = $(field_clear, this.el); @@ -68,6 +70,11 @@ this.ckan.module('image-upload', function($) { this._('Upload') + '') .insertAfter(this.input); + if (this.previousUpload) { + $('
' + + this._('Please select the file to upload again') + '
').appendTo(this.field_image); + } + // Button for resetting the form when there is a URL set var removeText = this._('Remove'); $('' diff --git a/ckan/public/base/less/forms.less b/ckan/public/base/less/forms.less index 79cc069b63f..41c33932b31 100644 --- a/ckan/public/base/less/forms.less +++ b/ckan/public/base/less/forms.less @@ -752,6 +752,12 @@ select[data-module="autocomplete"] { margin-right: 0; } } + + .error-inline { + margin-top: 5px; + margin-left: 2px; + font-weight: bold; + } } .add-member-form .control-label { diff --git a/ckan/templates/macros/form.html b/ckan/templates/macros/form.html index dd9a3fef6ff..ff01a076c02 100644 --- a/ckan/templates/macros/form.html +++ b/ckan/templates/macros/form.html @@ -422,10 +422,21 @@ {% set placeholder = placeholder if placeholder else _('http://example.com/my-image.jpg') %} {% set url_label = url_label or _('Image URL') %} {% set upload_label = upload_label or _('Image') %} + {% set previous_upload = data['previous_upload'] %} {% if is_upload_enabled %} -
+ + +
{% endif %} diff --git a/ckan/tests/controllers/test_package.py b/ckan/tests/controllers/test_package.py index 462686a7ffe..4d18a637359 100644 --- a/ckan/tests/controllers/test_package.py +++ b/ckan/tests/controllers/test_package.py @@ -144,6 +144,8 @@ def test_complete_package_with_two_resources(self, app, user_env): assert pkg.resources[1].url == u"http://example.com/resource1" assert pkg.state == "active" + # resource upload is tested in TestExampleIUploaderPlugin + def test_previous_button_works(self, app, user_env): url = url_for("dataset.new") response = app.post(url, environ_overrides=user_env, data={ diff --git a/ckan/tests/controllers/test_user.py b/ckan/tests/controllers/test_user.py index e5f752419c1..9f25d915fa9 100644 --- a/ckan/tests/controllers/test_user.py +++ b/ckan/tests/controllers/test_user.py @@ -783,7 +783,7 @@ def test_request_reset_when_duplicate_emails(self, send_reset_link, app): emailed_users = [ call[0][0].name for call in send_reset_link.call_args_list ] - assert emailed_users == [user_a["name"], user_b["name"]] + assert sorted(emailed_users) == sorted([user_a["name"], user_b["name"]]) def test_request_reset_without_param(self, app): diff --git a/ckan/tests/lib/test_uploader.py b/ckan/tests/lib/test_uploader.py new file mode 100644 index 00000000000..b4117202939 --- /dev/null +++ b/ckan/tests/lib/test_uploader.py @@ -0,0 +1,90 @@ +# encoding: utf-8 +import mock +import tempfile +import __builtin__ as builtins +import flask +import paste.fileapp +import cStringIO + +from werkzeug.datastructures import FileStorage +from pyfakefs import fake_filesystem +from nose.tools import assert_equal as eq_ + +from ckan.common import config +import ckan.lib.uploader +from ckan.lib.uploader import ResourceUpload +from ckanext.example_iuploader.test.test_plugin import mock_open_if_open_fails + + +fs = fake_filesystem.FakeFilesystem() +fake_os = fake_filesystem.FakeOsModule(fs) + + +class TestInitResourceUpload(object): + @mock.patch.object(ckan.lib.uploader, u'os', fake_os) + @mock.patch.object(builtins, u'open', side_effect=mock_open_if_open_fails) + @mock.patch.object(flask, u'send_file', side_effect=[b'DATA']) + @mock.patch.object(paste.fileapp, u'os', fake_os) + @mock.patch.object(config[u'pylons.h'], u'uploads_enabled', + return_value=True) + @mock.patch.object(ckan.lib.uploader, u'_storage_path', new=u'/doesnt_exist') + def test_resource_without_upload_with_old_werkzeug( + self, mock_uploads_enabled, mock_open, send_file): + # this test data is based on real observation using a browser + # and werkzeug 0.14.1 + res = {u'clear_upload': u'true', + u'format': u'CSV', + u'url': u'https://example.com/data.csv', + u'description': u'', + u'upload': u'', + u'package_id': u'dataset1', + u'id': u'8a3a874e-5ee1-4e43-bdaf-e2569cf72344', + u'name': u'data.csv'} + res_upload = ResourceUpload(res) + + eq_(res_upload.filename, None) + + @mock.patch.object(ckan.lib.uploader, u'os', fake_os) + @mock.patch.object(builtins, u'open', side_effect=mock_open_if_open_fails) + @mock.patch.object(flask, u'send_file', side_effect=[b'DATA']) + @mock.patch.object(paste.fileapp, u'os', fake_os) + @mock.patch.object(config[u'pylons.h'], u'uploads_enabled', + return_value=True) + @mock.patch.object(ckan.lib.uploader, u'_storage_path', new=u'/doesnt_exist') + def test_resource_without_upload( + self, mock_uploads_enabled, mock_open, send_file): + # this test data is based on real observation using a browser + res = {u'clear_upload': u'true', + u'format': u'PNG', + u'url': u'https://example.com/data.csv', + u'description': u'', + u'upload': FileStorage(filename=u''), + u'package_id': u'dataset1', + u'id': u'8a3a874e-5ee1-4e43-bdaf-e2569cf72344', + u'name': u'data.csv'} + res_upload = ResourceUpload(res) + + eq_(res_upload.filename, None) + + @mock.patch.object(ckan.lib.uploader, u'os', fake_os) + @mock.patch.object(builtins, u'open', side_effect=mock_open_if_open_fails) + @mock.patch.object(flask, u'send_file', side_effect=[b'DATA']) + @mock.patch.object(paste.fileapp, u'os', fake_os) + @mock.patch.object(config[u'pylons.h'], u'uploads_enabled', + return_value=True) + @mock.patch.object(ckan.lib.uploader, u'_storage_path', new=u'/doesnt_exist') + def test_resource_with_upload( + self, mock_uploads_enabled, mock_open, send_file): + # this test data is based on real observation using a browser + res = {u'clear_upload': u'', + u'format': u'PNG', + u'url': u'https://example.com/data.csv', + u'description': u'', + u'upload': FileStorage(filename=u'data.csv', content_type=u'CSV'), + u'package_id': u'dataset1', + u'id': u'8a3a874e-5ee1-4e43-bdaf-e2569cf72344', + u'name': u'data.csv'} + res_upload = ResourceUpload(res) + + eq_(res_upload.filesize, 0) + eq_(res_upload.filename, u'data.csv') diff --git a/ckan/tests/logic/action/test_create.py b/ckan/tests/logic/action/test_create.py index 8bf5dc3e1d0..db433b34f74 100644 --- a/ckan/tests/logic/action/test_create.py +++ b/ckan/tests/logic/action/test_create.py @@ -6,6 +6,8 @@ import mock import pytest +from werkzeug.datastructures import FileStorage as FlaskFileStorage + import ckan import ckan.logic as logic import ckan.model as model @@ -29,9 +31,11 @@ fake_open = fake_filesystem.FakeFileOpen(fs) -class FakeFileStorage(cgi.FieldStorage): - def __init__(self, fp, filename): - self.file = fp +class FakeFileStorage(FlaskFileStorage): + content_type = None + + def __init__(self, stream, filename): + self.stream = stream self.filename = filename self.name = "upload" @@ -539,7 +543,7 @@ def test_mimetype_by_upload_by_file(self, monkeypatch): NAZKO,1C08,1070,2016/01/05,20,31,,76,16,JAN-01,41 """ ) - test_resource = FakeFileStorage(test_file, "") + test_resource = FakeFileStorage(test_file, "test.csv") context = {} params = { diff --git a/ckan/tests/logic/action/test_update.py b/ckan/tests/logic/action/test_update.py index 8eb082a5d75..58070ad369a 100644 --- a/ckan/tests/logic/action/test_update.py +++ b/ckan/tests/logic/action/test_update.py @@ -5,6 +5,8 @@ import mock import pytest +from werkzeug.datastructures import FileStorage as FlaskFileStorage + import ckan import ckan.lib.app_globals as app_globals import ckan.logic as logic @@ -28,6 +30,15 @@ fake_open = fake_filesystem.FakeFileOpen(fs) +class FakeFileStorage(FlaskFileStorage): + content_type = None + + def __init__(self, stream, filename): + self.stream = stream + self.filename = filename + self.name = "upload" + + def mock_open_if_open_fails(*args, **kwargs): try: return real_open(*args, **kwargs) @@ -814,18 +825,6 @@ def test_calling_with_only_id_doesnt_update_anything(self): @pytest.mark.ckan_config("ckan.plugins", "image_view recline_view") @pytest.mark.usefixtures("clean_db", "with_plugins") class TestResourceUpdate(object): - import cgi - - class FakeFileStorage(cgi.FieldStorage): - def __init__(self, fp, filename): - self.file = fp - self.filename = filename - self.name = "upload" - - def setup(self): - import ckan.model as model - - model.repo.rebuild_db() def test_url_only(self): dataset = factories.Dataset() @@ -1076,7 +1075,7 @@ def test_mimetype_by_upload_by_file(self, monkeypatch): NAZKO,1C08,1070,2016/01/05,20,31,,76,16,JAN-01,41 """ ) - update_resource = TestResourceUpdate.FakeFileStorage( + update_resource = FakeFileStorage( update_file, "update_test" ) monkeypatch.setattr(builtins, 'open', mock_open_if_open_fails) @@ -1125,7 +1124,7 @@ def test_mimetype_by_upload_by_filename(self, monkeypatch): } """ ) - test_resource = TestResourceUpdate.FakeFileStorage( + test_resource = FakeFileStorage( test_file, "test.json" ) dataset = factories.Dataset() @@ -1149,7 +1148,7 @@ def test_mimetype_by_upload_by_filename(self, monkeypatch): NAZKO,1C08,1070,2016/01/05,20,31,,76,16,JAN-01,41 """ ) - update_resource = TestResourceUpdate.FakeFileStorage( + update_resource = FakeFileStorage( update_file, "update_test.csv" ) with mock.patch("ckan.lib.helpers.url_for"): @@ -1219,7 +1218,7 @@ def test_size_of_resource_by_upload(self, monkeypatch): } """ ) - test_resource = TestResourceUpdate.FakeFileStorage( + test_resource = FakeFileStorage( test_file, "test.json" ) dataset = factories.Dataset() @@ -1243,7 +1242,7 @@ def test_size_of_resource_by_upload(self, monkeypatch): NAZKO,1C08,1070,2016/01/05,20,31,,76,16,JAN-01,41 """ ) - update_resource = TestResourceUpdate.FakeFileStorage( + update_resource = FakeFileStorage( update_file, "update_test.csv" ) with mock.patch("ckan.lib.helpers.url_for"): diff --git a/ckan/views/resource.py b/ckan/views/resource.py index 293c30f8a8c..274a648d8b6 100644 --- a/ckan/views/resource.py +++ b/ckan/views/resource.py @@ -243,6 +243,10 @@ def post(self, package_type, id): except ValidationError as e: errors = e.error_dict error_summary = e.error_summary + if data.get(u'url_type') == u'upload' and data.get(u'url'): + data[u'url'] = u'' + data[u'url_type'] = u'' + data[u'previous_upload'] = True return self.get(package_type, id, data, errors, error_summary) except NotAuthorized: return base.abort(403, _(u'Unauthorized to create a resource'))