Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft model and autosave on Project edit form #352

Merged
merged 65 commits into from
Feb 19, 2019
Merged
Show file tree
Hide file tree
Changes from 60 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
fd29b97
Use ES6 JS. Use webpack to build and transpile ES6 code.
vidya-ram Jan 15, 2019
5e0c866
Sync with master
vidya-ram Jan 15, 2019
29ad207
Add scroll active menu and swipe functionality.
vidya-ram Jan 23, 2019
468cc92
Sync with master.
vidya-ram Jan 24, 2019
e20f3f0
Sort schedule event days.
vidya-ram Jan 25, 2019
86b5449
Sync with master.
vidya-ram Jan 28, 2019
6db29ca
Change buttons design of the header.
vidya-ram Jan 28, 2019
45cc1ec
Add check to find if eventday is present.
vidya-ram Jan 29, 2019
21c55fe
Avoid using global names.
vidya-ram Jan 29, 2019
3404dc3
Add support to lazy load images on the home page
vidya-ram Jan 31, 2019
2ba252a
Merge branch 'master' into js-refactor
vidya-ram Jan 31, 2019
13bb27f
Add script folder. Change to lazy load on hasgeek index page.
vidya-ram Jan 31, 2019
2009366
Merge branch 'master' into js-refactor
vidya-ram Feb 1, 2019
4e3f037
Sync with master.
vidya-ram Feb 1, 2019
e32971e
added Makefile
Feb 1, 2019
cbae311
Updated package-lock.json.
vidya-ram Feb 4, 2019
22cae87
Fix the mui class name.
vidya-ram Feb 4, 2019
7154dd2
added newline
Feb 4, 2019
acdb758
Remove default lat and lon values.
vidya-ram Feb 4, 2019
beaa65f
Merge branch 'js-refactor' of github.com:hasgeek/funnel into js-refactor
vidya-ram Feb 4, 2019
bc25e3e
split makefile
Feb 4, 2019
f1fe677
Merge branch 'js-refactor' of github.com:hasgeek/funnel into js-refactor
Feb 4, 2019
94f25bb
Move view proposal btn to right.
vidya-ram Feb 4, 2019
07b1c5f
Merge branch 'js-refactor' of github.com:hasgeek/funnel into js-refactor
vidya-ram Feb 4, 2019
c2d1bc0
Add support for autosave (wip)
vidya-ram Feb 4, 2019
4d31f12
Merge branch 'master' into autosave
Feb 5, 2019
d25d236
Add autosave flag to form data.
vidya-ram Feb 5, 2019
51ac07f
added draft model and initial view
Feb 5, 2019
daf4270
Merge branch 'autosave' of github.com:hasgeek/funnel into autosave
Feb 5, 2019
5df6d23
fixed edit endpoint
Feb 5, 2019
3a7b9f2
Update server response flag in error cases.
vidya-ram Feb 5, 2019
34a5295
Merge branch 'autosave' of github.com:hasgeek/funnel into autosave
vidya-ram Feb 5, 2019
7f3ae6a
fixed creation of draft
Feb 5, 2019
51e6dfd
Merge branch 'autosave' of github.com:hasgeek/funnel into autosave
vidya-ram Feb 5, 2019
ae27841
Check form data changes before sending to backend.
vidya-ram Feb 5, 2019
059430d
Refactor checking for dirty fields of the form.
vidya-ram Feb 5, 2019
35715f2
added timestamp to draft model, fixed form submit workflow
Feb 5, 2019
2e773ed
removed revision id validation
Feb 5, 2019
e8497a1
removed print statement
Feb 5, 2019
8013eb2
removed unused import
Feb 5, 2019
7d460d3
updated draft model and using composite key
Feb 7, 2019
193416a
Send last_revision_id to render_form
vidya-ram Feb 7, 2019
2df3296
various fixes
Feb 7, 2019
e075640
update form post url
vidya-ram Feb 7, 2019
2ec4fb8
Merge branch 'autosave' of github.com:hasgeek/funnel into autosave
Feb 7, 2019
3d2b35f
Update form id
vidya-ram Feb 7, 2019
a9f1933
Merge branch 'autosave' of github.com:hasgeek/funnel into autosave
vidya-ram Feb 7, 2019
3bf1d4e
fixed action url, only updating fields sent in request
Feb 7, 2019
aadd9f8
Fix form field selector
vidya-ram Feb 7, 2019
a0b660e
Add missing semicolon.
vidya-ram Feb 8, 2019
90a12ff
Change to draft_revision. Use updated form review field.
vidya-ram Feb 9, 2019
03be511
added csrf check for draft autosave
Feb 10, 2019
49e91eb
fixed draft model
Feb 10, 2019
dea2a95
fixed draft delete and invalid revision ID handling
Feb 11, 2019
96af04a
Merge branch 'master' of github.com:hasgeek/funnel into autosave
Feb 13, 2019
b703846
updated down revision
Feb 13, 2019
82f87da
update down revision
Feb 13, 2019
e5050fd
fixed revision check logic
Feb 13, 2019
dd6499c
added DraftModelViewMixin
Feb 14, 2019
7866c6e
fixed draftmixin
Feb 15, 2019
2ab6a44
minor fixes
Feb 18, 2019
9f1c3f5
changed error format
Feb 19, 2019
5bb58c6
fixed multidict update
Feb 19, 2019
91f3d93
Display the error message from the server.
vidya-ram Feb 19, 2019
30d0b2e
removed redundant statement
Feb 19, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion funnel/forms/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from baseframe import __
import baseframe.forms as forms
from baseframe.forms.sqlalchemy import AvailableName, QuerySelectField
from ..models import RSVP_STATUS
from ..models import RSVP_STATUS, Project

__all__ = [
'EventForm', 'ProjectForm', 'ProjectTransitionForm', 'RsvpForm',
Expand Down Expand Up @@ -76,6 +76,8 @@ def set_queries(self):
self.admin_team.query = profile_teams
self.review_team.query = profile_teams
self.checkin_team.query = profile_teams
self.parent_project.query = Project.query.filter(
Project.profile == self.edit_obj.profile, Project.id != self.edit_obj.id, Project.parent_project == None) # NOQA

def validate_bg_color(self, field):
if not valid_color_re.match(field.data):
Expand Down
19 changes: 10 additions & 9 deletions funnel/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@

from coaster.sqlalchemy import (TimestampMixin, UuidMixin, BaseMixin, BaseNameMixin,
BaseScopedNameMixin, BaseScopedIdNameMixin, BaseIdNameMixin, MarkdownColumn,
JsonDict, CoordinatesMixin, make_timestamp_columns)
JsonDict, NoIdMixin, CoordinatesMixin, make_timestamp_columns)
from coaster.db import db

from .user import *
from .profile import *
from .commentvote import *
from .contact_exchange import *
from .draft import *
from .event import *
from .feedback import *
from .profile import *
from .project import *
from .section import *
from .usergroup import *
from .proposal import *
from .feedback import *
from .rsvp import *
from .section import *
from .session import *
from .user import *
from .usergroup import *
iambibhas marked this conversation as resolved.
Show resolved Hide resolved
from .venue import *
from .rsvp import *
from .event import *
from .contact_exchange import *
25 changes: 25 additions & 0 deletions funnel/models/draft.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# -*- coding: utf-8 -*-

from sqlalchemy_utils import UUIDType
from werkzeug.datastructures import MultiDict
from . import db, JsonDict, NoIdMixin

__all__ = ['Draft']


class Draft(NoIdMixin, db.Model):
"""Store for autosaved, unvalidated drafts on behalf of other models"""
__tablename__ = 'draft'

table = db.Column(db.UnicodeText, primary_key=True)
table_row_id = db.Column(UUIDType(binary=False), primary_key=True)
body = db.Column(JsonDict, nullable=False, server_default='{}')
revision = db.Column(UUIDType(binary=False))

@property
def formdata(self):
return MultiDict(self.body['form']) if 'form' in self.body else MultiDict({})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return MultiDict(self.body.get('form', {}))


@formdata.setter
def formdata(self, value):
self.body = {'form': value}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.body['form'] = value. Don't mess with anything else in self.body.

97 changes: 97 additions & 0 deletions funnel/templates/formlayout.html.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@
{%- endassets -%}
{% endblock %}

{% block contentwrapper %}
<div class="grid">
<div class="grid__col-xs-12">
{%- if autosave %}
<div><p class="mui--text-subhead mui--text-light mui--pull-right" id="autosave-msg"></p></div>
{% endif %}
{% block content %}{% endblock %}
</div>
</div>
{% endblock %}

{% block pagescripts %}
{% assets "js_codemirrormarkdown" -%}
<script type="text/javascript" src="{{ ASSET_URL }}"></script>
Expand All @@ -17,3 +28,89 @@
<script type="text/javascript" src="{{ ASSET_URL }}"></script>
{%- endassets -%}
{% endblock %}

{% block layoutscripts %}
{%- if autosave %}
<script type="text/javascript">
$(function() {
var typingTimer;
var typingWaitInterval = 1000; // wait till user stops typing for one second to send form data
var waitingForResponse = false;
var lastSavedData = '';

$('input[name="form.revision"]').val() ? $('#autosave-msg').text('These changes have not been published yet.') : '';

$('#{{ ref_id }}').on('change', function(e) {
autosaveForm();
});

$('#{{ ref_id }}').on('keyup', function(e) {
if(typingTimer) clearTimeout(typingTimer);
typingTimer = setTimeout(autosaveForm, typingWaitInterval);
});

function autosaveForm() {
var actionUrl = $('#{{ ref_id }}').attr('action');
var sep = (actionUrl.indexOf('?') === -1) ? '?' : '&';
if(!waitingForResponse && haveDirtyFields()) {
$.ajax({
type: 'POST',
url: actionUrl + sep + 'form.autosave=true',
data: $("#{{ ref_id }}").serialize(),
dataType: 'json',
timeout: 15000,
beforeSend: function() {
$('#autosave-msg').text('Autosaving...');
lastSavedData = $("#{{ ref_id }}").find('[type!="hidden"]').serialize();
waitingForResponse = true;
},
success: function (remoteData) {
// Todo: Update window.history.pushState for new form
$('#autosave-msg').text('Changes saved but not published');
if(remoteData.revision) {
$('input[name="form.revision"]').val(remoteData.revision);
}
waitingForResponse = false;
autosaveForm();
},
error: function (response) {
var errorMsg = '';
waitingForResponse = false;
if (response.readyState === 4) {
if (response.status === 500) {
errorMsg ='Internal Server Error. Please reload and try again.';
} else if(response.status === 400) {
// There is a version mismatch, notify user to reload the page.
waitingForResponse = true;
errorMsg = 'This page has been edited elsewhere. Please reload the page to avoid losing those changes.';
}
} else {
errorMsg = 'Unable to connect. Please reload and try again.';
}
$('#autosave-msg').text(errorMsg);
window.toastr.error(errorMsg);
},
});
}

function haveDirtyFields() {
var latestFormData = $('#{{ ref_id }}').find('[type!="hidden"]').serialize();
if (latestFormData !== lastSavedData) {
return true;
}
}

$(window).bind('beforeunload', function() {
if(haveDirtyFields()){
return 'You have unsaved changes on this page. Do you want to leave this page?';
}
});
}
});
</script>
{%- endif %}
{% block footerscripts %}{% endblock %}
{% endblock %}



84 changes: 82 additions & 2 deletions funnel/views/mixins.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
from uuid import uuid4
from flask import abort, g, redirect, request
from baseframe import _, forms
from coaster.utils import require_one_of
from ..models import (Project, Profile, ProjectRedirect, Proposal, ProposalRedirect, Session,
UserGroup, Venue, VenueRoom, Section)
from werkzeug.datastructures import MultiDict
from ..models import (Draft, Project, Profile, ProjectRedirect, Proposal, ProposalRedirect, Session,
UserGroup, Venue, VenueRoom, Section, db)


class ProjectViewMixin(object):
Expand Down Expand Up @@ -130,3 +133,80 @@ def loader(self, profile, project, section):
).first_or_404()
g.profile = section.project.profile
return section


class DraftViewMixin(object):
def get_draft(self, obj=None):
"""
Returns the draft object for `obj`. Defaults to `self.obj`.
`obj` is needed in case of multi-model views.
"""
obj = obj if obj is not None else self.obj
return Draft.query.get((self.model.__tablename__, obj.uuid))

def delete_draft(self, obj=None):
"""
Deletes draft for `obj`, or `self.obj` if `obj` is `None`.
"""
draft = self.get_draft(obj)
if draft is not None:
db.session.delete(draft)
else:
raise ValueError(_("There is no draft for the given object."))

def get_draft_data(self, obj=None):
"""
Returns a tuple of the current draft revision and the formdata needed to initialize forms
"""
draft = self.get_draft(obj)
if draft is not None:
return draft.revision, draft.formdata
else:
return None, None

def autosave_post(self, obj=None):
"""
Handles autosave POST requests
"""
obj = obj if obj is not None else self.obj
if 'form.revision' not in request.form:
# as form.autosave is true, the form should have `form.revision` field even if it's empty
return {'error': _("Form must contain a revision ID.")}, 400

# CSRF check
if forms.Form().validate_on_submit():
iambibhas marked this conversation as resolved.
Show resolved Hide resolved
incoming_data = MultiDict(request.form.items(multi=True))
client_revision = incoming_data.pop('form.revision')
incoming_data.pop('csrf_token', None)

# find the last draft
draft = self.get_draft(obj)

if draft is not None:
if client_revision is None or (client_revision is not None and str(draft.revision) != client_revision):
# draft exists, but the form did not send a revision ID,
# OR revision ID sent by client does not match the last revision ID
return {'error': _("There has been changes to this draft since you last edited it. Please reload.")}, 400
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There have been changes to this item since you last edited it. Please reload this page.

elif client_revision is not None and str(draft.revision) == client_revision:
# revision ID sent my client matches, save updated draft data and update revision ID
existing = draft.formdata
for key in incoming_data.keys():
if existing[key] != incoming_data[key]:
existing[key] = incoming_data[key]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not required. You can simply do existing.update(incoming_data)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that doesn't work as existing is a MultiDict and the update() method doesn't replace keys, it just adds and extends.

draft.body = {'form': existing}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set draft.formdata

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

draft.formdata = existing

draft.revision = uuid4()
elif draft is None and client_revision:
# The form contains a revision ID but no draft exists.
# Somebody is making autosave requests with an invalid draft ID.
return {'error': _("Invalid revision ID or the existing changes have been submitted already. Please reload.")}, 400
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can happen in the following scenario:

  1. Tab A is opened to edit the document
  2. Tab A makes some changes and creates a draft
  3. Tab B opens the same document and gets the draft
  4. Tab B makes some changes and updates the draft
  5. Tab B submits, updating the document and deleting the draft
  6. Tab A now edits and gets to this error

A variation of this happens if step 2 is skipped. Tab A will now make changes on an obsolete document, losing Tab B's changes. No error will be displayed. To fix this we have to track revision id of not just the draft, but the source document as well. This deserves to be a separate ticket since it's some way off from our original intent of providing autosave functionality.

else:
# no draft exists, create one
draft = Draft(
table=Project.__tablename__, table_row_id=obj.uuid,
formdata=incoming_data, revision=uuid4()
)
db.session.add(draft)
db.session.commit()
return {'revision': draft.revision}
else:
return {'error': _("Invalid CSRF token")}, 400
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recall we had established a convention for JSON responses earlier:

  1. Success: {"status": "ok", "result": {...}}
  2. Error: {"status": "error", "error": "error_identifier", "error_description": "English message"}

The first one I'm ambivalent about, since HTTP status (200 vs 400) is a sufficient indicator of whether the request was successful or not. The second is relevant. The front-end should be able to look up a static error code and invoke custom behaviour. The error_description is shown when the front-end does not recognise the error.

57 changes: 41 additions & 16 deletions funnel/views/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from baseframe import _, forms
from baseframe.forms import render_form
from coaster.auth import current_auth
from coaster.utils import getbool
from coaster.views import jsonp, route, render_with, requires_permission, UrlForView, ModelView

from .. import app, funnelapp, lastuser
Expand All @@ -16,7 +17,7 @@
from .schedule import schedule_data
from .venue import venue_data, room_data
from .section import section_data
from .mixins import ProjectViewMixin, ProfileViewMixin
from .mixins import ProjectViewMixin, ProfileViewMixin, DraftViewMixin
from .decorators import legacy_redirect


Expand Down Expand Up @@ -75,7 +76,7 @@ class FunnelProfileProjectView(ProfileProjectView):


@route('/<profile>/<project>/')
class ProjectView(ProjectViewMixin, UrlForView, ModelView):
class ProjectView(ProjectViewMixin, DraftViewMixin, UrlForView, ModelView):
__decorators__ = [legacy_redirect]

@route('')
Expand Down Expand Up @@ -129,23 +130,47 @@ def csv(self):
headers=[('Content-Disposition', 'attachment;filename="{project}.csv"'.format(project=self.obj.name))])

@route('edit', methods=['GET', 'POST'])
@render_with(json=True)
@lastuser.requires_login
@requires_permission('edit_project')
def edit(self):
if self.obj.parent_project:
form = SubprojectForm(obj=self.obj, model=Project)
else:
form = ProjectForm(obj=self.obj, parent=self.obj.profile, model=Project)
form.parent_project.query = Project.query.filter(Project.profile == self.obj.profile, Project.id != self.obj.id, Project.parent_project == None) # NOQA
if request.method == 'GET' and not self.obj.timezone:
form.timezone.data = current_app.config.get('TIMEZONE')
if form.validate_on_submit():
form.populate_obj(self.obj)
db.session.commit()
flash(_("Your changes have been saved"), 'info')
tag_locations.queue(self.obj.id)
return redirect(self.obj.url_for(), code=303)
return render_form(form=form, title=_("Edit project"), submit=_("Save changes"))
if request.method == 'GET':
# find draft if it exists
draft_revision, initial_formdata = self.get_draft_data()

# initialize forms with draft initial formdata.
# if no draft exists, initial_formdata is None. wtforms ignore formdata if it's None.
if self.obj.parent_project:
form = SubprojectForm(obj=self.obj, model=Project, formdata=initial_formdata)
else:
form = ProjectForm(obj=self.obj, parent=self.obj.profile, model=Project, formdata=initial_formdata)

if not self.obj.timezone:
form.timezone.data = current_app.config.get('TIMEZONE')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated fix, but this should default to the user's timezone, not the app's. Use current_auth.user.timezone (it falls back to the app if the user has not specified a timezone).


return render_form(form=form, title=_("Edit project"), submit=_("Save changes"), autosave=True, draft_revision=draft_revision)
elif request.method == 'POST':
if getbool(request.args.get('form.autosave')):
return self.autosave_post()
else:
if self.obj.parent_project:
form = SubprojectForm(obj=self.obj, model=Project)
else:
form = ProjectForm(obj=self.obj, parent=self.obj.profile, model=Project)
if form.validate_on_submit():
form.populate_obj(self.obj)
db.session.commit()
flash(_("Your changes have been saved"), 'info')
tag_locations.queue(self.obj.id)

# find and delete draft if it exists
if self.get_draft() is not None:
self.delete_draft()
db.session.commit()

return redirect(self.obj.url_for(), code=303)
else:
return render_form(form=form, title=_("Edit project"), submit=_("Save changes"), autosave=True)

@route('boxoffice_data', methods=['GET', 'POST'])
@lastuser.requires_login
Expand Down
32 changes: 32 additions & 0 deletions migrations/versions/94ce3a9b7a3a_draft_model.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"""draft model

Revision ID: 94ce3a9b7a3a
Revises: c3069d33419a
Create Date: 2019-02-06 20:48:34.700795

"""

revision = '94ce3a9b7a3a'
down_revision = 'a9cb0e1c52ed'

from alembic import op
import sqlalchemy as sa

from sqlalchemy_utils.types.uuid import UUIDType
from coaster.sqlalchemy.columns import JsonDict


def upgrade():
op.create_table('draft',
sa.Column('created_at', sa.DateTime(), nullable=False),
sa.Column('updated_at', sa.DateTime(), nullable=False),
sa.Column('table', sa.UnicodeText(), nullable=False),
sa.Column('table_row_id', UUIDType(binary=False), nullable=False),
sa.Column('body', JsonDict(), server_default='{}', nullable=False),
sa.Column('revision', UUIDType(binary=False), nullable=True),
sa.PrimaryKeyConstraint('table', 'table_row_id')
)


def downgrade():
op.drop_table('draft')