DEV: Rework static controllers/routes (#19466)

The issues fixed:

1. Previously all static pages (e.g. login-required landing page, /tos, /privacy, forgot-password) were wrapped in the faq-read-tracking component
2. All these pages shared one controller with methods that were relevant to one route
3. There were two route-generating functions: `static-route-builder` and `build-static-route` 🤣 
4. They were using the deprecated `renderTemplate()` API
5. A slight misuse of Ember API (`controllerFor()`)
6. Small mark-faq-read related bugs

app/assets/javascripts/discourse/app/components/watch-read.js

@@ -1,39 +1,43 @@
-import { bind } from "discourse-common/utils/decorators";
 import Component from "@ember/component";
+import { inject as service } from "@ember/service";
+import { bind } from "discourse-common/utils/decorators";
 import isElementInViewport from "discourse/lib/is-element-in-viewport";
+import { ajax } from "discourse/lib/ajax";
+import { userPath } from "discourse/lib/url";
+
+export default class WatchRead extends Component {
+  @service currentUser;
 
-export default Component.extend({
   didInsertElement() {
-    this._super(...arguments);
-    const currentUser = this.currentUser;
-    if (!currentUser) {
+    super.didInsertElement(...arguments);
+
+    if (!this.currentUser || this.currentUser.read_faq) {
       return;
     }
 
-    const path = this.path;
-    if (path === "faq" || path === "guidelines") {
-      this._markRead();
-      window.addEventListener("resize", this._markRead, false);
-      window.addEventListener("scroll", this._markRead, false);
-    }
-  },
+    this._checkIfRead();
+    window.addEventListener("resize", this._checkIfRead, false);
+    window.addEventListener("scroll", this._checkIfRead, false);
+  }
 
   willDestroyElement() {
-    this._super(...arguments);
+    super.willDestroyElement(...arguments);
 
-    window.removeEventListener("resize", this._markRead);
-    window.removeEventListener("scroll", this._markRead);
-  },
+    window.removeEventListener("resize", this._checkIfRead);
+    window.removeEventListener("scroll", this._checkIfRead);
+  }
 
   @bind
-  _markRead() {
-    const faqUnread = !this.currentUser.read_faq;
+  async _checkIfRead() {
+    const lastParagraph = document.querySelector(
+      "[itemprop='mainContentOfPage'] > *:last-child"
+    );
 
-    if (
-      faqUnread &&
-      isElementInViewport(document.querySelector(".contents p:last-child"))
-    ) {
-      this.action();
+    if (!isElementInViewport(lastParagraph)) {
+      return;
     }
-  },
-});
+
+    await ajax(userPath("read-faq"), { type: "POST" });
+    this.currentUser.set("read_faq", true);
+  }
+}

app/assets/javascripts/discourse/app/controllers/login-page.js

@@ -0,0 +1,5 @@
+import Controller, { inject as controller } from "@ember/controller";
+
+export default class LoginPageController extends Controller {
+  @controller application;
+}

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

@@ -1,37 +0,0 @@
-import Controller, { inject as controller } from "@ember/controller";
-import { ajax } from "discourse/lib/ajax";
-import discourseComputed from "discourse-common/utils/decorators";
-import { equal, or } from "@ember/object/computed";
-import { userPath } from "discourse/lib/url";
-import { wavingHandURL } from "discourse/lib/waving-hand-url";
-
-export default Controller.extend({
-  application: controller(),
-
-  showLoginButton: equal("model.path", "login"),
-  anyButtons: or("showLoginButton", "showSignupButton"),
-
-  @discourseComputed("model.path")
-  bodyClass: (path) => `static-${path}`,
-
-  @discourseComputed("model.path")
-  showSignupButton() {
-    return (
-      this.get("model.path") === "login" && this.get("application.canSignUp")
-    );
-  },
-
-  @discourseComputed
-  wavingHandURL: () => wavingHandURL(),
-
-  actions: {
-    markFaqRead() {
-      const currentUser = this.currentUser;
-      if (currentUser) {
-        ajax(userPath("read-faq"), { type: "POST" }).then(() => {
-          currentUser.set("read_faq", true);
-        });
-      }
-    },
-  },
-});

app/assets/javascripts/discourse/app/lib/static-route-builder.js

@@ -1,50 +0,0 @@
-import DiscourseURL from "discourse/lib/url";
-import DiscourseRoute from "discourse/routes/discourse";
-import I18n from "I18n";
-import StaticPage from "discourse/models/static-page";
-
-const configs = {
-  faq: "faq_url",
-  tos: "tos_url",
-  privacy: "privacy_policy_url",
-};
-
-export default function (page) {
-  return DiscourseRoute.extend({
-    renderTemplate() {
-      this.render("static");
-    },
-
-    beforeModel(transition) {
-      const configKey = configs[page];
-      if (configKey && this.siteSettings[configKey].length > 0) {
-        transition.abort();
-        DiscourseURL.redirectTo(this.siteSettings[configKey]);
-      }
-    },
-
-    activate() {
-      this._super(...arguments);
-      DiscourseURL.jumpToElement(document.location.hash.slice(1));
-    },
-
-    model() {
-      return StaticPage.find(page);
-    },
-
-    setupController(controller, model) {
-      this.controllerFor("static").set("model", model);
-    },
-
-    titleToken() {
-      return I18n.t(page);
-    },
-
-    actions: {
-      didTransition() {
-        this.controllerFor("application").set("showFooter", true);
-        return true;
-      },
-    },
-  });
-}

app/assets/javascripts/discourse/app/routes/app-route-map.js

@@ -205,12 +205,14 @@ export default function () {
   this.route("associate-account", { path: "/associate/:token" });
   this.route("login-preferences");
   this.route("forgot-password", { path: "/password-reset" });
+
   this.route("faq", { path: "/faq" });
+  this.route("guidelines", { path: "/guidelines" });
+  this.route("conduct", { path: "/conduct" });
+  this.route("rules", { path: "/rules" });
+
   this.route("tos", { path: "/tos" });
   this.route("privacy", { path: "/privacy" });
-  this.route("guidelines", { path: "/guidelines" });
-  this.route("rules", { path: "/rules" });
-  this.route("conduct", { path: "/conduct" });
 
   this.route("new-topic", { path: "/new-topic" });
   this.route("new-message", { path: "/new-message" });

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

@@ -129,7 +129,7 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
     ),
 
     showForgotPassword() {
-      this.controllerFor("forgot-password").setProperties({
+      getOwner(this).lookup("controller:forgot-password").setProperties({
         offerHelp: null,
         helpSeen: false,
       });
@@ -259,7 +259,7 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
       window.location = getURL("/session/sso?return_path=" + returnPath);
     } else {
       this._autoLogin("login", {
-        notAuto: () => this.controllerFor("login").resetForm(),
+        notAuto: () => getOwner(this).lookup("controller:login").resetForm(),
       });
     }
   },
@@ -289,9 +289,11 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, {
     const methods = findAll();
 
     if (!this.siteSettings.enable_local_logins && methods.length === 1) {
-      this.controllerFor("login").send("externalLogin", methods[0], {
-        signup,
-      });
+      getOwner(this)
+        .lookup("controller:login")
+        .send("externalLogin", methods[0], {
+          signup,
+        });
     } else {
       showModal(modal, { modalClass, titleAriaElementId });
       notAuto?.();

app/assets/javascripts/discourse/app/routes/build-static-route.js

@@ -1,19 +0,0 @@
-import DiscourseRoute from "discourse/routes/discourse";
-import StaticPage from "discourse/models/static-page";
-
-export default function (pageName) {
-  const route = {
-    model() {
-      return StaticPage.find(pageName);
-    },
-
-    renderTemplate() {
-      this.render("static");
-    },
-
-    setupController(controller, model) {
-      this.controllerFor("static").set("model", model);
-    },
-  };
-  return DiscourseRoute.extend(route);
-}

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

@@ -1,3 +1,6 @@
-import staticRouteBuilder from "discourse/lib/static-route-builder";
+import FaqRoute from "./faq";
 
-export default staticRouteBuilder("conduct");
+// Conduct is effectively a faq alias
+export default class ConductRoute extends FaqRoute {
+  pageId = "conduct";
+}

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

@@ -1,3 +1,39 @@
-import staticRouteBuilder from "discourse/lib/static-route-builder";
+import DiscourseRoute from "discourse/routes/discourse";
+import { inject as service } from "@ember/service";
+import DiscourseURL from "discourse/lib/url";
+import StaticPage from "discourse/models/static-page";
+import I18n from "I18n";
+import { action } from "@ember/object";
 
-export default staticRouteBuilder("faq");
+export default class FaqRoute extends DiscourseRoute {
+  @service siteSettings;
+
+  pageId = "faq";
+  templateName = "faq";
+
+  activate() {
+    super.activate(...arguments);
+    DiscourseURL.jumpToElement(document.location.hash.slice(1));
+  }
+
+  beforeModel(transition) {
+    if (this.pageId === "faq" && this.siteSettings.faq_url) {
+      transition.abort();
+      DiscourseURL.redirectTo(this.siteSettings.faq_url);
+    }
+  }
+
+  model() {
+    return StaticPage.find(this.pageId);
+  }
+
+  titleToken() {
+    return I18n.t(this.pageId);
+  }
+
+  @action
+  didTransition() {
+    this.controllerFor("application").set("showFooter", true);
+    return true;
+  }
+}

app/assets/javascripts/discourse/app/routes/forgot-password.js

@@ -1,19 +1,15 @@
-import buildStaticRoute from "discourse/routes/build-static-route";
+import DiscourseRoute from "discourse/routes/discourse";
 import { defaultHomepage } from "discourse/lib/utilities";
 import { next } from "@ember/runloop";
 
-const ForgotPasswordRoute = buildStaticRoute("password-reset");
+export default class ForgotPasswordRoute extends DiscourseRoute {
+  async beforeModel() {
+    const { loginRequired } = this.controllerFor("application");
 
-ForgotPasswordRoute.reopen({
-  beforeModel() {
-    const loginRequired =
-      this.controllerFor("application").get("loginRequired");
-    this.replaceWith(
+    const e = await this.replaceWith(
       loginRequired ? "login" : `discovery.${defaultHomepage()}`
-    ).then((e) => {
-      next(() => e.send("showForgotPassword"));
-    });
-  },
-});
+    );
 
-export default ForgotPasswordRoute;
+    next(() => e.send("showForgotPassword"));
+  }
+}

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

@@ -1,3 +1,6 @@
-import staticRouteBuilder from "discourse/lib/static-route-builder";
+import FaqRoute from "./faq";
 
-export default staticRouteBuilder("guidelines");
+// Guidelines is a faq alias
+export default class GuidelinesRoute extends FaqRoute {
+  pageId = "guidelines";
+}

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

@@ -1,17 +1,25 @@
-import buildStaticRoute from "discourse/routes/build-static-route";
+import DiscourseRoute from "discourse/routes/discourse";
+import { inject as service } from "@ember/service";
 import { defaultHomepage } from "discourse/lib/utilities";
 import { next } from "@ember/runloop";
+import StaticPage from "discourse/models/static-page";
 
-const LoginRoute = buildStaticRoute("login");
+export default class LoginRoute extends DiscourseRoute {
+  @service siteSettings;
+
+  // `login-page` because `login` controller is the one for
+  // the login modal
+  controllerName = "login-page";
 
-LoginRoute.reopen({
   beforeModel() {
     if (!this.siteSettings.login_required) {
       this.replaceWith(`/${defaultHomepage()}`).then((e) => {
         next(() => e.send("showLogin"));
       });
     }
-  },
-});
+  }
 
-export default LoginRoute;
+  model() {
+    return StaticPage.find("login");
+  }
+}

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

@@ -1,3 +1,36 @@
-import staticRouteBuilder from "discourse/lib/static-route-builder";
+import DiscourseRoute from "discourse/routes/discourse";
+import { inject as service } from "@ember/service";
+import DiscourseURL from "discourse/lib/url";
+import StaticPage from "discourse/models/static-page";
+import I18n from "I18n";
+import { action } from "@ember/object";
 
-export default staticRouteBuilder("privacy");
+export default class PrivacyRoute extends DiscourseRoute {
+  @service siteSettings;
+
+  activate() {
+    super.activate(...arguments);
+    DiscourseURL.jumpToElement(document.location.hash.slice(1));
+  }
+
+  beforeModel(transition) {
+    if (this.siteSettings.privacy_policy_url) {
+      transition.abort();
+      DiscourseURL.redirectTo(this.siteSettings.privacy_policy_url);
+    }
+  }
+
+  model() {
+    return StaticPage.find("privacy");
+  }
+
+  titleToken() {
+    return I18n.t("privacy");
+  }
+
+  @action
+  didTransition() {
+    this.controllerFor("application").set("showFooter", true);
+    return true;
+  }
+}

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

@@ -1,3 +1,6 @@
-import staticRouteBuilder from "discourse/lib/static-route-builder";
+import FaqRoute from "./faq";
 
-export default staticRouteBuilder("rules");
+// Rules is effectively a faq alias
+export default class RulesRoute extends FaqRoute {
+  pageId = "rules";
+}

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

@@ -1,26 +1,22 @@
-import buildStaticRoute from "discourse/routes/build-static-route";
+import DiscourseRoute from "discourse/routes/discourse";
 import { next } from "@ember/runloop";
 
-const SignupRoute = buildStaticRoute("signup");
-
-SignupRoute.reopen({
+export default class SignupRoute extends DiscourseRoute {
   beforeModel() {
-    let canSignUp = this.controllerFor("application").get("canSignUp");
+    const { canSignUp } = this.controllerFor("application");
 
-    if (!this.siteSettings.login_required) {
-      this.replaceWith("discovery.latest").then((e) => {
-        if (canSignUp) {
-          next(() => e.send("showCreateAccount"));
-        }
-      });
-    } else {
+    if (this.siteSettings.login_required) {
       this.replaceWith("login").then((e) => {
         if (canSignUp) {
           next(() => e.send("showCreateAccount"));
         }
       });
+    } else {
+      this.replaceWith("discovery.latest").then((e) => {
+        if (canSignUp) {
+          next(() => e.send("showCreateAccount"));
+        }
+      });
     }
-  },
-});
-
-export default SignupRoute;
+  }
+}

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

@@ -1,3 +1,36 @@
-import staticRouteBuilder from "discourse/lib/static-route-builder";
+import DiscourseRoute from "discourse/routes/discourse";
+import { inject as service } from "@ember/service";
+import DiscourseURL from "discourse/lib/url";
+import StaticPage from "discourse/models/static-page";
+import I18n from "I18n";
+import { action } from "@ember/object";
 
-export default staticRouteBuilder("tos");
+export default class TosRoute extends DiscourseRoute {
+  @service siteSettings;
+
+  activate() {
+    super.activate(...arguments);
+    DiscourseURL.jumpToElement(document.location.hash.slice(1));
+  }
+
+  beforeModel(transition) {
+    if (this.siteSettings.tos_url) {
+      transition.abort();
+      DiscourseURL.redirectTo(this.siteSettings.tos_url);
+    }
+  }
+
+  model() {
+    return StaticPage.find("tos");
+  }
+
+  titleToken() {
+    return I18n.t("tos");
+  }
+
+  @action
+  didTransition() {
+    this.controllerFor("application").set("showFooter", true);
+    return true;
+  }
+}

app/assets/javascripts/discourse/app/templates/faq.hbs

@@ -0,0 +1,7 @@
+<DSection @bodyClass="static-{{this.model.path}}" @class="container">
+  <WatchRead>
+    <div class="contents clearfix body-page">
+      {{html-safe this.model.html}}
+    </div>
+  </WatchRead>
+</DSection>

app/assets/javascripts/discourse/app/templates/login.hbs

@@ -0,0 +1,28 @@
+<DSection @bodyClass="static-login" @class="container">
+  <div class="contents clearfix body-page">
+    <div class="login-welcome">
+      <PluginOutlet @name="above-login" @outletArgs={{hash model=this.model}} />
+      <div class="login-content">
+        {{html-safe this.model.html}}
+      </div>
+      <PluginOutlet @name="below-login" @outletArgs={{hash model=this.model}} />
+
+      <div class="body-page-button-container">
+        {{#if this.application.canSignUp}}
+          <DButton
+            @action={{route-action "showCreateAccount"}}
+            @class="btn-primary sign-up-button"
+            @label="sign_up"
+          />
+        {{/if}}
+
+        <DButton
+          @action={{route-action "showLogin"}}
+          @class="btn-primary login-button"
+          @icon="user"
+          @label="log_in"
+        />
+      </div>
+    </div>
+  </div>
+</DSection>

app/assets/javascripts/discourse/app/templates/privacy.hbs

@@ -0,0 +1,5 @@
+<DSection @bodyClass="static-privacy" @class="container">
+  <div class="contents clearfix body-page">
+    {{html-safe this.model.html}}
+  </div>
+</DSection>

app/assets/javascripts/discourse/app/templates/static.hbs

@@ -1,37 +0,0 @@
-<DSection @bodyClass={{this.bodyClass}} @class="container">
-  <WatchRead @action={{action "markFaqRead"}} @path={{this.model.path}}>
-    <div class="contents clearfix body-page">
-
-      <span>
-        <PluginOutlet @name="above-static" @connectorTagName="div" />
-      </span>
-
-      <div class="login-welcome">
-        {{html-safe this.model.html}}
-
-        <PluginOutlet @name="below-static" @connectorTagName="div" />
-
-        {{#if this.anyButtons}}
-          <div class="body-page-button-container">
-            {{#if this.showSignupButton}}
-              <DButton
-                @action={{route-action "showCreateAccount"}}
-                @class="btn-primary sign-up-button"
-                @label="sign_up"
-              />
-            {{/if}}
-
-            {{#if this.showLoginButton}}
-              <DButton
-                @action={{route-action "showLogin"}}
-                @class="btn-primary login-button"
-                @icon="user"
-                @label="log_in"
-              />
-            {{/if}}
-          </div>
-        {{/if}}
-      </div>
-    </div>
-  </WatchRead>
-</DSection>

app/assets/javascripts/discourse/app/templates/tos.hbs

@@ -0,0 +1,5 @@
+<DSection @bodyClass="static-tos" @class="container">
+  <div class="contents clearfix body-page">
+    {{html-safe this.model.html}}
+  </div>
+</DSection>

app/assets/javascripts/discourse/tests/acceptance/static-test.js

@@ -1,42 +1,97 @@
-import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers";
+import { acceptance } from "discourse/tests/helpers/qunit-helpers";
 import { currentRouteName, visit } from "@ember/test-helpers";
 import { test } from "qunit";
 
-acceptance("Static", function () {
-  test("Static Pages", async function (assert) {
+acceptance("Static pages", function () {
+  test("/faq", async function (assert) {
     await visit("/faq");
-    assert.ok(
+    assert.true(
       document.body.classList.contains("static-faq"),
-      "has the body class"
+      "/faq has the body class"
     );
-    assert.ok(exists(".body-page"), "The content is present");
+    assert.dom(".body-page").exists("The content is present");
+  });
 
+  test("/guidelines", async function (assert) {
     await visit("/guidelines");
-    assert.ok(
+    assert.true(
       document.body.classList.contains("static-guidelines"),
       "has the body class"
     );
-    assert.ok(exists(".body-page"), "The content is present");
+    assert.dom(".body-page").exists("The content is present");
+  });
 
+  test("/conduct", async function (assert) {
+    await visit("/conduct");
+    assert.true(
+      document.body.classList.contains("static-conduct"),
+      "has the body class"
+    );
+    assert.dom(".body-page").exists("The content is present");
+  });
+
+  test("/tos", async function (assert) {
     await visit("/tos");
-    assert.ok(
+    assert.true(
       document.body.classList.contains("static-tos"),
       "has the body class"
     );
-    assert.ok(exists(".body-page"), "The content is present");
+    assert.dom(".body-page").exists("The content is present");
+  });
 
+  test("/privacy", async function (assert) {
     await visit("/privacy");
-    assert.ok(
+    assert.true(
       document.body.classList.contains("static-privacy"),
       "has the body class"
     );
-    assert.ok(exists(".body-page"), "The content is present");
+    assert.dom(".body-page").exists("The content is present");
+  });
 
+  test("/rules", async function (assert) {
+    await visit("/rules");
+    assert.true(
+      document.body.classList.contains("static-rules"),
+      "has the body class"
+    );
+    assert.dom(".body-page").exists("The content is present");
+  });
+
+  test("Login redirect", async function (assert) {
     await visit("/login");
+
     assert.strictEqual(
       currentRouteName(),
       "discovery.latest",
-      "it redirects them to latest unless `login_required`"
+      "it redirects to /latest"
     );
   });
+
+  test("Login-required page", async function (assert) {
+    this.siteSettings.login_required = true;
+    await visit("/login");
+
+    assert.strictEqual(currentRouteName(), "login");
+    assert.dom(".body-page").exists("The content is present");
+    assert.dom(".sign-up-button").exists();
+    assert.dom(".login-button").exists();
+  });
+
+  test("Signup redirect", async function (assert) {
+    await visit("/signup");
+
+    assert.strictEqual(
+      currentRouteName(),
+      "discovery.latest",
+      "it redirects to /latest"
+    );
+  });
+
+  test("Signup redirect with login_required", async function (assert) {
+    this.siteSettings.login_required = true;
+    await visit("/signup");
+
+    assert.strictEqual(currentRouteName(), "login");
+    assert.dom(".body-page").exists("The content is present");
+  });
 });

config/routes.rb

@@ -416,18 +416,17 @@ Discourse::Application.routes.draw do
     get "composer_messages" => "composer_messages#index"
     get "composer_messages/user_not_seen_in_a_while" => "composer_messages#user_not_seen_in_a_while"
 
-    resources :static
     post "login" => "static#enter"
-    get "login" => "static#show", :id => "login"
-    get "password-reset" => "static#show", :id => "password_reset"
-    get "faq" => "static#show", :id => "faq"
-    get "tos" => "static#show", :id => "tos", :as => "tos"
-    get "privacy" => "static#show", :id => "privacy", :as => "privacy"
-    get "signup" => "static#show", :id => "signup"
-    get "login-preferences" => "static#show", :id => "login"
 
-    %w[guidelines rules conduct].each do |faq_alias|
-      get faq_alias => "static#show", :id => "guidelines", :as => faq_alias
+    get "login" => "static#show", :id => "login"
+    get "login-preferences" => "static#show", :id => "login"
+    get "signup" => "static#show", :id => "signup"
+    get "password-reset" => "static#show", :id => "password_reset"
+    get "privacy" => "static#show", :id => "privacy", :as => "privacy"
+    get "tos" => "static#show", :id => "tos", :as => "tos"
+    get "faq" => "static#show", :id => "faq"
+    %w[guidelines rules conduct].each do |guidelines_alias|
+      get guidelines_alias => "static#show", :id => "guidelines", :as => guidelines_alias
     end
 
     get "my/*path", to: "users#my_redirect"