Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
FEATURE: Add site setting to show more detailed 404 errors. (#8014)
If the setting is turned on, then the user will receive information
about the subject: if it was deleted or requires some special access to
a group (only if the group is public). Otherwise, the user will receive
a generic #404 error message. For now, this change affects only the
topics and categories controller.

This commit also tries to refactor some of the code related to error
handling. To make error pages more consistent (design-wise), the actual
error page will be rendered server-side.
  • Loading branch information
udan11 committed Oct 8, 2019
1 parent d2bceff commit fdb1d34
Show file tree
Hide file tree
Showing 23 changed files with 755 additions and 646 deletions.
42 changes: 1 addition & 41 deletions app/assets/javascripts/discourse/controllers/topic.js.es6
Expand Up @@ -945,46 +945,6 @@ export default Ember.Controller.extend(bufferedProperty("model"), {
}
},

joinGroup() {
const groupId = this.get("model.group.id");
if (groupId) {
if (this.get("model.group.allow_membership_requests")) {
const groupName = this.get("model.group.name");
return ajax(`/groups/${groupName}/request_membership`, {
type: "POST",
data: {
topic_id: this.get("model.id")
}
})
.then(() => {
bootbox.alert(
I18n.t("topic.group_request_sent", {
group_name: this.get("model.group.full_name")
}),
() =>
this.previousURL
? DiscourseURL.routeTo(this.previousURL)
: DiscourseURL.routeTo("/")
);
})
.catch(popupAjaxError);
} else {
const topic = this.model;
return ajax(`/groups/${groupId}/members`, {
type: "PUT",
data: { user_id: this.get("currentUser.id") }
})
.then(() =>
topic.reload().then(() => {
topic.set("view_hidden", false);
topic.postStream.refresh();
})
)
.catch(popupAjaxError);
}
}
},

replyAsNewTopic(post, quotedText) {
const composerController = this.composer;

Expand Down Expand Up @@ -1182,7 +1142,7 @@ export default Ember.Controller.extend(bufferedProperty("model"), {
}
},

hasError: Ember.computed.or("model.notFoundHtml", "model.message"),
hasError: Ember.computed.or("model.errorHtml", "model.errorMessage"),
noErrorYet: Ember.computed.not("hasError"),

categories: Ember.computed.alias("site.categoriesList"),
Expand Down
27 changes: 6 additions & 21 deletions app/assets/javascripts/discourse/models/post-stream.js.es6
Expand Up @@ -1067,31 +1067,16 @@ export default RestModel.extend({
// Handles an error loading a topic based on a HTTP status code. Updates
// the text to the correct values.
errorLoading(result) {
const status = result.jqXHR.status;

const topic = this.topic;
this.set("loadingFilter", false);
topic.set("errorLoading", true);

// If the result was 404 the post is not found
// If it was 410 the post is deleted and the user should not see it
if (status === 404 || status === 410) {
topic.set("notFoundHtml", result.jqXHR.responseText);
return;
}

// If the result is 403 it means invalid access
if (status === 403) {
topic.set("noRetry", true);
if (Discourse.User.current()) {
topic.set("message", I18n.t("topic.invalid_access.description"));
} else {
topic.set("message", I18n.t("topic.invalid_access.login_required"));
}
return;
const json = result.jqXHR.responseJSON;
if (json && json.extras && json.extras.html) {
topic.set("errorHtml", json.extras.html);
} else {
topic.set("errorMessage", I18n.t("topic.server_error.description"));
topic.set("noRetry", result.jqXHR.status === 403);
}

// Otherwise supply a generic error message
topic.set("message", I18n.t("topic.server_error.description"));
}
});
5 changes: 2 additions & 3 deletions app/assets/javascripts/discourse/models/topic.js.es6
Expand Up @@ -17,16 +17,15 @@ import {
} from "ember-addons/ember-computed-decorators";

export function loadTopicView(topic, args) {
const topicId = topic.get("id");
const data = _.merge({}, args);
const url = `${Discourse.getURL("/t/")}${topicId}`;
const url = `${Discourse.getURL("/t/")}${topic.id}`;
const jsonUrl = (data.nearPost ? `${url}/${data.nearPost}` : url) + ".json";

delete data.nearPost;
delete data.__type;
delete data.store;

return PreloadStore.getAndRemove(`topic_${topicId}`, () =>
return PreloadStore.getAndRemove(`topic_${topic.id}`, () =>
ajax(jsonUrl, { data })
).then(json => {
topic.updateFromJson(json);
Expand Down
Expand Up @@ -198,6 +198,18 @@ export default (filterArg, params) => {
},

actions: {
error(err) {
const json = err.jqXHR.responseJSON;
if (json && json.extras && json.extras.html) {
this.controllerFor("discovery").set(
"errorHtml",
err.jqXHR.responseJSON.extras.html
);
} else {
this.replaceWith("exception");
}
},

setNotification(notification_level) {
this.currentModel.setNotification(notification_level);
},
Expand Down
48 changes: 26 additions & 22 deletions app/assets/javascripts/discourse/templates/discovery.hbs
@@ -1,32 +1,36 @@
<div class="container">
{{discourse-banner user=currentUser banner=site.banner}}
</div>

<div class="list-controls">
{{#if errorHtml}}
{{{errorHtml}}}
{{else}}
<div class="container">
{{outlet "navigation-bar"}}
{{discourse-banner user=currentUser banner=site.banner}}
</div>
</div>

{{conditional-loading-spinner condition=loading}}
<div class="list-controls">
<div class="container">
{{outlet "navigation-bar"}}
</div>
</div>

<div class="container list-container {{if loading "hidden"}}">
<div class="row">
<div class="full-width">
<div id="header-list-area">
{{outlet "header-list-container"}}
{{conditional-loading-spinner condition=loading}}

<div class="container list-container {{if loading "hidden"}}">
<div class="row">
<div class="full-width">
<div id="header-list-area">
{{outlet "header-list-container"}}
</div>
</div>
</div>
</div>
<div class="row">
<div class="full-width">
<div id="list-area">
{{plugin-outlet name="discovery-list-container-top"
args=(hash category=category listLoading=loading)}}
{{outlet "list-container"}}
<div class="row">
<div class="full-width">
<div id="list-area">
{{plugin-outlet name="discovery-list-container-top"
args=(hash category=category listLoading=loading)}}
{{outlet "list-container"}}
</div>
</div>
</div>
</div>
</div>

{{plugin-outlet name="discovery-below"}}
{{plugin-outlet name="discovery-below"}}
{{/if}}

1 comment on commit fdb1d34

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/you-need-to-request-membership-but-im-already-a-member/128502/5

Please sign in to comment.