FIX: various fixes to draft system

- destroyDraft which is called when we cancel a draft is now async,
  removing race conditions when you click "reply" to a post and are
  already editing. We used to trigger double dialogs for cancelling
  drafts which was confusing.

- Remove reply as new topic / reply as pm keys, they are no longer
  used and only caused confustion. For example we used to pop up a
  warning when you are composing a reply and flick to reply as
  new topic

- Remove createTopic key, this was a bug that proliferated. Whenever
  creating a topic via the C shortcut or clicking on new topic on full
  screen search the correct new topic draft key will be used
  consistently

- When abandoning an edit we now say "Are you sure you want to discard
  your changes" (instead of abandon your post which is confusing)
This commit is contained in:
Sam Saffron 2019-10-21 08:57:55 +11:00
parent 858cf5836c
commit 98d6cee7c7
11 changed files with 85 additions and 84 deletions

View File

@ -590,12 +590,7 @@ export default Ember.Component.extend({
_warnCannotSeeMention($preview) { _warnCannotSeeMention($preview) {
const composerDraftKey = this.get("composer.draftKey"); const composerDraftKey = this.get("composer.draftKey");
if ( if (composerDraftKey === Composer.NEW_PRIVATE_MESSAGE_KEY) {
composerDraftKey === Composer.CREATE_TOPIC ||
composerDraftKey === Composer.NEW_PRIVATE_MESSAGE_KEY ||
composerDraftKey === Composer.REPLY_AS_NEW_TOPIC_KEY ||
composerDraftKey === Composer.REPLY_AS_NEW_PRIVATE_MESSAGE_KEY
) {
return; return;
} }

View File

@ -78,14 +78,6 @@ export default Ember.Controller.extend({
topicController: Ember.inject.controller("topic"), topicController: Ember.inject.controller("topic"),
router: Ember.inject.service(), router: Ember.inject.service(),
replyAsNewTopicDraft: Ember.computed.equal(
"model.draftKey",
Composer.REPLY_AS_NEW_TOPIC_KEY
),
replyAsNewPrivateMessageDraft: Ember.computed.equal(
"model.draftKey",
Composer.REPLY_AS_NEW_PRIVATE_MESSAGE_KEY
),
checkedMessages: false, checkedMessages: false,
messageCount: null, messageCount: null,
showEditReason: false, showEditReason: false,
@ -677,47 +669,54 @@ export default Ember.Controller.extend({
} }
} }
this.destroyDraft(); return this.destroyDraft().then(() => {
this.close(); this.close();
this.appEvents.trigger("post-stream:refresh"); this.appEvents.trigger("post-stream:refresh");
return result; return result;
});
} }
// If user "created a new topic/post" or "replied as a new topic" successfully, remove the draft. // If user "created a new topic/post" or "replied as a new topic" successfully, remove the draft.
if ( let destroyDraftPromise;
result.responseJson.action === "create_post" ||
this.replyAsNewTopicDraft ||
this.replyAsNewPrivateMessageDraft
) {
this.destroyDraft();
}
if (this.get("model.editingPost")) {
this.appEvents.trigger("post-stream:refresh", {
id: parseInt(result.responseJson.id)
});
if (result.responseJson.post.post_number === 1) {
this.appEvents.trigger("header:update-topic", composer.topic);
}
} else {
this.appEvents.trigger("post-stream:refresh");
}
if (result.responseJson.action === "create_post") { if (result.responseJson.action === "create_post") {
this.appEvents.trigger("post:highlight", result.payload.post_number); destroyDraftPromise = this.destroyDraft();
}
this.close();
const currentUser = this.currentUser;
if (composer.creatingTopic) {
currentUser.set("topic_count", currentUser.topic_count + 1);
} else { } else {
currentUser.set("reply_count", currentUser.reply_count + 1); destroyDraftPromise = Ember.RSVP.Promise.resolve();
} }
const post = result.target; return destroyDraftPromise.then(() => {
if (post && !staged) { if (this.get("model.editingPost")) {
DiscourseURL.routeTo(post.url); this.appEvents.trigger("post-stream:refresh", {
} id: parseInt(result.responseJson.id)
});
if (result.responseJson.post.post_number === 1) {
this.appEvents.trigger("header:update-topic", composer.topic);
}
} else {
this.appEvents.trigger("post-stream:refresh");
}
if (result.responseJson.action === "create_post") {
this.appEvents.trigger(
"post:highlight",
result.payload.post_number
);
}
this.close();
const currentUser = this.currentUser;
if (composer.creatingTopic) {
currentUser.set("topic_count", currentUser.topic_count + 1);
} else {
currentUser.set("reply_count", currentUser.reply_count + 1);
}
const post = result.target;
if (post && !staged) {
DiscourseURL.routeTo(post.url);
}
});
}) })
.catch(error => { .catch(error => {
composer.set("disableDrafts", false); composer.set("disableDrafts", false);
@ -783,11 +782,7 @@ export default Ember.Controller.extend({
}); });
// Scope the categories drop down to the category we opened the composer with. // Scope the categories drop down to the category we opened the composer with.
if ( if (opts.categoryId && !opts.disableScopedCategory) {
opts.categoryId &&
opts.draftKey !== "reply_as_new_topic" &&
!opts.disableScopedCategory
) {
const category = this.site.categories.findBy("id", opts.categoryId); const category = this.site.categories.findBy("id", opts.categoryId);
if (category) { if (category) {
this.set("scopedCategoryId", opts.categoryId); this.set("scopedCategoryId", opts.categoryId);
@ -847,7 +842,6 @@ export default Ember.Controller.extend({
data.draft = undefined; data.draft = undefined;
return data; return data;
} }
return this.confirmDraftAbandon(data); return this.confirmDraftAbandon(data);
}) })
.then(data => { .then(data => {
@ -862,7 +856,9 @@ export default Ember.Controller.extend({
// otherwise, do the draft check async // otherwise, do the draft check async
else if (!opts.draft && !opts.skipDraftCheck) { else if (!opts.draft && !opts.skipDraftCheck) {
Draft.get(opts.draftKey) Draft.get(opts.draftKey)
.then(data => this.confirmDraftAbandon(data)) .then(data => {
return this.confirmDraftAbandon(data);
})
.then(data => { .then(data => {
if (data.draft) { if (data.draft) {
opts.draft = data.draft; opts.draft = data.draft;
@ -943,9 +939,11 @@ export default Ember.Controller.extend({
this.send("clearTopicDraft"); this.send("clearTopicDraft");
} }
Draft.clear(key, this.get("model.draftSequence")).then(() => return Draft.clear(key, this.get("model.draftSequence")).then(() =>
this.appEvents.trigger("draft:destroyed", key) this.appEvents.trigger("draft:destroyed", key)
); );
} else {
return Ember.RSVP.Promise.resolve();
} }
}, },
@ -985,13 +983,16 @@ export default Ember.Controller.extend({
}, },
cancelComposer(differentDraft = false) { cancelComposer(differentDraft = false) {
const keyPrefix =
this.model.action === "edit" ? "post.abandon_edit" : "post.abandon";
return new Ember.RSVP.Promise(resolve => { return new Ember.RSVP.Promise(resolve => {
if (this.get("model.hasMetaData") || this.get("model.replyDirty")) { if (this.get("model.hasMetaData") || this.get("model.replyDirty")) {
bootbox.dialog(I18n.t("post.abandon.confirm"), [ bootbox.dialog(I18n.t(keyPrefix + ".confirm"), [
{ {
label: differentDraft label: differentDraft
? I18n.t("post.abandon.no_save_draft") ? I18n.t(keyPrefix + ".no_save_draft")
: I18n.t("post.abandon.no_value"), : I18n.t(keyPrefix + ".no_value"),
callback: () => { callback: () => {
// cancel composer without destroying draft on new draft context // cancel composer without destroying draft on new draft context
if (differentDraft) { if (differentDraft) {
@ -1002,24 +1003,26 @@ export default Ember.Controller.extend({
} }
}, },
{ {
label: I18n.t("post.abandon.yes_value"), label: I18n.t(keyPrefix + ".yes_value"),
class: "btn-danger", class: "btn-danger",
callback: result => { callback: result => {
if (result) { if (result) {
this.destroyDraft(); this.destroyDraft().then(() => {
this.model.clearState(); this.model.clearState();
this.close(); this.close();
resolve(); resolve();
});
} }
} }
} }
]); ]);
} else { } else {
// it is possible there is some sort of crazy draft with no body ... just give up on it // it is possible there is some sort of crazy draft with no body ... just give up on it
this.destroyDraft(); this.destroyDraft().then(() => {
this.model.clearState(); this.model.clearState();
this.close(); this.close();
resolve(); resolve();
});
} }
}); });
}, },

View File

@ -285,7 +285,7 @@ export default Ember.Controller.extend({
} }
this.composer.open({ this.composer.open({
action: Composer.CREATE_TOPIC, action: Composer.CREATE_TOPIC,
draftKey: Composer.CREATE_TOPIC, draftKey: Composer.NEW_TOPIC_KEY,
topicCategory topicCategory
}); });
}, },

View File

@ -965,13 +965,13 @@ export default Ember.Controller.extend(bufferedProperty("model"), {
options = { options = {
action: Composer.PRIVATE_MESSAGE, action: Composer.PRIVATE_MESSAGE,
archetypeId: "private_message", archetypeId: "private_message",
draftKey: Composer.REPLY_AS_NEW_PRIVATE_MESSAGE_KEY, draftKey: post.topic.draft_key,
usernames: usernames usernames: usernames
}; };
} else { } else {
options = { options = {
action: Composer.CREATE_TOPIC, action: Composer.CREATE_TOPIC,
draftKey: Composer.REPLY_AS_NEW_TOPIC_KEY, draftKey: post.topic.draft_key,
categoryId: this.get("model.category.id") categoryId: this.get("model.category.id")
}; };
} }

View File

@ -241,7 +241,7 @@ export default {
this.container.lookup("controller:composer").open({ this.container.lookup("controller:composer").open({
action: Composer.CREATE_TOPIC, action: Composer.CREATE_TOPIC,
draftKey: Composer.CREATE_TOPIC draftKey: Composer.NEW_TOPIC_KEY
}); });
}, },

View File

@ -15,7 +15,7 @@ export default Ember.Mixin.create({
this.controllerFor("composer").open({ this.controllerFor("composer").open({
categoryId, categoryId,
action: Composer.CREATE_TOPIC, action: Composer.CREATE_TOPIC,
draftKey: controller.get("model.draft_key") || Composer.CREATE_TOPIC, draftKey: controller.get("model.draft_key") || Composer.NEW_TOPIC_KEY,
draftSequence: controller.get("model.draft_sequence") || 0 draftSequence: controller.get("model.draft_sequence") || 0
}); });
}, },

View File

@ -17,12 +17,10 @@ export const CREATE_TOPIC = "createTopic",
CREATE_SHARED_DRAFT = "createSharedDraft", CREATE_SHARED_DRAFT = "createSharedDraft",
EDIT_SHARED_DRAFT = "editSharedDraft", EDIT_SHARED_DRAFT = "editSharedDraft",
PRIVATE_MESSAGE = "privateMessage", PRIVATE_MESSAGE = "privateMessage",
NEW_PRIVATE_MESSAGE_KEY = "new_private_message",
NEW_TOPIC_KEY = "new_topic",
REPLY = "reply", REPLY = "reply",
EDIT = "edit", EDIT = "edit",
REPLY_AS_NEW_TOPIC_KEY = "reply_as_new_topic", NEW_PRIVATE_MESSAGE_KEY = "new_private_message",
REPLY_AS_NEW_PRIVATE_MESSAGE_KEY = "reply_as_new_private_message"; NEW_TOPIC_KEY = "new_topic";
function isEdit(action) { function isEdit(action) {
return action === EDIT || action === EDIT_SHARED_DRAFT; return action === EDIT || action === EDIT_SHARED_DRAFT;
@ -1151,8 +1149,7 @@ Composer.reopenClass({
// Draft key // Draft key
NEW_PRIVATE_MESSAGE_KEY, NEW_PRIVATE_MESSAGE_KEY,
REPLY_AS_NEW_TOPIC_KEY, NEW_TOPIC_KEY
REPLY_AS_NEW_PRIVATE_MESSAGE_KEY
}); });
export default Composer; export default Composer;

View File

@ -82,7 +82,7 @@ const ApplicationRoute = Discourse.Route.extend(OpenComposer, {
action: Composer.PRIVATE_MESSAGE, action: Composer.PRIVATE_MESSAGE,
usernames: recipient, usernames: recipient,
archetypeId: "private_message", archetypeId: "private_message",
draftKey: "new_private_message", draftKey: Composer.NEW_PRIVATE_MESSAGE_KEY,
reply, reply,
title title
}); });

View File

@ -1,4 +1,5 @@
import Draft from "discourse/models/draft"; import Draft from "discourse/models/draft";
import Composer from "discourse/models/composer";
export default Discourse.Route.extend({ export default Discourse.Route.extend({
renderTemplate() { renderTemplate() {
@ -17,7 +18,7 @@ export default Discourse.Route.extend({
if (data.draft) { if (data.draft) {
composerController.open({ composerController.open({
draft: data.draft, draft: data.draft,
draftKey: "new_private_message", draftKey: Composer.NEW_PRIVATE_MESSAGE_KEY,
ignoreIfChanged: true, ignoreIfChanged: true,
draftSequence: data.draft_sequence draftSequence: data.draft_sequence
}); });

View File

@ -5,8 +5,7 @@ import {
CREATE_TOPIC, CREATE_TOPIC,
CREATE_SHARED_DRAFT, CREATE_SHARED_DRAFT,
REPLY, REPLY,
EDIT, EDIT
NEW_PRIVATE_MESSAGE_KEY
} from "discourse/models/composer"; } from "discourse/models/composer";
// Component can get destroyed and lose state // Component can get destroyed and lose state
@ -209,7 +208,6 @@ export default DropdownSelectBoxComponent.extend({
_replyFromExisting(options, post, topic) { _replyFromExisting(options, post, topic) {
this.closeComposer(); this.closeComposer();
this.openComposer(options, post, topic); this.openComposer(options, post, topic);
}, },
@ -244,6 +242,7 @@ export default DropdownSelectBoxComponent.extend({
options.action = CREATE_TOPIC; options.action = CREATE_TOPIC;
options.categoryId = this.get("composerModel.topic.category.id"); options.categoryId = this.get("composerModel.topic.category.id");
options.disableScopedCategory = true; options.disableScopedCategory = true;
options.skipDraftCheck = true;
this._replyFromExisting(options, _postSnapshot, _topicSnapshot); this._replyFromExisting(options, _postSnapshot, _topicSnapshot);
}, },
@ -269,7 +268,7 @@ export default DropdownSelectBoxComponent.extend({
options.action = PRIVATE_MESSAGE; options.action = PRIVATE_MESSAGE;
options.usernames = usernames; options.usernames = usernames;
options.archetypeId = "private_message"; options.archetypeId = "private_message";
options.draftKey = NEW_PRIVATE_MESSAGE_KEY; options.skipDraftCheck = true;
this._replyFromExisting(options, _postSnapshot, _topicSnapshot); this._replyFromExisting(options, _postSnapshot, _topicSnapshot);
}, },

View File

@ -2444,6 +2444,12 @@ en:
attachment_upload_not_allowed_for_new_user: "Sorry, new users can not upload attachments." attachment_upload_not_allowed_for_new_user: "Sorry, new users can not upload attachments."
attachment_download_requires_login: "Sorry, you need to be logged in to download attachments." attachment_download_requires_login: "Sorry, you need to be logged in to download attachments."
abandon_edit:
confirm: "Are you sure you want to discard your changes?"
no_value: "No, keep"
no_save_draft: "No, save draft"
yes_value: "Yes, discard edit"
abandon: abandon:
confirm: "Are you sure you want to abandon your post?" confirm: "Are you sure you want to abandon your post?"
no_value: "No, keep" no_value: "No, keep"