From 8ce6aa3e7d721e78bdd9993a2c1df9811b7db54b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= <1108771+megothss@users.noreply.github.com> Date: Thu, 5 Dec 2024 14:21:29 -0300 Subject: [PATCH] DEV: Ensure composer opens properly when the post topic is not yet loaded (#30114) This PR fixes an error that would be thrown in some edge cases where the composer is opened for a post instance without an associated topic model already loaded. An example of such edge cases would be, a plugin trying to edit a post outside the topic view. This was causing an error that would prevent the composer from being opened. --- .../discourse/app/models/composer.js | 97 +++++++++---------- .../javascripts/discourse/app/models/topic.js | 9 +- .../discourse/tests/unit/models/topic-test.js | 9 ++ 3 files changed, 64 insertions(+), 51 deletions(-) diff --git a/app/assets/javascripts/discourse/app/models/composer.js b/app/assets/javascripts/discourse/app/models/composer.js index ae9d5d21153..2808bf508bc 100644 --- a/app/assets/javascripts/discourse/app/models/composer.js +++ b/app/assets/javascripts/discourse/app/models/composer.js @@ -23,6 +23,7 @@ import discourseComputed from "discourse-common/utils/decorators"; import { i18n } from "discourse-i18n"; let _customizations = []; + export function registerCustomizationCallback(cb) { _customizations.push(cb); } @@ -804,29 +805,29 @@ export default class Composer extends RestModel { } /** - Open a composer + Open a composer - @method open - @param {Object} opts - @param {String} opts.action The action we're performing: edit, reply, createTopic, createSharedDraft, privateMessage - @param {String} opts.draftKey - @param {String} opts.draftSequence - @param {Post} [opts.post] The post we're replying to, if present - @param {Topic} [opts.topic] The topic we're replying to, if present - @param {String} [opts.quote] If we're opening a reply from a quote, the quote we're making - @param {String} [opts.reply] - @param {String} [opts.recipients] - @param {Number} [opts.composerTime] - @param {Number} [opts.typingTime] - @param {Boolean} [opts.whisper] - @param {Boolean} [opts.noBump] - @param {String} [opts.archetypeId] One of `site.archetypes` e.g. `regular` or `private_message` - @param {Object} [opts.metaData] - @param {Number} [opts.categoryId] - @param {Number} [opts.postId] - @param {Number} [opts.destinationCategoryId] - @param {String} [opts.title] - **/ + @method open + @param {Object} opts + @param {String} opts.action The action we're performing: edit, reply, createTopic, createSharedDraft, privateMessage + @param {String} opts.draftKey + @param {String} opts.draftSequence + @param {Post} [opts.post] The post we're replying to, if present + @param {Topic} [opts.topic] The topic we're replying to, if present + @param {String} [opts.quote] If we're opening a reply from a quote, the quote we're making + @param {String} [opts.reply] + @param {String} [opts.recipients] + @param {Number} [opts.composerTime] + @param {Number} [opts.typingTime] + @param {Boolean} [opts.whisper] + @param {Boolean} [opts.noBump] + @param {String} [opts.archetypeId] One of `site.archetypes` e.g. `regular` or `private_message` + @param {Object} [opts.metaData] + @param {Number} [opts.categoryId] + @param {Number} [opts.postId] + @param {Number} [opts.destinationCategoryId] + @param {String} [opts.title] + **/ open(opts) { let promise = Promise.resolve(); @@ -886,7 +887,18 @@ export default class Composer extends RestModel { }); if (!this.topic) { - this.set("topic", opts.post.topic); + if (opts.post.topic) { + this.set("topic", opts.post.topic); + } else { + // handles the edge cases where the topic model is not loaded in the post model and the store does not have a + // topic for the post, e.g., make a post then edit right away, edit a post outside the post stream, etc. + promise = promise.then(async () => { + const data = await Topic.find(opts.post.topic_id, {}); + const topic = this.store.createRecord("topic", data); + this.post.set("topic", topic); + this.set("topic", topic); + }); + } } } else if (opts.postId) { promise = promise.then(() => @@ -935,37 +947,22 @@ export default class Composer extends RestModel { } this.setProperties(topicProps); - promise = promise.then(() => { - let rawPromise = this.store.find("post", opts.post.id).then((post) => { - this.setProperties({ - post, - reply: post.raw, - originalText: post.raw, - }); - - if (post.post_number === 1 && this.canEditTitle) { - this.setProperties({ - originalTitle: post.topic.title, - originalTags: post.topic.tags, - }); - } + promise = promise.then(async () => { + const post = await this.store.find("post", opts.post.id); + this.setProperties({ + post, + reply: post.raw, + originalText: post.raw, }); - // edge case ... make a post then edit right away - // store does not have topic for the post - if (this.topic && this.topic.id === this.post.topic_id) { - // nothing to do ... we have the right topic - } else { - rawPromise = this.store - .find("topic", this.post.topic_id) - .then((topic) => { - this.set("topic", topic); - }); + if (post.post_number === 1 && this.canEditTitle) { + this.setProperties({ + originalTitle: this.topic.title, + originalTags: this.topic.tags, + }); } - return rawPromise.then(() => { - this.appEvents.trigger("composer:reply-reloaded", this); - }); + this.appEvents.trigger("composer:reply-reloaded", this); }); } else if (opts.action === REPLY && opts.quote) { this.set("reply", opts.quote); diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index b44c6075620..118b54549b2 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -19,6 +19,7 @@ import ActionSummary from "discourse/models/action-summary"; import Bookmark from "discourse/models/bookmark"; import RestModel from "discourse/models/rest"; import Site from "discourse/models/site"; +import TopicDetails from "discourse/models/topic-details"; import { flushMap } from "discourse/services/store"; import deprecated from "discourse-common/lib/deprecated"; import getURL from "discourse-common/lib/get-url"; @@ -461,7 +462,13 @@ export default class Topic extends RestModel { } set details(value) { - this._details = value; + if (value instanceof TopicDetails) { + this._details = value; + return; + } + + // we need to ensure that details is an instance of TopicDetails + this._details = this.store.createRecord("topicDetails", value); } @discourseComputed("visible") diff --git a/app/assets/javascripts/discourse/tests/unit/models/topic-test.js b/app/assets/javascripts/discourse/tests/unit/models/topic-test.js index 15b112ee26c..d13bd425cfc 100644 --- a/app/assets/javascripts/discourse/tests/unit/models/topic-test.js +++ b/app/assets/javascripts/discourse/tests/unit/models/topic-test.js @@ -4,6 +4,7 @@ import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; import { module, test } from "qunit"; import Category from "discourse/models/category"; import Topic from "discourse/models/topic"; +import TopicDetails from "discourse/models/topic-details"; module("Unit | Model | topic", function (hooks) { setupTest(hooks); @@ -108,6 +109,10 @@ module("Unit | Model | topic", function (hooks) { const topic = this.store.createRecord("topic", { id: 1234 }); const topicDetails = topic.details; + assert.true( + topicDetails instanceof TopicDetails, + "topicDetails is an instance of TopicDetails" + ); assert.present(topicDetails, "a topic has topicDetails after we create it"); assert.strictEqual( topicDetails.topic, @@ -165,6 +170,10 @@ module("Unit | Model | topic", function (hooks) { }); assert.blank(topic.post_stream, "it does not update post_stream"); + assert.true( + topic.details instanceof TopicDetails, + "topicDetails is an instance of TopicDetails" + ); assert.strictEqual(topic.details.hello, "world", "it updates the details"); assert.strictEqual(topic.cool, "property", "it updates other properties"); assert.strictEqual(topic.category, category);