diff --git a/ckanext/odsh/plugin.py b/ckanext/odsh/plugin.py index 83323b95f9315f4e05a1db1d9ba15283b16099b7..a6382b31ea41a8996e4f739fd212905023a2f4d6 100644 --- a/ckanext/odsh/plugin.py +++ b/ckanext/odsh/plugin.py @@ -392,6 +392,9 @@ class OdshPlugin(plugins.SingletonPlugin, DefaultTranslation, DefaultDatasetForm }) schema.update({'__extras': [toolkit.get_converter('odsh_validate_extras')] }) + ## only to make sure the spatial field is there for validation + # schema.update({'spatial': [toolkit.get_converter('convert_from_extras')]}) + def create_package_schema(self): schema = super(OdshPlugin, self).create_package_schema() self._update_schema(schema) diff --git a/ckanext/odsh/templates/package/snippets/package_basic_fields.html b/ckanext/odsh/templates/package/snippets/package_basic_fields.html index d65f806f1a93616875aa41f811035b93dc98a94f..042bb05fe99ab012033c7d07448baa617f453b3b 100644 --- a/ckanext/odsh/templates/package/snippets/package_basic_fields.html +++ b/ckanext/odsh/templates/package/snippets/package_basic_fields.html @@ -191,6 +191,7 @@ is_required=true,placeholder=_('Enter title')) }} <!-- field spatial_uri --> + {{errors}} {% set field = 'spatial_uri' %} {% set value = h.odsh_extract_value_from_extras(data.extras,field) %} {% set error = h.odsh_extract_error(field, errors) %} @@ -210,13 +211,14 @@ is_required=true,placeholder=_('Enter title')) }} </div> </div> + {# {% set spatial_extends_available = h.odsh_spatial_extends_available() %} <div class="control-group"> <label class="control-label" for="field-{{field}}">{{_('Spatial uri')}}: <span title="Dieses Feld ist erforderlich" class="control-required">*</span> </label> <div class="controls"> <div class="row-fluid"> <div class="span6"> - <select id="field-{{field}}" data-module="autocomplete"> + <select id="field-{{field}}" data-module="autocomplete" data-module-items="10"> {% for extend in spatial_extends_available%} <option value="{{ extend }}" {% if selected_extend %} selected="selected" {% endif %}>{{extend}}</option> {% endfor %} @@ -225,6 +227,7 @@ is_required=true,placeholder=_('Enter title')) }} </div> </div> </div> + #} <!-- field private --> <div class="control-group"> diff --git a/ckanext/odsh/tests/test_upload.py b/ckanext/odsh/tests/test_upload.py index 115fb3eb51f8e0bb5d0002747da26c50d7d9a47b..c7c8c2d3214223e9a322323d43664fc734659056 100644 --- a/ckanext/odsh/tests/test_upload.py +++ b/ckanext/odsh/tests/test_upload.py @@ -9,6 +9,7 @@ from routes import url_for from nose.tools import assert_true, assert_false, assert_equal, assert_in from ckanext.odsh.helpers import odsh_create_checksum webtest_submit = helpers.webtest_submit +import pdb class TestUpload(helpers.FunctionalTestBase): @@ -43,6 +44,60 @@ class TestUpload(helpers.FunctionalTestBase): # assert response.mustcontain('spatial_uri: uri unknown') + @odsh_test() + def test_upload_empty_spatial_uri(self): + # arrange + form = self._get_package_new_form() + + # act + form[self._get_field_name('spatial_uri')] = '' + response = self._submit_form(form) + + # assert + assert 'spatial_uri: empty not allowed' in response + + @odsh_test() + def test_edit_empty_spatial_uri(self): + # arrange + dataset = self._create_dataset() + + form = self._get_package_update_form(dataset['id']) + + # act + form[self._get_field_name('spatial_uri')] = '' + response = self._submit_form(form) + + # assert + assert 'spatial_uri: empty not allowed' in response + + @odsh_test() + def test_edit_empty_spatial_uri_but_spatial(self): + # arrange + extras = [ + {'key': 'temporal_start', 'value': '2000-01-27'}, + {'key': 'temporal_end', 'value': '2000-01-27'}, + {'key': 'issued', 'value': '2000-01-27'}, + {'key': 'groups', 'value': 'soci'}, + {'key': 'licenseAttributionByText', 'value': 'text'}, + {'key': 'spatial_uri', 'value': ''}, + {'key': 'spatial', 'value': '{"type": "Point", "coordinates": [9.511769, 53.928028]}'}, + ] + dataset = self._create_dataset(extras=extras) + # pdb.set_trace() + + form = self._get_package_update_form(dataset['id']) + + # # act + # form[self._get_field_name('spatial_uri')] = '' + # response = self._submit_form(form) + + # # assert + # assert 'spatial_uri: empty not allowed' not in response + response = self._submit_and_follow_form(form) + + # assert + response.mustcontain('Manage Dataset') + @odsh_test() def test_upload_empty_wrong_date_temporal_start(self): # arrange @@ -84,6 +139,7 @@ class TestUpload(helpers.FunctionalTestBase): 'spatial_uri')] = 'http://dcat-ap.de/def/politicalGeocoding/districtKey/01001' form[self._get_field_name('issued')] = '2019-01-29' form[self._get_field_name('temporal_start')] = '2019-01-29' + form[self._get_field_name('groups')] = 'soci' form[self._get_field_name('temporal_end')] = '2019-02-02' form['license_id'] = 'http://dcat-ap.de/def/licenses/dl-by-de/2.0' form[self._get_field_name('licenseAttributionByText')].value = 'text' @@ -136,3 +192,36 @@ class TestUpload(helpers.FunctionalTestBase): extra_environ=self.env, ) return response.forms['dataset-edit'] + + def _get_package_update_form(self, id): + app = self._get_test_app() + # user = factories.User() + response = app.get( + url=url_for(controller='package', action='edit', id=id), + extra_environ=self.env, + ) + return response.forms['dataset-edit'] + + def _create_dataset(self, name='my-own-dataset', temporal_start='2000-01-27', temporal_end='2000-01-27',title='title', extras=None): + user = factories.User() + self.org = factories.Organization( + name="my-org", + users=[{'name': user['id'], 'capacity': 'admin'}] + ) + self.env = {'REMOTE_USER': user['name'].encode('ascii')} + if not extras: + extras = [ + {'key': 'temporal_start', 'value': temporal_start}, + {'key': 'temporal_end', 'value': temporal_end}, + {'key': 'issued', 'value': '2000-01-27'}, + {'key': 'spatial_uri', 'value': 'http://dcat-ap.de/def/politicalGeocoding/districtKey/01001'}, + {'key': 'groups', 'value': 'soci'}, + {'key': 'licenseAttributionByText', 'value': 'text'} + ] + return factories.Dataset(user=user, + name=name, + title=title, + issued='27-01-2000', + extras=extras, + license_id='http://dcat-ap.de/def/licenses/dl-by-de/2.0') + diff --git a/ckanext/odsh/tests/test_validation.py b/ckanext/odsh/tests/test_validation.py index 0309f913fc204494ec3f8789f59088e605d97cdd..d631545ab34a2a0cd10094ab55ff22f7904970f6 100644 --- a/ckanext/odsh/tests/test_validation.py +++ b/ckanext/odsh/tests/test_validation.py @@ -1,3 +1,7 @@ +from ckanext.odsh.validation import * +import ckan.plugins.toolkit as toolkit +import pylons +import ckan.model as modelMock import sys import json from nose.tools import * @@ -7,13 +11,19 @@ from mock import MagicMock, Mock, patch def mockInvalid(*args, **kwargs): return Exception(*args, **kwargs) + def mock_(s): return s + m = MagicMock() + + class MissingMock: pass -m.Missing=MissingMock + + +m.Missing = MissingMock sys.modules['ckan'] = MagicMock() sys.modules['ckan.plugins'] = MagicMock() @@ -24,21 +34,16 @@ sys.modules['ckan.lib.navl'] = MagicMock() sys.modules['ckan.lib.navl.dictization_functions'] = m sys.modules['pylons'] = MagicMock() -import ckan.model as modelMock -import pylons -import ckan.plugins.toolkit as toolkit toolkit.Invalid = mockInvalid toolkit._ = mock_ -from ckanext.odsh.validation import * - - def test_get_validators(): assert get_validators() +# @patch('toolkit.get_validator', side_effect=lambda a: None) def test_tag_string_convert(): # arrange data = {'tag_string': 'tag1,tag2'} @@ -74,6 +79,28 @@ def test_known_spatial_uri(url_mock, get_mock, csv_mock): assert data[('extras', 2, 'value')] == '0' +@raises(Exception) +@patch('urllib2.urlopen') +@patch('pylons.config.get', side_effect='foo') +@patch('csv.reader', side_effect=[[['uri', 'text', json.dumps({"geometry": 0})]]]) +def test_known_spatial_uri_without_uri(url_mock, get_mock, csv_mock): + # arrange + data = {('extras', 0, 'key'): 'spatial_uri', + ('extras', 0, 'value'): ''} + # act + known_spatial_uri('spatial_uri', data, {}, None) + + +def test_known_spatial_uri_without_uri_with_spatial(): + # arrange + data = {('extras', 0, 'key'): 'spatial', + ('extras', 0, 'value'): 'value', + ('extras', 1, 'key'): 'spatial_uri', + ('extras', 1, 'value'): ''} + # act + known_spatial_uri('spatial_uri', data, {}, None) + + def test_validate_licenseAttributionByText(): # arrange def get_licenses(): diff --git a/ckanext/odsh/validation.py b/ckanext/odsh/validation.py index 90efa8f2e412a31e617345280c9ce7001584417a..099e92ee71337c66568a2e0656030ada5cf764f9 100644 --- a/ckanext/odsh/validation.py +++ b/ckanext/odsh/validation.py @@ -1,4 +1,5 @@ # This Python file uses the following encoding: utf-8 +import logging import csv import re import urllib2 @@ -12,11 +13,13 @@ from ckan.lib.navl.dictization_functions import Missing from pylons import config +import pdb + _ = toolkit._ -import logging log = logging.getLogger(__name__) + def _extract_value(data, field): key = None for k in data.keys(): @@ -27,48 +30,51 @@ def _extract_value(data, field): return None return data[(key[0], key[1], 'value')] + def validate_extra_groups(data, requireAtLeastOne, errors): value = _extract_value(data, 'groups') if value != None: # 'value != None' means the extra key 'groups' was found, - # so the dataset came from manual editing via the web-frontend. + # so the dataset came from manual editing via the web-frontend. if not value: if requireAtLeastOne: - errors['groups']= 'at least one group needed' - data[('groups', 0, 'id')] = '' - return + errors['groups'] = 'at least one group needed' + data[('groups', 0, 'id')] = '' + return groups = [g.strip() for g in value.split(',') if value.strip()] for k in data.keys(): if len(k) == 3 and k[0] == 'groups': - data[k]='' + data[k] = '' # del data[k] - if len(groups)==0: + if len(groups) == 0: if requireAtLeastOne: - errors['groups']= 'at least one group needed' - return + errors['groups'] = 'at least one group needed' + return for num, group in zip(range(len(groups)), groups): data[('groups', num, 'id')] = group - else: # no extra-field 'groups' + else: # no extra-field 'groups' # dataset might come from a harvest process if not data.get(('groups', 0, 'id'), False) and \ not data.get(('groups', 0, 'name'), False): - errors['groups']= 'at least one group needed' + errors['groups'] = 'at least one group needed' + def validate_extras(key, data, errors, context): extra_errors = {} isStaNord = ('id',) in data and data[('id',)][:7] == 'StaNord' - validate_extra_groups(data, True, extra_errors) validate_extra_date_new(key, 'issued', data, isStaNord, extra_errors) - validate_extra_date_new(key, 'temporal_start', data, isStaNord, extra_errors) + validate_extra_date_new(key, 'temporal_start', + data, isStaNord, extra_errors) validate_extra_date_new(key, 'temporal_end', data, True, extra_errors) if len(extra_errors.values()): raise toolkit.Invalid(extra_errors) + def _set_value(data, field, value): key = None for k in data.keys(): @@ -79,6 +85,7 @@ def _set_value(data, field, value): return None data[(key[0], key[1], 'value')] = value + def validate_extra_date_new(key, field, data, optional, errors): value = _extract_value(data, field) @@ -89,22 +96,23 @@ def validate_extra_date_new(key, field, data, optional, errors): else: if re.match(r'\d\d\d\d-\d\d-\d\d', value): try: - dt=parse(value) + dt = parse(value) _set_value(data, field, dt.isoformat()) return except ValueError: pass errors[field] = 'not a valid date' -def validate_licenseAttributionByText(key, data, errors,context): + +def validate_licenseAttributionByText(key, data, errors, context): register = model.Package.get_license_register() - isByLicense=False + isByLicense = False for k in data: if len(k) > 0 and k[0] == 'license_id' and data[k] and not isinstance(data[k], Missing) and \ - 'Namensnennung' in register[data[k]].title: + 'Namensnennung' in register[data[k]].title: isByLicense = True break - hasAttribution=False + hasAttribution = False for k in data: if data[k] == 'licenseAttributionByText': if isinstance(data[(k[0], k[1], 'value')], Missing) or (k[0], k[1], 'value') not in data: @@ -136,13 +144,25 @@ def known_spatial_uri(key, data, errors, context): value = _extract_value(data, 'spatial_uri') if not value: + poly = None + # some harvesters might import a polygon directly... + # pdb.set_trace() poly = _extract_value(data, 'spatial') - if not poly: + + has_old_uri = False + pkg = context.get('package', None) + if pkg: + old_uri = pkg.extras.get('spatial_uri', None) + has_old_uri = old_uri != None and len(old_uri) > 0 + if not poly: + poly = pkg.extras.get('spatial', None) + if not poly or has_old_uri: + # pdb.set_trace() raise toolkit.Invalid('spatial_uri: empty not allowed') else: - return - + return + mapping_file = config.get('ckanext.odsh.spatial.mapping') try: mapping_file = urllib2.urlopen(mapping_file) @@ -164,17 +184,21 @@ def known_spatial_uri(key, data, errors, context): raise toolkit.Invalid( 'spatial_uri: uri unknown') - # Get the current extras index - current_indexes = [k[1] for k in data.keys() - if len(k) > 1 and k[0] == 'extras'] - - new_index = max(current_indexes) + 1 if current_indexes else 0 + new_index = next_extra_index(data) data[('extras', new_index, 'key')] = 'spatial_text' data[('extras', new_index, 'value')] = spatial_text data[('extras', new_index+1, 'key')] = 'spatial' data[('extras', new_index+1, 'value')] = spatial + +def next_extra_index(data): + current_indexes = [k[1] for k in data.keys() + if len(k) > 1 and k[0] == 'extras'] + + return max(current_indexes) + 1 if current_indexes else 0 + + def tag_name_validator(value, context): tagname_match = re.compile('[\w \-.\:\(\)\ยด\`]*$', re.UNICODE) if not tagname_match.match(value): @@ -182,6 +206,7 @@ def tag_name_validator(value, context): 'characters or symbols: -_.:()') % (value)) return value + def tag_string_convert(key, data, errors, context): '''Takes a list of tags that is a comma-separated string (in data[key]) and parses tag names. These are added to the data dict, enumerated. They @@ -193,7 +218,6 @@ def tag_string_convert(key, data, errors, context): else: tags = data[key] - current_index = max([int(k[1]) for k in data.keys() if len(k) == 3 and k[0] == 'tags'] + [-1]) @@ -207,8 +231,8 @@ def tag_string_convert(key, data, errors, context): def get_validators(): return { - 'known_spatial_uri': known_spatial_uri, - 'odsh_tag_name_validator': tag_name_validator, - 'odsh_validate_extras':validate_extras, - 'validate_licenseAttributionByText':validate_licenseAttributionByText - } + 'known_spatial_uri': known_spatial_uri, + 'odsh_tag_name_validator': tag_name_validator, + 'odsh_validate_extras': validate_extras, + 'validate_licenseAttributionByText': validate_licenseAttributionByText + }