From 6b3f2576c35d6e8d10e27a8befd66eb35f33a6f9 Mon Sep 17 00:00:00 2001 From: Frank Date: Thu, 17 Nov 2022 14:03:38 +0800 Subject: [PATCH] FEATURE: add a global setting to support custom docs url path (#107) * FEATURE: add a global setting to support custom docs url path This commit adds a GlobalSetting `docs_path` to support custom docs url path for sites that do not want docs page to live at `/docs` and have a customized path. * Fixed the route declaration * Test and linting * Update server.en.yml * Fixed doc test * Fixed linting. * Testing qunit test fix * Fixed tests * Prettified tests * Changed the implementation from SiteSetting to GlobalSetting instead. * Fixed tests * Cleanup * Using Site instead of .js.erb to pass GlobalSetting.docs_url to the front end. Also fixed front end tests * Remove references to obsolete site setting * remove unused fixture file * Rename `docs_url` to `docs_path` and use camelCase in JavaScript * Add serializer tests Co-authored-by: Arpit Jalan --- .../discourse/components/docs-topic-list.js | 2 + .../javascripts/discourse/docs-route-map.js | 6 ++- .../discourse/initializers/setup-docs.js | 8 +++- assets/javascripts/discourse/models/docs.js | 4 +- .../templates/components/docs-topic-list.hbs | 2 +- .../discourse/templates/docs-topic-link.hbr | 2 +- .../templates/docs-topic-list-item.hbr | 2 +- assets/javascripts/lib/get-docs.js | 5 +++ lib/docs/engine.rb | 4 +- lib/docs/query.rb | 2 +- plugin.rb | 8 +++- spec/requests/docs_controller_spec.rb | 43 ++++++++++--------- spec/requests/robots_txt_controller_spec.rb | 3 +- spec/serializers/site_serializer_spec.rb | 26 +++++++++++ .../acceptance/docs-sidebar-test.js | 16 +++++-- test/javascripts/acceptance/docs-test.js | 12 ++++-- 16 files changed, 104 insertions(+), 41 deletions(-) create mode 100644 assets/javascripts/lib/get-docs.js create mode 100644 spec/serializers/site_serializer_spec.rb diff --git a/assets/javascripts/discourse/components/docs-topic-list.js b/assets/javascripts/discourse/components/docs-topic-list.js index 55a6ebc..0b34362 100644 --- a/assets/javascripts/discourse/components/docs-topic-list.js +++ b/assets/javascripts/discourse/components/docs-topic-list.js @@ -1,9 +1,11 @@ import Component from "@ember/component"; import { action } from "@ember/object"; import discourseComputed from "discourse-common/utils/decorators"; +import { getDocs } from "../../lib/get-docs"; export default Component.extend({ classNames: "docs-topic-list", + urlPath: getDocs(), @discourseComputed("order") sortTitle(order) { diff --git a/assets/javascripts/discourse/docs-route-map.js b/assets/javascripts/discourse/docs-route-map.js index 28b9e1a..069cea4 100644 --- a/assets/javascripts/discourse/docs-route-map.js +++ b/assets/javascripts/discourse/docs-route-map.js @@ -1,5 +1,9 @@ +import { getDocs } from "../lib/get-docs"; + export default function () { - this.route("docs", { path: "/docs" }, function () { + const docsPath = getDocs(); + + this.route("docs", { path: "/" + docsPath }, function () { this.route("index", { path: "/" }); }); } diff --git a/assets/javascripts/discourse/initializers/setup-docs.js b/assets/javascripts/discourse/initializers/setup-docs.js index 92d4c24..f0036f3 100644 --- a/assets/javascripts/discourse/initializers/setup-docs.js +++ b/assets/javascripts/discourse/initializers/setup-docs.js @@ -1,16 +1,20 @@ import { withPluginApi } from "discourse/lib/plugin-api"; import I18n from "I18n"; +import { getDocs } from "../../lib/get-docs"; function initialize(api, container) { const siteSettings = container.lookup("site-settings:main"); + const docsPath = getDocs(); - api.addKeyboardShortcut("g e", "", { path: "/docs" }); + api.addKeyboardShortcut("g e", "", { + path: "/" + docsPath, + }); if (siteSettings.docs_add_to_top_menu) { api.addNavigationBarItem({ name: "docs", displayName: I18n.t("docs.title"), - href: "/docs", + href: "/" + docsPath, }); } } diff --git a/assets/javascripts/discourse/models/docs.js b/assets/javascripts/discourse/models/docs.js index e8e8d00..48508f2 100644 --- a/assets/javascripts/discourse/models/docs.js +++ b/assets/javascripts/discourse/models/docs.js @@ -1,8 +1,10 @@ import EmberObject from "@ember/object"; import { ajax } from "discourse/lib/ajax"; import Topic from "discourse/models/topic"; +import { getDocs } from "../../lib/get-docs"; const Docs = EmberObject.extend({}); +const docsPath = getDocs(); Docs.reopenClass({ list(params) { @@ -34,7 +36,7 @@ Docs.reopenClass({ filters.push("track_visit=true"); } - return ajax(`/docs.json?${filters.join("&")}`).then((data) => { + return ajax(`/${docsPath}.json?${filters.join("&")}`).then((data) => { data.topics.topic_list.topics = data.topics.topic_list.topics.map( (topic) => Topic.create(topic) ); diff --git a/assets/javascripts/discourse/templates/components/docs-topic-list.hbs b/assets/javascripts/discourse/templates/components/docs-topic-list.hbs index 9a92c92..8cb2ddc 100644 --- a/assets/javascripts/discourse/templates/components/docs-topic-list.hbs +++ b/assets/javascripts/discourse/templates/components/docs-topic-list.hbs @@ -28,7 +28,7 @@ {{#each topics as |topic|}} - {{raw "docs-topic-list-item" topic=topic}} + {{raw "docs-topic-list-item" topic=topic urlPath=urlPath}} {{/each}} diff --git a/assets/javascripts/discourse/templates/docs-topic-link.hbr b/assets/javascripts/discourse/templates/docs-topic-link.hbr index a0e9a1c..ab7afc2 100644 --- a/assets/javascripts/discourse/templates/docs-topic-link.hbr +++ b/assets/javascripts/discourse/templates/docs-topic-link.hbr @@ -1 +1 @@ -{{{topic.fancyTitle}}} +{{{topic.fancyTitle}}} diff --git a/assets/javascripts/discourse/templates/docs-topic-list-item.hbr b/assets/javascripts/discourse/templates/docs-topic-list-item.hbr index 195212a..c8f2a8c 100644 --- a/assets/javascripts/discourse/templates/docs-topic-list-item.hbr +++ b/assets/javascripts/discourse/templates/docs-topic-list-item.hbr @@ -2,7 +2,7 @@ {{~raw "topic-status" topic=topic}} - {{~raw "docs-topic-link" topic=topic}} + {{~raw "docs-topic-link" topic=topic urlPath=urlPath}} {{category-link topic.category}} diff --git a/assets/javascripts/lib/get-docs.js b/assets/javascripts/lib/get-docs.js new file mode 100644 index 0000000..0fb7190 --- /dev/null +++ b/assets/javascripts/lib/get-docs.js @@ -0,0 +1,5 @@ +import Site from "discourse/models/site"; + +export function getDocs() { + return Site.currentProp("docs_path") || "docs"; +} diff --git a/lib/docs/engine.rb b/lib/docs/engine.rb index f276eb4..7d88fe3 100644 --- a/lib/docs/engine.rb +++ b/lib/docs/engine.rb @@ -6,8 +6,8 @@ module ::Docs config.after_initialize do Discourse::Application.routes.append do - mount ::Docs::Engine, at: '/docs' - get '/knowledge-explorer', to: redirect("/docs") + mount ::Docs::Engine, at: "/#{GlobalSetting.docs_path}" + get '/knowledge-explorer', to: redirect("/#{GlobalSetting.docs_path}") end end end diff --git a/lib/docs/query.rb b/lib/docs/query.rb index 12396ef..41c576f 100644 --- a/lib/docs/query.rb +++ b/lib/docs/query.rb @@ -170,7 +170,7 @@ module Docs filters.push('page=1') end - "/docs.json?#{filters.join('&')}" + "/#{GlobalSetting.docs_path}.json?#{filters.join('&')}" end end end diff --git a/plugin.rb b/plugin.rb index 563e588..46d4fb0 100644 --- a/plugin.rb +++ b/plugin.rb @@ -20,6 +20,8 @@ register_svg_icon 'sort-numeric-down' load File.expand_path('lib/docs/engine.rb', __dir__) load File.expand_path('lib/docs/query.rb', __dir__) +GlobalSetting.add_default :docs_path, "docs" + after_initialize do require_dependency 'search' @@ -55,6 +57,10 @@ after_initialize do end any_user_agent[:disallow] ||= [] - any_user_agent[:disallow] << "/docs/" + any_user_agent[:disallow] << "/#{GlobalSetting.docs_path}/" + end + + add_to_serializer(:site, :docs_path) do + GlobalSetting.docs_path end end diff --git a/spec/requests/docs_controller_spec.rb b/spec/requests/docs_controller_spec.rb index 136a9d1..6a0715d 100644 --- a/spec/requests/docs_controller_spec.rb +++ b/spec/requests/docs_controller_spec.rb @@ -13,12 +13,13 @@ describe Docs::DocsController do SiteSetting.docs_enabled = true SiteSetting.docs_categories = category.id.to_s SiteSetting.docs_tags = 'test' + GlobalSetting.stubs(:docs_path).returns('docs') end describe 'docs data' do context 'when any user' do it 'should return the right response' do - get '/docs.json' + get "/#{GlobalSetting.docs_path}.json" expect(response.status).to eq(200) @@ -31,7 +32,7 @@ describe Docs::DocsController do end it 'should return a topic count' do - get '/docs.json' + get "/#{GlobalSetting.docs_path}.json" json = response.parsed_body topic_count = json['topic_count'] @@ -50,7 +51,7 @@ describe Docs::DocsController do end it 'should not show topics in private categories without permissions' do - get '/docs.json' + get "/#{GlobalSetting.docs_path}.json" json = JSON.parse(response.body) topics = json['topics']['topic_list']['topics'] @@ -62,7 +63,7 @@ describe Docs::DocsController do admin = Fabricate(:admin) sign_in(admin) - get '/docs.json' + get "/#{GlobalSetting.docs_path}.json" json = JSON.parse(response.body) topics = json['topics']['topic_list']['topics'] @@ -76,7 +77,7 @@ describe Docs::DocsController do fab!(:tag3) { Fabricate(:tag, topics: [topic], name: 'test3') } it 'should return a list filtered by tag' do - get '/docs.json?tags=test' + get "/#{GlobalSetting.docs_path}.json?tags=test" expect(response.status).to eq(200) @@ -87,7 +88,7 @@ describe Docs::DocsController do end it 'should properly filter with more than two tags' do - get '/docs.json?tags=test%7ctest2%7ctest3' + get "/#{GlobalSetting.docs_path}.json?tags=test%7ctest2%7ctest3" expect(response.status).to eq(200) @@ -109,7 +110,7 @@ describe Docs::DocsController do end it 'should return a list filtered by category' do - get "/docs.json?category=#{category2.id}" + get "/#{GlobalSetting.docs_path}.json?category=#{category2.id}" expect(response.status).to eq(200) @@ -122,7 +123,7 @@ describe Docs::DocsController do end it 'ignores category filter when incorrect argument' do - get "/docs.json?category=hack" + get "/#{GlobalSetting.docs_path}.json?category=hack" expect(response.status).to eq(200) @@ -139,7 +140,7 @@ describe Docs::DocsController do context 'when ordering results' do describe 'by title' do it 'should return the list ordered descending' do - get "/docs.json?order=title" + get "/#{GlobalSetting.docs_path}.json?order=title" expect(response.status).to eq(200) @@ -151,7 +152,7 @@ describe Docs::DocsController do end it 'should return the list ordered ascending with an additional parameter' do - get "/docs.json?order=title&ascending=true" + get "/#{GlobalSetting.docs_path}.json?order=title&ascending=true" expect(response.status).to eq(200) @@ -169,7 +170,7 @@ describe Docs::DocsController do end it 'should return the list ordered descending' do - get "/docs.json?order=activity" + get "/#{GlobalSetting.docs_path}.json?order=activity" expect(response.status).to eq(200) @@ -181,7 +182,7 @@ describe Docs::DocsController do end it 'should return the list ordered ascending with an additional parameter' do - get "/docs.json?order=activity&ascending=true" + get "/#{GlobalSetting.docs_path}.json?order=activity&ascending=true" expect(response.status).to eq(200) @@ -211,7 +212,7 @@ describe Docs::DocsController do end it 'should correctly filter topics' do - get "/docs.json?search=banana" + get "/#{GlobalSetting.docs_path}.json?search=banana" expect(response.status).to eq(200) @@ -225,7 +226,7 @@ describe Docs::DocsController do expect(topics.size).to eq(2) - get "/docs.json?search=walk" + get "/#{GlobalSetting.docs_path}.json?search=walk" json = JSON.parse(response.body) topics = json['topics']['topic_list']['topics'] @@ -239,13 +240,13 @@ describe Docs::DocsController do let!(:non_ke_topic) { Fabricate(:topic) } it 'should correctly grab the topic' do - get "/docs.json?topic=#{topic.id}" + get "/#{GlobalSetting.docs_path}.json?topic=#{topic.id}" expect(response.parsed_body['topic']['id']).to eq(topic.id) end it 'should get topics matching a selected docs tag or category' do - get "/docs.json?topic=#{non_ke_topic.id}" + get "/#{GlobalSetting.docs_path}.json?topic=#{non_ke_topic.id}" expect(response.parsed_body['topic']).to be_blank end @@ -253,7 +254,7 @@ describe Docs::DocsController do it 'should return a docs topic when only tags are added to settings' do SiteSetting.docs_categories = nil - get "/docs.json?topic=#{topic.id}" + get "/#{GlobalSetting.docs_path}.json?topic=#{topic.id}" expect(response.parsed_body['topic']['id']).to eq(topic.id) end @@ -261,7 +262,7 @@ describe Docs::DocsController do it 'should return a docs topic when only categories are added to settings' do SiteSetting.docs_tags = nil - get "/docs.json?topic=#{topic.id}" + get "/#{GlobalSetting.docs_path}.json?topic=#{topic.id}" expect(response.parsed_body['topic']['id']).to eq(topic.id) end @@ -270,19 +271,19 @@ describe Docs::DocsController do admin = Fabricate(:admin) sign_in(admin) expect do - get "/docs.json?topic=#{topic.id}" + get "/#{GlobalSetting.docs_path}.json?topic=#{topic.id}" end.to change { TopicViewItem.count }.by(1) end it 'should create TopicUser if authenticated' do expect do - get "/docs.json?topic=#{topic.id}&track_visit=true" + get "/#{GlobalSetting.docs_path}.json?topic=#{topic.id}&track_visit=true" end.not_to change { TopicUser.count } admin = Fabricate(:admin) sign_in(admin) expect do - get "/docs.json?topic=#{topic.id}&track_visit=true" + get "/#{GlobalSetting.docs_path}.json?topic=#{topic.id}&track_visit=true" end.to change { TopicUser.count }.by(1) end end diff --git a/spec/requests/robots_txt_controller_spec.rb b/spec/requests/robots_txt_controller_spec.rb index 3a8536d..66671ef 100644 --- a/spec/requests/robots_txt_controller_spec.rb +++ b/spec/requests/robots_txt_controller_spec.rb @@ -5,12 +5,13 @@ require 'rails_helper' describe RobotsTxtController do before do SiteSetting.docs_enabled = true + GlobalSetting.stubs(:docs_path).returns('docs') end it 'adds /docs/ to robots.txt' do get '/robots.txt' expect(response.body).to include('User-agent: *') - expect(response.body).to include('Disallow: /docs/') + expect(response.body).to include("Disallow: /#{GlobalSetting.docs_path}/") end end diff --git a/spec/serializers/site_serializer_spec.rb b/spec/serializers/site_serializer_spec.rb new file mode 100644 index 0000000..eed71cb --- /dev/null +++ b/spec/serializers/site_serializer_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe SiteSerializer do + fab!(:user) { Fabricate(:user) } + let(:guardian) { Guardian.new(user) } + + before do + SiteSetting.docs_enabled = true + GlobalSetting.stubs(:docs_path).returns('docs') + end + + it 'returns correct default value' do + data = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + + expect(data[:docs_path]).to eq("docs") + end + + it 'returns custom path based on global setting' do + GlobalSetting.stubs(:docs_path).returns('custom_path') + data = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + + expect(data[:docs_path]).to eq("custom_path") + end +end diff --git a/test/javascripts/acceptance/docs-sidebar-test.js b/test/javascripts/acceptance/docs-sidebar-test.js index 8dbd554..f66733a 100644 --- a/test/javascripts/acceptance/docs-sidebar-test.js +++ b/test/javascripts/acceptance/docs-sidebar-test.js @@ -9,9 +9,11 @@ import { import docsFixtures from "../fixtures/docs"; import { cloneJSON } from "discourse-common/lib/object"; +let DOCS_URL_PATH = "docs"; + acceptance("Docs - Sidebar with docs disabled", function (needs) { needs.user(); - + needs.site({ docs_path: DOCS_URL_PATH }); needs.settings({ docs_enabled: false, enable_experimental_sidebar_hamburger: true, @@ -34,7 +36,7 @@ acceptance("Docs - Sidebar with docs disabled", function (needs) { acceptance("Docs - Sidebar with docs enabled", function (needs) { needs.user(); - + needs.site({ docs_path: DOCS_URL_PATH }); needs.settings({ docs_enabled: true, enable_experimental_sidebar_hamburger: true, @@ -42,7 +44,9 @@ acceptance("Docs - Sidebar with docs enabled", function (needs) { }); needs.pretender((server, helper) => { - server.get("/docs.json", () => helper.response(cloneJSON(docsFixtures))); + server.get("/" + DOCS_URL_PATH + ".json", () => + helper.response(cloneJSON(docsFixtures)) + ); }); test("clicking on docs link", async function (assert) { @@ -66,6 +70,10 @@ acceptance("Docs - Sidebar with docs enabled", function (needs) { await click(".sidebar-section-link-docs"); - assert.strictEqual(currentURL(), "/docs", "it navigates to the right page"); + assert.strictEqual( + currentURL(), + "/" + DOCS_URL_PATH, + "it navigates to the right page" + ); }); }); diff --git a/test/javascripts/acceptance/docs-test.js b/test/javascripts/acceptance/docs-test.js index 9d299b2..58cbbdd 100644 --- a/test/javascripts/acceptance/docs-test.js +++ b/test/javascripts/acceptance/docs-test.js @@ -8,14 +8,17 @@ import { test } from "qunit"; import docsFixtures from "../fixtures/docs"; import { click, visit } from "@ember/test-helpers"; +let DOCS_URL_PATH = "docs"; + acceptance("Docs", function (needs) { needs.user(); + needs.site({ docs_path: DOCS_URL_PATH }); needs.settings({ docs_enabled: true, }); needs.pretender((server, helper) => { - server.get("/docs.json", (request) => { + server.get("/" + DOCS_URL_PATH + ".json", (request) => { if (request.queryParams.category === "1") { const fixture = JSON.parse(JSON.stringify(docsFixtures)); @@ -54,7 +57,7 @@ acceptance("Docs", function (needs) { }); test("selecting a category", async function (assert) { - await visit("/docs"); + await visit("/" + DOCS_URL_PATH); assert.equal(count(".docs-category.selected"), 0); await click(".docs-item.docs-category"); @@ -64,12 +67,13 @@ acceptance("Docs", function (needs) { acceptance("Docs - empty state", function (needs) { needs.user(); + needs.site({ docs_path: DOCS_URL_PATH }); needs.settings({ docs_enabled: true, }); needs.pretender((server, helper) => { - server.get("/docs.json", () => { + server.get("/" + DOCS_URL_PATH + ".json", () => { const response = { tags: [], categories: [], @@ -90,7 +94,7 @@ acceptance("Docs - empty state", function (needs) { }); test("shows the empty state panel when there are no docs", async function (assert) { - await visit("/docs"); + await visit("/" + DOCS_URL_PATH); assert.ok(exists("div.empty-state")); }); });