Skip to content

Commit d99e6c9

Browse files
authored
Merge pull request #153 from open-data/feature/lowercase-sheetnames
Lowercase Uploaded Worksheet Names
2 parents 25044c1 + fe1d770 commit d99e6c9

File tree

7 files changed

+233
-9
lines changed

7 files changed

+233
-9
lines changed

.github/workflows/pytest.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,20 @@ jobs:
188188
pip install -r test-requirements.txt
189189
python3 setup.py develop
190190
cd /srv/app/src/ckan
191+
pip install -e .
191192
python3 setup.py develop
192193
pip install setuptools==70.0.0
193194
cd /srv/app
194195
- name: Setup Databases
195196
run: |
196197
source /srv/app/venv/bin/activate
197198
199+
cd /srv/app/src/ckan
200+
pip install -e .
201+
python3 setup.py develop
202+
pip install setuptools==70.0.0
203+
cd /srv/app
204+
198205
ln -s /srv/app/src/ckan/test-core.ini /srv/app/src/ckanext-recombinant/links/test-core.ini
199206
ln -s /srv/app/src/ckan/who.ini /srv/app/src/ckanext-recombinant/links/who.ini
200207
mkdir -p /srv/app/src/ckanext-recombinant/links/ckanext/datastore/tests

changes/153.changes

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Worksheet names will now always be processed in lower case when uploading Excel files.

ckanext/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# this is a namespace package
22
try:
3-
import pkg_resources
3+
# type_ignore_reason: try/catch
4+
import pkg_resources # type: ignore
45
# type_ignore_reason: reportAttributeAccessIssue
56
pkg_resources.declare_namespace(__name__)
67
except ImportError:

ckanext/recombinant/read_excel.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,21 @@ def read_excel(f: Union[str, FlaskFileStorage, FieldStorage],
3535
"""
3636
# type_ignore_reason: incomplete typing
3737
wb = load_workbook(f, read_only=True) # type: ignore
38-
3938
for sheetname in wb.sheetnames:
40-
if sheetname == 'reference':
39+
40+
# NOTE: we lowercase uploaded worksheet names in the scenario
41+
# that users or Excel rename the worksheet
42+
_sheetname = sheetname.lower()
43+
44+
if _sheetname == 'reference':
4145
return
42-
if sheetname in bad_sheet_names:
46+
if _sheetname in bad_sheet_names:
4347
raise BadExcelData(_('Invalid file for this data type. ' +
4448
'Sheet must be labeled "{0}", ' +
4549
'but you supplied a sheet labeled "{1}"').format(
4650
'"/"'.join(sorted(expected_sheet_names)),
47-
sheetname))
48-
if sheetname not in expected_sheet_names:
51+
_sheetname))
52+
if _sheetname not in expected_sheet_names:
4953
# NOTE: some Excel extensions and Macros create fully hidden
5054
# worksheets that act as a sort of database/index cache
5155
# for other sheets or external services such as Geo Services.
@@ -65,7 +69,7 @@ def read_excel(f: Union[str, FlaskFileStorage, FieldStorage],
6569
if org_name and names_row[0].value != 'v3':
6670
# v2 template
6771
yield (
68-
sheetname,
72+
_sheetname,
6973
org_name,
7074
[c.value for c in names_row],
7175
# type_ignore_reason: incomplete typing
@@ -79,7 +83,7 @@ def read_excel(f: Union[str, FlaskFileStorage, FieldStorage],
7983
raise BadExcelData('Example record on row 5 is missing')
8084

8185
yield (
82-
sheetname,
86+
_sheetname,
8387
names_row[1].value,
8488
[c.value for c in names_row[2:]],
8589
_filter_bumf((row[2:] for row in rowiter), HEADER_ROWS_V3))

ckanext/recombinant/tests/__init__.py

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from ckanapi import LocalCKAN
2+
13
from ckan.tests.helpers import reset_db
24
from ckan.lib.search import clear_all
35

@@ -10,3 +12,118 @@ def setup_method(self, method):
1012
"""
1113
reset_db()
1214
clear_all()
15+
16+
lc = LocalCKAN()
17+
18+
lc.action.datastore_function_create(
19+
name='required_error',
20+
or_replace=True,
21+
arguments=[
22+
{'argname': 'value', 'argtype': 'text'},
23+
{'argname': 'field_name', 'argtype': 'text'}],
24+
rettype='_text',
25+
definition='''
26+
BEGIN
27+
IF (value = '') IS NOT FALSE THEN
28+
RETURN ARRAY[[field_name,
29+
'This field must not be empty']];
30+
END IF;
31+
RETURN NULL;
32+
END;
33+
''')
34+
lc.action.datastore_function_create(
35+
name='required_error',
36+
or_replace=True,
37+
arguments=[
38+
{'argname': 'value', 'argtype': '_text'},
39+
{'argname': 'field_name', 'argtype': 'text'}],
40+
rettype='_text',
41+
definition='''
42+
BEGIN
43+
IF value IS NULL OR value = '{}' THEN
44+
return ARRAY[[field_name,
45+
'This field must not be empty']];
46+
END IF;
47+
RETURN NULL;
48+
END;
49+
''')
50+
lc.action.datastore_function_create(
51+
name='required_error',
52+
or_replace=True,
53+
arguments=[
54+
{'argname': 'value', 'argtype': 'date'},
55+
{'argname': 'field_name', 'argtype': 'text'}],
56+
rettype='_text',
57+
definition='''
58+
BEGIN
59+
IF value IS NULL THEN
60+
RETURN ARRAY[[field_name,
61+
'This field must not be empty']];
62+
END IF;
63+
RETURN NULL;
64+
END;
65+
''')
66+
lc.action.datastore_function_create(
67+
name='required_error',
68+
or_replace=True,
69+
arguments=[
70+
{'argname': 'value', 'argtype': 'numeric'},
71+
{'argname': 'field_name', 'argtype': 'text'}],
72+
rettype='_text',
73+
definition='''
74+
BEGIN
75+
IF value IS NULL THEN
76+
RETURN ARRAY[[field_name,
77+
'This field must not be empty']];
78+
END IF;
79+
RETURN NULL;
80+
END;
81+
''')
82+
lc.action.datastore_function_create(
83+
name='required_error',
84+
or_replace=True,
85+
arguments=[
86+
{'argname': 'value', 'argtype': 'int4'},
87+
{'argname': 'field_name', 'argtype': 'text'}],
88+
rettype='_text',
89+
definition='''
90+
BEGIN
91+
IF value IS NULL THEN
92+
RETURN ARRAY[[field_name,
93+
'This field must not be empty']];
94+
END IF;
95+
RETURN NULL;
96+
END;
97+
''')
98+
lc.action.datastore_function_create(
99+
name='required_error',
100+
or_replace=True,
101+
arguments=[
102+
{'argname': 'value', 'argtype': 'money'},
103+
{'argname': 'field_name', 'argtype': 'text'}],
104+
rettype='_text',
105+
definition='''
106+
BEGIN
107+
IF value IS NULL THEN
108+
RETURN ARRAY[[field_name,
109+
'This field must not be empty']];
110+
END IF;
111+
RETURN NULL;
112+
END;
113+
''')
114+
lc.action.datastore_function_create(
115+
name='required_error',
116+
or_replace=True,
117+
arguments=[
118+
{'argname': 'value', 'argtype': 'boolean'},
119+
{'argname': 'field_name', 'argtype': 'text'}],
120+
rettype='_text',
121+
definition='''
122+
BEGIN
123+
IF value IS NULL THEN
124+
RETURN ARRAY[[field_name,
125+
'This field must not be empty']];
126+
END IF;
127+
RETURN NULL;
128+
END;
129+
''')
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import pytest
2+
import flask
3+
import mock
4+
from io import BytesIO
5+
from ckanapi import LocalCKAN
6+
7+
from ckan.tests.factories import Organization, Sysadmin
8+
from ckanext.recombinant.tests import RecombinantTestBase
9+
10+
from ckan import model
11+
from ckan.plugins.toolkit import config
12+
from ckanext.recombinant.tables import _get_plugin, get_chromo
13+
from ckanext.recombinant.logic import _action_get_dataset
14+
from ckanext.recombinant.write_excel import (
15+
excel_template,
16+
append_data
17+
)
18+
from ckanext.recombinant.views import _process_upload_file
19+
20+
21+
@pytest.mark.usefixtures('with_request_context')
22+
class TestRecombinantExcel(RecombinantTestBase):
23+
@classmethod
24+
def setup_method(self, method):
25+
"""Method is called at class level before EACH test methods of the class are called.
26+
Setup any state specific to the execution of the given class methods.
27+
"""
28+
super(TestRecombinantExcel, self).setup_method(method)
29+
30+
self.sysadmin = Sysadmin()
31+
self.org = Organization()
32+
self.lc = LocalCKAN()
33+
34+
def test_excel_template(self, app):
35+
"""
36+
Should be able to write and read and Excel template based
37+
on the Schema and DataStore records.
38+
"""
39+
_get_plugin().update_config(config)
40+
41+
# setup sample
42+
self.lc.action.recombinant_create(dataset_type='sample',
43+
owner_org=self.org['name'])
44+
_lc, _geno, dataset = _action_get_dataset({'ignore_auth': True,
45+
'user': self.sysadmin['name']},
46+
{'dataset_type': 'sample',
47+
'owner_org': self.org['name']})
48+
org = self.lc.action.organization_show(
49+
id=self.org['id'],
50+
include_datasets=False)
51+
52+
records = [
53+
{'reference_number': 'sheet_test_1', 'year': 2026},
54+
{'reference_number': 'sheet_test_2', 'year': 2025},
55+
{'reference_number': 'sheet_test_3', 'year': 2024},
56+
]
57+
58+
# setup sample ds data
59+
self.lc.action.datastore_upsert(
60+
resource_id=dataset['resources'][0]['id'],
61+
force=True,
62+
method='insert',
63+
records=records)
64+
65+
# reference_number is primary key in sample, can update year
66+
for r in records:
67+
r['year'] = 2001
68+
records.append({'reference_number': 'sheet_test_new', 'year': 2026})
69+
70+
# write excel file, should not raise any exceptions
71+
chromo = get_chromo(dataset['resources'][0]['name'])
72+
book = excel_template(dataset['type'], org)
73+
append_data(book, records, chromo)
74+
blob = BytesIO()
75+
book.save(blob)
76+
77+
# read excel file, should not raise any exceptions
78+
current_user = model.User.get(self.sysadmin['name'])
79+
with mock.patch('ckan.lib.helpers.current_user', current_user):
80+
flask.g.user = self.sysadmin['name']
81+
_process_upload_file(self.lc, dataset, blob, {}, dry_run=False)
82+
83+
result = self.lc.action.datastore_search(
84+
resource_id=dataset['resources'][0]['id'])
85+
86+
expected_records = [
87+
{'_id': 1, 'reference_number': 'sheet_test_1', 'year': 2001},
88+
{'_id': 2, 'reference_number': 'sheet_test_2', 'year': 2001},
89+
{'_id': 3, 'reference_number': 'sheet_test_3', 'year': 2001},
90+
{'_id': 4, 'reference_number': 'sheet_test_new', 'year': 2026},
91+
]
92+
93+
assert result['total'] == 4
94+
assert result['records'] == expected_records

ckanext/recombinant/write_excel.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1026,7 +1026,7 @@ def org_title_lang_hack(title: str):
10261026
"""
10271027
try:
10281028
lang = h.lang()
1029-
except TypeError:
1029+
except (TypeError, RuntimeError): # RuntimeError for outside flask request context
10301030
lang = 'en'
10311031
if lang == 'fr':
10321032
return title.split(' | ')[-1]

0 commit comments

Comments
 (0)