change way in which badges are handled

Supplying multiple inputs and retrieving them all is probably better
than the weird badge-1, badge-2, ... hack we used.

4 files changed, 46 insertions, 41 deletions

diff --git a/fietsboek/templates/edit_form.jinja2 b/fietsboek/templates/edit_form.jinja2
index 6c3cd25..aebd37f 100644
--- a/fietsboek/templates/edit_form.jinja2
+++ b/fietsboek/templates/edit_form.jinja2
@@ -75,7 +75,7 @@
   <div>{{ _("page.track.form.badges") }}</div>
   {% for (state, badge) in badges %}
   <div class="form-check">
-    <input class="form-check-input" type="checkbox" value="marked" id="badge-{{ badge.id }}" name="badge-{{ badge.id }}"{% if state %} checked{% endif %}>
+    <input class="form-check-input" type="checkbox" value="{{ badge.id }}" id="badge-{{ badge.id }}" name="badge[]"{% if state %} checked{% endif %}>
     <label class="form-check-label" for="badge-{{ badge.id }}">
       {{ badge.title }}
     </label>
diff --git a/fietsboek/util.py b/fietsboek/util.py
index 0f9cc5d..9a8e596 100644
--- a/fietsboek/util.py
+++ b/fietsboek/util.py
@@ -8,7 +8,9 @@ import bleach
 import gpxpy
 
 from pyramid.i18n import TranslationString as _
+from pyramid.httpexceptions import HTTPBadRequest
 from markupsafe import Markup
+from sqlalchemy import select
 
 
 ALLOWED_TAGS = (bleach.sanitizer.ALLOWED_TAGS +
@@ -100,22 +102,36 @@ def random_alphanum_string(length=20):
     return ''.join(random.choice(candidates) for _ in range(length))
 
 
-def parse_badges(badges, params, prefix='badge-'):
-    """Parses a reply to extract which badges have been checked.
+def retrieve_multiple(dbsession, model, params, name):
+    """Parses a reply to retrieve multiple database objects.
 
-    :param badges: List of available badges.
-    :type badges: list[fietsboek.models.badge.Badge]
+    This is usable for arrays sent by HTML forms, for example to retrieve all
+    badges or all tagged friends.
+
+    If an object could not be found, this raises a
+    :class:`~pyramid.httpexceptions.HTTPBadRequest`.
+
+    :raises pyramid.httpexceptions.HTTPBadRequest: If an object could not be
+        found.
+    :param dbsession: The database session.
+    :type dbsession: sqlalchemy.orm.session.Session
+    :param model: The model class to retrieve.
+    :type model: class
     :param params: The form parameters.
     :type params: webob.multidict.NestedMultiDict
-    :param prefix: Prefix of the checkbox names.
-    :type prefix: str
-    :return: A list of active badges.
-    :rtype: list[fietsboek.models.badge.Badge]
+    :param name: Name of the parameter to look for.
+    :type name: str
+    :return: A list of elements found.
+    :rtype: list[model]
     """
-    return [
-        badge for badge in badges
-        if params.get(f'{prefix}{badge.id}')
-    ]
+    objects = []
+    for obj_id in params.getall(name):
+        query = select(model).filter_by(id=obj_id)
+        obj = dbsession.execute(query).scalar_one_or_none()
+        if obj is None:
+            raise HTTPBadRequest()
+        objects.append(obj)
+    return objects
 
 
 def check_password_constraints(password, repeat_password=None):
diff --git a/fietsboek/views/edit.py b/fietsboek/views/edit.py
index 21aeeb2..4b842fb 100644
--- a/fietsboek/views/edit.py
+++ b/fietsboek/views/edit.py
@@ -41,31 +41,26 @@ def do_edit(request):
     :return: The HTTP response.
     :rtype: pyramid.response.Response
     """
+    # pylint: disable=duplicate-code
     query = select(models.Track).filter_by(id=request.matchdict["id"])
     track = request.dbsession.execute(query).scalar_one()
     if request.identity != track.owner:
         return HTTPForbidden()
 
     user_friends = request.identity.get_friends()
-    badges = request.dbsession.execute(select(models.Badge)).scalars()
-    active_badges = util.parse_badges(badges, request.params)
+    badges = util.retrieve_multiple(request.dbsession, models.Badge, request.params, "badge[]")
+    tagged_people = util.retrieve_multiple(request.dbsession, models.User,
+                                           request.params, "tagged-friend[]")
 
-    tagged_people = request.params.getall("tagged-friend[]")
-    new_tagged_people = []
-    for friend_id in tagged_people:
-        user = request.dbsession.execute(
-            select(models.User).filter_by(id=friend_id)).scalar_one_or_none()
-        if user is None:
-            return HTTPBadRequest()
-        if user in track.tagged_people or user in user_friends:
-            new_tagged_people.append(user)
-    track.tagged_people = new_tagged_people
+    if any(user not in track.tagged_people and user not in user_friends for user in tagged_people):
+        return HTTPBadRequest()
 
+    track.tagged_people = tagged_people
     track.title = request.params["title"]
     track.visibility = Visibility[request.params["visibility"]]
     track.description = request.params["description"]
     track.date = datetime.datetime.fromisoformat(request.params["date"])
-    track.badges = active_badges
+    track.badges = badges
     tags = set(filter(bool, request.params["tags"].split(" ")))
     track.sync_tags(tags)
 
diff --git a/fietsboek/views/upload.py b/fietsboek/views/upload.py
index 567c9ba..00b2a67 100644
--- a/fietsboek/views/upload.py
+++ b/fietsboek/views/upload.py
@@ -114,7 +114,7 @@ def finish_upload(request):
     return {
         'preview_id': upload.id,
         'upload_title': gpx.name,
-        'upload_date': gpx.time,
+        'upload_date': gpx.time or datetime.datetime.now(),
         'upload_visibility': Visibility.PRIVATE,
         'upload_description': gpx.description,
         'upload_tags': set(),
@@ -138,18 +138,12 @@ def do_finish_upload(request):
         return HTTPForbidden()
 
     user_friends = request.identity.get_friends()
-    badges = request.dbsession.execute(select(models.Badge)).scalars()
-    active_badges = util.parse_badges(badges, request.params)
-
-    tagged_people = request.params.getall("tagged-friend[]")
-    new_tagged_people = []
-    for friend_id in tagged_people:
-        user = request.dbsession.execute(
-            select(models.User).filter_by(id=friend_id)).scalar_one_or_none()
-        if user is None:
-            return HTTPBadRequest()
-        if user in user_friends:
-            new_tagged_people.append(user)
+    badges = util.retrieve_multiple(request.dbsession, models.Badge, request.params, "badge[]")
+    tagged_people = util.retrieve_multiple(request.dbsession, models.User,
+                                           request.params, "tagged-friend[]")
+
+    if any(user not in user_friends for user in tagged_people):
+        return HTTPBadRequest()
 
     track = models.Track(
         owner=request.identity,
@@ -157,9 +151,9 @@ def do_finish_upload(request):
         date=datetime.datetime.fromisoformat(request.params["date"]),
         visibility = Visibility[request.params["visibility"]],
         description=request.params["description"],
-        badges=active_badges,
+        badges=badges,
         link_secret=util.random_alphanum_string(),
-        tagged_people=new_tagged_people,
+        tagged_people=tagged_people,
     )
     tags = set(filter(bool, request.params["tags"].split(" ")))
     track.sync_tags(tags)