diff --git a/amivapi/events/model.py b/amivapi/events/model.py
index 65cd8d2a..aafc1ecf 100644
--- a/amivapi/events/model.py
+++ b/amivapi/events/model.py
@@ -512,7 +512,8 @@
'data_relation': {
'resource': 'events',
- 'embeddable': True
+ 'embeddable': True,
+ 'cascade_delete': True,
},
'not_patchable': True,
'required': True,
diff --git a/amivapi/studydocs/__init__.py b/amivapi/studydocs/__init__.py
index 5f5ecfa7..de161a59 100644
--- a/amivapi/studydocs/__init__.py
+++ b/amivapi/studydocs/__init__.py
@@ -8,11 +8,10 @@
Contains settings for eve resource, special validation and email_confirmation
logic needed for signup of non members to events.
"""
-from amivapi.studydocs.authorization import (
- add_uploader_on_bulk_insert,
- add_uploader_on_insert
-)
+from amivapi.studydocs.authorization import add_uploader_on_insert
from amivapi.studydocs.summary import add_summary
+from amivapi.studydocs.rating import (
+ init_rating, update_rating_post, update_rating_patch, update_rating_delete)
from amivapi.studydocs.model import studydocdomain, StudyDocValidator
from amivapi.utils import register_domain, register_validator
@@ -22,7 +21,14 @@ def init_app(app):
register_domain(app, studydocdomain)
register_validator(app, StudyDocValidator)
- app.on_insert_item_studydocuments += add_uploader_on_insert
- app.on_insert_studydocuments += add_uploader_on_bulk_insert
+ # Uploader
+ app.on_insert_studydocuments += add_uploader_on_insert
+ # Rating
+ app.on_insert_studydocuments += init_rating
+ app.on_inserted_studydocumentratings += update_rating_post
+ app.on_updated_studydocumentratings += update_rating_patch
+ app.on_deleted_item_studydocumentratings += update_rating_delete
+
+ # Meta summary
app.on_fetched_resource_studydocuments += add_summary
diff --git a/amivapi/studydocs/authorization.py b/amivapi/studydocs/authorization.py
index 159dd1bc..41e50f3a 100644
--- a/amivapi/studydocs/authorization.py
+++ b/amivapi/studydocs/authorization.py
@@ -20,12 +20,22 @@ def has_resource_write_permission(self, user_id):
return True
-def add_uploader_on_insert(item):
- """Add the _author field before inserting studydocs"""
- item['uploader'] = g.get('current_user')
+class StudydocratingsAuth(AmivTokenAuth):
+ def has_item_write_permission(self, user_id, item):
+ """Allow users to modify only their own ratings."""
+ # item['user'] is Objectid, convert to str
+ return user_id == str(get_id(item['user']))
+
+ def create_user_lookup_filter(self, user_id):
+ """Allow users to only see their own ratings."""
+ return {'user': user_id}
+
+ def has_resource_write_permission(self, user_id):
+ # All users can rate studydocs
+ return True
-def add_uploader_on_bulk_insert(items):
+def add_uploader_on_insert(items):
"""Add the _author field before inserting studydocs"""
for item in items:
- add_uploader_on_insert(item)
+ item['uploader'] = g.get('current_user')
diff --git a/amivapi/studydocs/model.py b/amivapi/studydocs/model.py
index 55c66079..3d135313 100644
--- a/amivapi/studydocs/model.py
+++ b/amivapi/studydocs/model.py
@@ -4,9 +4,10 @@
# you to buy us beer if we meet and you like the software.
"""Model for studydocuments."""
-from amivapi.settings import DEPARTMENT_LIST
+from flask import g
-from .authorization import StudydocsAuth
+from amivapi.settings import DEPARTMENT_LIST
+from .authorization import StudydocsAuth, StudydocratingsAuth
description = ("""
@@ -23,16 +24,18 @@
-## Security
+## Ratings
-In addition to the usual
-[permissions](#section/Authentication-and-Authorization/Authorization),
-file uploaders have additional permissions:
+Every **User** can rate study documents by either up- or downvoting, using the
+[Study Document Ratings](#tag/Study-Document-Rating) endpoint.
-- **Users** can modify all items they uploaded themselves (identified by the
- user id in the `uploader` field).
+Ratings are not simply averages, but take the number of votes into account.
+Concretely, the rating is the lower bound of the wilson confidence interval.
+
+[You can read more here][1] if you are interested!
+
+[1]: http://www.evanmiller.org/how-not-to-sort-by-average-rating.html
-- **Admins** can modify items for all users.
@@ -58,6 +61,26 @@
The summary is only computed for documents matching the current `where` query,
e.g. when searching for ITET documents, only professors related to ITET
documents will show up in the summary.
+
+
+
+## Security
+
+In addition to the usual
+[permissions](#section/Authentication-and-Authorization/Authorization),
+file uploaders have additional permissions, and rating access is restricted:
+
+- **Users** can modify all items they uploaded themselves (identified by the
+ user id in the `uploader` field). They can give ratings (only for themselves,
+ not for other users) and see their own ratings.
+
+- **Admins** can see and modify items and ratings for all users.
+
+""")
+
+
+description_rating = ("""
+
""")
@@ -67,6 +90,20 @@ class StudyDocValidator(object):
def _validate_allow_summary(self, *args, **kwargs):
"""{'type': 'boolean'}"""
+ def _validate_only_self(self, enabled, field, value):
+ """Validate if the id can be used for a rating.
+
+ Users can only sign up themselves
+ Moderators and admins can sign up everyone
+
+ The rule's arguments are validated against this schema:
+ {'type': 'boolean'}
+ """
+ user = g.get('current_user')
+ if enabled and not g.get('resource_admin') and (str(value) != user):
+ self._error(field, "You can only rate with your own id."
+ "(Your id: %s)" % (user))
+
studydocdomain = {
'studydocuments': {
@@ -196,7 +233,73 @@ def _validate_allow_summary(self, *args, **kwargs):
'description': 'The year in which the course *was taken*, '
'to separate older from newer files.',
'allow_summary': True,
- }
+ },
+
+ 'rating': {
+ 'title': 'Study Document Rating',
+ 'description': 'The study document rating as a fraction of '
+ 'upvotes divided by all votes. Computed using '
+ 'a confidence interval. Null if no votes have '
+ 'been cast.',
+ 'example': '0.9',
+
+ 'type': 'float',
+ 'readonly': True,
+ },
+ },
+ },
+
+ 'studydocumentratings': {
+ 'resource_title': "Study Document Ratings",
+ 'item_title': "Study Document Rating",
+
+ 'description': "A rating for a [Study Document](#tag/Study-Document).",
+
+ 'resource_methods': ['GET', 'POST'],
+ 'item_methods': ['GET', 'PATCH', 'DELETE'],
+
+ 'authentication': StudydocratingsAuth,
+
+ 'schema': {
+ 'user': {
+ 'description': 'The rating user. You can only use your own id.',
+ 'example': '679ff66720812cdc2da4fb4a',
+
+ 'type': 'objectid',
+ 'data_relation': {
+ 'resource': 'users',
+ 'embeddable': True,
+ 'cascade_delete': True,
+ },
+ 'not_patchable': True,
+ 'required': True,
+ 'only_self': True,
+ 'unique_combination': ['studydocument'],
+ },
+
+ 'studydocument': {
+ 'title': 'Study Document',
+ 'description': 'The rated study document.',
+ 'example': '10d8e50e303049ecb856ae9b',
+
+ 'data_relation': {
+ 'resource': 'studydocuments',
+ 'embeddable': True,
+ 'cascade_delete': True,
+ },
+ 'not_patchable': True,
+ 'required': True,
+ 'type': 'objectid',
+ },
+ 'rating': {
+ 'description': 'The given rating, can be an up- or downvote.',
+ 'example': 'up',
+
+ 'type': 'string',
+ 'allowed': ['up', 'down'],
+ 'required': True
+ },
},
}
+
}
diff --git a/amivapi/studydocs/rating.py b/amivapi/studydocs/rating.py
new file mode 100644
index 00000000..54fff3c0
--- /dev/null
+++ b/amivapi/studydocs/rating.py
@@ -0,0 +1,81 @@
+# -*- coding: utf-8 -*-
+#
+# license: AGPLv3, see LICENSE for details. In addition we strongly encourage
+# you to buy us beer if we meet and you like the software.
+"""Studydoc rating computation.
+
+For the rating, we use the lower bound of a confidence interval around
+the 'naive' average vote, which accounts for the number of votes.
+
+The rating for a study document is updated each time a study doc rating is
+for the respective document is POSTed or PATCHed. The value is then written
+to the database to allow sorting of study documents by rating.
+"""
+
+from flask import current_app
+from math import sqrt
+
+from amivapi.utils import get_id
+
+
+def compute_rating(upvotes, downvotes, z=1.28):
+ """Compute the rating.
+
+ Concretely, the lower bound of the wilson confidence interval is returned,
+ which takes the number of votes into account. [1]
+
+ We use z = 1.28 by default, which corresponds to a 80% confidence interval
+ (As there are only a few votes, we do not want to be overly cautious).
+
+ [1]: http://www.evanmiller.org/how-not-to-sort-by-average-rating.html
+ """
+ total = upvotes + downvotes
+ if not total:
+ return
+
+ p = upvotes / total
+ bound = ((p + (z**2)/(2*total) -
+ z * sqrt((p * (1-p) + (z**2)/(4*total)) / total)) /
+ (1 + (z**2)/total))
+
+ # Ensure that the bound is not below 0
+ return max(bound, 0)
+
+
+def _update_rating(studydoc_id):
+ """Computes the rating for a study document."""
+ docs = current_app.data.driver.db['studydocuments']
+ ratings = current_app.data.driver.db['studydocumentratings']
+ lookup = {'studydocument': studydoc_id}
+
+ # Check votes
+ upvotes = ratings.count_documents({'rating': 'up', **lookup})
+ downvotes = ratings.count_documents({'rating': 'down', **lookup})
+
+ # Compute rating and write to database
+ rating = compute_rating(upvotes, downvotes)
+ docs.update_one({'_id': studydoc_id}, {'$set': {'rating': rating}})
+
+
+def init_rating(items):
+ """On creating of a study-document, set the rating to None."""
+ for item in items:
+ item['rating'] = None
+
+
+def update_rating_post(items):
+ """Rating update hook for POST requests."""
+ for item in items:
+ _update_rating(get_id(item['studydocument']))
+
+
+def update_rating_patch(updates, original):
+ """Rating update hook for PATCH requests."""
+ _update_rating(get_id(updates['studydocument'])
+ if 'studydocument' in updates else
+ get_id(original['studydocument']))
+
+
+def update_rating_delete(item):
+ """Rating update hook for DELETE requests."""
+ _update_rating(get_id(item['studydocument']))
diff --git a/amivapi/tests/studydocs/test_rating.py b/amivapi/tests/studydocs/test_rating.py
new file mode 100644
index 00000000..df11763a
--- /dev/null
+++ b/amivapi/tests/studydocs/test_rating.py
@@ -0,0 +1,229 @@
+# -*- coding: utf-8 -*-
+#
+# license: AGPLv3, see LICENSE for details. In addition we strongly encourage
+# you to buy us beer if we meet and you like the software.
+"""Tests for studydocuments rating."""
+
+from amivapi.studydocs.rating import compute_rating
+from amivapi.tests.utils import WebTest, WebTestNoAuth
+
+
+class StudydocsRatingTest(WebTestNoAuth):
+ """Test rating computation."""
+
+ def test_no_rating(self):
+ """Without any votes, the `rating` is in the document, and is None."""
+ doc = self.new_object('studydocuments')
+
+ response = self.api.get('/studydocuments/' + str(doc['_id']),
+ status_code=200)
+
+ self.assertIsNone(response.json['rating'])
+
+ def test_bound(self):
+ """Without any votes, the rating is None."""
+ for upvotes, downvotes, rating in [
+ (1, 0, 0.38),
+ (5, 0, 0.75),
+ (10, 0, 0.86),
+ (1, 1, 0.16),
+ (5, 2, 0.47),
+ (5, 5, 0.31),
+ (50, 50, 0.44),
+ (0, 1, 0.0),
+ (0, 5, 0.0),
+ (0, 10, 0.0),
+ ]:
+ bound = compute_rating(upvotes, downvotes)
+ self.assertAlmostEqual(rating, bound, 2)
+
+ def test_rating(self):
+ """Test that the rating is correctly attached to the studydoc."""
+ doc = self.new_object('studydocuments')
+ lookup = {'studydocument': str(doc['_id'])}
+
+ response = self.api.get('/studydocuments/' + str(doc['_id']),
+ status_code=200)
+ # No ratings yet
+ self.assertIsNone(response.json['rating'])
+
+ self.load_fixture({
+ 'users': [{}],
+ 'studydocumentratings': [{'rating': 'up', **lookup}],
+ })
+
+ response = self.api.get('/studydocuments/' + str(doc['_id']),
+ status_code=200)
+ self.assertAlmostEqual(response.json['rating'], 0.38, 2)
+
+ def test_sort_by_rating(self):
+ """Ensure that sorting by rating works."""
+ ids = [24 * '0', 24 * '1', 24 * '2']
+ self.load_fixture({
+ 'users': [{}],
+ 'studydocuments': [
+ {'_id': ids[0]}, {'_id': ids[1]}, {'_id': ids[2]}
+ ],
+ 'studydocumentratings': [
+ # Rate such that:
+ # 0: No rating, 1: Low rating, 2: High rating
+ {'studydocument': ids[2], 'rating': 'up'},
+ {'studydocument': ids[1], 'rating': 'down'},
+ ],
+ })
+ response = self.api.get('/studydocuments?sort=rating', status_code=200)
+ for (item, expected_id) in zip(response.json['_items'], ids):
+ self.assertEqual(item['_id'], expected_id)
+
+ # Also test reverse order, to ensure that it was no accident
+ response = self.api.get('/studydocuments?sort=-rating', status_code=200)
+ for (item, expected_id) in zip(response.json['_items'], ids[::-1]):
+ self.assertEqual(item['_id'], expected_id)
+
+ def _assert_rating(self, studydoc_id, rating):
+ response = self.api.get('/studydocuments/' + studydoc_id,
+ status_code=200)
+ self.assertAlmostEqual(response.json['rating'], rating, 2)
+
+ def test_new_vote(self):
+ """POSTing a new vote updates the rating."""
+ user = str(self.new_object('users')['_id'])
+ doc = str(self.new_object('studydocuments')['_id'])
+ self._assert_rating(doc, None)
+
+ data = {'user': user, 'studydocument': doc, 'rating': 'up'}
+ self.api.post('/studydocumentratings', data=data, status_code=201)
+ self._assert_rating(doc, 0.38)
+
+ def test_change_vote(self):
+ """PATCHing an existing vote updates the rating."""
+ self.new_object('users')
+ doc = str(self.new_object('studydocuments')['_id'])
+ rating = self.new_object('studydocumentratings', rating='up')
+ self._assert_rating(doc, 0.38)
+
+ headers = {'If-Match': rating['_etag']}
+ data = {'rating': 'down'}
+ self.api.patch('/studydocumentratings/' + str(rating['_id']),
+ data=data, headers=headers, status_code=200)
+ self._assert_rating(doc, 0.0)
+
+ def test_delete_vote(self):
+ """DELETEing votes update the rating."""
+ user_1, user_2 = self.load_fixture({'users': [{}, {}]})
+ doc = str(self.new_object('studydocuments')['_id'])
+ rating_1, rating_2 = self.load_fixture({
+ 'studydocumentratings': [
+ {'user': str(user_1['_id']), 'rating': 'up'},
+ {'user': str(user_2['_id']), 'rating': 'down'}
+ ]
+ })
+ self._assert_rating(doc, 0.16)
+
+ self.api.delete('/studydocumentratings/' + str(rating_2['_id']),
+ headers={'If-Match': rating_2['_etag']},
+ status_code=204)
+ self._assert_rating(doc, 0.38)
+
+ # After the last rating get's deleted, the rating is `None` again
+ self.api.delete('/studydocumentratings/' + str(rating_1['_id']),
+ headers={'If-Match': rating_1['_etag']},
+ status_code=204)
+ self._assert_rating(doc, None)
+
+ def test_rate_once(self):
+ """Every user can rate a document only once."""
+ user = str(self.new_object('users')['_id'])
+ doc = str(self.new_object('studydocuments')['_id'])
+
+ data = {'user': user, 'studydocument': doc, 'rating': 'up'}
+ self.api.post('/studydocumentratings', data=data, status_code=201)
+ self.api.post('/studydocumentratings', data=data, status_code=422)
+
+
+class StudydocsRatingAuthTest(WebTest):
+ """Test rating permissions and visibility."""
+
+ def test_see_own(self):
+ """Users can only see their own ratings."""
+ user_1, user_2 = self.load_fixture({'users': [{}, {}]})
+ self.new_object('studydocuments')
+ rating_1, rating_2 = self.load_fixture({
+ 'studydocumentratings': [
+ {'user': str(user_1['_id']), 'rating': 'up'},
+ {'user': str(user_2['_id']), 'rating': 'down'}
+ ]
+ })
+
+ token = self.get_user_token(rating_1['user'])
+
+ # Resource
+ response = self.api.get('/studydocumentratings',
+ token=token, status_code=200)
+ ids = [item['_id'] for item in response.json['_items']]
+ self.assertIn(str(rating_1['_id']), ids)
+ self.assertNotIn(str(rating_2['_id']), ids)
+
+ # Item
+ self.api.get('/studydocumentratings/' + str(rating_1['_id']),
+ token=token, status_code=200)
+ self.api.get('/studydocumentratings/' + str(rating_2['_id']),
+ token=token, status_code=404)
+
+ def test_create(self):
+ """Users can only create ratings for themselves."""
+ user_1, user_2 = self.load_fixture({'users': [{}, {}]})
+ doc = str(self.new_object('studydocuments')['_id'])
+
+ data_1 = {'studydocument': doc, 'user': str(user_1['_id']),
+ 'rating': 'up'}
+ data_2 = {'studydocument': doc, 'user': str(user_2['_id']),
+ 'rating': 'up'}
+ token = self.get_user_token(user_1['_id'])
+ self.api.post('/studydocumentratings', data=data_1, token=token,
+ status_code=201)
+ self.api.post('/studydocumentratings', data=data_2, token=token,
+ status_code=422)
+
+ def test_modify_own(self):
+ """Users can only patch their own ratings."""
+ user_1, user_2 = self.load_fixture({'users': [{}, {}]})
+ self.new_object('studydocuments')
+ rating_1, rating_2 = self.load_fixture({
+ 'studydocumentratings': [
+ {'user': str(user_1['_id']), 'rating': 'down'},
+ {'user': str(user_2['_id']), 'rating': 'down'}
+ ]
+ })
+
+ token = self.get_user_token(rating_1['user'])
+ updates = {'rating': 'up'}
+
+ self.api.patch('/studydocumentratings/' + str(rating_1['_id']),
+ data=updates, token=token, status_code=200,
+ headers={'If-Match': rating_1['_etag']})
+ # Cannot even see other rating, so a 404 is returned
+ self.api.patch('/studydocumentratings/' + str(rating_2['_id']),
+ data=updates, token=token, status_code=404,
+ headers={'If-Match': rating_2['_etag']})
+
+ def test_delete_own(self):
+ """Users can only delete their own ratings."""
+ user_1, user_2 = self.load_fixture({'users': [{}, {}]})
+ self.new_object('studydocuments')
+ rating_1, rating_2 = self.load_fixture({
+ 'studydocumentratings': [
+ {'user': str(user_1['_id']), 'rating': 'down'},
+ {'user': str(user_2['_id']), 'rating': 'down'}
+ ]
+ })
+
+ token = self.get_user_token(rating_1['user'])
+
+ self.api.delete('/studydocumentratings/' + str(rating_1['_id']),
+ token=token, status_code=204,
+ headers={'If-Match': rating_1['_etag']})
+ # Cannot even see other rating, so a 404 is returned
+ self.api.delete('/studydocumentratings/' + str(rating_2['_id']),
+ token=token, status_code=404,
+ headers={'If-Match': rating_2['_etag']})