Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Draft model and autosave on Project edit form #352
Changes from 41 commits
fd29b97
5e0c866
29ad207
468cc92
e20f3f0
86b5449
6db29ca
45cc1ec
21c55fe
3404dc3
2ba252a
13bb27f
2009366
4e3f037
e32971e
cbae311
22cae87
7154dd2
acdb758
beaa65f
bc25e3e
f1fe677
94f25bb
07b1c5f
c2d1bc0
4d31f12
d25d236
51ac07f
daf4270
5df6d23
3a7b9f2
34a5295
7f3ae6a
51e6dfd
ae27841
059430d
35715f2
2e773ed
e8497a1
8013eb2
7d460d3
193416a
2df3296
e075640
2ec4fb8
3d2b35f
a9f1933
3bf1d4e
aadd9f8
a0b660e
90a12ff
03be511
49e91eb
dea2a95
96af04a
b703846
82f87da
e5050fd
dd6499c
7866c6e
2ab6a44
9f1c3f5
5bb58c6
91f3d93
30d0b2e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is checking the entire form. Is this efficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vidya-ram ^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is event to detect user typing into input fields and event to detect form changes for fields like select, radio. Incase user selects browser's autocomplete, both these events are triggered and we end up sending two ajax events. This was added to avoid this.
We are actually comparing two longs strings since
serialize
returns text string in standard URL-encoded notation.There was a problem hiding this comment.
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).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will not have a revision if there was no draft when the user submitted. This can happen if the user submits without editing, or JavaScript is disabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first autosave will not have a revision. There is nothing to revise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
request.form.items(multi=True)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code assumes the client is sending the entire form for autosave. Ideally it should only be sending dirty fields, and we should be updating here instead of replacing. (I'm okay with this inefficient implementation for a first pass, but it will need to be fixed.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated it to only take in changes available in data sent by client. check once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
Form().validate_on_submit()
(the base Form object has nothing but a CSRF field).