Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DEV: Refactor tag-show route (#16217) 

Previously we were loading almost all the data in an afterModel hook, storing it temporarily in route properties, and then passing it to the controller in `setupController`.

This does not follow Ember best-practices, and causes a number of unexpected behaviours. For example, Ember only calls `setupController` **when the model value changes**. Since `model()` was only returning the tag, that meant that category changes and `additionalTag` changes wouldn't always trigger a `setupController` call, and things would get into a very weird state. This is visible when using the 'loading-slider' component because the category navigation dropdown gets 'stuck' when switching categories.

This commit moves all the data-fetching into `model()`. To make things cleaner, it also:
- removes most uses of route-level variables
- introduces async/await in the model() function
- removes some unneeded `get()` usage
- re-uses DiscoverySortableController for queryParam default handling
- Removes override of `renderTemplate()` so that queryParams are correctly passed through to the controller
- Removes some `transitionToRoute` hacks which were working around the queryParams issue
- Switches to `@action`
This commit is contained in:
David Taylor 2022-03-21 12:20:51 +00:00 committed by GitHub
parent 0832cad803
commit f7b5ff39cf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 180 additions and 213 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

211
app/assets/javascripts/discourse/app/controllers/tag-show.js
View File

@ -1,177 +1,144 @@
import Controller, { inject as controller } from "@ember/controller";
import DiscoverySortableController from "discourse/controllers/discovery-sortable";
import { inject as controller } from "@ember/controller";
import discourseComputed, { observes } from "discourse-common/utils/decorators";
import BulkTopicSelection from "discourse/mixins/bulk-topic-selection";
import FilterModeMixin from "discourse/mixins/filter-mode";
import I18n from "I18n";
import NavItem from "discourse/models/nav-item";
import Topic from "discourse/models/topic";
import { alias } from "@ember/object/computed";
import { readOnly } from "@ember/object/computed";
import bootbox from "bootbox";
import { queryParams } from "discourse/controllers/discovery-sortable";
import { endWith } from "discourse/lib/computed";
import { action } from "@ember/object";
export default Controller.extend(BulkTopicSelection, FilterModeMixin, {
application: controller(),
export default DiscoverySortableController.extend(
BulkTopicSelection,
FilterModeMixin,
{
application: controller(),
tag: null,
additionalTags: null,
list: null,
canAdminTag: alias("currentUser.staff"),
navMode: "latest",
loading: false,
canCreateTopic: false,
order: "default",
ascending: false,
status: null,
state: null,
search: null,
max_posts: null,
q: null,
showInfo: false,
top: endWith("list.filter", "top"),
period: alias("list.for_period"),
tag: null,
additionalTags: null,
list: null,
canAdminTag: readOnly("currentUser.staff"),
navMode: "latest",
loading: false,
canCreateTopic: false,
showInfo: false,
top: endWith("list.filter", "top"),
@discourseComputed(
"canCreateTopic",
"category",
"canCreateTopicOnCategory",
"tag",
"canCreateTopicOnTag"
)
createTopicDisabled(
canCreateTopic,
category,
canCreateTopicOnCategory,
tag,
canCreateTopicOnTag
) {
return (
!canCreateTopic ||
(category && !canCreateTopicOnCategory) ||
(tag && !canCreateTopicOnTag)
);
},
@discourseComputed(
"canCreateTopic",
"category",
"canCreateTopicOnCategory",
"tag",
"canCreateTopicOnTag"
)
createTopicDisabled(
canCreateTopic,
category,
canCreateTopicOnCategory,
tag,
canCreateTopicOnTag
) {
return (
!canCreateTopic ||
(category && !canCreateTopicOnCategory) ||
(tag && !canCreateTopicOnTag)
);
},
queryParams: Object.keys(queryParams),
@discourseComputed("category", "tag.id", "filterType", "noSubcategories")
navItems(category, tagId, filterType, noSubcategories) {
return NavItem.buildList(category, {
tagId,
filterType,
noSubcategories,
siteSettings: this.siteSettings,
});
},
@discourseComputed("category")
showTagFilter() {
return true;
},
@observes("list.canLoadMore")
_showFooter() {
this.set("application.showFooter", !this.get("list.canLoadMore"));
},
@discourseComputed("navMode", "list.topics.length", "loading")
footerMessage(navMode, listTopicsLength, loading) {
if (loading) {
return;
}
if (listTopicsLength === 0) {
return I18n.t(`tagging.topics.none.${navMode}`, {
tag: this.get("tag.id"),
@discourseComputed("category", "tag.id", "filterType", "noSubcategories")
navItems(category, tagId, filterType, noSubcategories) {
return NavItem.buildList(category, {
tagId,
filterType,
noSubcategories,
siteSettings: this.siteSettings,
});
} else {
return I18n.t(`topics.bottom.tag`, {
tag: this.get("tag.id"),
});
}
},
},
@discourseComputed("list.filter", "list.topics.length")
showDismissRead(filter, topicsLength) {
return this._isFilterPage(filter, "unread") && topicsLength > 0;
},
@observes("list.canLoadMore")
_showFooter() {
this.set("application.showFooter", !this.list?.canLoadMore);
},
@discourseComputed("list.filter", "list.topics.length")
showResetNew(filter, topicsLength) {
return this._isFilterPage(filter, "new") && topicsLength > 0;
},
@discourseComputed("navMode", "list.topics.length", "loading")
footerMessage(navMode, listTopicsLength, loading) {
if (loading) {
return;
}
actions: {
if (listTopicsLength === 0) {
return I18n.t(`tagging.topics.none.${navMode}`, {
tag: this.tag?.id,
});
} else {
return I18n.t("topics.bottom.tag", {
tag: this.tag?.id,
});
}
},
@discourseComputed("list.filter", "list.topics.length")
showDismissRead(filter, topicsLength) {
return this._isFilterPage(filter, "unread") && topicsLength > 0;
},
@discourseComputed("list.filter", "list.topics.length")
showResetNew(filter, topicsLength) {
return this._isFilterPage(filter, "new") && topicsLength > 0;
},
@action
resetNew() {
const tracked =
(this.router.currentRoute.queryParams["f"] ||
this.router.currentRoute.queryParams["filter"]) === "tracked";
let topicIds = this.selected
? this.selected.map((topic) => topic.id)
: null;
let topicIds = this.selected ? this.selected.mapBy("id") : null;
Topic.resetNew(this.category, !this.noSubcategories, {
tracked,
tag: this.tag,
topicIds,
}).then(() =>
this.send(
"refresh",
tracked ? { skipResettingParams: ["filter", "f"] } : {}
)
this.refresh(tracked ? { skipResettingParams: ["filter", "f"] } : {})
);
},
@action
showInserted() {
const tracker = this.topicTrackingState;
this.list.loadBefore(tracker.get("newIncoming"), true);
this.list.loadBefore(tracker.newIncoming, true);
tracker.resetTracking();
return false;
},
@action
changeSort(order) {
if (order === this.order) {
this.toggleProperty("ascending");
} else {
this.setProperties({ order, ascending: false });
}
let params = { order, ascending: this.ascending };
if (this.period) {
params.period = this.period;
}
this.transitionToRoute({
queryParams: params,
});
},
@action
changePeriod(p) {
this.set("period", p);
let params = { period: this.period };
if (!(this.order === "default" && this.ascending === false)) {
params = Object.assign(params, {
order: this.order,
ascending: this.ascending,
});
}
this.transitionToRoute({
queryParams: params,
});
},
@action
toggleInfo() {
this.toggleProperty("showInfo");
},
@action
refresh() {
return this.store
.findFiltered("topicList", {
filter: this.get("list.filter"),
filter: this.list?.filter,
})
.then((list) => {
this.set("list", list);
@ -179,6 +146,7 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, {
});
},
@action
deleteTag(tagInfo) {
const numTopics =
this.get("list.topic_list.tags.firstObject.topic_count") || 0;
@ -208,6 +176,7 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, {
});
},
@action
changeTagNotificationLevel(notificationLevel) {
this.tagNotification
.update({ notification_level: notificationLevel })
@ -222,5 +191,5 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, {
);
});
},
},
});
}
);

182
app/assets/javascripts/discourse/app/routes/tag-show.js
View File

@ -23,63 +23,56 @@ export default DiscourseRoute.extend(FilterModeMixin, {
queryParams,
renderTemplate() {
const controller = this.controllerFor("tag.show");
this.render("tags.show", { controller });
},
controllerName: "tag.show",
templateName: "tag.show",
model(params) {
const tag = this.store.createRecord("tag", {
id: escapeExpression(params.tag_id),
});
if (params.additional_tags) {
this.set(
"additionalTags",
params.additional_tags.split("/").map((t) => {
return this.store.createRecord("tag", {
id: escapeExpression(t),
}).id;
})
);
} else {
this.set("additionalTags", null);
}
this.set("filterType", this.navMode.split("/")[0]);
this.set("categorySlugPathWithID", params.category_slug_path_with_id);
if (tag && tag.get("id") !== "none" && this.currentUser) {
// If logged in, we should get the tag's user settings
return this.store
.find("tagNotification", tag.get("id").toLowerCase())
.then((tn) => {
this.set("tagNotification", tn);
return tag;
});
}
return tag;
},
afterModel(tag, transition) {
beforeModel() {
const controller = this.controllerFor("tag.show");
controller.setProperties({
loading: true,
showInfo: false,
});
},
const params = filterQueryParams(transition.to.queryParams, {});
const category = this.categorySlugPathWithID
? Category.findBySlugPathWithID(this.categorySlugPathWithID)
async model(params, transition) {
const tag = this.store.createRecord("tag", {
id: escapeExpression(params.tag_id),
});
let additionalTags;
if (params.additional_tags) {
additionalTags = params.additional_tags.split("/").map((t) => {
return this.store.createRecord("tag", {
id: escapeExpression(t),
}).id;
});
}
const filterType = this.navMode.split("/")[0];
let tagNotification;
if (tag && tag.id !== "none" && this.currentUser) {
// If logged in, we should get the tag's user settings
tagNotification = await this.store.find(
"tagNotification",
tag.id.toLowerCase()
);
}
const category = params.category_slug_path_with_id
? Category.findBySlugPathWithID(params.category_slug_path_with_id)
: null;
const filteredQueryParams = filterQueryParams(
transition.to.queryParams,
{}
);
const topicFilter = this.navMode;
const tagId = tag ? tag.id.toLowerCase() : "none";
let filter;
if (category) {
category.setupGroupsAndPermissions();
this.set("category", category);
filter = `tags/c/${Category.slugFor(category)}/${category.id}`;
if (this.noSubcategories) {
@ -87,36 +80,55 @@ export default DiscourseRoute.extend(FilterModeMixin, {
}
filter += `/${tagId}/l/${topicFilter}`;
} else if (this.additionalTags) {
this.set("category", null);
filter = `tags/intersection/${tagId}/${this.additionalTags.join("/")}`;
} else if (additionalTags) {
filter = `tags/intersection/${tagId}/${additionalTags.join("/")}`;
} else {
this.set("category", null);
filter = `tag/${tagId}/l/${topicFilter}`;
}
return findTopicList(this.store, this.topicTrackingState, filter, params, {
cached: this.isPoppedState(transition),
}).then((list) => {
if (list.topic_list.tags && list.topic_list.tags.length === 1) {
// Update name of tag (case might be different)
tag.setProperties({
id: list.topic_list.tags[0].name,
staff: list.topic_list.tags[0].staff,
});
const list = await findTopicList(
this.store,
this.topicTrackingState,
filter,
filteredQueryParams,
{
cached: this.isPoppedState(transition),
}
);
setTopicList(list);
controller.setProperties({
list,
canCreateTopic: list.get("can_create_topic"),
loading: false,
canCreateTopicOnCategory:
this.get("category.permission") === PermissionType.FULL,
canCreateTopicOnTag: !tag.get("staff") || this.get("currentUser.staff"),
if (list.topic_list.tags && list.topic_list.tags.length === 1) {
// Update name of tag (case might be different)
tag.setProperties({
id: list.topic_list.tags[0].name,
staff: list.topic_list.tags[0].staff,
});
}
setTopicList(list);
return {
tag,
category,
list,
additionalTags,
filterType,
tagNotification,
canCreateTopic: list.can_create_topic,
canCreateTopicOnCategory: category?.permission === PermissionType.FULL,
canCreateTopicOnTag: !tag.staff || this.currentUser?.staff,
};
},
setupController(controller, model) {
this.controllerFor("tag.show").setProperties({
model: model.tag,
...model,
period: model.list.for_period,
navMode: this.navMode,
noSubcategories: this.noSubcategories,
loading: false,
});
this.searchService.set("searchContext", model.tag.searchContext);
},
titleToken() {
@ -125,24 +137,24 @@ export default DiscourseRoute.extend(FilterModeMixin, {
);
const controller = this.controllerFor("tag.show");
if (controller.get("model.id")) {
if (this.category) {
if (controller.tag?.id) {
if (controller.category) {
return I18n.t("tagging.filters.with_category", {
filter: filterText,
tag: controller.get("model.id"),
category: this.get("category.name"),
tag: controller.tag.id,
category: controller.category.name,
});
} else {
return I18n.t("tagging.filters.without_category", {
filter: filterText,
tag: controller.get("model.id"),
tag: controller.tag.id,
});
}
} else {
if (this.category) {
if (controller.category) {
return I18n.t("tagging.filters.untagged_with_category", {
filter: filterText,
category: this.get("category.name"),
category: controller.category.name,
});
} else {
return I18n.t("tagging.filters.untagged_without_category", {
@ -152,20 +164,6 @@ export default DiscourseRoute.extend(FilterModeMixin, {
}
},
setupController(controller, model) {
this.controllerFor("tag.show").setProperties({
model,
tag: model,
additionalTags: this.additionalTags,
category: this.category,
filterType: this.filterType,
navMode: this.navMode,
tagNotification: this.tagNotification,
noSubcategories: this.noSubcategories,
});
this.searchService.set("searchContext", model.get("searchContext"));
},
deactivate() {
this._super(...arguments);
this.searchService.set("searchContext", null);
@ -178,21 +176,21 @@ export default DiscourseRoute.extend(FilterModeMixin, {
@action
createTopic() {
if (this.get("currentUser.has_topic_draft")) {
if (this.currentUser?.has_topic_draft) {
this.openTopicDraft();
} else {
const controller = this.controllerFor("tag.show");
const composerController = this.controllerFor("composer");
composerController
.open({
categoryId: controller.get("category.id"),
categoryId: controller.category?.id,
action: Composer.CREATE_TOPIC,
draftKey: Composer.NEW_TOPIC_KEY,
})
.then(() => {
// Pre-fill the tags input field
if (composerController.canEditTags && controller.get("model.id")) {
const composerModel = this.controllerFor("composer").get("model");
if (composerController.canEditTags && controller.tag?.id) {
const composerModel = this.controllerFor("composer").model;
composerModel.set(
"tags",
[
@ -215,9 +213,9 @@ export default DiscourseRoute.extend(FilterModeMixin, {
dismissRead(operationType) {
const controller = this.controllerFor("tags-show");
let options = {
tagName: controller.get("tag.id"),
tagName: controller.tag?.id,
};
const categoryId = controller.get("category.id");
const categoryId = controller.category?.id;
if (categoryId) {
options = Object.assign({}, options, {

0
app/assets/javascripts/discourse/app/templates/tags/show.hbs → app/assets/javascripts/discourse/app/templates/tag/show.hbs
View File