From cf60a5bf5ff705030cf7d1a27e30e67fca6a15d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Krienb=C3=BChl?= Date: Mon, 1 Jun 2015 14:39:02 +0200 Subject: [PATCH] Divides the submissions into 'pending' and 'complete'. --- HISTORY.rst | 7 +++ onegov/form/__init__.py | 9 +++- onegov/form/collection.py | 58 ++++++++++++++++++++-- onegov/form/errors.py | 4 ++ onegov/form/models.py | 36 ++++++++++++-- onegov/form/parser/core.py | 13 +++-- onegov/form/tests/test_collection.py | 74 ++++++++++++++++++++++++++-- onegov/form/tests/test_core.py | 2 +- onegov/form/validators.py | 2 +- 9 files changed, 187 insertions(+), 18 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 839cd5e..2869035 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -4,6 +4,13 @@ Changelog Unreleased ~~~~~~~~~~ +- Divides the submissions into 'pending' and 'complete'. + + Pending submissions are temporary and possibly invalid. Complete submissions + are final and always valid. + + [href] + - Compresses uploaded files before storing them on the database. [href] diff --git a/onegov/form/__init__.py b/onegov/form/__init__.py index 176460c..91a377b 100644 --- a/onegov/form/__init__.py +++ b/onegov/form/__init__.py @@ -1,11 +1,18 @@ from onegov.form.collection import FormCollection from onegov.form.core import Form, with_options -from onegov.form.models import FormDefinition, FormSubmission +from onegov.form.models import ( + FormDefinition, + FormSubmission, + PendingFormSubmission, + CompleteFormSubmission +) __all__ = [ 'Form', 'FormCollection', 'FormDefinition', 'FormSubmission', + 'PendingFormSubmission', + 'CompleteFormSubmission', 'with_options' ] diff --git a/onegov/form/collection.py b/onegov/form/collection.py index a8204a1..bc6cb41 100644 --- a/onegov/form/collection.py +++ b/onegov/form/collection.py @@ -1,5 +1,8 @@ +from delorean import Delorean +from datetime import datetime, timedelta from onegov.core.utils import normalize_for_url from onegov.form.models import FormDefinition, FormSubmission +from sqlalchemy import inspect class FormCollection(object): @@ -73,7 +76,7 @@ def __init__(self, session): def query(self): return self.session.query(FormSubmission) - def add(self, form_name, form): + def add(self, form_name, form, state): """ Takes a form filled-out form instance and stores the submission in the database. The form instance is expected to have a ``_source`` parameter, which contains the source used to build the form (as only @@ -84,12 +87,23 @@ def add(self, form_name, form): assert hasattr(form, '_source') # this should happen way earlier, we just double check here - assert form.validate() + if state == 'complete': + assert form.validate() - submission = FormSubmission() + # look up the right class depending on the type + _mapper = inspect(FormSubmission).polymorphic_map.get(state) + submission = (_mapper and _mapper.class_ or FormSubmission)() submission.name = form_name submission.definition = form._source submission.data = form.data + submission.state = state + + # pending submissions are not necessarily valid, however we don't need + # to store invalid state as it is wiped out anyway + if state == 'pending': + form.validate() + for field_id in form.errors: + del submission.data[field_id] # never include the csrf token if form.meta.csrf and form.meta.csrf_field_name in submission.data: @@ -98,8 +112,46 @@ def add(self, form_name, form): self.session.add(submission) self.session.flush() + # whenever we add a form submission, we remove all the old ones + # which were never completed (this is way easier than having to use + # some kind of cronjob ;) + self.remove_old_pending_submissions( + older_than=datetime.utcnow() - timedelta(days=1) + ) + return submission + def remove_old_pending_submissions(self, older_than): + """ Removes all pending submissions older than the given date. The + date is expected to be in UTC! + + """ + if older_than.tzinfo is None: + older_than = Delorean(older_than, timezone='UTC').datetime + + query = self.query() + + # delete the ones that were never modified + query = query.filter(FormSubmission.state == 'pending') + query = query.filter(FormSubmission.modified == None) + query = query.filter(FormSubmission.created < older_than) + query.delete() + + # delete the ones that were modified + query = query.filter(FormSubmission.state == 'pending') + query = query.filter(FormSubmission.modified != None) + query = query.filter(FormSubmission.modified < older_than) + query.delete() + def by_form_name(self, name): """ Return all submissions for the given form-name. """ return self.query().filter(FormSubmission.name == name).all() + + def by_id(self, id, state=None): + """ Return the submission by id. """ + query = self.query().filter(FormSubmission.id == id) + + if state is not None: + query = query.filter(FormSubmission.state == state) + + return query.first() diff --git a/onegov/form/errors.py b/onegov/form/errors.py index 7f47478..72c7041 100644 --- a/onegov/form/errors.py +++ b/onegov/form/errors.py @@ -13,3 +13,7 @@ def __repr__(self): class InvalidMimeType(FormError): pass + + +class UnableToComplete(FormError): + pass diff --git a/onegov/form/models.py b/onegov/form/models.py index 475dc0b..182feee 100644 --- a/onegov/form/models.py +++ b/onegov/form/models.py @@ -1,12 +1,14 @@ from onegov.core.orm import Base from onegov.core.orm.mixins import TimestampMixin -from onegov.core.orm.types import JSON +from onegov.core.orm.types import JSON, UUID +from onegov.form.errors import UnableToComplete from onegov.form.parser import parse_form -from sqlalchemy import Column, ForeignKey, Integer, Text +from sqlalchemy import Column, Enum, ForeignKey, Text from sqlalchemy.orm import ( deferred, relationship, ) +from uuid import uuid4 class FormDefinition(Base, TimestampMixin): @@ -53,8 +55,8 @@ class FormSubmission(Base, TimestampMixin): __tablename__ = 'submissions' - #: internal id of the form submission - id = Column(Integer, primary_key=True) + #: id of the form submission + id = Column(UUID, primary_key=True, default=uuid4) #: name of the form this submission belongs to name = Column(Text, ForeignKey(FormDefinition.name), nullable=False) @@ -67,8 +69,34 @@ class FormSubmission(Base, TimestampMixin): #: the submission data data = Column(JSON, nullable=False) + #: the state of the submission + state = Column( + Enum('pending', 'complete', name='submission_state'), + nullable=False + ) + + __mapper_args__ = { + "polymorphic_on": 'state' + } + @property def form_class(self): """ Parses the form definition and returns a form class. """ return parse_form(self.definition) + + def complete(self): + """ Changes the state to 'complete', if the data is valid. """ + + if not self.form_class(data=self.data).validate(): + raise UnableToComplete() + + self.state = 'complete' + + +class PendingFormSubmission(FormSubmission): + __mapper_args__ = {'polymorphic_identity': 'pending'} + + +class CompleteFormSubmission(FormSubmission): + __mapper_args__ = {'polymorphic_identity': 'complete'} diff --git a/onegov/form/parser/core.py b/onegov/form/parser/core.py index 09a5803..929bf11 100644 --- a/onegov/form/parser/core.py +++ b/onegov/form/parser/core.py @@ -280,7 +280,7 @@ TextAreaField ) from wtforms.fields.html5 import DateField, DateTimeLocalField, EmailField -from wtforms.validators import InputRequired, Length +from wtforms.validators import DataRequired, Length from wtforms.widgets import TextArea from wtforms_components import Email, If @@ -709,12 +709,19 @@ def add_field(self, field_class, label, required, validators = kwargs.pop('validators', []) if required: + + # we use the DataRequired check instead of InputRequired, since + # InputRequired only works if the data comes over the wire. We + # also want to load forms with data from the database, where + # InputRequired will fail, but DataRequired will not. + # + # As a consequence, falsey values can't be submitted for now. if dependency is None: - validators.insert(0, InputRequired()) + validators.insert(0, DataRequired()) else: # set the requried flag, even if it's not always required # as it's better to show it too often, than not often enough - validator = If(dependency.fulfilled, InputRequired()) + validator = If(dependency.fulfilled, DataRequired()) validator.field_flags = ('required', ) validators.insert(0, validator) diff --git a/onegov/form/tests/test_collection.py b/onegov/form/tests/test_collection.py index fb0457f..e9724cd 100644 --- a/onegov/form/tests/test_collection.py +++ b/onegov/form/tests/test_collection.py @@ -1,6 +1,8 @@ import pytest -from onegov.form import FormCollection +from datetime import datetime, timedelta +from onegov.form import FormCollection, PendingFormSubmission +from onegov.form.errors import UnableToComplete from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import FlushError from textwrap import dedent @@ -47,7 +49,7 @@ def test_submit_form(session): ]) submitted_form = form.form_class(data) - collection.submissions.add('tps-report', submitted_form) + collection.submissions.add('tps-report', submitted_form, state='complete') form = collection.definitions.by_name('tps-report') submission = form.submissions[0] @@ -57,6 +59,66 @@ def test_submit_form(session): assert submitted_form.data == stored_form.data +def test_submit_pending(session): + collection = FormCollection(session) + + form = collection.definitions.add('tweet', definition=dedent(""" + handle * = ___ + tweet * = ___[140] + """)) + + data = MultiDict([ + ('handle', '@href'), + ('tweet', ( + "I think I found a way to tweet more than 140 characters! " + "Finally I can tell my life's story in a single tweet. " + "#hackersgonnahack #infosec #yolo" + )) + ]) + + # pending forms may be submitted even if the are not valid + submitted_form = form.form_class(data) + assert not submitted_form.validate() + + submission = collection.submissions.add( + 'tweet', submitted_form, state='pending') + assert isinstance(submission, PendingFormSubmission) + + # but invalid data is purged from the submission + assert 'handle' in submission.data + assert 'tweet' not in submission.data + + with pytest.raises(UnableToComplete): + submission.complete() + + submission.data['tweet'] = "Nevermind, it didn't work #mybad" + submission.complete() + + +def test_remove_old_pending_submissions(session): + collection = FormCollection(session) + + signup = collection.definitions.add('Signup', definition="E-Mail = @@@") + + data = MultiDict([('e_mail', 'info@example.org')]) + form = signup.form_class(data) + + collection.submissions.add('signup', form, state='complete') + collection.submissions.add('signup', form, state='pending') + + assert collection.submissions.query().count() == 2 + + collection.submissions.remove_old_pending_submissions( + datetime.utcnow() - timedelta(hours=1)) + + assert collection.submissions.query().count() == 2 + + collection.submissions.remove_old_pending_submissions( + datetime.utcnow() + timedelta(hours=1)) + + assert collection.submissions.query().count() == 1 + + def test_no_store_csrf_token(session): collection = FormCollection(session) @@ -73,7 +135,7 @@ def generate_csrf_token(self, csrf_token_field): ]) form = signup.form_class(data, meta=dict(csrf=True, csrf_class=MockCSRF)) - submission = collection.submissions.add('signup', form) + submission = collection.submissions.add('signup', form, state='complete') assert 'e_mail' in submission.data assert 'csrf_token' not in submission.data @@ -85,7 +147,8 @@ def test_delete_without_submissions(session): form = collection.definitions.add('Newsletter', definition="E-Mail *= @@@") data = MultiDict([('e_mail', 'billg@microsoft.com')]) - collection.submissions.add('newsletter', form.form_class(data)) + collection.submissions.add( + 'newsletter', form.form_class(data), state='complete') with pytest.raises(IntegrityError): collection.definitions.delete('newsletter') @@ -97,7 +160,8 @@ def test_delete_with_submissions(session): form = collection.definitions.add('Newsletter', definition="E-Mail *= @@@") data = MultiDict([('e_mail', 'billg@microsoft.com')]) - collection.submissions.add('newsletter', form.form_class(data)) + collection.submissions.add( + 'newsletter', form.form_class(data), state='complete') collection.definitions.delete('newsletter', with_submissions=True) assert collection.submissions.query().count() == 0 diff --git a/onegov/form/tests/test_core.py b/onegov/form/tests/test_core.py index 3376462..45c8cdf 100644 --- a/onegov/form/tests/test_core.py +++ b/onegov/form/tests/test_core.py @@ -29,7 +29,7 @@ def _value(self): def test_submitted(): class TestForm(Form): - test = StringField("Test", [validators.InputRequired()]) + test = StringField("Test", [validators.DataRequired()]) request = DummyRequest({}) assert not TestForm(request.POST).submitted(request) diff --git a/onegov/form/validators.py b/onegov/form/validators.py index f052d37..51b60ea 100644 --- a/onegov/form/validators.py +++ b/onegov/form/validators.py @@ -17,7 +17,7 @@ def __init__(self, format): def __call__(self, form, field): # only do a check for filled out values, to check for the existance - # of any value use InputRequired! + # of any value use DataRequired! if not field.data: return