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.
This commit is contained in:
Sérgio Saquetim 2024-12-05 14:21:29 -03:00 committed by GitHub
parent 68e57190df
commit 8ce6aa3e7d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 64 additions and 51 deletions

View File

@ -23,6 +23,7 @@ import discourseComputed from "discourse-common/utils/decorators";
import { i18n } from "discourse-i18n"; import { i18n } from "discourse-i18n";
let _customizations = []; let _customizations = [];
export function registerCustomizationCallback(cb) { export function registerCustomizationCallback(cb) {
_customizations.push(cb); _customizations.push(cb);
} }
@ -804,29 +805,29 @@ export default class Composer extends RestModel {
} }
/** /**
Open a composer Open a composer
@method open @method open
@param {Object} opts @param {Object} opts
@param {String} opts.action The action we're performing: edit, reply, createTopic, createSharedDraft, privateMessage @param {String} opts.action The action we're performing: edit, reply, createTopic, createSharedDraft, privateMessage
@param {String} opts.draftKey @param {String} opts.draftKey
@param {String} opts.draftSequence @param {String} opts.draftSequence
@param {Post} [opts.post] The post we're replying to, if present @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 {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.quote] If we're opening a reply from a quote, the quote we're making
@param {String} [opts.reply] @param {String} [opts.reply]
@param {String} [opts.recipients] @param {String} [opts.recipients]
@param {Number} [opts.composerTime] @param {Number} [opts.composerTime]
@param {Number} [opts.typingTime] @param {Number} [opts.typingTime]
@param {Boolean} [opts.whisper] @param {Boolean} [opts.whisper]
@param {Boolean} [opts.noBump] @param {Boolean} [opts.noBump]
@param {String} [opts.archetypeId] One of `site.archetypes` e.g. `regular` or `private_message` @param {String} [opts.archetypeId] One of `site.archetypes` e.g. `regular` or `private_message`
@param {Object} [opts.metaData] @param {Object} [opts.metaData]
@param {Number} [opts.categoryId] @param {Number} [opts.categoryId]
@param {Number} [opts.postId] @param {Number} [opts.postId]
@param {Number} [opts.destinationCategoryId] @param {Number} [opts.destinationCategoryId]
@param {String} [opts.title] @param {String} [opts.title]
**/ **/
open(opts) { open(opts) {
let promise = Promise.resolve(); let promise = Promise.resolve();
@ -886,7 +887,18 @@ export default class Composer extends RestModel {
}); });
if (!this.topic) { 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) { } else if (opts.postId) {
promise = promise.then(() => promise = promise.then(() =>
@ -935,37 +947,22 @@ export default class Composer extends RestModel {
} }
this.setProperties(topicProps); this.setProperties(topicProps);
promise = promise.then(() => { promise = promise.then(async () => {
let rawPromise = this.store.find("post", opts.post.id).then((post) => { const post = await this.store.find("post", opts.post.id);
this.setProperties({ this.setProperties({
post, post,
reply: post.raw, reply: post.raw,
originalText: post.raw, originalText: post.raw,
});
if (post.post_number === 1 && this.canEditTitle) {
this.setProperties({
originalTitle: post.topic.title,
originalTags: post.topic.tags,
});
}
}); });
// edge case ... make a post then edit right away if (post.post_number === 1 && this.canEditTitle) {
// store does not have topic for the post this.setProperties({
if (this.topic && this.topic.id === this.post.topic_id) { originalTitle: this.topic.title,
// nothing to do ... we have the right topic originalTags: this.topic.tags,
} else { });
rawPromise = this.store
.find("topic", this.post.topic_id)
.then((topic) => {
this.set("topic", topic);
});
} }
return rawPromise.then(() => { this.appEvents.trigger("composer:reply-reloaded", this);
this.appEvents.trigger("composer:reply-reloaded", this);
});
}); });
} else if (opts.action === REPLY && opts.quote) { } else if (opts.action === REPLY && opts.quote) {
this.set("reply", opts.quote); this.set("reply", opts.quote);

View File

@ -19,6 +19,7 @@ import ActionSummary from "discourse/models/action-summary";
import Bookmark from "discourse/models/bookmark"; import Bookmark from "discourse/models/bookmark";
import RestModel from "discourse/models/rest"; import RestModel from "discourse/models/rest";
import Site from "discourse/models/site"; import Site from "discourse/models/site";
import TopicDetails from "discourse/models/topic-details";
import { flushMap } from "discourse/services/store"; import { flushMap } from "discourse/services/store";
import deprecated from "discourse-common/lib/deprecated"; import deprecated from "discourse-common/lib/deprecated";
import getURL from "discourse-common/lib/get-url"; import getURL from "discourse-common/lib/get-url";
@ -461,7 +462,13 @@ export default class Topic extends RestModel {
} }
set details(value) { 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") @discourseComputed("visible")

View File

@ -4,6 +4,7 @@ import { IMAGE_VERSION as v } from "pretty-text/emoji/version";
import { module, test } from "qunit"; import { module, test } from "qunit";
import Category from "discourse/models/category"; import Category from "discourse/models/category";
import Topic from "discourse/models/topic"; import Topic from "discourse/models/topic";
import TopicDetails from "discourse/models/topic-details";
module("Unit | Model | topic", function (hooks) { module("Unit | Model | topic", function (hooks) {
setupTest(hooks); setupTest(hooks);
@ -108,6 +109,10 @@ module("Unit | Model | topic", function (hooks) {
const topic = this.store.createRecord("topic", { id: 1234 }); const topic = this.store.createRecord("topic", { id: 1234 });
const topicDetails = topic.details; 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.present(topicDetails, "a topic has topicDetails after we create it");
assert.strictEqual( assert.strictEqual(
topicDetails.topic, topicDetails.topic,
@ -165,6 +170,10 @@ module("Unit | Model | topic", function (hooks) {
}); });
assert.blank(topic.post_stream, "it does not update post_stream"); 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.details.hello, "world", "it updates the details");
assert.strictEqual(topic.cool, "property", "it updates other properties"); assert.strictEqual(topic.cool, "property", "it updates other properties");
assert.strictEqual(topic.category, category); assert.strictEqual(topic.category, category);