From 68f0092c8f3f6f29d1cbdd4511aef84875519fbe Mon Sep 17 00:00:00 2001 From: "Vitaliy @blackst0ne Klachkov" Date: Tue, 28 Nov 2017 14:51:03 +1100 Subject: [PATCH] Limit autocomplete menu to applied labels --- app/assets/javascripts/gfm_auto_complete.js | 75 ++++++++++--- .../autocomplete_sources_controller.rb | 16 +-- app/services/projects/autocomplete_service.rb | 21 +++- .../layouts/_init_auto_complete.html.haml | 2 +- ...d-limit-autocomplete-to-applied-labels.yml | 5 + spec/features/issues/gfm_autocomplete_spec.rb | 106 ++++++++++++++++++ 6 files changed, 199 insertions(+), 26 deletions(-) create mode 100644 changelogs/unreleased/22680-unlabel-slash-command-limit-autocomplete-to-applied-labels.yml diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js index a642464c920..d918d80df8d 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js @@ -287,6 +287,10 @@ class GfmAutoComplete { } setupLabels($input) { + const fetchData = this.fetchData.bind(this); + const LABEL_COMMAND = { LABEL: '/label', UNLABEL: '/unlabel', RELABEL: '/relabel' }; + let command = ''; + $input.atwho({ at: '~', alias: 'labels', @@ -309,8 +313,45 @@ class GfmAutoComplete { title: sanitize(m.title), color: m.color, search: m.title, + set: m.set, })); }, + matcher(flag, subtext) { + const match = GfmAutoComplete.defaultMatcher(flag, subtext, this.app.controllers); + const subtextNodes = subtext.split(/\n+/g).pop().split(GfmAutoComplete.regexSubtext); + + // Check if ~ is followed by '/label', '/relabel' or '/unlabel' commands. + command = subtextNodes.find((node) => { + if (node === LABEL_COMMAND.LABEL || + node === LABEL_COMMAND.RELABEL || + node === LABEL_COMMAND.UNLABEL) { return node; } + return null; + }); + + return match && match.length ? match[1] : null; + }, + filter(query, data, searchKey) { + if (GfmAutoComplete.isLoading(data)) { + fetchData(this.$inputor, this.at); + return data; + } + + if (data === GfmAutoComplete.defaultLoadingData) { + return $.fn.atwho.default.callbacks.filter(query, data, searchKey); + } + + // The `LABEL_COMMAND.RELABEL` is intentionally skipped + // because we want to return all the labels (unfiltered) for that command. + if (command === LABEL_COMMAND.LABEL) { + // Return labels with set: undefined. + return data.filter(label => !label.set); + } else if (command === LABEL_COMMAND.UNLABEL) { + // Return labels with set: true. + return data.filter(label => label.set); + } + + return data; + }, }, }); } @@ -346,20 +387,7 @@ class GfmAutoComplete { return resultantValue; }, matcher(flag, subtext) { - // The below is taken from At.js source - // Tweaked to commands to start without a space only if char before is a non-word character - // https://github.com/ichord/At.js - const atSymbolsWithBar = Object.keys(this.app.controllers).join('|'); - const atSymbolsWithoutBar = Object.keys(this.app.controllers).join(''); - const targetSubtext = subtext.split(/\s+/g).pop(); - const resultantFlag = flag.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&'); - - const accentAChar = decodeURI('%C3%80'); - const accentYChar = decodeURI('%C3%BF'); - - const regexp = new RegExp(`^(?:\\B|[^a-zA-Z0-9_${atSymbolsWithoutBar}]|\\s)${resultantFlag}(?!${atSymbolsWithBar})((?:[A-Za-z${accentAChar}-${accentYChar}0-9_'.+-]|[^\\x00-\\x7a])*)$`, 'gi'); - - const match = regexp.exec(targetSubtext); + const match = GfmAutoComplete.defaultMatcher(flag, subtext, this.app.controllers); if (match) { return match[1]; @@ -420,8 +448,27 @@ class GfmAutoComplete { return dataToInspect && (dataToInspect === loadingState || dataToInspect.name === loadingState); } + + static defaultMatcher(flag, subtext, controllers) { + // The below is taken from At.js source + // Tweaked to commands to start without a space only if char before is a non-word character + // https://github.com/ichord/At.js + const atSymbolsWithBar = Object.keys(controllers).join('|'); + const atSymbolsWithoutBar = Object.keys(controllers).join(''); + const targetSubtext = subtext.split(GfmAutoComplete.regexSubtext).pop(); + const resultantFlag = flag.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&'); + + const accentAChar = decodeURI('%C3%80'); + const accentYChar = decodeURI('%C3%BF'); + + const regexp = new RegExp(`^(?:\\B|[^a-zA-Z0-9_${atSymbolsWithoutBar}]|\\s)${resultantFlag}(?!${atSymbolsWithBar})((?:[A-Za-z${accentAChar}-${accentYChar}0-9_'.+-]|[^\\x00-\\x7a])*)$`, 'gi'); + + return regexp.exec(targetSubtext); + } } +GfmAutoComplete.regexSubtext = new RegExp(/\s+/g); + GfmAutoComplete.defaultLoadingData = ['loading']; GfmAutoComplete.atTypeMap = { diff --git a/app/controllers/projects/autocomplete_sources_controller.rb b/app/controllers/projects/autocomplete_sources_controller.rb index ffb54390965..45c66b63ea5 100644 --- a/app/controllers/projects/autocomplete_sources_controller.rb +++ b/app/controllers/projects/autocomplete_sources_controller.rb @@ -2,7 +2,7 @@ class Projects::AutocompleteSourcesController < Projects::ApplicationController before_action :load_autocomplete_service, except: [:members] def members - render json: ::Projects::ParticipantsService.new(@project, current_user).execute(noteable) + render json: ::Projects::ParticipantsService.new(@project, current_user).execute(target) end def issues @@ -14,7 +14,7 @@ class Projects::AutocompleteSourcesController < Projects::ApplicationController end def labels - render json: @autocomplete_service.labels + render json: @autocomplete_service.labels(target) end def milestones @@ -22,7 +22,7 @@ class Projects::AutocompleteSourcesController < Projects::ApplicationController end def commands - render json: @autocomplete_service.commands(noteable, params[:type]) + render json: @autocomplete_service.commands(target, params[:type]) end private @@ -31,13 +31,13 @@ class Projects::AutocompleteSourcesController < Projects::ApplicationController @autocomplete_service = ::Projects::AutocompleteService.new(@project, current_user) end - def noteable - case params[:type] - when 'Issue' + def target + case params[:type]&.downcase + when 'issue' IssuesFinder.new(current_user, project_id: @project.id).execute.find_by(iid: params[:type_id]) - when 'MergeRequest' + when 'mergerequest' MergeRequestsFinder.new(current_user, project_id: @project.id).execute.find_by(iid: params[:type_id]) - when 'Commit' + when 'commit' @project.commit(params[:type_id]) end end diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index 724a77c873a..1ae2c40872a 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -20,8 +20,23 @@ module Projects MergeRequestsFinder.new(current_user, project_id: project.id, state: 'opened').execute.select([:iid, :title]) end - def labels - LabelsFinder.new(current_user, project_id: project.id).execute.select([:title, :color]) + def labels(target = nil) + labels = LabelsFinder.new(current_user, project_id: project.id).execute.select([:color, :title]) + + return labels unless target&.respond_to?(:labels) + + issuable_label_titles = target.labels.pluck(:title) + + if issuable_label_titles + labels = labels.as_json(only: [:title, :color]) + + issuable_label_titles.each do |issuable_label_title| + found_label = labels.find { |label| label['title'] == issuable_label_title } + found_label[:set] = true if found_label + end + end + + labels end def commands(noteable, type) @@ -33,7 +48,7 @@ module Projects @project.merge_requests.build end - return [] unless noteable && noteable.is_a?(Issuable) + return [] unless noteable&.is_a?(Issuable) opts = { project: project, diff --git a/app/views/layouts/_init_auto_complete.html.haml b/app/views/layouts/_init_auto_complete.html.haml index fe0ec35d003..4276e6ee4bb 100644 --- a/app/views/layouts/_init_auto_complete.html.haml +++ b/app/views/layouts/_init_auto_complete.html.haml @@ -10,7 +10,7 @@ members: "#{members_project_autocomplete_sources_path(project, type: noteable_type, type_id: params[:id])}", issues: "#{issues_project_autocomplete_sources_path(project)}", mergeRequests: "#{merge_requests_project_autocomplete_sources_path(project)}", - labels: "#{labels_project_autocomplete_sources_path(project)}", + labels: "#{labels_project_autocomplete_sources_path(project, type: noteable_type, type_id: params[:id])}", milestones: "#{milestones_project_autocomplete_sources_path(project)}", commands: "#{commands_project_autocomplete_sources_path(project, type: noteable_type, type_id: params[:id])}" }; diff --git a/changelogs/unreleased/22680-unlabel-slash-command-limit-autocomplete-to-applied-labels.yml b/changelogs/unreleased/22680-unlabel-slash-command-limit-autocomplete-to-applied-labels.yml new file mode 100644 index 00000000000..6d7f8655282 --- /dev/null +++ b/changelogs/unreleased/22680-unlabel-slash-command-limit-autocomplete-to-applied-labels.yml @@ -0,0 +1,5 @@ +--- +title: Limit autocomplete menu to applied labels +merge_request: 11110 +author: Vitaliy @blackst0ne Klachkov +type: added diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb index 95d637265e0..c31b636d67f 100644 --- a/spec/features/issues/gfm_autocomplete_spec.rb +++ b/spec/features/issues/gfm_autocomplete_spec.rb @@ -220,6 +220,89 @@ feature 'GFM autocomplete', :js do end end + # This context has jsut one example in each contexts in order to improve spec performance. + context 'labels' do + let!(:backend) { create(:label, project: project, title: 'backend') } + let!(:bug) { create(:label, project: project, title: 'bug') } + let!(:feature_proposal) { create(:label, project: project, title: 'feature proposal') } + + context 'when no labels are assigned' do + it 'shows labels' do + note = find('#note-body') + + # It should show all the labels on "~". + type(note, '~') + expect_labels(shown: [backend, bug, feature_proposal]) + + # It should show all the labels on "/label ~". + type(note, '/label ~') + expect_labels(shown: [backend, bug, feature_proposal]) + + # It should show all the labels on "/relabel ~". + type(note, '/relabel ~') + expect_labels(shown: [backend, bug, feature_proposal]) + + # It should show no labels on "/unlabel ~". + type(note, '/unlabel ~') + expect_labels(not_shown: [backend, bug, feature_proposal]) + end + end + + context 'when some labels are assigned' do + before do + issue.labels << [backend] + end + + it 'shows labels' do + note = find('#note-body') + + # It should show all the labels on "~". + type(note, '~') + expect_labels(shown: [backend, bug, feature_proposal]) + + # It should show only unset labels on "/label ~". + type(note, '/label ~') + expect_labels(shown: [bug, feature_proposal], not_shown: [backend]) + + # It should show all the labels on "/relabel ~". + type(note, '/relabel ~') + expect_labels(shown: [backend, bug, feature_proposal]) + + # It should show only set labels on "/unlabel ~". + type(note, '/unlabel ~') + expect_labels(shown: [backend], not_shown: [bug, feature_proposal]) + end + end + + context 'when all labels are assigned' do + before do + issue.labels << [backend, bug, feature_proposal] + end + + it 'shows labels' do + note = find('#note-body') + + # It should show all the labels on "~". + type(note, '~') + expect_labels(shown: [backend, bug, feature_proposal]) + + # It should show no labels on "/label ~". + type(note, '/label ~') + expect_labels(not_shown: [backend, bug, feature_proposal]) + + # It should show all the labels on "/relabel ~". + type(note, '/relabel ~') + expect_labels(shown: [backend, bug, feature_proposal]) + + # It should show all the labels on "/unlabel ~". + type(note, '/unlabel ~') + expect_labels(shown: [backend, bug, feature_proposal]) + end + end + end + + private + def expect_to_wrap(should_wrap, item, note, value) expect(item).to have_content(value) expect(item).not_to have_content("\"#{value}\"") @@ -232,4 +315,27 @@ feature 'GFM autocomplete', :js do expect(note.value).not_to include("\"#{value}\"") end end + + def expect_labels(shown: nil, not_shown: nil) + page.within('.atwho-container') do + if shown + expect(page).to have_selector('.atwho-view li', count: shown.size) + shown.each { |label| expect(page).to have_content(label.title) } + end + + if not_shown + expect(page).not_to have_selector('.atwho-view li') unless shown + not_shown.each { |label| expect(page).not_to have_content(label.title) } + end + end + end + + # `note` is a textarea where the given text should be typed. + # We don't want to find it each time this function gets called. + def type(note, text) + page.within('.timeline-content-form') do + note.set('') + note.native.send_keys(text) + end + end end -- GitLab