From 34e4d82f1a505405b1518020bfef89f0bde7be87 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Wed, 14 Nov 2018 13:56:25 +0200 Subject: [PATCH] FEATURE: Report edit conflicts when saving draft. (#6585) --- .../discourse/models/composer.js.es6 | 31 ++++++++++++++----- .../javascripts/discourse/models/draft.js.es6 | 8 +++-- .../discourse/templates/composer.hbs | 3 ++ app/controllers/draft_controller.rb | 9 ++++++ config/locales/client.en.yml | 1 + spec/requests/draft_controller_spec.rb | 28 +++++++++++++++++ 6 files changed, 69 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/discourse/models/composer.js.es6 b/app/assets/javascripts/discourse/models/composer.js.es6 index aae9a31055f..2ebbd8b1a67 100644 --- a/app/assets/javascripts/discourse/models/composer.js.es6 +++ b/app/assets/javascripts/discourse/models/composer.js.es6 @@ -977,6 +977,7 @@ const Composer = RestModel.extend({ const data = { reply: this.get("reply"), + originalText: this.get("originalText"), action: this.get("action"), title: this.get("title"), categoryId: this.get("categoryId"), @@ -991,22 +992,35 @@ const Composer = RestModel.extend({ noBump: this.get("noBump") }; - this.set("draftStatus", I18n.t("composer.saving_draft_tip")); - - const composer = this; + this.setProperties({ + draftStatus: I18n.t("composer.saving_draft_tip"), + draftConflictUser: null + }); if (this._clearingStatus) { Em.run.cancel(this._clearingStatus); this._clearingStatus = null; } - // try to save the draft return Draft.save(this.get("draftKey"), this.get("draftSequence"), data) - .then(function() { - composer.set("draftStatus", I18n.t("composer.saved_draft_tip")); + .then(result => { + if (result.conflict_user) { + this.setProperties({ + draftStatus: I18n.t("composer.edit_conflict"), + draftConflictUser: result.conflict_user + }); + } else { + this.setProperties({ + draftStatus: I18n.t("composer.saved_draft_tip"), + draftConflictUser: null + }); + } }) - .catch(function() { - composer.set("draftStatus", I18n.t("composer.drafts_offline")); + .catch(() => { + this.setProperties({ + draftStatus: I18n.t("composer.drafts_offline"), + draftConflictUser: null + }); }); }, @@ -1019,6 +1033,7 @@ const Composer = RestModel.extend({ this, function() { self.set("draftStatus", null); + self.set("draftConflictUser", null); self._clearingStatus = null; }, 1000 diff --git a/app/assets/javascripts/discourse/models/draft.js.es6 b/app/assets/javascripts/discourse/models/draft.js.es6 index 86a912f37d8..2f92256ca66 100644 --- a/app/assets/javascripts/discourse/models/draft.js.es6 +++ b/app/assets/javascripts/discourse/models/draft.js.es6 @@ -25,13 +25,15 @@ Draft.reopenClass({ }, save(key, sequence, data) { - data = typeof data === "string" ? data : JSON.stringify(data); + const dataJson = typeof data === "string" ? dataJson : JSON.stringify(data); return ajax("/draft.json", { type: "POST", data: { draft_key: key, - data: data, - sequence: sequence + data: dataJson, + sequence, + post_id: data.postId, + original_text: data.originalText } }); } diff --git a/app/assets/javascripts/discourse/templates/composer.hbs b/app/assets/javascripts/discourse/templates/composer.hbs index d60ed2e804d..f3e50c7561c 100644 --- a/app/assets/javascripts/discourse/templates/composer.hbs +++ b/app/assets/javascripts/discourse/templates/composer.hbs @@ -149,6 +149,9 @@ {{/if}}
+ {{#if model.draftConflictUser}} + {{avatar model.draftConflictUser imageSize="small"}} + {{/if}} {{model.draftStatus}}
diff --git a/app/controllers/draft_controller.rb b/app/controllers/draft_controller.rb index ebd0dd1bccb..9491c833e2a 100644 --- a/app/controllers/draft_controller.rb +++ b/app/controllers/draft_controller.rb @@ -10,6 +10,15 @@ class DraftController < ApplicationController def update Draft.set(current_user, params[:draft_key], params[:sequence].to_i, params[:data]) + + if params[:post_id] && params[:original_text] + post = Post.find_by(id: params[:post_id]) + if post && post.raw != params[:original_text] + conflict_user = BasicUserSerializer.new(post.last_editor, root: false) + return render json: success_json.merge(conflict_user: conflict_user) + end + end + render json: success_json end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 47658fd6c9b..9760176d738 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1356,6 +1356,7 @@ en: saved_local_draft_tip: "saved locally" similar_topics: "Your topic is similar to..." drafts_offline: "drafts offline" + edit_conflict: 'edit conflict' group_mentioned_limit: "Warning! You mentioned {{group}}, however this group has more members than the administrator configured mention limit of {{max}} users. Nobody will be notified. " group_mentioned: diff --git a/spec/requests/draft_controller_spec.rb b/spec/requests/draft_controller_spec.rb index 55f471a44d3..0c66de5ee46 100644 --- a/spec/requests/draft_controller_spec.rb +++ b/spec/requests/draft_controller_spec.rb @@ -13,6 +13,34 @@ describe DraftController do expect(Draft.get(user, 'xyz', 0)).to eq('my data') end + it 'checks for an conflict on update' do + user = sign_in(Fabricate(:user)) + post = Fabricate(:post, user: user) + + post "/draft.json", params: { + username: user.username, + draft_key: "topic", + sequence: 0, + data: "{}", + post_id: post.id, + original_text: post.raw + } + + expect(JSON.parse(response.body)['conflict_user']).to eq(nil) + + post "/draft.json", params: { + username: user.username, + draft_key: "topic", + sequence: 0, + data: "{}", + post_id: post.id, + original_text: "something else" + } + + expect(JSON.parse(response.body)['conflict_user']['id']).to eq(post.last_editor.id) + expect(JSON.parse(response.body)['conflict_user']).to include('avatar_template') + end + it 'destroys drafts when required' do user = sign_in(Fabricate(:user)) Draft.set(user, 'xxx', 0, 'hi')