diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index e33973cd44..f1520eb5b2 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -12,13 +12,9 @@ import { fetchUnseenMentions } from "discourse/lib/link-mentions"; import { - linkSeenCategoryHashtags, - fetchUnseenCategoryHashtags -} from "discourse/lib/link-category-hashtags"; -import { - linkSeenTagHashtags, - fetchUnseenTagHashtags -} from "discourse/lib/link-tag-hashtag"; + linkSeenHashtags, + fetchUnseenHashtags +} from "discourse/lib/link-hashtags"; import Composer from "discourse/models/composer"; import { load, LOADING_ONEBOX_CSS_CLASS } from "pretty-text/oneboxer"; import { applyInlineOneboxes } from "pretty-text/inline-oneboxer"; @@ -520,16 +516,13 @@ export default Component.extend({ }); }, - _renderUnseenCategoryHashtags($preview, unseen) { - fetchUnseenCategoryHashtags(unseen).then(() => { - linkSeenCategoryHashtags($preview); - }); - }, - - _renderUnseenTagHashtags($preview, unseen) { - fetchUnseenTagHashtags(unseen).then(() => { - linkSeenTagHashtags($preview); - }); + _renderUnseenHashtags($preview) { + const unseen = linkSeenHashtags($preview); + if (unseen.length > 0) { + fetchUnseenHashtags(unseen).then(() => { + linkSeenHashtags($preview); + }); + } }, _loadInlineOneboxes(inline) { @@ -927,30 +920,10 @@ export default Component.extend({ this._warnMentionedGroups($preview); this._warnCannotSeeMention($preview); - // Paint category hashtags - const unseenCategoryHashtags = linkSeenCategoryHashtags($preview); - if (unseenCategoryHashtags.length) { - debounce( - this, - this._renderUnseenCategoryHashtags, - $preview, - unseenCategoryHashtags, - 450 - ); - } - - // Paint tag hashtags - if (this.siteSettings.tagging_enabled) { - const unseenTagHashtags = linkSeenTagHashtags($preview); - if (unseenTagHashtags.length) { - debounce( - this, - this._renderUnseenTagHashtags, - $preview, - unseenTagHashtags, - 450 - ); - } + // Paint category and tag hashtags + const unseenHashtags = linkSeenHashtags($preview); + if (unseenHashtags.length > 0) { + debounce(this, this._renderUnseenHashtags, $preview, 450); } // Paint oneboxes diff --git a/app/assets/javascripts/discourse/app/lib/link-category-hashtags.js b/app/assets/javascripts/discourse/app/lib/link-category-hashtags.js deleted file mode 100644 index f074c69cc7..0000000000 --- a/app/assets/javascripts/discourse/app/lib/link-category-hashtags.js +++ /dev/null @@ -1,61 +0,0 @@ -import { schedule } from "@ember/runloop"; -import { ajax } from "discourse/lib/ajax"; -import { replaceSpan } from "discourse/lib/category-hashtags"; - -const validCategoryHashtags = {}; -const checkedCategoryHashtags = []; -const testedClass = `hashtag-category-tested`; - -function updateFound($hashtags, categorySlugs) { - schedule("afterRender", () => { - $hashtags.each((index, hashtag) => { - const categorySlug = categorySlugs[index]; - const link = validCategoryHashtags[categorySlug]; - const $hashtag = $(hashtag); - - if (link) { - if ($hashtag.data("type") !== "tag") { - replaceSpan($hashtag, categorySlug, link, "category"); - } - } else if (checkedCategoryHashtags.indexOf(categorySlug) !== -1) { - if (hashtag.tagName !== "A") { - $hashtag.addClass(testedClass); - } - } - }); - }); -} - -export function linkSeenCategoryHashtags($elem) { - const $hashtags = $(`span.hashtag:not(.${testedClass}), a.hashtag`, $elem); - const unseen = []; - - if ($hashtags.length) { - const categorySlugs = $hashtags.map((_, hashtag) => - $(hashtag) - .text() - .substr(1) - ); - if (categorySlugs.length) { - _.uniq(categorySlugs).forEach(categorySlug => { - if (checkedCategoryHashtags.indexOf(categorySlug) === -1) { - unseen.push(categorySlug); - } - }); - } - updateFound($hashtags, categorySlugs); - } - - return unseen; -} - -export function fetchUnseenCategoryHashtags(categorySlugs) { - return ajax("/category_hashtags/check", { - data: { category_slugs: categorySlugs } - }).then(response => { - response.valid.forEach(category => { - validCategoryHashtags[category.slug] = category.url; - }); - checkedCategoryHashtags.push.apply(checkedCategoryHashtags, categorySlugs); - }); -} diff --git a/app/assets/javascripts/discourse/app/lib/link-hashtags.js b/app/assets/javascripts/discourse/app/lib/link-hashtags.js new file mode 100644 index 0000000000..6456238ed8 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/link-hashtags.js @@ -0,0 +1,51 @@ +import { schedule } from "@ember/runloop"; +import { ajax } from "discourse/lib/ajax"; +import { replaceSpan } from "discourse/lib/category-hashtags"; +import { TAG_HASHTAG_POSTFIX } from "discourse/lib/tag-hashtags"; + +const categoryHashtags = {}; +const tagHashtags = {}; +const checkedHashtags = new Set(); + +export function linkSeenHashtags($elem) { + const $hashtags = $elem.find("span.hashtag"); + if ($hashtags.length === 0) { + return []; + } + + const slugs = [...$hashtags.map((_, hashtag) => hashtag.innerText.substr(1))]; + + schedule("afterRender", () => { + $hashtags.each((index, hashtag) => { + let slug = slugs[index]; + const hasTagSuffix = slug.endsWith(TAG_HASHTAG_POSTFIX); + if (hasTagSuffix) { + slug = slug.substr(0, slug.length - TAG_HASHTAG_POSTFIX.length); + } + + if (categoryHashtags[slug] && !hasTagSuffix) { + replaceSpan($(hashtag), slug, categoryHashtags[slug]); + } else if (tagHashtags[slug]) { + replaceSpan($(hashtag), slug, tagHashtags[slug]); + } + }); + }); + + return slugs.uniq().filter(slug => !checkedHashtags.has(slug)); +} + +export function fetchUnseenHashtags(slugs) { + return ajax("/hashtags", { + data: { slugs } + }).then(response => { + Object.keys(response.categories).forEach(slug => { + categoryHashtags[slug] = response.categories[slug]; + }); + + Object.keys(response.tags).forEach(slug => { + tagHashtags[slug] = response.tags[slug]; + }); + + slugs.forEach(checkedHashtags.add, checkedHashtags); + }); +} diff --git a/app/assets/javascripts/discourse/app/lib/link-tag-hashtag.js b/app/assets/javascripts/discourse/app/lib/link-tag-hashtag.js deleted file mode 100644 index 1d689fefdd..0000000000 --- a/app/assets/javascripts/discourse/app/lib/link-tag-hashtag.js +++ /dev/null @@ -1,62 +0,0 @@ -import { schedule } from "@ember/runloop"; -import { ajax } from "discourse/lib/ajax"; -import { replaceSpan } from "discourse/lib/category-hashtags"; -import { TAG_HASHTAG_POSTFIX } from "discourse/lib/tag-hashtags"; - -const validTagHashtags = {}; -const checkedTagHashtags = []; -const testedClass = "hashtag-tag-tested"; - -function updateFound($hashtags, tagValues) { - schedule("afterRender", () => { - $hashtags.each((index, hashtag) => { - const tagValue = tagValues[index]; - const link = validTagHashtags[tagValue]; - const $hashtag = $(hashtag); - - if (link) { - if (!$hashtag.data("type") || $hashtag.data("type") === "tag") { - replaceSpan($hashtag, tagValue, link, $hashtag.data("type")); - } - } else if (checkedTagHashtags.indexOf(tagValue) !== -1) { - $hashtag.addClass(testedClass); - } - }); - }); -} - -export function linkSeenTagHashtags($elem) { - const $hashtags = $(`span.hashtag:not(.${testedClass})`, $elem); - const unseen = []; - - if ($hashtags.length) { - const tagValues = $hashtags.map((_, hashtag) => { - let text = $(hashtag).text(); - if (text.endsWith(TAG_HASHTAG_POSTFIX)) { - text = text.slice(0, -TAG_HASHTAG_POSTFIX.length); - $(hashtag).data("type", "tag"); - } - return text.substr(1); - }); - - if (tagValues.length) { - _.uniq(tagValues).forEach(tagValue => { - if (checkedTagHashtags.indexOf(tagValue) === -1) unseen.push(tagValue); - }); - } - updateFound($hashtags, tagValues); - } - - return unseen; -} - -export function fetchUnseenTagHashtags(tagValues) { - return ajax("/tags/check", { data: { tag_values: tagValues } }).then( - response => { - response.valid.forEach(tag => { - validTagHashtags[tag.value] = tag.url; - }); - checkedTagHashtags.push.apply(checkedTagHashtags, tagValues); - } - ); -} diff --git a/app/controllers/category_hashtags_controller.rb b/app/controllers/category_hashtags_controller.rb deleted file mode 100644 index b40b9f6823..0000000000 --- a/app/controllers/category_hashtags_controller.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -class CategoryHashtagsController < ApplicationController - requires_login - - def check - category_slugs = params[:category_slugs] - - ids = category_slugs.map { |category_slug| Category.query_from_hashtag_slug(category_slug).try(:id) } - - slugs_and_urls = {} - - Category.secured(guardian).where(id: ids).each do |category| - slugs_and_urls[category.slug_path.last(2).join(':')] ||= category.url - end - - render json: { - valid: slugs_and_urls.map { |slug, url| { slug: slug, url: url } } - } - end -end diff --git a/app/controllers/hashtags_controller.rb b/app/controllers/hashtags_controller.rb new file mode 100644 index 0000000000..6fa0cf282c --- /dev/null +++ b/app/controllers/hashtags_controller.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +class HashtagsController < ApplicationController + requires_login + + HASHTAGS_PER_REQUEST = 20 + + def show + raise Discourse::InvalidParameters.new(:slugs) if !params[:slugs].is_a?(Array) + + all_slugs = [] + tag_slugs = [] + + params[:slugs][0..HASHTAGS_PER_REQUEST].each do |slug| + if slug.end_with?(PrettyText::Helpers::TAG_HASHTAG_POSTFIX) + tag_slugs << slug.chomp(PrettyText::Helpers::TAG_HASHTAG_POSTFIX) + else + all_slugs << slug + end + end + + # Try to resolve hashtags as categories first + category_slugs_and_ids = all_slugs.map { |slug| [slug, Category.query_from_hashtag_slug(slug)&.id] }.to_h + category_ids_and_urls = Category + .secured(guardian) + .select(:id, :slug, :parent_category_id) # fields required for generating category URL + .where(id: category_slugs_and_ids.values) + .map { |c| [c.id, c.url] } + .to_h + categories_hashtags = {} + category_slugs_and_ids.each do |slug, id| + if category_url = category_ids_and_urls[id] + categories_hashtags[slug] = category_url + end + end + + # Resolve remaining hashtags as tags + tag_hashtags = {} + if SiteSetting.tagging_enabled + tag_slugs += (all_slugs - categories_hashtags.keys) + DiscourseTagging.filter_visible(Tag.where_name(tag_slugs), guardian).each do |tag| + tag_hashtags[tag.name] = tag.full_url + end + end + + render json: { categories: categories_hashtags, tags: tag_hashtags } + end +end diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 1e86d108f2..136a2e8467 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -12,7 +12,6 @@ class TagsController < ::ApplicationController :show, :tag_feed, :search, - :check_hashtag, :info, Discourse.anonymous_filters.map { |f| :"show_#{f}" } ].flatten @@ -283,14 +282,6 @@ class TagsController < ::ApplicationController render json: { notification_level: level, tag_id: tag.id } end - def check_hashtag - valid_tags = Tag.where_name(params[:tag_values]).map do |tag| - { value: tag.name, url: tag.full_url } - end.compact - - render json: { valid: valid_tags } - end - def personal_messages guardian.ensure_can_tag_pms! allowed_user = fetch_user_from_params diff --git a/config/routes.rb b/config/routes.rb index a64c15e82b..2848255f58 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -692,7 +692,7 @@ Discourse::Application.routes.draw do get "/" => "list#category_default", as: "category_default" end - get "category_hashtags/check" => "category_hashtags#check" + get "hashtags" => "hashtags#show" TopTopic.periods.each do |period| get "top/#{period}.rss" => "list#top_#{period}_feed", format: :rss @@ -887,7 +887,6 @@ Discourse::Application.routes.draw do get '/' => 'tags#index' get '/filter/list' => 'tags#index' get '/filter/search' => 'tags#search' - get '/check' => 'tags#check_hashtag' get '/personal_messages/:username' => 'tags#personal_messages' post '/upload' => 'tags#upload' get '/unused' => 'tags#list_unused' diff --git a/lib/pretty_text/helpers.rb b/lib/pretty_text/helpers.rb index 9633a4e8ea..62f3f63e8d 100644 --- a/lib/pretty_text/helpers.rb +++ b/lib/pretty_text/helpers.rb @@ -4,6 +4,8 @@ module PrettyText module Helpers extend self + TAG_HASHTAG_POSTFIX = "::tag" + # functions here are available to v8 def t(key, opts) key = "js." + key @@ -102,13 +104,12 @@ module PrettyText end def category_tag_hashtag_lookup(text) - tag_postfix = '::tag' - is_tag = text =~ /#{tag_postfix}$/ + is_tag = text =~ /#{TAG_HASHTAG_POSTFIX}$/ if !is_tag && category = Category.query_from_hashtag_slug(text) [category.url, text] elsif (!is_tag && tag = Tag.find_by(name: text)) || - (is_tag && tag = Tag.find_by(name: text.gsub!("#{tag_postfix}", ''))) + (is_tag && tag = Tag.find_by(name: text.gsub!(TAG_HASHTAG_POSTFIX, ''))) ["#{Discourse.base_url}/tag/#{tag.name}", text] else nil diff --git a/spec/requests/category_hashtags_controller_spec.rb b/spec/requests/hashtags_controller_spec.rb similarity index 55% rename from spec/requests/category_hashtags_controller_spec.rb rename to spec/requests/hashtags_controller_spec.rb index 707a5aa720..16d736d5c2 100644 --- a/spec/requests/category_hashtags_controller_spec.rb +++ b/spec/requests/hashtags_controller_spec.rb @@ -2,11 +2,20 @@ require "rails_helper" -describe CategoryHashtagsController do +describe HashtagsController do fab!(:category) { Fabricate(:category) } + fab!(:tag) { Fabricate(:tag) } - let(:group) { Fabricate(:group) } - let(:private_category) { Fabricate(:private_category, group: group) } + fab!(:group) { Fabricate(:group) } + fab!(:private_category) { Fabricate(:private_category, group: group) } + + fab!(:hidden_tag) { Fabricate(:tag, name: "hidden") } + let(:tag_group) { Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [hidden_tag.name]) } + + before do + SiteSetting.tagging_enabled = true + tag_group + end describe "#check" do context "when logged in" do @@ -15,20 +24,21 @@ describe CategoryHashtagsController do sign_in(Fabricate(:user)) end - it "returns only valid categories" do - get "/category_hashtags/check.json", params: { category_slugs: [category.slug, private_category.slug, "none"] } + it "returns only valid categories and tags" do + get "/hashtags.json", params: { slugs: [category.slug, private_category.slug, "none", tag.name, hidden_tag.name] } expect(response.status).to eq(200) expect(response.parsed_body).to eq( - "valid" => [{ "slug" => category.slug, "url" => category.url }] + "categories" => { category.slug => category.url }, + "tags" => { tag.name => tag.full_url } ) end - it "does not return restricted categories" do - get "/category_hashtags/check.json", params: { category_slugs: [private_category.slug] } + it "does not return restricted categories or hidden tags" do + get "/hashtags.json", params: { slugs: [private_category.slug, hidden_tag.name] } expect(response.status).to eq(200) - expect(response.parsed_body).to eq("valid" => []) + expect(response.parsed_body).to eq("categories" => {}, "tags" => {}) end end @@ -39,14 +49,15 @@ describe CategoryHashtagsController do sign_in(admin) end - it "returns restricted categories" do + it "returns restricted categories and hidden tagss" do group.add(admin) - get "/category_hashtags/check.json", params: { category_slugs: [private_category.slug] } + get "/hashtags.json", params: { slugs: [private_category.slug, hidden_tag.name] } expect(response.status).to eq(200) expect(response.parsed_body).to eq( - "valid" => [{ "slug" => private_category.slug, "url" => private_category.url }] + "categories" => { private_category.slug => private_category.url }, + "tags" => { hidden_tag.name => hidden_tag.full_url } ) end end @@ -66,8 +77,8 @@ describe CategoryHashtagsController do quxbar = Fabricate(:category_with_definition, slug: "bar", parent_category_id: qux.id) quxbarbaz = Fabricate(:category_with_definition, slug: "baz", parent_category_id: quxbar.id) - get "/category_hashtags/check.json", params: { - category_slugs: [ + get "/hashtags.json", params: { + slugs: [ ":", # should not work "foo", "bar", # should not work @@ -82,12 +93,12 @@ describe CategoryHashtagsController do } expect(response.status).to eq(200) - expect(response.parsed_body["valid"]).to contain_exactly( - { "slug" => "foo", "url" => foo.url }, - { "slug" => "foo:bar", "url" => foobar.url }, - { "slug" => "bar:baz", "url" => foobarbaz.id < quxbarbaz.id ? foobarbaz.url : quxbarbaz.url }, - { "slug" => "qux", "url" => qux.url }, - { "slug" => "qux:bar", "url" => quxbar.url } + expect(response.parsed_body["categories"]).to eq( + "foo" => foo.url, + "foo:bar" => foobar.url, + "bar:baz" => foobarbaz.id < quxbarbaz.id ? foobarbaz.url : quxbarbaz.url, + "qux" => qux.url, + "qux:bar" => quxbar.url ) end end @@ -95,7 +106,7 @@ describe CategoryHashtagsController do context "when not logged in" do it "returns invalid access" do - get "/category_hashtags/check.json", params: { category_slugs: [] } + get "/hashtags.json", params: { slugs: [] } expect(response.status).to eq(403) end end diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb index 3dbaab4ed9..38528e5d49 100644 --- a/spec/requests/tags_controller_spec.rb +++ b/spec/requests/tags_controller_spec.rb @@ -328,19 +328,6 @@ describe TagsController do end end - describe '#check_hashtag' do - fab!(:tag) { Fabricate(:tag) } - - it "should return the right response" do - get "/tags/check.json", params: { tag_values: [tag.name] } - - expect(response.status).to eq(200) - - response_tag = response.parsed_body["valid"].first - expect(response_tag["value"]).to eq(tag.name) - end - end - describe "#update" do fab!(:tag) { Fabricate(:tag) } diff --git a/test/javascripts/acceptance/category-hashtag-test.js b/test/javascripts/acceptance/category-hashtag-test.js deleted file mode 100644 index 080ff313ba..0000000000 --- a/test/javascripts/acceptance/category-hashtag-test.js +++ /dev/null @@ -1,18 +0,0 @@ -import { acceptance } from "helpers/qunit-helpers"; - -acceptance("Category hashtag", { loggedIn: true }); - -QUnit.test("category hashtag is cooked properly", async assert => { - await visit("/t/internationalization-localization/280"); - await click("#topic-footer-buttons .btn.create"); - - await fillIn(".d-editor-input", "this is a category hashtag #bug"); - - // TODO: Test that the autocomplete shows - assert.equal( - find(".d-editor-preview:visible") - .html() - .trim(), - '
this is a category hashtag #bug
' - ); -}); diff --git a/test/javascripts/acceptance/hashtags-test.js b/test/javascripts/acceptance/hashtags-test.js new file mode 100644 index 0000000000..5a9af1214c --- /dev/null +++ b/test/javascripts/acceptance/hashtags-test.js @@ -0,0 +1,40 @@ +import { acceptance } from "helpers/qunit-helpers"; + +acceptance("Category and Tag Hashtags", { + loggedIn: true, + settings: { tagging_enabled: true }, + pretend(server, helper) { + server.get("/hashtags", () => { + return helper.response({ + categories: { bug: "/c/bugs" }, + tags: { + monkey: "/tag/monkey", + bug: "/tag/bug" + } + }); + }); + } +}); + +QUnit.test("hashtags are cooked properly", async assert => { + await visit("/t/internationalization-localization/280"); + await click("#topic-footer-buttons .btn.create"); + + await fillIn( + ".d-editor-input", + `this is a category hashtag #bug + +this is a tag hashtag #monkey + +category vs tag: #bug vs #bug::tag` + ); + + assert.equal( + find(".d-editor-preview:visible") + .html() + .trim(), + `this is a category hashtag #bug
+this is a tag hashtag #monkey
+` + ); +}); diff --git a/test/javascripts/acceptance/tag-hashtag-test.js b/test/javascripts/acceptance/tag-hashtag-test.js deleted file mode 100644 index 37be13e726..0000000000 --- a/test/javascripts/acceptance/tag-hashtag-test.js +++ /dev/null @@ -1,45 +0,0 @@ -import { acceptance } from "helpers/qunit-helpers"; - -acceptance("Tag Hashtag", { - loggedIn: true, - settings: { tagging_enabled: true }, - pretend(server, helper) { - server.get("/tags/check", () => { - return helper.response({ - valid: [ - { value: "monkey", url: "/tag/monkey" }, - { value: "bug", url: "/tag/bug" } - ] - }); - }); - } -}); - -QUnit.test("tag is cooked properly", async assert => { - await visit("/t/internationalization-localization/280"); - await click("#topic-footer-buttons .btn.create"); - - await fillIn(".d-editor-input", "this is a tag hashtag #monkey"); - assert.equal( - find(".d-editor-preview:visible") - .html() - .trim(), - 'this is a tag hashtag #monkey
' - ); -}); - -QUnit.test( - "tags and categories with same name are cooked properly", - async assert => { - await visit("/t/internationalization-localization/280"); - await click("#topic-footer-buttons .btn.create"); - - await fillIn(".d-editor-input", "#bug vs #bug::tag"); - assert.equal( - find(".d-editor-preview:visible") - .html() - .trim(), - '' - ); - } -); diff --git a/test/javascripts/helpers/create-pretender.js b/test/javascripts/helpers/create-pretender.js index b09965fb96..cc30a0accd 100644 --- a/test/javascripts/helpers/create-pretender.js +++ b/test/javascripts/helpers/create-pretender.js @@ -300,10 +300,6 @@ export function applyDefaultHandlers(pretender) { return response({ post_reply_histories: [{ id: 1234, cooked: "wat" }] }); }); - pretender.get("/category_hashtags/check", () => { - return response({ valid: [{ slug: "bug", url: "/c/bugs" }] }); - }); - pretender.get("/categories_and_latest", () => response(fixturesByUrl["/categories_and_latest.json"]) );