Skip to content

Commit 06d6caa

Browse files
committed
Merge branch 'update-dependencies-jt'
This will be release 1.4. Addresses issues #4, #16, #27, #29, #30, and #40.
2 parents 03e5b09 + 755dfa9 commit 06d6caa

File tree

99 files changed

+802670
-558823
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

99 files changed

+802670
-558823
lines changed

README.md

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,51 +1046,61 @@ file is in `.gitignore` for that reason. There is a
10461046
Early in development we implemented a series of tests using the built-in Django
10471047
test runner to do some simple sanity-checking to make sure the Django ORM
10481048
models for Sierra match the structures actually in the production database.
1049-
You can still run these tests after you first install the catalog-api to ensure
1050-
the models match your Sierra setup. Systems may differ from institution to
1049+
We have since converted these to run via pytest: see
1050+
`django/sierra/base/tests/test_database.py`.
1051+
1052+
When you run the full test suite, as [described below](#unit-tests), these run
1053+
against the test Sierra database—which is useful. But, there are times
1054+
that you'll want to run these tests against your live database to make sure the
1055+
models are accurate. For instance, systems may differ from institution to
10511056
institution based on what products you have, so you may end up needing to fork
10521057
this project and update the models so they work with your own setup. It may
10531058
also be worth running these tests after Sierra upgrades so that you can make
10541059
sure there were no changes made to the database that break the models.
10551060

1056-
If using Docker, run:
1061+
If using Docker, run _only_ the database tests using the following:
1062+
1063+
./docker-compose.sh run --rm live-db-test
10571064

1058-
./docker-compose.sh run manage-dev test base
1065+
If not using Docker, you can use the below command, instead. If applicable,
1066+
replace the value of the `--ds` option with whatever your DEV settings file is.
10591067

1060-
If not using Docker, you can run:
1068+
cd <project_root>
1069+
pytest --ds=sierra.settings.dev django/sierra/base/tests/test_database.py
10611070

1062-
cd <project_root>/django/sierra
1063-
manage.py test base
1071+
Note: Some of these tests may fail simply because the models are generally more
1072+
restrictive than the live Sierra database. We are forcing ForeignKey-type
1073+
relationships on a lot of fields that don't seem to have actual
1074+
database-enforced keys in Sierra. E.g., from what I can gather, `id` fields are
1075+
usually proper keys, while `code` fields may not be&mdash;but `code` fields are
1076+
frequently used in a foreign-key-like capacity. I think this leads to a lot of
1077+
the invalid codes you have in Sierra, where you have a code in a record that
1078+
_should_ point to some entry in an administrative table (like a location), but
1079+
it doesn't because the administrative table entry was deleted and the record
1080+
was never updated. And there are other cases, as well. E.g., a code might use
1081+
the string `none` instead of a null value, but there is no corresponding entry
1082+
for `none` in the related table. Bib locations use the string `multi` to
1083+
indicate that they have multiple locations, but there is no corresponding
1084+
`multi` record in the location table. Etc.
10641085

1065-
Note: If any `*_maps_to_database` tests fail, it indicates that there are
1066-
fields on the model that aren't present in the database. These are more
1067-
serious (but often easier to fix) than `*_sanity_check` tests, which test
1068-
to ensure that relationship fields work properly. In either case, you can
1069-
check the models against the SierraDNA documentation and your own database
1070-
to see where the problems lie and decide if they're worth trying to fix in
1071-
the models. If tests fail on models that are central, like `RecordMetadata`
1072-
or `BibRecord`, then it's a problem. If tests fail on models that are more
1073-
peripheral, like `LocationChange`, then finding and fixing those problems
1074-
may be less of a priority, especially if you never intend to use those
1075-
models in your API.
1086+
Ultimately, even though these `code` relationships aren't database-enforced
1087+
keys, we do still want the ORM to handle the relationships for us in the
1088+
general case where you _can_ match a code with the entry that describes it.
1089+
Otherwise we'd have to do the matching manually, which would somewhat reduce
1090+
the utility of the ORM.
10761091

1077-
Presumably because the Sierra data that customers have access to is
1078-
implemented in views instead of tables, proper data integrity is lacking in
1079-
a few cases. (E.g., the views sometimes don't implement proper primary key
1080-
/ foreign key relationships.) This can cause `*_sanity_check` tests to fail
1081-
in practice on live data when they should work in theory.
10821092

10831093
### <a name="unit-tests"></a>Running Unit(ish) Tests
10841094

10851095
More recently we've been working on adding unit (and integration) tests that
1086-
run with py.test. We recommend running these tests via Docker, but it *is*
1096+
run with pytest. We recommend running these tests via Docker, but it *is*
10871097
possible to run them outside of Docker if you're motivated enough.
10881098

10891099
If you followed the [Docker setup](#installation-docker),
10901100

10911101
./docker-compose.sh run --rm test
10921102

1093-
will run all available py.test tests.
1103+
will run all available pytest tests.
10941104

10951105
If you didn't follow the Docker setup, then you should still be able to create
10961106
a comparable test environment:
@@ -1108,7 +1118,7 @@ relevant connection details.
11081118

11091119
Spin up all of the needed test databases, and then run
11101120

1111-
py.test
1121+
pytest
11121122

11131123

11141124
### <a name="testing-exports"></a>Testing Sierra Exports Manually

django/sierra/api/__init__.py

Lines changed: 0 additions & 164 deletions
Original file line numberDiff line numberDiff line change
@@ -1,164 +0,0 @@
1-
import csv
2-
import re
3-
4-
from .models import APIUser
5-
from django.contrib.auth.models import User
6-
7-
8-
class APIUserManagerError(Exception):
9-
pass
10-
11-
12-
class UserExists(APIUserManagerError):
13-
pass
14-
15-
16-
class UserDoesNotExist(APIUserManagerError):
17-
pass
18-
19-
20-
def get_api_user(username):
21-
api_user = None
22-
try:
23-
api_user = APIUser.objects.get(user__username=username)
24-
except APIUser.DoesNotExist:
25-
raise APIUserManagerError('APIUser {} does not exist.'
26-
''.format(username))
27-
28-
return api_user
29-
30-
31-
def create_api_user(username, secret, permissions=None, default=False,
32-
email='', django_password=None, first_name='',
33-
last_name=''):
34-
user = None
35-
api_user = None
36-
37-
try:
38-
api_user = get_api_user(username)
39-
except APIUserManagerError:
40-
pass
41-
else:
42-
raise UserExists('APIUser {} already exists.'.format(username))
43-
44-
try:
45-
user = User.objects.get(username=username)
46-
except User.DoesNotExist:
47-
if not django_password:
48-
raise UserDoesNotExist('Django User {} does not exist and '
49-
'must be created. You must supply a '
50-
'django_password argument.'
51-
''.format(username))
52-
user = User.objects.create_user(username, email, django_password)
53-
user.first_name = first_name
54-
user.last_name = last_name
55-
user.save()
56-
57-
api_user = APIUser(user=user)
58-
api_user.set_secret(secret)
59-
api_user.set_permissions(permissions=permissions, default=default)
60-
return api_user
61-
62-
63-
def update_api_user(username, secret=None, permissions=None, default=False,
64-
email='', django_password=None, first_name='',
65-
last_name=''):
66-
try:
67-
api_user = get_api_user(username)
68-
except APIUserManagerError:
69-
raise UserDoesNotExist('API User {} does not exist.'.format(username))
70-
71-
if (secret):
72-
api_user.set_secret(secret)
73-
if (permissions):
74-
api_user.set_permissions(permissions=permissions, default=default)
75-
76-
django_user_changed = False
77-
if (email and email != api_user.user.email):
78-
api_user.user.email = email
79-
django_user_changed = True
80-
if (first_name and first_name != api_user.user.first_name):
81-
api_user.user.first_name = first_name
82-
django_user_changed = True
83-
if (last_name and last_name != api_user.user.last_name):
84-
api_user.user.last_name = last_name
85-
django_user_changed = True
86-
if (django_password and not api_user.user.check_password(django_password)):
87-
api_user.user.set_password(django_password)
88-
django_user_changed = True
89-
90-
if django_user_changed:
91-
api_user.user.save()
92-
return api_user
93-
94-
95-
def set_api_user_secret(username, secret):
96-
api_user = get_api_user(username)
97-
api_user.set_secret(secret)
98-
return api_user
99-
100-
101-
def set_api_user_permissions(username, permissions, default=False):
102-
api_user = get_api_user(username)
103-
api_user.set_permissions(permissions=permissions, default=default)
104-
return api_user
105-
106-
107-
def batch_create_update_api_users(filepath, default=None):
108-
"""
109-
This will open a csv file provided by filepath and batch create API
110-
users based on its contents. Column names should be provided that
111-
match the args to create_api_user (except default). If the API user
112-
already exists, then this will attempt to update the secret and the
113-
permissions for that user.
114-
"""
115-
data = []
116-
with open(filepath, 'r') as csvfile:
117-
permreader = csv.DictReader(csvfile)
118-
for row in permreader:
119-
row_data = {}
120-
try:
121-
row_data['username'] = row['username']
122-
except KeyError:
123-
raise APIUserManagerError('Your csv file must include a '
124-
'"username" column.')
125-
row_data['secret'] = row.get('secret', None)
126-
row_data['django_password'] = row.get('django_password', None)
127-
row_data['first_name'] = row.get('first_name', None)
128-
row_data['last_name'] = row.get('last_name', None)
129-
row_data['email'] = row.get('email', None)
130-
131-
permissions = {}
132-
133-
for key, val in row.iteritems():
134-
if re.match(r'^permission_', key):
135-
if val in ('True', 'true', 'TRUE', 'T', 't', '1'):
136-
val = True
137-
else:
138-
val = False
139-
permissions[re.sub(r'^permission_', '', key)] = val
140-
141-
if not permissions and default is not None:
142-
permissions = default
143-
144-
row_data['permissions'] = permissions
145-
data.append(row_data)
146-
147-
for row in data:
148-
try:
149-
create_api_user(row['username'], row['secret'],
150-
permissions=row['permissions'],
151-
email=row['email'],
152-
django_password=row['django_password'],
153-
first_name=row['first_name'],
154-
last_name=row['last_name'], default=default)
155-
except UserExists:
156-
update_api_user(row['username'], row['secret'],
157-
permissions=row['permissions'],
158-
email=row['email'],
159-
django_password=row['django_password'],
160-
first_name=row['first_name'],
161-
last_name=row['last_name'], default=default)
162-
except UserDoesNotExist:
163-
print ('User {} does not exist and no django_password was '
164-
'provided. Skipping.'.format(row['username']))

django/sierra/api/exceptions.py

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,34 @@
1-
'''
1+
"""
22
Implements REST API exceptions.
3-
'''
3+
"""
44
from rest_framework import exceptions
55
from rest_framework import views
66

7-
def sierra_exception_handler(exc):
8-
'''
7+
8+
def sierra_exception_handler(exc, context):
9+
"""
910
Custom exception handler. Defines what gets returned in the
1011
response when clients encounter errors.
11-
'''
12-
response = views.exception_handler(exc)
12+
"""
13+
response = views.exception_handler(exc, context)
1314
if response is not None:
1415
response.data['status'] = response.status_code
15-
1616
return response
1717

1818

1919
class BadQuery(exceptions.APIException):
2020
status_code = 400
2121
default_detail = ('One of the query parameters was not correctly '
22-
'formatted.')
22+
'formatted.')
23+
2324

2425
class BadUpdate(exceptions.APIException):
2526
status_code = 400
2627
default_detail = ('The requested resource could not be updated because '
2728
'the content received was invalid.')
29+
30+
2831
class ReadOnlyEdit(exceptions.APIException):
2932
status_code = 400
3033
default_detail = ('The requested resource could not be updated because '
31-
'the request attempted to update read-only content.')
34+
'the request attempted to update read-only content.')

0 commit comments

Comments
 (0)