DEV: Improve UX when user profiles are hidden from public (#26293)

Previously, we had an instant redirect back to the homepage, and clicking avatars would do nothing. This made things feel 'broken' for anon when 'hide_user_profiles_from_public' was enabled.

This commit does a few things to resolve this:

1. Improve our 'exception' system for routes so that developers can deliberately trigger it without an ajax error

2. Improve 'exception' system so that the browser URL bar is updated correctly, and the 'back' button works as expected

3. Replace the redirect-to-home with an 'access denied' error page, with specific copy for 'You must log in to view user profiles'

4. Update user-card logic to display this new page instead of doing nothing on click

app/assets/javascripts/discourse/app/controllers/exception.js

@@ -7,6 +7,22 @@ import DiscourseURL from "discourse/lib/url";
 import discourseComputed, { on } from "discourse-common/utils/decorators";
 import I18n from "discourse-i18n";
 
+/**
+ * You can throw an instance of this error during a route's beforeModel/model/afterModel hooks.
+ * It will be caught by the top-level ApplicationController, and cause this Exception controller/template
+ * to be rendered without changing the URL.
+ */
+export class RouteException {
+  status;
+  reason;
+
+  constructor({ status, reason, desc }) {
+    this.status = status;
+    this.reason = reason;
+    this.desc = desc;
+  }
+}
+
 // The controller for the nice error page
 export default Controller.extend({
   thrown: null,
@@ -48,7 +64,9 @@ export default Controller.extend({
 
   @discourseComputed("isNetwork", "thrown.status", "thrown")
   reason(isNetwork, thrownStatus, thrown) {
-    if (isNetwork) {
+    if (thrown.reason) {
+      return thrown.reason;
+    } else if (isNetwork) {
       return I18n.t("errors.reasons.network");
     } else if (thrownStatus >= 500) {
       return I18n.t("errors.reasons.server");
@@ -74,7 +92,9 @@ export default Controller.extend({
     "thrown"
   )
   desc(networkFixed, isNetwork, thrownStatus, thrownStatusText, thrown) {
-    if (networkFixed) {
+    if (thrown.desc) {
+      return thrown.desc;
+    } else if (networkFixed) {
       return I18n.t("errors.desc.network_fixed");
     } else if (isNetwork) {
       return I18n.t("errors.desc.network");
@@ -138,8 +158,7 @@ export default Controller.extend({
   back() {
     // Strip off subfolder
     const currentURL = DiscourseURL.router.location.getURL();
-    if (this.lastTransition && currentURL !== "/exception") {
-      this.lastTransition.abort();
+    if (this.lastTransition?.method === "replace") {
       this.setProperties({ lastTransition: null, thrown: null });
       // Can't use routeTo because it handles navigation to the same page
       DiscourseURL.handleURL(currentURL);

app/assets/javascripts/discourse/app/lib/url.js

@@ -177,6 +177,11 @@ const DiscourseURL = EmberObject.extend({
     }
   },
 
+  pushState(path) {
+    path = withoutPrefix(path);
+    this.router._routerMicrolib.updateURL(path);
+  },
+
   routeToTag(a) {
     // skip when we are provided nowhere to route to
     if (!a || !a.href) {

app/assets/javascripts/discourse/app/mixins/card-contents-base.js

@@ -45,7 +45,7 @@ export default Mixin.create({
   _show(username, target, event) {
     // No user card for anon
     if (this.siteSettings.hide_user_profiles_from_public && !this.currentUser) {
-      return false;
+      return true;
     }
 
     username = escapeExpression(username.toString());
@@ -157,9 +157,15 @@ export default Mixin.create({
         return true;
       }
 
-      event.preventDefault();
-      event.stopPropagation();
-      return this._show(transformText(matchingEl), matchingEl, event);
+      const shouldBubble = this._show(
+        transformText(matchingEl),
+        matchingEl,
+        event
+      );
+      if (!shouldBubble) {
+        event.preventDefault();
+        event.stopPropagation();
+      }
     }
 
     return false;

app/assets/javascripts/discourse/app/routes/application.js

@@ -4,6 +4,7 @@ import CreateAccount from "discourse/components/modal/create-account";
 import ForgotPassword from "discourse/components/modal/forgot-password";
 import KeyboardShortcutsHelp from "discourse/components/modal/keyboard-shortcuts-help";
 import LoginModal from "discourse/components/modal/login";
+import { RouteException } from "discourse/controllers/exception";
 import { setting } from "discourse/lib/computed";
 import cookie from "discourse/lib/cookie";
 import logout from "discourse/lib/logout";
@@ -125,23 +126,27 @@ const ApplicationRoute = DiscourseRoute.extend({
     error(err, transition) {
       const xhrOrErr = err.jqXHR ? err.jqXHR : err;
       const exceptionController = this.controllerFor("exception");
+      let shouldBubble = false;
 
       const themeOrPluginSource = identifySource(err);
 
-      // eslint-disable-next-line no-console
-      console.error(
-        ...[consolePrefix(err, themeOrPluginSource), xhrOrErr].filter(Boolean)
-      );
-
-      if (xhrOrErr && xhrOrErr.status === 404) {
-        return this.router.transitionTo("exception-unknown");
-      }
-
-      if (themeOrPluginSource) {
-        this.clientErrorHandler.displayErrorNotice(
-          "Error loading route",
-          themeOrPluginSource
+      if (!(xhrOrErr instanceof RouteException)) {
+        shouldBubble = true;
+        // eslint-disable-next-line no-console
+        console.error(
+          ...[consolePrefix(err, themeOrPluginSource), xhrOrErr].filter(Boolean)
         );
+
+        if (xhrOrErr && xhrOrErr.status === 404) {
+          return this.router.transitionTo("exception-unknown");
+        }
+
+        if (themeOrPluginSource) {
+          this.clientErrorHandler.displayErrorNotice(
+            "Error loading route",
+            themeOrPluginSource
+          );
+        }
       }
 
       exceptionController.setProperties({
@@ -149,8 +154,16 @@ const ApplicationRoute = DiscourseRoute.extend({
         thrown: xhrOrErr,
       });
 
+      if (transition.intent.url) {
+        if (transition.method === "replace") {
+          DiscourseURL.replaceState(transition.intent.url);
+        } else {
+          DiscourseURL.pushState(transition.intent.url);
+        }
+      }
+
       this.intermediateTransitionTo("exception");
-      return true;
+      return shouldBubble;
     },
 
     showLogin: unlessStrictlyReadOnly(

app/assets/javascripts/discourse/app/routes/user.js

@@ -1,8 +1,10 @@
 import { action } from "@ember/object";
 import { service } from "@ember/service";
+import { RouteException } from "discourse/controllers/exception";
 import User from "discourse/models/user";
 import DiscourseRoute from "discourse/routes/discourse";
 import { bind } from "discourse-common/utils/decorators";
+import I18n from "discourse-i18n";
 
 export default DiscourseRoute.extend({
   router: service(),
@@ -12,7 +14,10 @@ export default DiscourseRoute.extend({
 
   beforeModel() {
     if (this.siteSettings.hide_user_profiles_from_public && !this.currentUser) {
-      this.router.replaceWith("discovery");
+      throw new RouteException({
+        status: 403,
+        desc: I18n.t("user.login_to_view_profile"),
+      });
     }
   },
 

config/locales/client.en.yml

@@ -1164,6 +1164,7 @@ en:
           warning: "Are you sure you want to clear your featured topic?"
       use_current_timezone: "Use Current Timezone"
       profile_hidden: "This user's public profile is hidden."
+      login_to_view_profile: "You must log in to view user profiles"
       inactive_user: "This user is no longer active."
       expand_profile: "Expand"
       sr_expand_profile: "Expand profile details"

spec/system/user_page/hide_from_public_spec.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+describe "hide_user_profiles_from_public", type: :system do
+  let(:user) { Fabricate(:user) }
+  before { SiteSetting.hide_user_profiles_from_public = true }
+
+  it "displays an error when navigating straight to a profile" do
+    visit("/u/#{user.username}")
+    expect(page).to have_css(".error-page .reason", text: I18n.t("js.errors.reasons.forbidden"))
+    expect(page).to have_css(".error-page .desc", text: I18n.t("js.user.login_to_view_profile"))
+  end
+
+  it "displays an error when navigating from an internal link" do
+    Fabricate(:post, user: user)
+    visit("/latest")
+    find("[data-user-card='#{user.username}']").click
+
+    expect(page).to have_css(".error-page .reason", text: I18n.t("js.errors.reasons.forbidden"))
+    expect(page).to have_css(".error-page .desc", text: I18n.t("js.user.login_to_view_profile"))
+    expect(page).to have_current_path("/u/#{user.username}")
+
+    find(".error-page .buttons .btn-primary", text: "Back").click
+    expect(page).to have_current_path("/latest")
+  end
+end