Draft model and autosave on Project edit form #352
Conversation
funnel/views/project.py
Outdated
| elif request.method == 'POST': | ||
| if 'autosave' in request.form and request.form['autosave'] == 'true': | ||
| if 'revision' not in request.form: | ||
| return {'error': _("Form must contain a valid revision ID.")}, 400 |
There was a problem hiding this comment.
The first autosave will not have a revision. There is nothing to revise.
funnel/views/project.py
Outdated
| table=Project.__tablename__, table_row_id=self.obj.uuid, | ||
| body={'form': request.form.items(multi=True)}, revision=uuid4() | ||
| ) | ||
| db.session.add(draft) |
There was a problem hiding this comment.
This requires a CSRF check. Without the check, a malicious third party website can dump junk content into the Draft model by performing a cross-site POST (which is what CSRF protects from).
- Do a CSRF check using
Form().validate_on_submit()(the base Form object has nothing but a CSRF field). - Remove the CSRF field when saving to the Draft model. It will cause problems when restored later.
|
Change on this comment: #352 (comment)
|
funnel/models/draft.py
Outdated
|
|
||
| @property | ||
| def formdata(self): | ||
| return MultiDict(self.body['form']) if 'form' in self.body else MultiDict({}) |
There was a problem hiding this comment.
return MultiDict(self.body.get('form', {}))
funnel/models/draft.py
Outdated
|
|
||
| @formdata.setter | ||
| def formdata(self, value): | ||
| self.body = {'form': value} |
There was a problem hiding this comment.
self.body['form'] = value. Don't mess with anything else in self.body.
funnel/views/mixins.py
Outdated
| 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 |
There was a problem hiding this comment.
There have been changes to this item since you last edited it. Please reload this page.
funnel/views/mixins.py
Outdated
| for key in incoming_data.keys(): | ||
| if existing[key] != incoming_data[key]: | ||
| existing[key] = incoming_data[key] | ||
| draft.body = {'form': existing} |
| existing = draft.formdata | ||
| for key in incoming_data.keys(): | ||
| if existing[key] != incoming_data[key]: | ||
| existing[key] = incoming_data[key] |
There was a problem hiding this comment.
This is not required. You can simply do existing.update(incoming_data)
There was a problem hiding this comment.
that doesn't work as existing is a MultiDict and the update() method doesn't replace keys, it just adds and extends.
funnel/views/mixins.py
Outdated
| 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 |
There was a problem hiding this comment.
This can happen in the following scenario:
- Tab A is opened to edit the document
- Tab A makes some changes and creates a draft
- Tab B opens the same document and gets the draft
- Tab B makes some changes and updates the draft
- Tab B submits, updating the document and deleting the draft
- 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.
funnel/views/mixins.py
Outdated
| db.session.commit() | ||
| return {'revision': draft.revision} | ||
| else: | ||
| return {'error': _("Invalid CSRF token")}, 400 |
There was a problem hiding this comment.
I recall we had established a convention for JSON responses earlier:
- Success:
{"status": "ok", "result": {...}} - 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.
funnel/views/project.py
Outdated
| 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') |
There was a problem hiding this comment.
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).
* Use ES6 JS. Use webpack to build and transpile ES6 code. * Add scroll active menu and swipe functionality. * Sort schedule event days. * Change buttons design of the header. * Add check to find if eventday is present. * Avoid using global names. * Add support to lazy load images on the home page * Add script folder. Change to lazy load on hasgeek index page. * Sync with master. * added Makefile * Updated package-lock.json. * Fix the mui class name. * added newline * Remove default lat and lon values. * split makefile * Move view proposal btn to right. * Add support for autosave (wip) * Add autosave flag to form data. * added draft model and initial view * fixed edit endpoint * Update server response flag in error cases. * fixed creation of draft * Check form data changes before sending to backend. * Refactor checking for dirty fields of the form. * added timestamp to draft model, fixed form submit workflow * removed revision id validation * removed print statement * removed unused import * updated draft model and using composite key * Send last_revision_id to render_form * various fixes * update form post url * Update form id * fixed action url, only updating fields sent in request * Fix form field selector * Add missing semicolon. * Change to draft_revision. Use updated form review field. * added csrf check for draft autosave * fixed draft model * fixed draft delete and invalid revision ID handling * updated down revision * update down revision * fixed revision check logic * added DraftModelViewMixin * fixed draftmixin * minor fixes * changed error format * fixed multidict update * Display the error message from the server. * removed redundant statement
This PR introduces Draft model as proposed in hasgeek/baseframe#207 in
Projectedit form.Depends on hasgeek/baseframe#208