From 7c4d4331bccff13cb86ca0ba09a3401c9258f5ae Mon Sep 17 00:00:00 2001 From: Gerhard Schlager <mail@gerhard-schlager.at> Date: Sat, 10 Nov 2018 01:17:07 +0100 Subject: [PATCH] FEATURE: Better handling of quotation marks in site text search It also matches 3 dots with the ellipsis symbol. --- .../admin/components/site-text-summary.js.es6 | 15 ++++++- .../admin/templates/site-text-index.hbs | 2 +- .../admin/site_texts_controller.rb | 4 ++ lib/freedom_patches/translate_accelerator.rb | 2 +- lib/i18n/backend/discourse_i18n.rb | 13 +++++- .../admin/site_texts_controller_spec.rb | 40 +++++++++++++++++-- 6 files changed, 69 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/admin/components/site-text-summary.js.es6 b/app/assets/javascripts/admin/components/site-text-summary.js.es6 index 0ebd139614e..48ceb2fefd5 100644 --- a/app/assets/javascripts/admin/components/site-text-summary.js.es6 +++ b/app/assets/javascripts/admin/components/site-text-summary.js.es6 @@ -6,7 +6,8 @@ export default Ember.Component.extend({ @on("didInsertElement") highlightTerm() { - const term = this.get("term"); + const term = this._searchTerm(); + if (term) { this.$(".site-text-id, .site-text-value").highlight(term, { className: "text-highlight" @@ -19,6 +20,18 @@ export default Ember.Component.extend({ this.send("edit"); }, + _searchTerm() { + const regex = this.get("searchRegex"); + const siteText = this.get("siteText"); + + if (regex && siteText) { + const matches = siteText.value.match(new RegExp(regex, "i")); + if (matches) return matches[0]; + } + + return this.get("term"); + }, + actions: { edit() { this.sendAction("editAction", this.get("siteText")); diff --git a/app/assets/javascripts/admin/templates/site-text-index.hbs b/app/assets/javascripts/admin/templates/site-text-index.hbs index 7caf9b124e8..6d0589f9cd8 100644 --- a/app/assets/javascripts/admin/templates/site-text-index.hbs +++ b/app/assets/javascripts/admin/templates/site-text-index.hbs @@ -18,6 +18,6 @@ {{/if}} {{#each siteTexts as |siteText|}} - {{site-text-summary siteText=siteText editAction="edit" term=q}} + {{site-text-summary siteText=siteText editAction="edit" term=q searchRegex=siteTexts.extras.regex}} {{/each}} {{/conditional-loading-spinner}} diff --git a/app/controllers/admin/site_texts_controller.rb b/app/controllers/admin/site_texts_controller.rb index ffc3633203c..b00aae3ad47 100644 --- a/app/controllers/admin/site_texts_controller.rb +++ b/app/controllers/admin/site_texts_controller.rb @@ -28,6 +28,10 @@ class Admin::SiteTextsController < Admin::AdminController results << record_for(k, v) end + unless translations.empty? + extras[:regex] = I18n::Backend::DiscourseI18n.create_search_regexp(query, as_string: true) + end + results.sort! do |x, y| if x[:value].casecmp(query) == 0 -1 diff --git a/lib/freedom_patches/translate_accelerator.rb b/lib/freedom_patches/translate_accelerator.rb index eaa07a678d3..0d5090b80b4 100644 --- a/lib/freedom_patches/translate_accelerator.rb +++ b/lib/freedom_patches/translate_accelerator.rb @@ -70,7 +70,7 @@ module I18n target = opts[:backend] || backend results = opts[:overridden] ? {} : target.search(config.locale, query) - regexp = /#{Regexp.escape(query)}/i + regexp = I18n::Backend::DiscourseI18n.create_search_regexp(query) (overrides_by_locale(locale) || {}).each do |k, v| results.delete(k) results[k] = v if (k =~ regexp || v =~ regexp) diff --git a/lib/i18n/backend/discourse_i18n.rb b/lib/i18n/backend/discourse_i18n.rb index 74c082c390c..46ca58d26ee 100644 --- a/lib/i18n/backend/discourse_i18n.rb +++ b/lib/i18n/backend/discourse_i18n.rb @@ -39,11 +39,22 @@ module I18n false end + def self.create_search_regexp(query, as_string: false) + regexp = Regexp.escape(query) + + regexp.gsub!(/['‘’‚‹›]/, "['‘’‚‹›]") + regexp.gsub!(/["“”„«»]/, '["“”„«»]') + regexp.gsub!(/(?:\\\.\\\.\\\.|…)/, '(?:\.\.\.|…)') + + as_string ? regexp : /#{regexp}/i + end + def search(locale, query) results = {} + regexp = self.class.create_search_regexp(query) fallbacks(locale).each do |fallback| - find_results(/#{Regexp.escape(query)}/i, results, translations[fallback]) + find_results(regexp, results, translations[fallback]) end results diff --git a/spec/requests/admin/site_texts_controller_spec.rb b/spec/requests/admin/site_texts_controller_spec.rb index 0ee4383e23e..e7703d334ca 100644 --- a/spec/requests/admin/site_texts_controller_spec.rb +++ b/spec/requests/admin/site_texts_controller_spec.rb @@ -33,17 +33,51 @@ RSpec.describe Admin::SiteTextsController do end end - context "when logged in as amin" do + context "when logged in as admin" do before do sign_in(admin) end describe '#index' do it 'returns json' do - get "/admin/customize/site_texts.json", params: { q: 'title' } + get "/admin/customize/site_texts.json", params: { q: 'title' } expect(response.status).to eq(200) - expect(::JSON.parse(response.body)).to be_present + expect(JSON.parse(response.body)['site_texts']).to include(include("id" => "title")) end + + it 'normalizes quotes during search' do + value = %q|“That’s a ‘magic’ sock.”| + put "/admin/customize/site_texts/title.json", params: { site_text: { value: value } } + + [ + %q|That's a 'magic' sock.|, + %q|That’s a ‘magic’ sock.|, + %q|“That's a 'magic' sock.”|, + %q|"That's a 'magic' sock."|, + %q|«That's a 'magic' sock.»|, + %q|„That’s a ‚magic‘ sock.“| + ].each do |search_term| + get "/admin/customize/site_texts.json", params: { q: search_term } + expect(response.status).to eq(200) + expect(JSON.parse(response.body)['site_texts']).to include(include("id" => "title", "value" => value)) + end + end + + it 'normalizes ellipsis' do + value = "Loading Discussion…" + put "/admin/customize/site_texts/embed.loading.json", params: { site_text: { value: value } } + + [ + "Loading Discussion", + "Loading Discussion...", + "Loading Discussion…" + ].each do |search_term| + get "/admin/customize/site_texts.json", params: { q: search_term } + expect(response.status).to eq(200) + expect(JSON.parse(response.body)['site_texts']).to include(include("id" => "embed.loading", "value" => value)) + end + end + end describe '#show' do