diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index 1d3ccc6452..f08549f397 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -18,6 +18,10 @@ import { fetchUnseenHashtags, linkSeenHashtags, } from "discourse/lib/link-hashtags"; +import { + fetchUnseenHashtagsInContext, + linkSeenHashtagsInContext, +} from "discourse/lib/hashtag-autocomplete"; import { cannotSee, fetchUnseenMentions, @@ -187,6 +191,10 @@ export default Component.extend(ComposerUploadUppy, { } } }, + + hashtagTypesInPriorityOrder: + this.site.hashtag_configurations["topic-composer"], + hashtagIcons: this.site.hashtag_icons, }; }, @@ -477,11 +485,24 @@ export default Component.extend(ComposerUploadUppy, { }, _renderUnseenHashtags(preview) { - const unseen = linkSeenHashtags(preview); + let unseen; + const hashtagContext = this.site.hashtag_configurations["topic-composer"]; + if (this.siteSettings.enable_experimental_hashtag_autocomplete) { + unseen = linkSeenHashtagsInContext(hashtagContext, preview); + } else { + unseen = linkSeenHashtags(preview); + } + if (unseen.length > 0) { - fetchUnseenHashtags(unseen).then(() => { - linkSeenHashtags(preview); - }); + if (this.siteSettings.enable_experimental_hashtag_autocomplete) { + fetchUnseenHashtagsInContext(hashtagContext, unseen).then(() => { + linkSeenHashtagsInContext(hashtagContext, preview); + }); + } else { + fetchUnseenHashtags(unseen).then(() => { + linkSeenHashtags(preview); + }); + } } }, @@ -858,8 +879,14 @@ export default Component.extend(ComposerUploadUppy, { this._warnMentionedGroups(preview); this._warnCannotSeeMention(preview); - // Paint category and tag hashtags - const unseenHashtags = linkSeenHashtags(preview); + // Paint category, tag, and other data source hashtags + let unseenHashtags; + const hashtagContext = this.site.hashtag_configurations["topic-composer"]; + if (this.siteSettings.enable_experimental_hashtag_autocomplete) { + unseenHashtags = linkSeenHashtagsInContext(hashtagContext, preview); + } else { + unseenHashtags = linkSeenHashtags(preview); + } if (unseenHashtags.length > 0) { discourseDebounce(this, this._renderUnseenHashtags, preview, 450); } diff --git a/app/assets/javascripts/discourse/app/components/d-editor.js b/app/assets/javascripts/discourse/app/components/d-editor.js index d31c7c5fe9..64175e030f 100644 --- a/app/assets/javascripts/discourse/app/components/d-editor.js +++ b/app/assets/javascripts/discourse/app/components/d-editor.js @@ -258,7 +258,7 @@ export default Component.extend(TextareaTextManipulation, { this._textarea = this.element.querySelector("textarea.d-editor-input"); this._$textarea = $(this._textarea); this._applyEmojiAutocomplete(this._$textarea); - this._applyCategoryHashtagAutocomplete(this._$textarea); + this._applyHashtagAutocomplete(this._$textarea); scheduleOnce("afterRender", this, this._readyNow); @@ -459,9 +459,9 @@ export default Component.extend(TextareaTextManipulation, { } }, - _applyCategoryHashtagAutocomplete() { + _applyHashtagAutocomplete() { setupHashtagAutocomplete( - "topic-composer", + this.site.hashtag_configurations["topic-composer"], this._$textarea, this.siteSettings, (value) => { diff --git a/app/assets/javascripts/discourse/app/initializers/composer-hashtag-autocomplete.js b/app/assets/javascripts/discourse/app/initializers/composer-hashtag-autocomplete.js deleted file mode 100644 index b46fab8cd2..0000000000 --- a/app/assets/javascripts/discourse/app/initializers/composer-hashtag-autocomplete.js +++ /dev/null @@ -1,16 +0,0 @@ -import { withPluginApi } from "discourse/lib/plugin-api"; - -export default { - name: "composer-hashtag-autocomplete", - - initialize(container) { - const siteSettings = container.lookup("service:site-settings"); - - withPluginApi("1.4.0", (api) => { - if (siteSettings.enable_experimental_hashtag_autocomplete) { - api.registerHashtagSearchParam("category", "topic-composer", 100); - api.registerHashtagSearchParam("tag", "topic-composer", 50); - } - }); - }, -}; diff --git a/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js b/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js index e62e0ea507..094a689bdf 100644 --- a/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js +++ b/app/assets/javascripts/discourse/app/lib/hashtag-autocomplete.js @@ -12,28 +12,38 @@ import { } from "discourse/lib/utilities"; import { search as searchCategoryTag } from "discourse/lib/category-tag-search"; +/** + * Sets up a textarea using the jQuery autocomplete plugin, specifically + * to match on the hashtag (#) character for autocompletion of categories, + * tags, and other resource data types. + * + * @param {Array} contextualHashtagConfiguration - The hashtag datasource types in priority order + * that should be used when searching for or looking up hashtags from the server, determines + * the order of search results and the priority for looking up conflicting hashtags. See also + * Site.hashtag_configurations. + * @param {$Element} $textarea - jQuery element to use for the autocompletion + * plugin to attach to, this is what will watch for the # matcher when the user is typing. + * @param {Hash} siteSettings - The clientside site settings. + * @param {Function} afterComplete - Called with the selected autocomplete option once it is selected. + **/ export function setupHashtagAutocomplete( - context, + contextualHashtagConfiguration, $textArea, siteSettings, afterComplete ) { if (siteSettings.enable_experimental_hashtag_autocomplete) { - _setupExperimental(context, $textArea, siteSettings, afterComplete); + _setupExperimental( + contextualHashtagConfiguration, + $textArea, + siteSettings, + afterComplete + ); } else { _setup($textArea, siteSettings, afterComplete); } } -const contextBasedParams = {}; - -export function registerHashtagSearchParam(param, context, priority) { - if (!contextBasedParams[context]) { - contextBasedParams[context] = {}; - } - contextBasedParams[context][param] = priority; -} - export function hashtagTriggerRule(textarea, opts) { const result = caretRowCol(textarea); const row = result.rowNum; @@ -62,7 +72,61 @@ export function hashtagTriggerRule(textarea, opts) { return true; } -function _setupExperimental(context, $textArea, siteSettings, afterComplete) { +const checkedHashtags = new Set(); +let seenHashtags = {}; + +// NOTE: For future maintainers, the hashtag lookup here does not take +// into account mixed contexts -- for instance, a chat quote inside a post +// or a post quote inside a chat message, so this may +// not provide an accurate priority lookup for hashtags without a ::type suffix in those +// cases. +export function fetchUnseenHashtagsInContext( + contextualHashtagConfiguration, + slugs +) { + return ajax("/hashtags", { + data: { slugs, order: contextualHashtagConfiguration }, + }).then((response) => { + Object.keys(response).forEach((type) => { + seenHashtags[type] = seenHashtags[type] || {}; + response[type].forEach((item) => { + seenHashtags[type][item.ref] = seenHashtags[type][item.ref] || item; + }); + }); + slugs.forEach(checkedHashtags.add, checkedHashtags); + }); +} + +export function linkSeenHashtagsInContext( + contextualHashtagConfiguration, + elem +) { + const hashtagSpans = [...(elem?.querySelectorAll("span.hashtag-raw") || [])]; + if (hashtagSpans.length === 0) { + return []; + } + const slugs = [...hashtagSpans.mapBy("innerText")]; + + hashtagSpans.forEach((hashtagSpan, index) => { + _findAndReplaceSeenHashtagPlaceholder( + slugs[index], + contextualHashtagConfiguration, + hashtagSpan + ); + }); + + return slugs + .map((slug) => slug.toLowerCase()) + .uniq() + .filter((slug) => !checkedHashtags.has(slug)); +} + +function _setupExperimental( + contextualHashtagConfiguration, + $textArea, + siteSettings, + afterComplete +) { $textArea.autocomplete({ template: findRawTemplate("hashtag-autocomplete"), key: "#", @@ -73,7 +137,7 @@ function _setupExperimental(context, $textArea, siteSettings, afterComplete) { if (term.match(/\s/)) { return null; } - return _searchGeneric(term, siteSettings, context); + return _searchGeneric(term, siteSettings, contextualHashtagConfiguration); }, triggerRule: (textarea, opts) => hashtagTriggerRule(textarea, opts), }); @@ -105,7 +169,7 @@ function _updateSearchCache(term, results) { return results; } -function _searchGeneric(term, siteSettings, context) { +function _searchGeneric(term, siteSettings, contextualHashtagConfiguration) { if (currentSearch) { currentSearch.abort(); currentSearch = null; @@ -133,16 +197,16 @@ function _searchGeneric(term, siteSettings, context) { discourseDebounce(this, _searchRequest, q, ctx, resultFunc, INPUT_DELAY); }; - debouncedSearch(term, context, (result) => { + debouncedSearch(term, contextualHashtagConfiguration, (result) => { cancel(timeoutPromise); resolve(_updateSearchCache(term, result)); }); }); } -function _searchRequest(term, context, resultFunc) { +function _searchRequest(term, contextualHashtagConfiguration, resultFunc) { currentSearch = ajax("/hashtags/search.json", { - data: { term, order: _sortedContextParams(context) }, + data: { term, order: contextualHashtagConfiguration }, }); currentSearch .then((r) => { @@ -154,8 +218,30 @@ function _searchRequest(term, context, resultFunc) { return currentSearch; } -function _sortedContextParams(context) { - return Object.entries(contextBasedParams[context]) - .sort((a, b) => b[1] - a[1]) - .map((item) => item[0]); +function _findAndReplaceSeenHashtagPlaceholder( + slug, + contextualHashtagConfiguration, + hashtagSpan +) { + contextualHashtagConfiguration.forEach((type) => { + // remove type suffixes + const typePostfix = `::${type}`; + if (slug.endsWith(typePostfix)) { + slug = slug.slice(0, slug.length - typePostfix.length); + } + + // replace raw span for the hashtag with a cooked one + const matchingSeenHashtag = seenHashtags[type]?.[slug]; + if (matchingSeenHashtag) { + // NOTE: When changing the HTML structure here, you must also change + // it in the hashtag-autocomplete markdown rule, and vice-versa. + const link = document.createElement("a"); + link.classList.add("hashtag-cooked"); + link.href = matchingSeenHashtag.relative_url; + link.dataset.type = type; + link.dataset.slug = matchingSeenHashtag.slug; + link.innerHTML = `${matchingSeenHashtag.text}`; + hashtagSpan.replaceWith(link); + } + }); } diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js index 30ddedef54..13faa140b0 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.js @@ -104,7 +104,6 @@ import DiscourseURL from "discourse/lib/url"; import { registerNotificationTypeRenderer } from "discourse/lib/notification-types-manager"; import { registerUserMenuTab } from "discourse/lib/user-menu/tab"; import { registerModelTransformer } from "discourse/lib/model-transformers"; -import { registerHashtagSearchParam } from "discourse/lib/hashtag-autocomplete"; // If you add any methods to the API ensure you bump up the version number // based on Semantic Versioning 2.0.0. Please update the changelog at @@ -1995,35 +1994,6 @@ class PluginApi { registerModelTransformer(modelName, transformer) { registerModelTransformer(modelName, transformer); } - - /** - * EXPERIMENTAL. Do not use. - * - * When initiating a search inside the composer or other designated inputs - * with the `#` key, we search records based on params registered with - * this function, and order them by type using the priority here. Since - * there can be many different inputs that use `#` and some may need to - * weight different types higher in priority, we also require a context - * parameter. - * - * For example, the topic composer may wish to search for categories - * and tags, with categories appearing first in the results. The usage - * looks like this: - * - * api.registerHashtagSearchParam("category", "topic-composer", 100); - * api.registerHashtagSearchParam("tag", "topic-composer", 50); - * - * Additional types of records used for the hashtag search results - * can be registered via the #register_hashtag_data_source plugin API - * method. - * - * @param {string} param - The type of record to be fetched. - * @param {string} context - Where the hashtag search is being initiated using `#` - * @param {number} priority - Used for ordering types of records. Priority order is descending. - */ - registerHashtagSearchParam(param, context, priority) { - registerHashtagSearchParam(param, context, priority); - } } // from http://stackoverflow.com/questions/6832596/how-to-compare-software-version-number-using-js-only-number diff --git a/app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js index 9c7475e196..aedd0baef9 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js +++ b/app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js @@ -697,7 +697,9 @@ export default { can_revoke: true, }, ], - displayed_about_plugin_stat_groups: ["chat_messages"] + displayed_about_plugin_stat_groups: ["chat_messages"], + hashtag_configurations: { "topic-composer": ["category", "tag"] }, + hashtag_icons: ["folder", "tag"] }, }, }; diff --git a/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js b/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js index e614bf66a1..c806dc0ffd 100644 --- a/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js +++ b/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js @@ -546,6 +546,8 @@ export function setup(opts, siteSettings, state) { markdownTypographerQuotationMarks: siteSettings.markdown_typographer_quotation_marks, markdownLinkifyTlds: siteSettings.markdown_linkify_tlds, + enableExperimentalHashtagAutocomplete: + siteSettings.enable_experimental_hashtag_autocomplete, }; const markdownitOpts = { diff --git a/app/assets/javascripts/pretty-text/addon/pretty-text.js b/app/assets/javascripts/pretty-text/addon/pretty-text.js index a6b0e5c69d..631a9fb7f8 100644 --- a/app/assets/javascripts/pretty-text/addon/pretty-text.js +++ b/app/assets/javascripts/pretty-text/addon/pretty-text.js @@ -46,6 +46,9 @@ export function buildOptions(state) { featuresOverride, markdownItRules, additionalOptions, + hashtagTypesInPriorityOrder, + hashtagIcons, + hashtagLookup, } = state; let features = {}; @@ -88,6 +91,9 @@ export function buildOptions(state) { featuresOverride, markdownItRules, additionalOptions, + hashtagTypesInPriorityOrder, + hashtagIcons, + hashtagLookup, }; // note, this will mutate options due to the way the API is designed diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/category-hashtag.js b/app/assets/javascripts/pretty-text/engines/discourse-markdown/category-hashtag.js index 1616d90985..65595603b8 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/category-hashtag.js +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/category-hashtag.js @@ -1,3 +1,7 @@ +// TODO (martin) Remove this once enable_experimental_hashtag_autocomplete +// (and by extension enableExperimentalHashtagAutocomplete) is not required +// anymore, the new hashtag-autocomplete rule replaces it. + function addHashtag(buffer, matches, state) { const options = state.md.options.discourse; const slug = matches[1]; @@ -46,11 +50,16 @@ function addHashtag(buffer, matches, state) { export function setup(helper) { helper.registerPlugin((md) => { - const rule = { - matcher: /#([\u00C0-\u1FFF\u2C00-\uD7FF\w:-]{1,101})/, - onMatch: addHashtag, - }; + if ( + !md.options.discourse.limitedSiteSettings + .enableExperimentalHashtagAutocomplete + ) { + const rule = { + matcher: /#([\u00C0-\u1FFF\u2C00-\uD7FF\w:-]{1,101})/, + onMatch: addHashtag, + }; - md.core.textPostProcess.ruler.push("category-hashtag", rule); + md.core.textPostProcess.ruler.push("category-hashtag", rule); + } }); } diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/hashtag-autocomplete.js b/app/assets/javascripts/pretty-text/engines/discourse-markdown/hashtag-autocomplete.js new file mode 100644 index 0000000000..8b143448a7 --- /dev/null +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/hashtag-autocomplete.js @@ -0,0 +1,132 @@ +// NOTE: For future maintainers, the hashtag lookup here does not take +// into account mixed contexts -- for instance, a chat quote inside a post +// or a post quote inside a chat message, so hashtagTypesInPriorityOrder may +// not provide an accurate lookup for hashtags without a ::type suffix in those +// cases if there are conflcting types of resources with the same slug. + +function addHashtag(buffer, matches, state) { + const options = state.md.options.discourse; + const slug = matches[1]; + const hashtagLookup = options.hashtagLookup; + + // NOTE: The lookup function is only run when cooking + // server-side, and will only return a single result based on the + // slug lookup. + const result = + hashtagLookup && + hashtagLookup( + slug, + options.currentUser, + options.hashtagTypesInPriorityOrder + ); + + // NOTE: When changing the HTML structure here, you must also change + // it in the placeholder HTML code inside lib/hashtag-autocomplete, and vice-versa. + let token; + if (result) { + token = new state.Token("link_open", "a", 1); + token.attrs = [ + ["class", "hashtag-cooked"], + ["href", result.relative_url], + ["data-type", result.type], + ["data-slug", result.slug], + ]; + token.block = false; + buffer.push(token); + + token = new state.Token("svg_open", "svg", 1); + token.block = false; + token.attrs = [ + ["class", `fa d-icon d-icon-${result.icon} svg-icon svg-node`], + ]; + buffer.push(token); + + token = new state.Token("use_open", "use", 1); + token.block = false; + token.attrs = [["href", `#${result.icon}`]]; + buffer.push(token); + + buffer.push(new state.Token("use_close", "use", -1)); + buffer.push(new state.Token("svg_close", "svg", -1)); + + token = new state.Token("span_open", "span", 1); + token.block = false; + buffer.push(token); + + token = new state.Token("text", "", 0); + token.content = result.text; + buffer.push(token); + + buffer.push(new state.Token("span_close", "span", -1)); + + buffer.push(new state.Token("link_close", "a", -1)); + } else { + token = new state.Token("span_open", "span", 1); + token.attrs = [["class", "hashtag-raw"]]; + buffer.push(token); + + token = new state.Token("svg_open", "svg", 1); + token.block = false; + token.attrs = [["class", `fa d-icon d-icon-hashtag svg-icon svg-node`]]; + buffer.push(token); + + token = new state.Token("use_open", "use", 1); + token.block = false; + token.attrs = [["href", `#hashtag`]]; + buffer.push(token); + + buffer.push(new state.Token("use_close", "use", -1)); + buffer.push(new state.Token("svg_close", "svg", -1)); + + token = new state.Token("span_open", "span", 1); + token = new state.Token("text", "", 0); + token.content = matches[0].replace("#", ""); + buffer.push(token); + token = new state.Token("span_close", "span", -1); + + token = new state.Token("span_close", "span", -1); + buffer.push(token); + } +} + +export function setup(helper) { + const opts = helper.getOptions(); + + // we do this because plugins can register their own hashtag data + // sources which specify an icon, and each icon must be allowlisted + // or it will not render in the markdown pipeline + const hashtagIconAllowList = opts.hashtagIcons + ? opts.hashtagIcons + .concat(["hashtag"]) + .map((icon) => { + return [ + `svg[class=fa d-icon d-icon-${icon} svg-icon svg-node]`, + `use[href=#${icon}]`, + ]; + }) + .flat() + : []; + + helper.registerPlugin((md) => { + if ( + md.options.discourse.limitedSiteSettings + .enableExperimentalHashtagAutocomplete + ) { + const rule = { + matcher: /#([\u00C0-\u1FFF\u2C00-\uD7FF\w:-]{1,101})/, + onMatch: addHashtag, + }; + + md.core.textPostProcess.ruler.push("hashtag-autocomplete", rule); + } + }); + + helper.allowList( + hashtagIconAllowList.concat([ + "a.hashtag-cooked", + "span.hashtag-raw", + "a[data-type]", + "a[data-slug]", + ]) + ); +} diff --git a/app/assets/stylesheets/common/base/tagging.scss b/app/assets/stylesheets/common/base/tagging.scss index 78d8411ccb..8050ece7ca 100644 --- a/app/assets/stylesheets/common/base/tagging.scss +++ b/app/assets/stylesheets/common/base/tagging.scss @@ -203,24 +203,6 @@ header .discourse-tag { } } -.hashtag-autocomplete { - .hashtag-autocomplete__option { - .hashtag-autocomplete__link { - align-items: center; - color: var(--primary-medium); - display: flex; - - .d-icon { - padding-right: 0.5em; - } - - .hashtag-autocomplete__text { - flex: 1; - } - } - } -} - .tags-admin-menu { margin-top: 20px; ul { diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss index d80440f25e..16970b72ab 100644 --- a/app/assets/stylesheets/common/base/topic-post.scss +++ b/app/assets/stylesheets/common/base/topic-post.scss @@ -1249,13 +1249,7 @@ blockquote > *:last-child { } a.mention { - display: inline-block; // https://bugzilla.mozilla.org/show_bug.cgi?id=1656119 - font-weight: bold; - font-size: 0.93em; - color: var(--primary-high-or-secondary-low); - padding: 0 4px 1px; - background: var(--primary-low); - border-radius: 8px; + @include mention; } span.mention { diff --git a/app/assets/stylesheets/common/components/hashtag.scss b/app/assets/stylesheets/common/components/hashtag.scss index e532721847..5b0f04ba05 100644 --- a/app/assets/stylesheets/common/components/hashtag.scss +++ b/app/assets/stylesheets/common/components/hashtag.scss @@ -13,3 +13,35 @@ a.hashtag { } } } + +.hashtag-autocomplete { + .hashtag-autocomplete__option { + .hashtag-autocomplete__link { + align-items: center; + color: var(--primary-medium); + display: flex; + + .d-icon { + padding-right: 0.5em; + } + + .hashtag-autocomplete__text { + flex: 1; + } + } + } +} + +.hashtag-raw, +.hashtag-cooked { + @include mention; + + &:visited, + &:hover { + color: var(--primary-high-or-secondary-low); + } + + .d-icon { + margin-right: 3px; + } +} diff --git a/app/assets/stylesheets/common/foundation/mixins.scss b/app/assets/stylesheets/common/foundation/mixins.scss index 8e62e10f6c..8f1227ba71 100644 --- a/app/assets/stylesheets/common/foundation/mixins.scss +++ b/app/assets/stylesheets/common/foundation/mixins.scss @@ -235,3 +235,13 @@ $hpad: 0.65em; @return url("#{$path}"); } } + +@mixin mention() { + display: inline-block; // https://bugzilla.mozilla.org/show_bug.cgi?id=1656119 + font-weight: bold; + font-size: 0.93em; + color: var(--primary-high-or-secondary-low); + padding: 0 4px 1px; + background: var(--primary-low); + border-radius: 8px; +} diff --git a/app/controllers/hashtags_controller.rb b/app/controllers/hashtags_controller.rb index ec625b7f22..b6cf86df59 100644 --- a/app/controllers/hashtags_controller.rb +++ b/app/controllers/hashtags_controller.rb @@ -3,15 +3,17 @@ class HashtagsController < ApplicationController requires_login - def show - raise Discourse::InvalidParameters.new(:slugs) if !params[:slugs].is_a?(Array) - render json: HashtagAutocompleteService.new(guardian).lookup(params[:slugs]) + def lookup + if SiteSetting.enable_experimental_hashtag_autocomplete + render json: HashtagAutocompleteService.new(guardian).lookup(params[:slugs], params[:order]) + else + render json: HashtagAutocompleteService.new(guardian).lookup_old(params[:slugs]) + end end def search params.require(:term) params.require(:order) - raise Discourse::InvalidParameters.new(:order) if !params[:order].is_a?(Array) results = HashtagAutocompleteService.new(guardian).search(params[:term], params[:order]) diff --git a/app/models/concerns/category_hashtag.rb b/app/models/concerns/category_hashtag.rb index 216c87a7f6..959216baf3 100644 --- a/app/models/concerns/category_hashtag.rb +++ b/app/models/concerns/category_hashtag.rb @@ -6,6 +6,8 @@ module CategoryHashtag SEPARATOR = ":" class_methods do + # TODO (martin) Remove this when enable_experimental_hashtag_autocomplete + # becomes the norm, it is reimplemented below for CategoryHashtagDataSourcee def query_from_hashtag_slug(category_slug) slug_path = category_slug.split(SEPARATOR) return nil if slug_path.empty? || slug_path.size > 2 @@ -22,5 +24,45 @@ module CategoryHashtag categories.where(parent_category_id: nil).first end end + + ## + # Finds any categories that match the provided slugs, supporting + # the parent:child format for category slugs (only one level of + # depth supported). + # + # @param {Array} category_slugs - Slug strings to look up, can also be in the parent:child format + # @param {Array} cached_categories - An array of Hashes representing categories, Site.categories + # should be used here since it is scoped to the Guardian. + def query_from_cached_categories(category_slugs, cached_categories) + category_slugs + .map(&:downcase) + .map do |slug| + slug_path = slug.split(":") + if SiteSetting.slug_generation_method == "encoded" + slug_path.map! { |slug| CGI.escape(slug) } + end + parent_slug, child_slug = slug_path.last(2) + + # Category slugs can be in the parent:child format, if there + # is no child then the "parent" part of the slug is just the + # entire slug we look for. + # + # Otherwise if the child slug is present, we find the parent + # by its slug then find the child by its slug and its parent's + # ID to make sure they match. + if child_slug.present? + parent_category = cached_categories.find { |cat| cat[:slug].downcase == parent_slug } + if parent_category.present? + cached_categories.find do |cat| + cat[:slug].downcase == child_slug && cat[:parent_category_id] == parent_category[:id] + end + end + else + cached_categories.find do |cat| + cat[:slug].downcase == parent_slug && cat[:parent_category_id].nil? + end + end + end.compact + end end end diff --git a/app/models/post.rb b/app/models/post.rb index 362ce9bf8b..39aab5700d 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -305,8 +305,13 @@ class Post < ActiveRecord::Base options = opts.dup options[:cook_method] = cook_method - post_user = self.user - options[:user_id] = post_user.id if post_user + # A rule in our Markdown pipeline may have Guardian checks that require a + # user to be present. The last editing user of the post will be more + # generally up to date than the creating user. For example, we use + # this when cooking #hashtags to determine whether we should render + # the found hashtag based on whether the user can access the category it + # is referencing. + options[:user_id] = self.last_editor_id options[:omit_nofollow] = true if omit_nofollow? if self.with_secure_uploads? diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index 30636fe61f..d460244b28 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -35,6 +35,8 @@ class SiteSerializer < ApplicationSerializer :watched_words_link, :categories, :markdown_additional_options, + :hashtag_configurations, + :hashtag_icons, :displayed_about_plugin_stat_groups, :show_welcome_topic_banner, :anonymous_default_sidebar_tags @@ -220,6 +222,14 @@ class SiteSerializer < ApplicationSerializer Site.markdown_additional_options end + def hashtag_configurations + HashtagAutocompleteService.contexts_with_ordered_types + end + + def hashtag_icons + HashtagAutocompleteService.data_source_icons + end + def displayed_about_plugin_stat_groups About.displayed_plugin_stat_groups end diff --git a/app/services/category_hashtag_data_source.rb b/app/services/category_hashtag_data_source.rb new file mode 100644 index 0000000000..40c37ba495 --- /dev/null +++ b/app/services/category_hashtag_data_source.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +# Used as a data source via HashtagAutocompleteService to provide category +# results when looking up a category slug via markdown or searching for +# categories via the # autocomplete character. +class CategoryHashtagDataSource + def self.icon + "folder" + end + + def self.category_to_hashtag_item(guardian_categories, category) + category = Category.new(category.slice(:id, :slug, :name, :parent_category_id)) + + HashtagAutocompleteService::HashtagItem.new.tap do |item| + item.text = category.name + item.slug = category.slug + item.icon = icon + item.relative_url = category.url + + # Single-level category heirarchy should be enough to distinguish between + # categories here. + item.ref = + if category.parent_category_id + parent_category = + guardian_categories.find { |cat| cat[:id] === category.parent_category_id } + !parent_category ? category.slug : "#{parent_category[:slug]}:#{category.slug}" + else + category.slug + end + end + end + + def self.lookup(guardian, slugs) + # We use Site here because it caches all the categories the + # user has access to. + guardian_categories = Site.new(guardian).categories + Category + .query_from_cached_categories(slugs, guardian_categories) + .map { |category| category_to_hashtag_item(guardian_categories, category) } + end + + def self.search(guardian, term, limit) + guardian_categories = Site.new(guardian).categories + + guardian_categories + .select do |category| + category[:name].downcase.include?(term) || category[:slug].downcase.include?(term) + end + .take(limit) + .map { |category| category_to_hashtag_item(guardian_categories, category) } + end +end diff --git a/app/services/hashtag_autocomplete_service.rb b/app/services/hashtag_autocomplete_service.rb index 1280f267df..838d5de4f1 100644 --- a/app/services/hashtag_autocomplete_service.rb +++ b/app/services/hashtag_autocomplete_service.rb @@ -5,71 +5,42 @@ class HashtagAutocompleteService SEARCH_MAX_LIMIT = 20 attr_reader :guardian - cattr_reader :data_sources + cattr_reader :data_sources, :contexts - def self.register_data_source(type, &block) - @@data_sources[type] = block + def self.register_data_source(type, klass) + @@data_sources[type] = klass end - def self.clear_data_sources + def self.clear_registered @@data_sources = {} + @@contexts = {} - register_data_source("category") do |guardian, term, limit| - guardian_categories = Site.new(guardian).categories + register_data_source("category", CategoryHashtagDataSource) + register_data_source("tag", TagHashtagDataSource) - guardian_categories - .select { |category| category[:name].downcase.include?(term) } - .take(limit) - .map do |category| - HashtagItem.new.tap do |item| - item.text = category[:name] - item.slug = category[:slug] - - # Single-level category heirarchy should be enough to distinguish between - # categories here. - item.ref = - if category[:parent_category_id] - parent_category = - guardian_categories.find { |c| c[:id] === category[:parent_category_id] } - category[:slug] if !parent_category - - parent_slug = parent_category[:slug] - "#{parent_slug}:#{category[:slug]}" - else - category[:slug] - end - item.icon = "folder" - end - end - end - - register_data_source("tag") do |guardian, term, limit| - if SiteSetting.tagging_enabled - tags_with_counts, _ = - DiscourseTagging.filter_allowed_tags( - guardian, - term: term, - with_context: true, - limit: limit, - for_input: true, - ) - TagsController - .tag_counts_json(tags_with_counts) - .take(limit) - .map do |tag| - HashtagItem.new.tap do |item| - item.text = "#{tag[:name]} x #{tag[:count]}" - item.slug = tag[:name] - item.icon = "tag" - end - end - else - [] - end - end + register_type_in_context("category", "topic-composer", 100) + register_type_in_context("tag", "topic-composer", 50) end - clear_data_sources + def self.register_type_in_context(type, context, priority) + @@contexts[context] = @@contexts[context] || {} + @@contexts[context][type] = priority + end + + def self.data_source_icons + @@data_sources.values.map(&:icon) + end + + def self.ordered_types_for_context(context) + return [] if @@contexts[context].blank? + @@contexts[context].sort_by { |param, priority| priority }.reverse.map(&:first) + end + + def self.contexts_with_ordered_types + Hash[@@contexts.keys.map { |context| [context, ordered_types_for_context(context)] }] + end + + clear_registered class HashtagItem # The text to display in the UI autocomplete menu for the item. @@ -89,13 +60,162 @@ class HashtagAutocompleteService # and must be unique so it can be used for lookups via the #lookup # method above. attr_accessor :ref + + # The relative URL for the resource that is represented by the autocomplete + # item, used for the cooked hashtags, e.g. /c/2/staff + attr_accessor :relative_url + + def to_h + { + relative_url: self.relative_url, + text: self.text, + icon: self.icon, + type: self.type, + ref: self.ref, + slug: self.slug + } + end end def initialize(guardian) @guardian = guardian end - def lookup(slugs) + ## + # Finds resources of the provided types by their exact slugs, unlike + # search which can search partial names, slugs, etc. Used for cooking + # fully formed #hashtags in the markdown pipeline. The @guardian handles + # permissions around which results should be returned here. + # + # @param {Array} slugs The fully formed slugs to look up, which can have + # ::type suffixes attached as well (e.g. ::category), + # and in the case of categories can have parent:child + # relationships. + # @param {Array} types_in_priority_order The resource types we are looking up + # and the priority order in which we should + # match them if they do not have type suffixes. + # @returns {Hash} A hash with the types as keys and an array of HashtagItem that + # matches the provided slugs. + def lookup(slugs, types_in_priority_order) + raise Discourse::InvalidParameters.new(:slugs) if !slugs.is_a?(Array) + raise Discourse::InvalidParameters.new(:order) if !types_in_priority_order.is_a?(Array) + + types_in_priority_order = + types_in_priority_order.select { |type| @@data_sources.keys.include?(type) } + lookup_results = Hash[types_in_priority_order.collect { |type| [type.to_sym, []] }] + limited_slugs = slugs[0..HashtagAutocompleteService::HASHTAGS_PER_REQUEST] + + slugs_without_suffixes = + limited_slugs.reject do |slug| + @@data_sources.keys.any? { |type| slug.ends_with?("::#{type}") } + end + slugs_with_suffixes = (limited_slugs - slugs_without_suffixes) + + # For all the slugs without a type suffix, we need to lookup in order, falling + # back to the next type if no results are returned for a slug for the current + # type. This way slugs without suffix make sense in context, e.g. in the topic + # composer we want a slug without a suffix to be a category first, tag second. + if slugs_without_suffixes.any? + types_in_priority_order.each do |type| + found_from_slugs = set_refs(@@data_sources[type].lookup(guardian, slugs_without_suffixes)) + found_from_slugs.each { |item| item.type = type }.sort_by! { |item| item.text.downcase } + lookup_results[type.to_sym] = lookup_results[type.to_sym].concat(found_from_slugs) + slugs_without_suffixes = slugs_without_suffixes - found_from_slugs.map(&:ref) + break if slugs_without_suffixes.empty? + end + end + + # We then look up the remaining slugs based on their type suffix, stripping out + # the type suffix first since it will not match the actual slug. + if slugs_with_suffixes.any? + types_in_priority_order.each do |type| + slugs_for_type = + slugs_with_suffixes + .select { |slug| slug.ends_with?("::#{type}") } + .map { |slug| slug.gsub("::#{type}", "") } + next if slugs_for_type.empty? + found_from_slugs = set_refs(@@data_sources[type].lookup(guardian, slugs_for_type)) + found_from_slugs.each { |item| item.type = type }.sort_by! { |item| item.text.downcase } + lookup_results[type.to_sym] = lookup_results[type.to_sym].concat(found_from_slugs) + end + end + + lookup_results + end + + ## + # Searches registered hashtag data sources using the provided term (data + # sources determine what is actually searched) and prioritises the results + # based on types_in_priority_order and the limit. For example, if 5 categories + # were returned for the term and the limit was 5, we would not even bother + # searching tags. The @guardian handles permissions around which results should + # be returned here. + # + # @param {String} term Search term, from the UI generally where the user is typing #has... + # @param {Array} types_in_priority_order The resource types we are searching for + # and the priority order in which we should + # return them. + # @param {Integer} limit The maximum number of search results to return, we don't + # bother searching subsequent types if the first types in + # the array already reach the limit. + # @returns {Array} The results as HashtagItems + def search(term, types_in_priority_order, limit: 5) + raise Discourse::InvalidParameters.new(:order) if !types_in_priority_order.is_a?(Array) + limit = [limit, SEARCH_MAX_LIMIT].min + + limited_results = [] + slugs_by_type = {} + term = term.downcase + types_in_priority_order = + types_in_priority_order.select { |type| @@data_sources.keys.include?(type) } + + # Search the data source for each type, validate and sort results, + # and break off from searching more data sources if we reach our limit + types_in_priority_order.each do |type| + search_results = + set_refs(@@data_sources[type].search(guardian, term, limit - limited_results.length)) + next if search_results.empty? + + all_data_items_valid = + search_results.all? do |item| + item.kind_of?(HashtagItem) && item.slug.present? && item.text.present? + end + next if !all_data_items_valid + + search_results.each { |item| item.type = type }.sort_by! { |item| item.text.downcase } + slugs_by_type[type] = search_results.map(&:slug) + + limited_results.concat(search_results) + break if limited_results.length >= limit + end + + # Any items that are _not_ the top-ranked type (which could possibly not be + # the same as the first item in the types_in_priority_order if there was + # no data for that type) that have conflicting slugs with other items for + # other types need to have a ::type suffix added to their ref. + # + # This will be used for the lookup method above if one of these items is + # chosen in the UI, otherwise there is no way to determine whether a hashtag is + # for a category or a tag etc. + # + # For example, if there is a category with the slug #general and a tag + # with the slug #general, then the tag will have its ref changed to #general::tag + top_ranked_type = slugs_by_type.keys.first + limited_results.each do |hashtag_item| + next if hashtag_item.type == top_ranked_type + + other_slugs = limited_results.reject { |r| r.type === hashtag_item.type }.map(&:slug) + if other_slugs.include?(hashtag_item.slug) + hashtag_item.ref = "#{hashtag_item.slug}::#{hashtag_item.type}" + end + end + + limited_results.take(limit) + end + + # TODO (martin) Remove this once plugins are not relying on the old lookup + # behavior via HashtagsController when enable_experimental_hashtag_autocomplete is removed + def lookup_old(slugs) raise Discourse::InvalidParameters.new(:slugs) if !slugs.is_a?(Array) all_slugs = [] @@ -138,58 +258,13 @@ class HashtagAutocompleteService { categories: categories_hashtags, tags: tag_hashtags } end - def search(term, types_in_priority_order, limit: 5) - raise Discourse::InvalidParameters.new(:order) if !types_in_priority_order.is_a?(Array) - limit = [limit, SEARCH_MAX_LIMIT].min + private - results = [] - slugs_by_type = {} - term = term.downcase - types_in_priority_order = - types_in_priority_order.select { |type| @@data_sources.keys.include?(type) } - - types_in_priority_order.each do |type| - data = @@data_sources[type].call(guardian, term, limit - results.length) - next if data.empty? - - all_data_items_valid = data.all? do |item| - item.kind_of?(HashtagItem) && item.slug.present? && item.text.present? - end - next if !all_data_items_valid - - data.each do |item| - item.type = type - item.ref = item.ref || item.slug - end - data.sort_by! { |item| item.text.downcase } - slugs_by_type[type] = data.map(&:slug) - - results.concat(data) - - break if results.length >= limit - end - - # Any items that are _not_ the top-ranked type (which could possibly not be - # the same as the first item in the types_in_priority_order if there was - # no data for that type) that have conflicting slugs with other items for - # other types need to have a ::type suffix added to their ref. - # - # This will be used for the lookup method above if one of these items is - # chosen in the UI, otherwise there is no way to determine whether a hashtag is - # for a category or a tag etc. - # - # For example, if there is a category with the slug #general and a tag - # with the slug #general, then the tag will have its ref changed to #general::tag - top_ranked_type = slugs_by_type.keys.first - results.each do |hashtag_item| - next if hashtag_item.type == top_ranked_type - - other_slugs = results.reject { |r| r.type === hashtag_item.type }.map(&:slug) - if other_slugs.include?(hashtag_item.slug) - hashtag_item.ref = "#{hashtag_item.slug}::#{hashtag_item.type}" - end - end - - results.take(limit) + # Sometimes a specific ref is required, e.g. for categories that have + # a parent their ref will be parent_slug:child_slug, though most of the + # time it will be the same as the slug. The ref can then be used for + # lookup in the UI. + def set_refs(hashtag_items) + hashtag_items.each { |item| item.ref ||= item.slug } end end diff --git a/app/services/tag_hashtag_data_source.rb b/app/services/tag_hashtag_data_source.rb new file mode 100644 index 0000000000..0323298d44 --- /dev/null +++ b/app/services/tag_hashtag_data_source.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +# Used as a data source via HashtagAutocompleteService to provide tag +# results when looking up a tag slug via markdown or searching for +# tags via the # autocomplete character. +class TagHashtagDataSource + def self.icon + "tag" + end + + def self.tag_to_hashtag_item(tag, include_count: false) + tag = Tag.new(tag.slice(:id, :name).merge(topic_count: tag[:count])) if tag.is_a?(Hash) + + HashtagAutocompleteService::HashtagItem.new.tap do |item| + if include_count + item.text = "#{tag.name} x #{tag.topic_count}" + else + item.text = tag.name + end + item.slug = tag.name + item.relative_url = tag.url + item.icon = icon + end + end + + def self.lookup(guardian, slugs) + return [] if !SiteSetting.tagging_enabled + + DiscourseTagging + .filter_visible(Tag.where_name(slugs), guardian) + .map { |tag| tag_to_hashtag_item(tag) } + end + + def self.search(guardian, term, limit) + return [] if !SiteSetting.tagging_enabled + + tags_with_counts, _ = + DiscourseTagging.filter_allowed_tags( + guardian, + term: term, + with_context: true, + limit: limit, + for_input: true, + ) + TagsController + .tag_counts_json(tags_with_counts) + .take(limit) + .map { |tag| tag_to_hashtag_item(tag, include_count: true) } + end +end diff --git a/config/routes.rb b/config/routes.rb index 3b36ba31d8..44c1570c88 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -774,7 +774,7 @@ Discourse::Application.routes.draw do get "/" => "list#category_default", as: "category_default" end - get "hashtags" => "hashtags#show" + get "hashtags" => "hashtags#lookup" get "hashtags/search" => "hashtags#search" TopTopic.periods.each do |period| diff --git a/lib/email/styles.rb b/lib/email/styles.rb index ed98e5e2a8..ba2b56fcaf 100644 --- a/lib/email/styles.rb +++ b/lib/email/styles.rb @@ -185,6 +185,7 @@ module Email correct_first_body_margin correct_footer_style correct_footer_style_highlight_first + decorate_hashtags reset_tables html_lang = SiteSetting.default_locale.sub("_", "-") @@ -323,6 +324,16 @@ module Email end end + def decorate_hashtags + @fragment.search(".hashtag-cooked").each do |hashtag| + hashtag_text = hashtag.search("span").first + hashtag_text.add_next_sibling(<<~HTML) + ##{hashtag["data-slug"]} + HTML + hashtag_text.remove + end + end + def make_all_links_absolute site_uri = URI(Discourse.base_url) @fragment.css("a").each do |link| diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index dfa943792d..0de0274f0e 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -1086,14 +1086,49 @@ class Plugin::Instance About.add_plugin_stat_group(plugin_stat_group_name, show_in_ui: show_in_ui, &block) end - # Registers a new record type to be searched via the HashtagAutocompleteService and the - # /hashtags/search endpoint. The data returned by the block must be an array - # with each item an instance of HashtagAutocompleteService::HashtagItem. + ## + # Used to register data sources for HashtagAutocompleteService to look + # up results based on a #hashtag string. # - # See also registerHashtagSearchParam in the plugin JS API, otherwise the - # clientside hashtag search code will use the new type registered here. - def register_hashtag_data_source(type, &block) - HashtagAutocompleteService.register_data_source(type, &block) + # @param {String} type - Roughly corresponding to a model, this is used as a unique + # key for the datasource and is also used when allowing different + # contexts to search for and lookup these types. The `category` + # and `tag` types are registered by default. + # @param {Class} klass - Must be a class that implements methods with the following + # signatures: + # + # @param {Guardian} guardian - Current user's guardian, used for permission-based filtering + # @param {Array} slugs - An array of strings that represent slugs to search this type for, + # e.g. category slugs. + # @returns {Hash} A hash with the slug as the key and the URL of the record as the value. + # def self.lookup(guardian, slugs) + # end + # + # @param {Guardian} guardian - Current user's guardian, used for permission-based filtering + # @param {String} term - The search term used to filter results + # @param {Integer} limit - The number of search results that should be returned by the query + # @returns {Array} An Array of HashtagAutocompleteService::HashtagItem + # def self.search(guardian, term, limit) + # end + def register_hashtag_data_source(type, klass) + HashtagAutocompleteService.register_data_source(type, klass) + end + + ## + # Used to set up the priority ordering of hashtag autocomplete results by + # type using HashtagAutocompleteService. + # + # @param {String} type - Roughly corresponding to a model, can only be registered once + # per context. The `category` and `tag` types are registered + # for the `topic-composer` context by default in that priority order. + # @param {String} context - The context in which the hashtag lookup or search is happening + # in. For example, the Discourse composer context is `topic-composer`. + # Different contexts may want to have different priority orderings + # for certain types of hashtag result. + # @param {Integer} priority - A number value for ordering type results when hashtag searches + # or lookups occur. Priority is ordered by DESCENDING order. + def register_hashtag_type_in_context(type, context, priority) + HashtagAutocompleteService.register_type_in_context(type, context, priority) end protected diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 0d4c8ee9b4..49effdc930 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -171,6 +171,9 @@ module PrettyText # user_id - User id for the post being cooked. # force_quote_link - Always create the link to the quoted topic for [quote] bbcode. Normally this only happens # if the topic_id provided is different from the [quote topic:X]. + # hashtag_context - Defaults to "topic-composer" if not supplied. Controls the order of #hashtag lookup results + # based on registered hashtag contexts from the `#register_hashtag_search_param` plugin API + # method. def self.markdown(text, opts = {}) # we use the exact same markdown converter as the client # TODO: use the same extensions on both client and server (in particular the template for mentions) @@ -201,6 +204,7 @@ module PrettyText __optInput.formatUsername = __formatUsername; __optInput.getTopicInfo = __getTopicInfo; __optInput.categoryHashtagLookup = __categoryLookup; + __optInput.hashtagLookup = __hashtagLookup; __optInput.customEmoji = #{custom_emoji.to_json}; __optInput.customEmojiTranslation = #{Plugin::CustomEmoji.translations.to_json}; __optInput.emojiUnicodeReplacer = __emojiUnicodeReplacer; @@ -221,8 +225,17 @@ module PrettyText if opts[:user_id] buffer << "__optInput.userId = #{opts[:user_id].to_i};\n" + buffer << "__optInput.currentUser = #{User.find(opts[:user_id]).to_json}\n" end + opts[:hashtag_context] = opts[:hashtag_context] || "topic-composer" + hashtag_types_as_js = HashtagAutocompleteService.ordered_types_for_context( + opts[:hashtag_context] + ).map { |t| "'#{t}'" }.join(",") + hashtag_icons_as_js = HashtagAutocompleteService.data_source_icons.map { |i| "'#{i}'" }.join(",") + buffer << "__optInput.hashtagTypesInPriorityOrder = [#{hashtag_types_as_js}];\n" + buffer << "__optInput.hashtagIcons = [#{hashtag_icons_as_js}];\n" + buffer << "__textOptions = __buildOptions(__optInput);\n" buffer << ("__pt = new __PrettyText(__textOptions);") diff --git a/lib/pretty_text/helpers.rb b/lib/pretty_text/helpers.rb index 8a4c13def9..97d2a79558 100644 --- a/lib/pretty_text/helpers.rb +++ b/lib/pretty_text/helpers.rb @@ -41,14 +41,6 @@ module PrettyText username end - def category_hashtag_lookup(category_slug) - if category = Category.query_from_hashtag_slug(category_slug) - [category.url, category_slug] - else - nil - end - end - def lookup_upload_urls(urls) map = {} result = {} @@ -103,6 +95,8 @@ module PrettyText end end + # TODO (martin) Remove this when everything is using hashtag_lookup + # after enable_experimental_hashtag_autocomplete is default. def category_tag_hashtag_lookup(text) is_tag = text =~ /#{TAG_HASHTAG_POSTFIX}$/ @@ -116,6 +110,29 @@ module PrettyText end end + def hashtag_lookup(slug, cooking_user, types_in_priority_order) + # This is _somewhat_ expected since we need to be able to cook posts + # etc. without a user sometimes, but it is still an edge case. + if cooking_user.blank? + cooking_user = Discourse.system_user + end + + cooking_user = User.new(cooking_user) if cooking_user.is_a?(Hash) + + result = HashtagAutocompleteService.new( + Guardian.new(cooking_user) + ).lookup([slug], types_in_priority_order) + + found_hashtag = nil + types_in_priority_order.each do |type| + if result[type.to_sym].any? + found_hashtag = result[type.to_sym].first.to_h + break + end + end + found_hashtag + end + def get_current_user(user_id) return unless user_id.is_a?(Integer) { staff: User.where(id: user_id).where("moderator OR admin").exists? } diff --git a/lib/pretty_text/shims.js b/lib/pretty_text/shims.js index 4032bff566..70c6ea5649 100644 --- a/lib/pretty_text/shims.js +++ b/lib/pretty_text/shims.js @@ -107,10 +107,16 @@ function __getTopicInfo(i) { return __helpers.get_topic_info(i); } +// TODO (martin) Remove this when everything is using hashtag_lookup +// after enable_experimental_hashtag_autocomplete is default. function __categoryLookup(c) { return __helpers.category_tag_hashtag_lookup(c); } +function __hashtagLookup(slug, cookingUser, typesInPriorityOrder) { + return __helpers.hashtag_lookup(slug, cookingUser, typesInPriorityOrder); +} + function __lookupAvatar(p) { return __utils.avatarImg( { size: "tiny", avatarTemplate: __helpers.avatar_template(p) }, diff --git a/plugins/chat/app/models/chat_channel.rb b/plugins/chat/app/models/chat_channel.rb index c2cbca6fd3..1a0e47d7f3 100644 --- a/plugins/chat/app/models/chat_channel.rb +++ b/plugins/chat/app/models/chat_channel.rb @@ -76,11 +76,11 @@ class ChatChannel < ActiveRecord::Base end def url - "#{Discourse.base_url}#{relative_url}" + "#{Discourse.base_url}/chat/channel/#{self.id}/#{self.slug || "-"}" end def relative_url - "/chat/channel/#{self.id}/#{self.slug || "-"}" + "#{Discourse.base_path}/chat/channel/#{self.id}/#{self.slug || "-"}" end private diff --git a/plugins/chat/app/models/chat_message.rb b/plugins/chat/app/models/chat_message.rb index 9b4159b3ef..2a73b0f179 100644 --- a/plugins/chat/app/models/chat_message.rb +++ b/plugins/chat/app/models/chat_message.rb @@ -33,7 +33,7 @@ class ChatMessage < ActiveRecord::Base scope :created_before, ->(date) { where("chat_messages.created_at < ?", date) } - before_save { self.last_editor_id ||= self.user_id } + before_save { ensure_last_editor_id } def validate_message(has_uploads:) WatchedWordsValidator.new(attributes: [:message]).validate(self) @@ -98,7 +98,15 @@ class ChatMessage < ActiveRecord::Base end def cook - self.cooked = self.class.cook(self.message) + ensure_last_editor_id + + # A rule in our Markdown pipeline may have Guardian checks that require a + # user to be present. The last editing user of the message will be more + # generally up to date than the creating user. For example, we use + # this when cooking #hashtags to determine whether we should render + # the found hashtag based on whether the user can access the channel it + # is referencing. + self.cooked = self.class.cook(self.message, user_id: self.last_editor_id) self.cooked_version = BAKED_VERSION end @@ -130,6 +138,7 @@ class ChatMessage < ActiveRecord::Base emojiShortcuts inlineEmoji html-img + hashtag-autocomplete mentions unicodeUsernames onebox @@ -164,6 +173,8 @@ class ChatMessage < ActiveRecord::Base features_override: MARKDOWN_FEATURES + DiscoursePluginRegistry.chat_markdown_features.to_a, markdown_it_rules: MARKDOWN_IT_RULES, force_quote_link: true, + user_id: opts[:user_id], + hashtag_context: "chat-composer" ) result = @@ -193,6 +204,10 @@ class ChatMessage < ActiveRecord::Base def message_too_short? message.length < SiteSetting.chat_minimum_message_length end + + def ensure_last_editor_id + self.last_editor_id ||= self.user_id + end end # == Schema Information diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js index bb9f68be19..2ef16eff45 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js @@ -357,7 +357,7 @@ export default Component.extend(TextareaTextManipulation, { _applyCategoryHashtagAutocomplete($textarea) { setupHashtagAutocomplete( - "chat-composer", + this.site.hashtag_configurations["chat-composer"], $textarea, this.siteSettings, (value) => { diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js index af4fc496da..cfbe931b49 100644 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js @@ -43,11 +43,6 @@ export default { }); } - if (this.siteSettings.enable_experimental_hashtag_autocomplete) { - api.registerHashtagSearchParam("category", "chat-composer", 100); - api.registerHashtagSearchParam("tag", "chat-composer", 50); - } - api.registerChatComposerButton({ label: "chat.emoji", id: "emoji", diff --git a/plugins/chat/assets/javascripts/discourse/services/chat.js b/plugins/chat/assets/javascripts/discourse/services/chat.js index bf8a59b839..ffb41e04b7 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat.js @@ -146,16 +146,19 @@ export default class Chat extends Service { return Promise.resolve(this.cook); } - const prettyTextFeatures = { + const markdownOptions = { featuresOverride: Site.currentProp( "markdown_additional_options.chat.limited_pretty_text_features" ), markdownItRules: Site.currentProp( "markdown_additional_options.chat.limited_pretty_text_markdown_rules" ), + hashtagTypesInPriorityOrder: + this.site.hashtag_configurations["chat-composer"], + hashtagIcons: this.site.hashtag_icons, }; - return generateCookFunction(prettyTextFeatures).then((cookFunction) => { + return generateCookFunction(markdownOptions).then((cookFunction) => { return this.set("cook", (raw) => { return simpleCategoryHashMentionTransform( cookFunction(raw), diff --git a/plugins/chat/assets/javascripts/lib/discourse-markdown/chat-transcript.js b/plugins/chat/assets/javascripts/lib/discourse-markdown/chat-transcript.js index 1f1d00fa87..236277a62f 100644 --- a/plugins/chat/assets/javascripts/lib/discourse-markdown/chat-transcript.js +++ b/plugins/chat/assets/javascripts/lib/discourse-markdown/chat-transcript.js @@ -155,6 +155,7 @@ const chatTranscriptRule = { // rendering chat message content with limited markdown rule subset const token = state.push("html_raw", "", 1); + token.content = customMarkdownCookFn(content); state.push("html_raw", "", -1); @@ -246,6 +247,10 @@ export function setup(helper) { { featuresOverride: chatAdditionalOpts.limited_pretty_text_features, markdownItRules, + hashtagLookup: opts.discourse.hashtagLookup, + hashtagTypesInPriorityOrder: + chatAdditionalOpts.hashtag_configurations["chat-composer"], + hashtagIcons: opts.discourse.hashtagIcons, }, (customCookFn) => { customMarkdownCookFn = customCookFn; diff --git a/plugins/chat/lib/chat_channel_fetcher.rb b/plugins/chat/lib/chat_channel_fetcher.rb index 714737043f..2fcb6774d1 100644 --- a/plugins/chat/lib/chat_channel_fetcher.rb +++ b/plugins/chat/lib/chat_channel_fetcher.rb @@ -76,13 +76,17 @@ module Chat::ChatChannelFetcher channels = channels.where(status: options[:status]) if options[:status].present? if options[:filter].present? - sql = "chat_channels.name ILIKE :filter OR categories.name ILIKE :filter" + sql = "chat_channels.name ILIKE :filter OR chat_channels.slug ILIKE :filter OR categories.name ILIKE :filter" channels = channels.where(sql, filter: "%#{options[:filter].downcase}%").order( "chat_channels.name ASC, categories.name ASC", ) end + if options.key?(:slugs) + channels = channels.where("chat_channels.slug IN (:slugs)", slugs: options[:slugs]) + end + if options.key?(:following) if options[:following] channels = diff --git a/plugins/chat/lib/chat_channel_hashtag_data_source.rb b/plugins/chat/lib/chat_channel_hashtag_data_source.rb new file mode 100644 index 0000000000..fac5d3bf2a --- /dev/null +++ b/plugins/chat/lib/chat_channel_hashtag_data_source.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class Chat::ChatChannelHashtagDataSource + def self.icon + "comment" + end + + def self.channel_to_hashtag_item(guardian, channel) + HashtagAutocompleteService::HashtagItem.new.tap do |item| + item.text = channel.title(guardian.user) + item.slug = channel.slug + item.icon = icon + item.relative_url = channel.relative_url + item.type = "channel" + end + end + + def self.lookup(guardian, slugs) + if SiteSetting.enable_experimental_hashtag_autocomplete + Chat::ChatChannelFetcher + .secured_public_channel_search(guardian, slugs: slugs) + .map { |channel| channel_to_hashtag_item(guardian, channel) } + else + [] + end + end + + def self.search(guardian, term, limit) + if SiteSetting.enable_experimental_hashtag_autocomplete + Chat::ChatChannelFetcher + .secured_public_channel_search(guardian, filter: term, limit: limit) + .map { |channel| channel_to_hashtag_item(guardian, channel) } + else + [] + end + end +end diff --git a/plugins/chat/lib/chat_message_creator.rb b/plugins/chat/lib/chat_message_creator.rb index 49b9f58b9f..629c07e390 100644 --- a/plugins/chat/lib/chat_message_creator.rb +++ b/plugins/chat/lib/chat_message_creator.rb @@ -31,6 +31,7 @@ class Chat::ChatMessageCreator ChatMessage.new( chat_channel: @chat_channel, user_id: @user.id, + last_editor_id: @user.id, in_reply_to_id: @in_reply_to_id, message: @content, ) diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index 933266ad5e..1544fba4f5 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -157,6 +157,7 @@ after_initialize do load File.expand_path("../app/serializers/user_chat_message_bookmark_serializer.rb", __FILE__) load File.expand_path("../app/serializers/reviewable_chat_message_serializer.rb", __FILE__) load File.expand_path("../lib/chat_channel_fetcher.rb", __FILE__) + load File.expand_path("../lib/chat_channel_hashtag_data_source.rb", __FILE__) load File.expand_path("../lib/chat_mailer.rb", __FILE__) load File.expand_path("../lib/chat_message_creator.rb", __FILE__) load File.expand_path("../lib/chat_message_processor.rb", __FILE__) @@ -228,10 +229,6 @@ after_initialize do ReviewableScore.add_new_types([:needs_review]) Site.preloaded_category_custom_fields << Chat::HAS_CHAT_ENABLED - Site.markdown_additional_options["chat"] = { - limited_pretty_text_features: ChatMessage::MARKDOWN_FEATURES, - limited_pretty_text_markdown_rules: ChatMessage::MARKDOWN_IT_RULES, - } Guardian.prepend Chat::GuardianExtensions UserNotifications.prepend Chat::UserNotificationsExtension @@ -719,6 +716,19 @@ after_initialize do register_about_stat_group("chat_channels") { Chat::Statistics.about_channels } register_about_stat_group("chat_users") { Chat::Statistics.about_users } + + # Make sure to update spec/system/hashtag_autocomplete_spec.rb when changing this. + register_hashtag_data_source("channel", Chat::ChatChannelHashtagDataSource) + register_hashtag_type_in_context("channel", "chat-composer", 200) + register_hashtag_type_in_context("category", "chat-composer", 100) + register_hashtag_type_in_context("tag", "chat-composer", 50) + register_hashtag_type_in_context("channel", "topic-composer", 10) + + Site.markdown_additional_options["chat"] = { + limited_pretty_text_features: ChatMessage::MARKDOWN_FEATURES, + limited_pretty_text_markdown_rules: ChatMessage::MARKDOWN_IT_RULES, + hashtag_configurations: HashtagAutocompleteService.contexts_with_ordered_types, + } end if Rails.env == "test" diff --git a/plugins/chat/spec/lib/chat_channel_fetcher_spec.rb b/plugins/chat/spec/lib/chat_channel_fetcher_spec.rb index 5a51d999d7..782062231d 100644 --- a/plugins/chat/spec/lib/chat_channel_fetcher_spec.rb +++ b/plugins/chat/spec/lib/chat_channel_fetcher_spec.rb @@ -3,7 +3,7 @@ describe Chat::ChatChannelFetcher do fab!(:category) { Fabricate(:category, name: "support") } fab!(:private_category) { Fabricate(:private_category, group: Fabricate(:group)) } - fab!(:category_channel) { Fabricate(:category_channel, chatable: category) } + fab!(:category_channel) { Fabricate(:category_channel, chatable: category, slug: "support") } fab!(:dm_channel1) { Fabricate(:direct_message) } fab!(:dm_channel2) { Fabricate(:direct_message) } fab!(:direct_message_channel1) { Fabricate(:direct_message_channel, chatable: dm_channel1) } @@ -170,6 +170,26 @@ describe Chat::ChatChannelFetcher do ).to match_array([category_channel.id]) end + it "can filter by an array of slugs" do + expect( + subject.secured_public_channels( + guardian, + memberships, + slugs: ["support"], + ).map(&:id), + ).to match_array([category_channel.id]) + end + + it "returns nothing if the array of slugs is empty" do + expect( + subject.secured_public_channels( + guardian, + memberships, + slugs: [], + ).map(&:id), + ).to eq([]) + end + it "can filter by status" do expect( subject.secured_public_channels(guardian, memberships, status: "closed").map(&:id), diff --git a/plugins/chat/spec/lib/chat_channel_hashtag_data_source_spec.rb b/plugins/chat/spec/lib/chat_channel_hashtag_data_source_spec.rb new file mode 100644 index 0000000000..b46157d00d --- /dev/null +++ b/plugins/chat/spec/lib/chat_channel_hashtag_data_source_spec.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +RSpec.describe Chat::ChatChannelHashtagDataSource do + fab!(:user) { Fabricate(:user) } + fab!(:category) { Fabricate(:category) } + fab!(:group) { Fabricate(:group) } + fab!(:private_category) { Fabricate(:private_category, group: group) } + fab!(:channel1) { Fabricate(:chat_channel, slug: "random", name: "Zany Things", chatable: category) } + fab!(:channel2) do + Fabricate(:chat_channel, slug: "secret", name: "Secret Stuff", chatable: private_category) + end + let!(:guardian) { Guardian.new(user) } + + before { SiteSetting.enable_experimental_hashtag_autocomplete = true } + + describe "#lookup" do + it "finds a channel by a slug" do + result = described_class.lookup(guardian, ["random"]).first + expect(result.to_h).to eq( + { + relative_url: channel1.relative_url, + text: "Zany Things", + icon: "comment", + type: "channel", + ref: nil, + slug: "random", + }, + ) + end + + it "does not return a channel that a user does not have permission to view" do + result = described_class.lookup(guardian, ["secret"]).first + expect(result).to eq(nil) + + GroupUser.create(user: user, group: group) + result = described_class.lookup(Guardian.new(user), ["secret"]).first + expect(result.to_h).to eq( + { + relative_url: channel2.relative_url, + text: "Secret Stuff", + icon: "comment", + type: "channel", + ref: nil, + slug: "secret", + }, + ) + end + + it "returns nothing if the slugs array is empty" do + result = described_class.lookup(guardian, []).first + expect(result).to eq(nil) + end + end + + describe "#search" do + it "finds a channel by category name" do + category.update!(name: "Randomizer") + result = described_class.search(guardian, "randomiz", 10).first + expect(result.to_h).to eq( + { + relative_url: channel1.relative_url, + text: "Zany Things", + icon: "comment", + type: "channel", + ref: nil, + slug: "random", + }, + ) + end + + it "finds a channel by slug" do + result = described_class.search(guardian, "rand", 10).first + expect(result.to_h).to eq( + { + relative_url: channel1.relative_url, + text: "Zany Things", + icon: "comment", + type: "channel", + ref: nil, + slug: "random", + }, + ) + end + + it "finds a channel by channel name" do + result = described_class.search(guardian, "aNY t", 10).first + expect(result.to_h).to eq( + { + relative_url: channel1.relative_url, + text: "Zany Things", + icon: "comment", + type: "channel", + ref: nil, + slug: "random", + }, + ) + end + + it "does not return channels the user does not have permission to view" do + result = described_class.search(guardian, "Sec", 10).first + expect(result).to eq(nil) + GroupUser.create(user: user, group: group) + result = described_class.search(Guardian.new(user), "Sec", 10).first + expect(result.to_h).to eq( + { + relative_url: channel2.relative_url, + text: "Secret Stuff", + icon: "comment", + type: "channel", + ref: nil, + slug: "secret", + }, + ) + end + end +end diff --git a/plugins/chat/spec/plugin_helper.rb b/plugins/chat/spec/plugin_helper.rb new file mode 100644 index 0000000000..09c4fbe37e --- /dev/null +++ b/plugins/chat/spec/plugin_helper.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module ChatSystemHelpers + def chat_system_bootstrap(user, channels_for_membership = []) + # ensures we have one valid registered admin/user + user.activate + + SiteSetting.chat_enabled = true + SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_1] + + channels_for_membership.each do |channel| + membership = channel.add(user) + if channel.chat_messages.any? + membership.update!(last_read_message_id: channel.chat_messages.last.id) + end + end + + Group.refresh_automatic_groups! + end +end + +RSpec.configure do |config| + config.include ChatSystemHelpers, type: :system +end diff --git a/plugins/chat/spec/system/hashtag_autocomplete_spec.rb b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb new file mode 100644 index 0000000000..203d2031f8 --- /dev/null +++ b/plugins/chat/spec/system/hashtag_autocomplete_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +describe "Using #hashtag autocompletion to search for and lookup channels", + type: :system, + js: true do + fab!(:user) { Fabricate(:user) } + fab!(:channel1) { Fabricate(:chat_channel, name: "Music Lounge", slug: "music") } + fab!(:channel2) { Fabricate(:chat_channel, name: "Random", slug: "random") } + fab!(:category) { Fabricate(:category, name: "Raspberry", slug: "raspberry-beret") } + fab!(:tag) { Fabricate(:tag, name: "razed") } + fab!(:topic) { Fabricate(:topic) } + fab!(:post) { Fabricate(:post, topic: topic) } + fab!(:message1) { Fabricate(:chat_message, chat_channel: channel1) } + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:chat_drawer_page) { PageObjects::Pages::ChatDrawer.new } + let(:chat_channel_page) { PageObjects::Pages::ChatChannel.new } + let(:topic_page) { PageObjects::Pages::Topic.new } + + before do + SiteSetting.enable_experimental_hashtag_autocomplete = true + + # This is annoying, but we need to reset the hashtag data sources inbetween + # tests, and since this is normally done in plugin.rb with the plugin API + # there is not an easier way to do this. + HashtagAutocompleteService.register_data_source("channel", Chat::ChatChannelHashtagDataSource) + HashtagAutocompleteService.register_type_in_context("channel", "chat-composer", 200) + HashtagAutocompleteService.register_type_in_context("category", "chat-composer", 100) + HashtagAutocompleteService.register_type_in_context("tag", "chat-composer", 50) + HashtagAutocompleteService.register_type_in_context("channel", "topic-composer", 10) + + chat_system_bootstrap(user, [channel1, channel2]) + sign_in(user) + end + + it "searches for channels, categories, and tags with # and prioritises channels in the results" do + chat_page.visit_channel(channel1) + expect(chat_channel_page).to have_no_loading_skeleton + chat_channel_page.type_in_composer("this is #ra") + expect(page).to have_css( + ".hashtag-autocomplete .hashtag-autocomplete__option .hashtag-autocomplete__link", + count: 3, + ) + hashtag_results = page.all(".hashtag-autocomplete__link", count: 3) + expect(hashtag_results.map(&:text)).to eq(["Random", "Raspberry", "razed x 0"]) + end + + it "searches for channels as well with # in a topic composer and deprioritises them" do + topic_page.visit_topic_and_open_composer(topic) + expect(topic_page).to have_expanded_composer + topic_page.type_in_composer("something #ra") + expect(page).to have_css( + ".hashtag-autocomplete .hashtag-autocomplete__option .hashtag-autocomplete__link", + count: 3, + ) + hashtag_results = page.all(".hashtag-autocomplete__link", count: 3) + expect(hashtag_results.map(&:text)).to eq(["Raspberry", "razed x 0", "Random"]) + end + + # TODO (martin) Commenting this out for now, we need to add the MessageBus + # last_message_id to our chat subscriptions in JS for this to work, since it + # relies on a MessageBus "sent" event to be published to substitute the + # staged message ID for the real one. + xit "cooks the hashtags for channels, categories, and tags serverside when the chat message is saved to the database" do + chat_page.visit_channel(channel1) + expect(chat_channel_page).to have_no_loading_skeleton + chat_channel_page.type_in_composer("this is #random and this is #raspberry and this is #razed which is cool") + chat_channel_page.click_send_message + + try_until_success do + expect(ChatMessage.exists?(user: user, message: "this is #random and this is #raspberry and this is #razed which is cool")).to eq(true) + end + message = ChatMessage.where(user: user).last + expect(chat_channel_page).to have_message(id: message.id) + + within chat_channel_page.message_by_id(message.id) do + cooked_hashtags = page.all(".hashtag-cooked", count: 3) + + expect(cooked_hashtags[0]["outerHTML"]).to eq(<<~HTML.chomp) + Random + HTML + expect(cooked_hashtags[1]["outerHTML"]).to eq(<<~HTML.chomp) + raspberry + HTML + expect(cooked_hashtags[2]["outerHTML"]).to eq(<<~HTML.chomp) + razed + HTML + end + end +end diff --git a/plugins/chat/spec/system/navigation_spec.rb b/plugins/chat/spec/system/navigation_spec.rb index 3c48cb2620..82a0f6ee45 100644 --- a/plugins/chat/spec/system/navigation_spec.rb +++ b/plugins/chat/spec/system/navigation_spec.rb @@ -13,14 +13,7 @@ RSpec.describe "Navigation", type: :system, js: true do let(:chat_drawer_page) { PageObjects::Pages::ChatDrawer.new } before do - # ensures we have one valid registered admin - user.activate - - SiteSetting.chat_enabled = true - SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] - category_channel.add(user) - category_channel_2.add(user) - + chat_system_bootstrap(user, [category_channel, category_channel_2]) sign_in(user) end diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb index 43c1529cd8..67ebe55f41 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat.rb @@ -11,6 +11,10 @@ module PageObjects visit("/chat") end + def visit_channel(channel) + visit(channel.url) + end + def minimize_full_page find(".open-drawer-btn").click end diff --git a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb new file mode 100644 index 0000000000..adbe0756a4 --- /dev/null +++ b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module PageObjects + module Pages + class ChatChannel < PageObjects::Pages::Base + def type_in_composer(input) + find(".chat-composer-input").send_keys(input) + end + + def fill_composer(input) + find(".chat-composer-input").fill_in(with: input) + end + + def click_send_message + find(".chat-composer .send-btn").click + end + + def message_by_id(id) + find(".chat-message-container[data-id=\"#{id}\"]") + end + + def has_no_loading_skeleton? + has_no_css?(".chat-skeleton") + end + + def has_message?(text: nil, id: nil) + if text + has_css?(".chat-message-text", text: text) + elsif id + has_css?(".chat-message-container[data-id=\"#{id}\"]", wait: 10) + end + end + end + end +end diff --git a/plugins/chat/test/javascripts/acceptance/chat-transcript-test.js b/plugins/chat/test/javascripts/acceptance/chat-transcript-test.js index 6321bbead1..00767d0071 100644 --- a/plugins/chat/test/javascripts/acceptance/chat-transcript-test.js +++ b/plugins/chat/test/javascripts/acceptance/chat-transcript-test.js @@ -164,6 +164,9 @@ function buildAdditionalOptions() { "blockquote", "emphasis", ], + hashtag_configurations: { + "chat-composer": ["channel", "category", "tag"], + }, }, }; } diff --git a/spec/lib/email/sender_spec.rb b/spec/lib/email/sender_spec.rb index dea27a342f..5dd1b5f3ef 100644 --- a/spec/lib/email/sender_spec.rb +++ b/spec/lib/email/sender_spec.rb @@ -535,6 +535,18 @@ RSpec.describe Email::Sender do .to contain_exactly(*[small_pdf, large_pdf, csv_file].map(&:original_filename)) end + it "changes the hashtags to the slug with a # symbol beforehand rather than the full name of the resource" do + SiteSetting.enable_experimental_hashtag_autocomplete = true + category = Fabricate(:category, slug: "dev") + reply.update!(raw: reply.raw + "\n wow this is #dev") + reply.rebake! + Email::Sender.new(message, :valid_type).send + expected = <<~HTML + #dev + HTML + expect(message.html_part.body.to_s).to include(expected.chomp) + end + context "when secure uploads enabled" do before do setup_s3 diff --git a/spec/lib/pretty_text/helpers_spec.rb b/spec/lib/pretty_text/helpers_spec.rb index 74544d18ee..ecb944b065 100644 --- a/spec/lib/pretty_text/helpers_spec.rb +++ b/spec/lib/pretty_text/helpers_spec.rb @@ -46,4 +46,101 @@ RSpec.describe PrettyText::Helpers do expect(PrettyText::Helpers.category_tag_hashtag_lookup("blah")).to eq(nil) end end + + describe ".hashtag_lookup" do + fab!(:tag) { Fabricate(:tag, name: "somecooltag") } + fab!(:category) do + Fabricate(:category, name: "Some Awesome Category", slug: "someawesomecategory") + end + fab!(:user) { Fabricate(:user) } + + it "handles tags and categories based on slug with type suffix" do + expect(PrettyText::Helpers.hashtag_lookup("somecooltag::tag", user, %w[category tag])).to eq( + { + relative_url: tag.url, + text: "somecooltag", + icon: "tag", + slug: "somecooltag", + ref: "somecooltag", + type: "tag", + }, + ) + expect(PrettyText::Helpers.hashtag_lookup("someawesomecategory::category", user, %w[category tag])).to eq( + { + relative_url: category.url, + text: "Some Awesome Category", + icon: "folder", + slug: "someawesomecategory", + ref: "someawesomecategory", + type: "category", + }, + ) + end + + it "handles categories based on slug" do + expect( + PrettyText::Helpers.hashtag_lookup("someawesomecategory", user, %w[category tag]), + ).to eq( + { + relative_url: category.url, + text: "Some Awesome Category", + icon: "folder", + slug: "someawesomecategory", + ref: "someawesomecategory", + type: "category", + }, + ) + end + + it "handles tags and categories based on slug without type suffix" do + expect(PrettyText::Helpers.hashtag_lookup("somecooltag", user, %w[category tag])).to eq( + { + relative_url: tag.url, + text: "somecooltag", + icon: "tag", + slug: "somecooltag", + ref: "somecooltag", + type: "tag", + }, + ) + expect(PrettyText::Helpers.hashtag_lookup("someawesomecategory", user, %w[category tag])).to eq( + { + relative_url: category.url, + text: "Some Awesome Category", + icon: "folder", + slug: "someawesomecategory", + ref: "someawesomecategory", + type: "category", + }, + ) + end + + it "does not include categories the cooking user does not have access to" do + group = Fabricate(:group) + private_category = Fabricate(:private_category, slug: "secretcategory", name: "Manager Hideout", group: group) + expect(PrettyText::Helpers.hashtag_lookup("secretcategory", user, %w[category tag])).to eq(nil) + + GroupUser.create(group: group, user: user) + expect(PrettyText::Helpers.hashtag_lookup("secretcategory", user, %w[category tag])).to eq( + { + relative_url: private_category.url, + text: "Manager Hideout", + icon: "folder", + slug: "secretcategory", + ref: "secretcategory", + type: "category", + }, + ) + end + + it "returns nil when no tag or category that matches exists" do + expect(PrettyText::Helpers.hashtag_lookup("blah", user, %w[category tag])).to eq(nil) + end + + it "uses the system user if the cooking_user is nil" do + guardian_system = Guardian.new(Discourse.system_user) + Guardian.expects(:new).with(Discourse.system_user).returns(guardian_system) + PrettyText::Helpers.hashtag_lookup("somecooltag", nil, %w[category tag]) + end + end end diff --git a/spec/lib/pretty_text_spec.rb b/spec/lib/pretty_text_spec.rb index 532f276d89..106ec628dd 100644 --- a/spec/lib/pretty_text_spec.rb +++ b/spec/lib/pretty_text_spec.rb @@ -1452,6 +1452,49 @@ RSpec.describe PrettyText do end + it "produces hashtag links when enable_experimental_hashtag_autocomplete is enabled" do + SiteSetting.enable_experimental_hashtag_autocomplete = true + + user = Fabricate(:user) + category = Fabricate(:category, name: 'testing') + category2 = Fabricate(:category, name: 'known') + Fabricate(:topic, tags: [Fabricate(:tag, name: 'known')]) + + cooked = PrettyText.cook(" #unknown::tag #known #known::tag #testing", user_id: user.id) + + [ + "unknown::tag", + "known", + "known", + "testing" + ].each do |element| + expect(cooked).to include(element) + end + + cooked = PrettyText.cook("[`a` #known::tag here](http://example.com)", user_id: user.id) + + html = <<~HTML +
+ HTML + + expect(cooked).to eq(html.strip) + + cooked = PrettyText.cook("`a` #known::tag here", user_id: user.id) + + expect(cooked).to eq(html.strip) + + cooked = PrettyText.cook("test #known::tag", user_id: user.id) + html = <<~HTML + + HTML + expect(cooked).to eq(html.strip) + + # ensure it does not fight with the autolinker + expect(PrettyText.cook(' http://somewhere.com/#known')).not_to include('hashtag') + expect(PrettyText.cook(' http://somewhere.com/?#known')).not_to include('hashtag') + expect(PrettyText.cook(' http://somewhere.com/?abc#known')).not_to include('hashtag') + end + it "can handle mixed lists" do # known bug in old md engine cooked = PrettyText.cook("* a\n\n1. b") diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index ead664ee12..a8c10b99a7 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -1001,6 +1001,21 @@ RSpec.describe Post do expect(post.cooked).to match(/noopener nofollow ugc/) end + it "passes the last_editor_id as the markdown user_id option" do + post.save + post.reload + PostAnalyzer.any_instance.expects(:cook).with( + post.raw, { cook_method: Post.cook_methods[:regular], user_id: post.last_editor_id } + ) + post.cook(post.raw) + user_editor = Fabricate(:user) + post.update!(last_editor_id: user_editor.id) + PostAnalyzer.any_instance.expects(:cook).with( + post.raw, { cook_method: Post.cook_methods[:regular], user_id: user_editor.id } + ) + post.cook(post.raw) + end + describe 'mentions' do fab!(:group) do Fabricate(:group, diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 0a057d4962..14e8ff951d 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -156,7 +156,7 @@ module TestSetup Bookmark.reset_bookmarkables # Make sure only the default category and tag hashtag data sources are registered. - HashtagAutocompleteService.clear_data_sources + HashtagAutocompleteService.clear_registered OmniAuth.config.test_mode = false end diff --git a/spec/requests/api/schemas/json/site_response.json b/spec/requests/api/schemas/json/site_response.json index 1401d22311..16bc845a5d 100644 --- a/spec/requests/api/schemas/json/site_response.json +++ b/spec/requests/api/schemas/json/site_response.json @@ -470,6 +470,12 @@ "markdown_additional_options" : { "type": "object" }, + "hashtag_configurations" : { + "type": "object" + }, + "hashtag_icons" : { + "type": "array" + }, "displayed_about_plugin_stat_groups" : { "type": "array" }, diff --git a/spec/requests/hashtags_controller_spec.rb b/spec/requests/hashtags_controller_spec.rb index a157e2a153..2eb51bfbc9 100644 --- a/spec/requests/hashtags_controller_spec.rb +++ b/spec/requests/hashtags_controller_spec.rb @@ -15,7 +15,7 @@ RSpec.describe HashtagsController do tag_group end - describe "#check" do + describe "#lookup" do context "when logged in" do context "as regular user" do before do @@ -119,4 +119,7 @@ RSpec.describe HashtagsController do end end end + + # TODO (martin) write a spec here for the new + # #lookup behaviour and the new #search behaviour end diff --git a/spec/services/hashtag_autocomplete_service_spec.rb b/spec/services/hashtag_autocomplete_service_spec.rb index 94281c4c66..06efda5347 100644 --- a/spec/services/hashtag_autocomplete_service_spec.rb +++ b/spec/services/hashtag_autocomplete_service_spec.rb @@ -10,8 +10,26 @@ RSpec.describe HashtagAutocompleteService do before { Site.clear_cache } - def register_bookmark_data_source - HashtagAutocompleteService.register_data_source("bookmark") do |guardian_scoped, term, limit| + class BookmarkDataSource + def self.icon + "bookmark" + end + + def self.lookup(guardian_scoped, slugs) + guardian_scoped + .user + .bookmarks + .where("LOWER(name) IN (:slugs)", slugs: slugs) + .map do |bm| + HashtagAutocompleteService::HashtagItem.new.tap do |item| + item.text = bm.name + item.slug = bm.name.gsub(" ", "-") + item.icon = icon + end + end + end + + def self.search(guardian_scoped, term, limit) guardian_scoped .user .bookmarks @@ -21,12 +39,31 @@ RSpec.describe HashtagAutocompleteService do HashtagAutocompleteService::HashtagItem.new.tap do |item| item.text = bm.name item.slug = bm.name.gsub(" ", "-") - item.icon = "bookmark" + item.icon = icon end end end end + describe ".contexts_with_ordered_types" do + it "returns a hash of all the registrered search contexts and their types in the defined priority order" do + expect(HashtagAutocompleteService.contexts_with_ordered_types).to eq( + { "topic-composer" => %w[category tag] }, + ) + HashtagAutocompleteService.register_type_in_context("category", "awesome-composer", 50) + HashtagAutocompleteService.register_type_in_context("tag", "awesome-composer", 100) + expect(HashtagAutocompleteService.contexts_with_ordered_types).to eq( + { "topic-composer" => %w[category tag], "awesome-composer" => %w[tag category] }, + ) + end + end + + describe ".data_source_icons" do + it "gets an array for all icons defined by data sources so they can be used for markdown allowlisting" do + expect(HashtagAutocompleteService.data_source_icons).to eq(%w[folder tag]) + end + end + describe "#search" do it "returns search results for tags and categories by default" do expect(subject.search("book", %w[category tag]).map(&:text)).to eq( @@ -41,7 +78,9 @@ RSpec.describe HashtagAutocompleteService do end it "respects the limit param" do - expect(subject.search("book", %w[tag category], limit: 1).map(&:text)).to eq(["great-books x 0"]) + expect(subject.search("book", %w[tag category], limit: 1).map(&:text)).to eq( + ["great-books x 0"], + ) end it "does not allow more than SEARCH_MAX_LIMIT results to be specified by the limit param" do @@ -59,7 +98,9 @@ RSpec.describe HashtagAutocompleteService do it "includes the tag count" do tag1.update!(topic_count: 78) - expect(subject.search("book", %w[tag category]).map(&:text)).to eq(["great-books x 78", "Book Club"]) + expect(subject.search("book", %w[tag category]).map(&:text)).to eq( + ["great-books x 78", "Book Club"], + ) end it "does case-insensitive search" do @@ -71,6 +112,11 @@ RSpec.describe HashtagAutocompleteService do ) end + it "can search categories by name or slug" do + expect(subject.search("book-club", %w[category]).map(&:text)).to eq(["Book Club"]) + expect(subject.search("Book C", %w[category]).map(&:text)).to eq(["Book Club"]) + end + it "does not include categories the user cannot access" do category1.update!(read_restricted: true) expect(subject.search("book", %w[tag category]).map(&:text)).to eq(["great-books x 0"]) @@ -86,20 +132,7 @@ RSpec.describe HashtagAutocompleteService do Fabricate(:bookmark, user: user, name: "cool rock song") guardian.user.reload - HashtagAutocompleteService.register_data_source("bookmark") do |guardian_scoped, term, limit| - guardian_scoped - .user - .bookmarks - .where("name ILIKE ?", "%#{term}%") - .limit(limit) - .map do |bm| - HashtagAutocompleteService::HashtagItem.new.tap do |item| - item.text = bm.name - item.slug = bm.name.dasherize - item.icon = "bookmark" - end - end - end + HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource) expect(subject.search("book", %w[category tag bookmark]).map(&:text)).to eq( ["Book Club", "great-books x 0", "read review of this fantasy book"], @@ -112,6 +145,7 @@ RSpec.describe HashtagAutocompleteService do expect(subject.search("book", %w[category tag]).map(&:ref)).to eq( %w[hobbies:book-club great-books], ) + category1.update!(parent_category: nil) end it "appends type suffixes for the ref on conflicting slugs on items that are not the top priority type" do @@ -123,7 +157,7 @@ RSpec.describe HashtagAutocompleteService do Fabricate(:bookmark, user: user, name: "book club") guardian.user.reload - register_bookmark_data_source + HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource) expect(subject.search("book", %w[category tag bookmark]).map(&:ref)).to eq( %w[book-club book-club::tag great-books book-club::bookmark], @@ -151,4 +185,156 @@ RSpec.describe HashtagAutocompleteService do end end end + + describe "#lookup_old" do + fab!(:tag2) { Fabricate(:tag, name: "fiction-books") } + + it "returns categories and tags in a hash format with the slug and url" do + result = subject.lookup_old(%w[book-club great-books fiction-books]) + expect(result[:categories]).to eq({ "book-club" => "/c/book-club/#{category1.id}" }) + expect(result[:tags]).to eq( + { + "fiction-books" => "http://test.localhost/tag/fiction-books", + "great-books" => "http://test.localhost/tag/great-books", + }, + ) + end + + it "does not include categories the user cannot access" do + category1.update!(read_restricted: true) + result = subject.lookup_old(%w[book-club great-books fiction-books]) + expect(result[:categories]).to eq({}) + end + + it "does not include tags the user cannot access" do + Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["great-books"]) + result = subject.lookup_old(%w[book-club great-books fiction-books]) + expect(result[:tags]).to eq({ "fiction-books" => "http://test.localhost/tag/fiction-books" }) + end + + it "handles tags which have the ::tag suffix" do + result = subject.lookup_old(%w[book-club great-books::tag fiction-books]) + expect(result[:tags]).to eq( + { + "fiction-books" => "http://test.localhost/tag/fiction-books", + "great-books" => "http://test.localhost/tag/great-books", + }, + ) + end + + context "when not tagging_enabled" do + before { SiteSetting.tagging_enabled = false } + + it "does not return tags" do + result = subject.lookup_old(%w[book-club great-books fiction-books]) + expect(result[:categories]).to eq({ "book-club" => "/c/book-club/#{category1.id}" }) + expect(result[:tags]).to eq({}) + end + end + end + + describe "#lookup" do + fab!(:tag2) { Fabricate(:tag, name: "fiction-books") } + + it "returns category and tag in a hash format with the slug and url" do + result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) + expect(result[:category].map(&:slug)).to eq(["book-club"]) + expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) + expect(result[:tag].map(&:slug)).to eq(%w[fiction-books great-books]) + expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/great-books]) + end + + it "does not include category the user cannot access" do + category1.update!(read_restricted: true) + result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) + expect(result[:category]).to eq([]) + end + + it "does not include tag the user cannot access" do + Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["great-books"]) + result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) + expect(result[:tag].map(&:slug)).to eq(%w[fiction-books]) + expect(result[:tag].map(&:relative_url)).to eq(["/tag/fiction-books"]) + end + + it "handles type suffixes for slugs" do + result = + subject.lookup(%w[book-club::category great-books::tag fiction-books], %w[category tag]) + expect(result[:category].map(&:slug)).to eq(["book-club"]) + expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) + expect(result[:tag].map(&:slug)).to eq(%w[fiction-books great-books]) + expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/great-books]) + end + + it "handles parent:child category lookups" do + parent_category = Fabricate(:category, name: "Media", slug: "media") + category1.update!(parent_category: parent_category) + result = subject.lookup(%w[media:book-club], %w[category tag]) + expect(result[:category].map(&:slug)).to eq(["book-club"]) + expect(result[:category].map(&:ref)).to eq(["media:book-club"]) + expect(result[:category].map(&:relative_url)).to eq(["/c/media/book-club/#{category1.id}"]) + category1.update!(parent_category: nil) + end + + it "does not return the category if the parent does not match the child" do + parent_category = Fabricate(:category, name: "Media", slug: "media") + category1.update!(parent_category: parent_category) + result = subject.lookup(%w[bad-parent:book-club], %w[category tag]) + expect(result[:category]).to be_empty + end + + it "for slugs without a type suffix it falls back in type order until a result is found or types are exhausted" do + result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) + expect(result[:category].map(&:slug)).to eq(["book-club"]) + expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) + expect(result[:tag].map(&:slug)).to eq(%w[fiction-books great-books]) + expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books /tag/great-books]) + + category2 = Fabricate(:category, name: "Great Books", slug: "great-books") + result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) + expect(result[:category].map(&:slug)).to eq(%w[book-club great-books]) + expect(result[:category].map(&:relative_url)).to eq( + ["/c/book-club/#{category1.id}", "/c/great-books/#{category2.id}"], + ) + expect(result[:tag].map(&:slug)).to eq(%w[fiction-books]) + expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/fiction-books]) + + category1.destroy! + Fabricate(:tag, name: "book-club") + result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) + expect(result[:category].map(&:slug)).to eq(["great-books"]) + expect(result[:category].map(&:relative_url)).to eq(["/c/great-books/#{category2.id}"]) + expect(result[:tag].map(&:slug)).to eq(%w[book-club fiction-books]) + expect(result[:tag].map(&:relative_url)).to eq(%w[/tag/book-club /tag/fiction-books]) + + result = subject.lookup(%w[book-club great-books fiction-books], %w[tag category]) + expect(result[:category]).to eq([]) + expect(result[:tag].map(&:slug)).to eq(%w[book-club fiction-books great-books]) + expect(result[:tag].map(&:relative_url)).to eq( + %w[/tag/book-club /tag/fiction-books /tag/great-books], + ) + end + + it "includes other data sources" do + Fabricate(:bookmark, user: user, name: "read review of this fantasy book") + Fabricate(:bookmark, user: user, name: "coolrock") + guardian.user.reload + + HashtagAutocompleteService.register_data_source("bookmark", BookmarkDataSource) + + result = subject.lookup(["coolrock"], %w[category tag bookmark]) + expect(result[:bookmark].map(&:slug)).to eq(["coolrock"]) + end + + context "when not tagging_enabled" do + before { SiteSetting.tagging_enabled = false } + + it "does not return tag" do + result = subject.lookup(%w[book-club great-books fiction-books], %w[category tag]) + expect(result[:category].map(&:slug)).to eq(["book-club"]) + expect(result[:category].map(&:relative_url)).to eq(["/c/book-club/#{category1.id}"]) + expect(result[:tag]).to eq([]) + end + end + end end diff --git a/spec/support/system_helpers.rb b/spec/support/system_helpers.rb index b7116b2a4c..e00d179cbd 100644 --- a/spec/support/system_helpers.rb +++ b/spec/support/system_helpers.rb @@ -17,9 +17,7 @@ module SystemHelpers backoff ||= frequency yield rescue RSpec::Expectations::ExpectationNotMetError - if Time.zone.now >= start + timeout.seconds - raise - end + raise if Time.zone.now >= start + timeout.seconds sleep backoff backoff += frequency retry diff --git a/spec/system/hashtag_autocomplete_spec.rb b/spec/system/hashtag_autocomplete_spec.rb new file mode 100644 index 0000000000..8fe3ca77a5 --- /dev/null +++ b/spec/system/hashtag_autocomplete_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +describe "Using #hashtag autocompletion to search for and lookup categories and tags", + type: :system, + js: true do + fab!(:user) { Fabricate(:user) } + fab!(:topic) { Fabricate(:topic) } + fab!(:post) { Fabricate(:post, topic: topic) } + fab!(:category) { Fabricate(:category, name: "Cool Category", slug: "cool-cat") } + fab!(:tag) { Fabricate(:tag, name: "cooltag") } + let(:topic_page) { PageObjects::Pages::Topic.new } + + before do + SiteSetting.enable_experimental_hashtag_autocomplete = true + sign_in user + end + + def visit_topic_and_initiate_autocomplete + topic_page.visit_topic_and_open_composer(topic) + expect(topic_page).to have_expanded_composer + topic_page.type_in_composer("something #co") + expect(page).to have_css( + ".hashtag-autocomplete .hashtag-autocomplete__option .hashtag-autocomplete__link", + count: 2, + ) + end + + it "searches for categories and tags with # and prioritises categories in the results" do + visit_topic_and_initiate_autocomplete + hashtag_results = page.all(".hashtag-autocomplete__link", count: 2) + expect(hashtag_results.map(&:text)).to eq(["Cool Category", "cooltag x 0"]) + end + + it "cooks the selected hashtag clientside with the correct url and icon" do + visit_topic_and_initiate_autocomplete + hashtag_results = page.all(".hashtag-autocomplete__link", count: 2) + hashtag_results[0].click + expect(page).to have_css(".hashtag-cooked") + cooked_hashtag = page.find(".hashtag-cooked") + expected = <<~HTML.chomp + Cool Category + HTML + expect(cooked_hashtag["outerHTML"].squish).to eq(expected) + + visit_topic_and_initiate_autocomplete + hashtag_results = page.all(".hashtag-autocomplete__link", count: 2) + hashtag_results[1].click + expect(page).to have_css(".hashtag-cooked") + cooked_hashtag = page.find(".hashtag-cooked") + expect(cooked_hashtag["outerHTML"].squish).to eq(<<~HTML.chomp) + cooltag + HTML + end + + it "cooks the hashtags for tag and category correctly serverside when the post is saved to the database" do + topic_page.visit_topic_and_open_composer(topic) + expect(topic_page).to have_expanded_composer + topic_page.type_in_composer("this is a #cool-cat category and a #cooltag tag") + topic_page.send_reply + expect(topic_page).to have_post_number(2) + + within topic_page.post_by_number(2) do + cooked_hashtags = page.all(".hashtag-cooked", count: 2) + + expect(cooked_hashtags[0]["outerHTML"]).to eq(<<~HTML.chomp) + Cool Category + HTML + expect(cooked_hashtags[1]["outerHTML"]).to eq(<<~HTML.chomp) + cooltag + HTML + end + end +end diff --git a/spec/system/page_objects/pages/topic.rb b/spec/system/page_objects/pages/topic.rb index aff0dc4ccf..5815fb760c 100644 --- a/spec/system/page_objects/pages/topic.rb +++ b/spec/system/page_objects/pages/topic.rb @@ -6,32 +6,58 @@ module PageObjects def initialize setup_component_classes!( post_show_more_actions: ".show-more-actions", - post_action_button_bookmark: ".bookmark.with-reminder" + post_action_button_bookmark: ".bookmark.with-reminder", + reply_button: ".topic-footer-main-buttons > .create", + composer: "#reply-control", + composer_textarea: "#reply-control .d-editor .d-editor-input", ) end + def visit_topic(topic) + page.visit "/t/#{topic.id}" + self + end + + def visit_topic_and_open_composer(topic) + visit_topic(topic) + click_reply_button + self + end + def has_post_content?(post) post_by_number(post).has_content? post.raw end + def has_post_number?(number) + has_css?("#post_#{number}") + end + + def post_by_number(post_or_number) + post_or_number = post_or_number.is_a?(Post) ? post_or_number.post_number : post_or_number + find("#post_#{post_or_number}") + end + def has_post_more_actions?(post) within post_by_number(post) do - has_css?(@component_classes[:post_show_more_actions]) + has_css?(".show-more-actions") end end def has_post_bookmarked?(post) within post_by_number(post) do - has_css?(@component_classes[:post_action_button_bookmark] + ".bookmarked") + has_css?(".bookmark.with-reminder.bookmarked") end end def expand_post_actions(post) - post_by_number(post).find(@component_classes[:post_show_more_actions]).click + post_by_number(post).find(".show-more-actions").click end def click_post_action_button(post, button) - post_by_number(post).find(@component_classes["post_action_button_#{button}".to_sym]).click + case button + when :bookmark + post_by_number(post).find(".bookmark.with-reminder").click + end end def click_topic_footer_button(button) @@ -46,15 +72,31 @@ module PageObjects find(topic_footer_button_id(button)) end + def click_reply_button + find(".topic-footer-main-buttons > .create").click + end + + def has_expanded_composer? + has_css?("#reply-control.open") + end + + def type_in_composer(input) + find("#reply-control .d-editor .d-editor-input").send_keys(input) + end + + def clear_composer + find("#reply-control .d-editor .d-editor-input").set("") + end + + def send_reply + within("#reply-control") { find(".save-or-cancel .create").click } + end + private def topic_footer_button_id(button) "#topic-footer-button-#{button}" end - - def post_by_number(post) - find("#post_#{post.post_number}") - end end end end