FIX: Always wait for promise when loading a topic (#10465)

It turns out that `setupController` doesn't always wait when returning a
promise, but the `model` hook does. This fixes issues with the
`page:changed` event firing before the transition has complete.
This commit is contained in:
Robin Ward 2020-08-18 11:11:02 -04:00 committed by GitHub
parent 234ae61ed5
commit bad7c287dd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -9,7 +9,29 @@ import { isTesting } from "discourse-common/config/environment";
export default DiscourseRoute.extend({ export default DiscourseRoute.extend({
// Avoid default model hook // Avoid default model hook
model(params) { model(params) {
return params; params = params || {};
params.track_visit = true;
const topic = this.modelFor("topic");
const postStream = topic.postStream;
// I sincerely hope no topic gets this many posts
if (params.nearPost === "last") {
params.nearPost = 999999999;
}
params.forceLoad = true;
return postStream
.refresh(params)
.then(() => params)
.catch(e => {
if (!isTesting()) {
// eslint-disable-next-line no-console
console.log("Could not view topic", e);
}
params._loading_error = true;
return params;
});
}, },
deactivate() { deactivate() {
@ -18,82 +40,68 @@ export default DiscourseRoute.extend({
}, },
setupController(controller, params, { _discourse_anchor }) { setupController(controller, params, { _discourse_anchor }) {
params = params || {}; // Don't do anything else if we couldn't load
params.track_visit = true; // TODO: Tests require this but it seems bad
if (params._loading_error) {
const topic = this.modelFor("topic"), return;
postStream = topic.postStream,
topicController = this.controllerFor("topic"),
composerController = this.controllerFor("composer");
// I sincerely hope no topic gets this many posts
if (params.nearPost === "last") {
params.nearPost = 999999999;
} }
params.forceLoad = true; const topicController = this.controllerFor("topic");
const composerController = this.controllerFor("composer");
const topic = this.modelFor("topic");
const postStream = topic.postStream;
return postStream // TODO we are seeing errors where closest post is null and this is exploding
.refresh(params) // we need better handling and logging for this condition.
.then(() => {
// TODO we are seeing errors where closest post is null and this is exploding
// we need better handling and logging for this condition.
// there are no closestPost for hidden topics // there are no closestPost for hidden topics
if (topic.view_hidden) { if (topic.view_hidden) {
return; return;
} }
// The post we requested might not exist. Let's find the closest post // The post we requested might not exist. Let's find the closest post
const closestPost = postStream.closestPostForPostNumber( const closestPost = postStream.closestPostForPostNumber(
params.nearPost || 1 params.nearPost || 1
); );
const closest = closestPost.post_number; const closest = closestPost.post_number;
topicController.setProperties({ topicController.setProperties({
"model.currentPost": closest, "model.currentPost": closest,
enteredIndex: topic.postStream.progressIndexOfPost(closestPost), enteredIndex: topic.postStream.progressIndexOfPost(closestPost),
enteredAt: Date.now().toString() enteredAt: Date.now().toString()
}); });
this.appEvents.trigger("page:topic-loaded", topic); this.appEvents.trigger("page:topic-loaded", topic);
topicController.subscribe(); topicController.subscribe();
// Highlight our post after the next render // Highlight our post after the next render
schedule("afterRender", () => schedule("afterRender", () =>
this.appEvents.trigger("post:highlight", closest) this.appEvents.trigger("post:highlight", closest)
); );
const opts = {}; const opts = {};
if (document.location.hash) { if (document.location.hash) {
opts.anchor = document.location.hash.substr(1); opts.anchor = document.location.hash.substr(1);
} else if (_discourse_anchor) { } else if (_discourse_anchor) {
opts.anchor = _discourse_anchor; opts.anchor = _discourse_anchor;
} }
DiscourseURL.jumpToPost(closest, opts); DiscourseURL.jumpToPost(closest, opts);
// completely clear out all the bookmark related attributes // completely clear out all the bookmark related attributes
// because they are not in the response if bookmarked == false // because they are not in the response if bookmarked == false
if (closestPost && !closestPost.bookmarked) { if (closestPost && !closestPost.bookmarked) {
closestPost.clearBookmark(); closestPost.clearBookmark();
} }
if (!isEmpty(topic.draft)) { if (!isEmpty(topic.draft)) {
composerController.open({ composerController.open({
draft: Draft.getLocal(topic.draft_key, topic.draft), draft: Draft.getLocal(topic.draft_key, topic.draft),
draftKey: topic.draft_key, draftKey: topic.draft_key,
draftSequence: topic.draft_sequence, draftSequence: topic.draft_sequence,
ignoreIfChanged: true, ignoreIfChanged: true,
topic topic
});
}
})
.catch(e => {
if (!isTesting()) {
// eslint-disable-next-line no-console
console.log("Could not view topic", e);
}
}); });
}
}, },
actions: { actions: {