Commit 50d2eae864d807695b16d7b369a1951bbcf3f63c
Exists in
spb-stable
and in
3 other branches
Merge branch 'refactor/search_context' of /home/git/repositories/gitlab/gitlabhq
Showing
11 changed files
with
137 additions
and
128 deletions
Show diff stats
app/assets/stylesheets/generic/nav.scss
| @@ -7,12 +7,9 @@ | @@ -7,12 +7,9 @@ | ||
| 7 | background: $primary_color; | 7 | background: $primary_color; |
| 8 | } | 8 | } |
| 9 | 9 | ||
| 10 | - > li > a { | ||
| 11 | - @include border-radius(0); | ||
| 12 | - } | ||
| 13 | - | ||
| 14 | &.nav-stacked { | 10 | &.nav-stacked { |
| 15 | > li > a { | 11 | > li > a { |
| 12 | + @include border-radius(0); | ||
| 16 | border-left: 4px solid #EEE; | 13 | border-left: 4px solid #EEE; |
| 17 | padding: 12px; | 14 | padding: 12px; |
| 18 | color: #777; | 15 | color: #777; |
| @@ -0,0 +1,40 @@ | @@ -0,0 +1,40 @@ | ||
| 1 | +module Search | ||
| 2 | + class GlobalContext | ||
| 3 | + attr_accessor :current_user, :params | ||
| 4 | + | ||
| 5 | + def initialize(user, params) | ||
| 6 | + @current_user, @params = user, params.dup | ||
| 7 | + end | ||
| 8 | + | ||
| 9 | + def execute | ||
| 10 | + query = params[:search] | ||
| 11 | + query = Shellwords.shellescape(query) if query.present? | ||
| 12 | + return result unless query.present? | ||
| 13 | + | ||
| 14 | + authorized_projects_ids = [] | ||
| 15 | + authorized_projects_ids += current_user.authorized_projects.pluck(:id) if current_user | ||
| 16 | + authorized_projects_ids += Project.public_or_internal_only(current_user).pluck(:id) | ||
| 17 | + | ||
| 18 | + group = Group.find_by_id(params[:group_id]) if params[:group_id].present? | ||
| 19 | + projects = Project.where(id: authorized_projects_ids) | ||
| 20 | + projects = projects.where(namespace_id: group.id) if group | ||
| 21 | + projects = projects.search(query) | ||
| 22 | + project_ids = projects.pluck(:id) | ||
| 23 | + | ||
| 24 | + result[:projects] = projects.limit(20) | ||
| 25 | + result[:merge_requests] = MergeRequest.in_projects(project_ids).search(query).order('updated_at DESC').limit(20) | ||
| 26 | + result[:issues] = Issue.where(project_id: project_ids).search(query).order('updated_at DESC').limit(20) | ||
| 27 | + result[:total_results] = %w(projects issues merge_requests).sum { |items| result[items.to_sym].size } | ||
| 28 | + result | ||
| 29 | + end | ||
| 30 | + | ||
| 31 | + def result | ||
| 32 | + @result ||= { | ||
| 33 | + projects: [], | ||
| 34 | + merge_requests: [], | ||
| 35 | + issues: [], | ||
| 36 | + total_results: 0, | ||
| 37 | + } | ||
| 38 | + end | ||
| 39 | + end | ||
| 40 | +end |
| @@ -0,0 +1,37 @@ | @@ -0,0 +1,37 @@ | ||
| 1 | +module Search | ||
| 2 | + class ProjectContext | ||
| 3 | + attr_accessor :project, :current_user, :params | ||
| 4 | + | ||
| 5 | + def initialize(project, user, params) | ||
| 6 | + @project, @current_user, @params = project, user, params.dup | ||
| 7 | + end | ||
| 8 | + | ||
| 9 | + def execute | ||
| 10 | + query = params[:search] | ||
| 11 | + query = Shellwords.shellescape(query) if query.present? | ||
| 12 | + return result unless query.present? | ||
| 13 | + | ||
| 14 | + if params[:search_code].present? | ||
| 15 | + blobs = project.repository.search_files(query, params[:repository_ref]) unless project.empty_repo? | ||
| 16 | + blobs = Kaminari.paginate_array(blobs).page(params[:page]).per(20) | ||
| 17 | + result[:blobs] = blobs | ||
| 18 | + result[:total_results] = blobs.total_count | ||
| 19 | + else | ||
| 20 | + result[:merge_requests] = project.merge_requests.search(query).order('updated_at DESC').limit(20) | ||
| 21 | + result[:issues] = project.issues.search(query).order('updated_at DESC').limit(20) | ||
| 22 | + result[:total_results] = %w(issues merge_requests).sum { |items| result[items.to_sym].size } | ||
| 23 | + end | ||
| 24 | + | ||
| 25 | + result | ||
| 26 | + end | ||
| 27 | + | ||
| 28 | + def result | ||
| 29 | + @result ||= { | ||
| 30 | + merge_requests: [], | ||
| 31 | + issues: [], | ||
| 32 | + blobs: [], | ||
| 33 | + total_results: 0, | ||
| 34 | + } | ||
| 35 | + end | ||
| 36 | + end | ||
| 37 | +end |
app/contexts/search_context.rb
| @@ -1,42 +0,0 @@ | @@ -1,42 +0,0 @@ | ||
| 1 | -class SearchContext | ||
| 2 | - attr_accessor :project_ids, :current_user, :params | ||
| 3 | - | ||
| 4 | - def initialize(project_ids, user, params) | ||
| 5 | - @project_ids, @current_user, @params = project_ids, user, params.dup | ||
| 6 | - end | ||
| 7 | - | ||
| 8 | - def execute | ||
| 9 | - query = params[:search] | ||
| 10 | - query = Shellwords.shellescape(query) if query.present? | ||
| 11 | - | ||
| 12 | - return result unless query.present? | ||
| 13 | - visibility_levels = @current_user ? [ Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC ] : [ Gitlab::VisibilityLevel::PUBLIC ] | ||
| 14 | - result[:projects] = Project.where("projects.id in (?) OR projects.visibility_level in (?)", project_ids, visibility_levels).search(query).limit(20) | ||
| 15 | - | ||
| 16 | - # Search inside single project | ||
| 17 | - single_project_search(Project.where(id: project_ids), query) | ||
| 18 | - result | ||
| 19 | - end | ||
| 20 | - | ||
| 21 | - def single_project_search(projects, query) | ||
| 22 | - project = projects.first if projects.length == 1 | ||
| 23 | - | ||
| 24 | - if params[:search_code].present? | ||
| 25 | - result[:blobs] = project.repository.search_files(query, params[:repository_ref]) unless project.empty_repo? | ||
| 26 | - else | ||
| 27 | - result[:merge_requests] = MergeRequest.in_projects(project_ids).search(query).order('updated_at DESC').limit(20) | ||
| 28 | - result[:issues] = Issue.where(project_id: project_ids).search(query).order('updated_at DESC').limit(20) | ||
| 29 | - result[:wiki_pages] = [] | ||
| 30 | - end | ||
| 31 | - end | ||
| 32 | - | ||
| 33 | - def result | ||
| 34 | - @result ||= { | ||
| 35 | - projects: [], | ||
| 36 | - merge_requests: [], | ||
| 37 | - issues: [], | ||
| 38 | - wiki_pages: [], | ||
| 39 | - blobs: [] | ||
| 40 | - } | ||
| 41 | - end | ||
| 42 | -end |
app/controllers/search_controller.rb
| 1 | class SearchController < ApplicationController | 1 | class SearchController < ApplicationController |
| 2 | def show | 2 | def show |
| 3 | - project_id = params[:project_id] | ||
| 4 | - group_id = params[:group_id] | ||
| 5 | - | ||
| 6 | - project_ids = find_project_ids(group_id, project_id) | ||
| 7 | - | ||
| 8 | - result = SearchContext.new(project_ids, current_user, params).execute | ||
| 9 | - | ||
| 10 | - @projects = result[:projects] | ||
| 11 | - @merge_requests = result[:merge_requests] | ||
| 12 | - @issues = result[:issues] | ||
| 13 | - @wiki_pages = result[:wiki_pages] | ||
| 14 | - @blobs = Kaminari.paginate_array(result[:blobs]).page(params[:page]).per(20) | ||
| 15 | - @total_results = @projects.count + @merge_requests.count + @issues.count + @wiki_pages.count + @blobs.total_count | ||
| 16 | - end | ||
| 17 | - | ||
| 18 | - private | ||
| 19 | - | ||
| 20 | - def find_project_ids(group_id, project_id) | ||
| 21 | - project_ids = current_user.authorized_projects.map(&:id) | ||
| 22 | - | ||
| 23 | - if group_id.present? | ||
| 24 | - @group = Group.find(group_id) | ||
| 25 | - group_project_ids = @group.projects.map(&:id) | ||
| 26 | - project_ids.select! { |id| group_project_ids.include?(id) } | ||
| 27 | - elsif project_id.present? | ||
| 28 | - @project = Project.find(project_id) | ||
| 29 | - project_ids = @project.public? ? [@project.id] : project_ids.select { |id| id == project_id.to_i } | 3 | + @project = Project.find_by_id(params[:project_id]) if params[:project_id].present? |
| 4 | + @group = Group.find_by_id(params[:group_id]) if params[:group_id].present? | ||
| 5 | + | ||
| 6 | + if @project | ||
| 7 | + return access_denied! unless can?(current_user, :download_code, @project) | ||
| 8 | + @search_results = Search::ProjectContext.new(@project, current_user, params).execute | ||
| 9 | + else | ||
| 10 | + @search_results = Search::GlobalContext.new(current_user, params).execute | ||
| 30 | end | 11 | end |
| 31 | - project_ids | ||
| 32 | end | 12 | end |
| 33 | end | 13 | end |
app/views/search/_filter.html.haml
| @@ -9,7 +9,7 @@ | @@ -9,7 +9,7 @@ | ||
| 9 | %b.caret | 9 | %b.caret |
| 10 | %ul.dropdown-menu | 10 | %ul.dropdown-menu |
| 11 | %li | 11 | %li |
| 12 | - = link_to search_path(group_id: nil) do | 12 | + = link_to search_path(group_id: nil, search: params[:search]) do |
| 13 | Any | 13 | Any |
| 14 | - current_user.authorized_groups.sort_by(&:name).each do |group| | 14 | - current_user.authorized_groups.sort_by(&:name).each do |group| |
| 15 | %li | 15 | %li |
| @@ -27,7 +27,7 @@ | @@ -27,7 +27,7 @@ | ||
| 27 | %b.caret | 27 | %b.caret |
| 28 | %ul.dropdown-menu | 28 | %ul.dropdown-menu |
| 29 | %li | 29 | %li |
| 30 | - = link_to search_path(project_id: nil) do | 30 | + = link_to search_path(project_id: nil, search: params[:search]) do |
| 31 | Any | 31 | Any |
| 32 | - current_user.authorized_projects.sort_by(&:name_with_namespace).each do |project| | 32 | - current_user.authorized_projects.sort_by(&:name_with_namespace).each do |project| |
| 33 | %li | 33 | %li |
app/views/search/_global_results.html.haml
| 1 | .search_results | 1 | .search_results |
| 2 | %ul.bordered-list | 2 | %ul.bordered-list |
| 3 | - = render partial: "search/results/project", collection: @projects | ||
| 4 | - = render partial: "search/results/merge_request", collection: @merge_requests | ||
| 5 | - = render partial: "search/results/issue", collection: @issues | 3 | + = render partial: "search/results/project", collection: @search_results[:projects] |
| 4 | + = render partial: "search/results/merge_request", collection: @search_results[:merge_requests] | ||
| 5 | + = render partial: "search/results/issue", collection: @search_results[:issues] |
app/views/search/_project_results.html.haml
| 1 | -%ul.nav.nav-pills | 1 | +%ul.nav.nav-tabs.append-bottom-10 |
| 2 | %li{class: ("active" if params[:search_code].present?)} | 2 | %li{class: ("active" if params[:search_code].present?)} |
| 3 | = link_to search_path(params.merge(search_code: true)) do | 3 | = link_to search_path(params.merge(search_code: true)) do |
| 4 | Repository Code | 4 | Repository Code |
| 5 | %li{class: ("active" if params[:search_code].blank?)} | 5 | %li{class: ("active" if params[:search_code].blank?)} |
| 6 | = link_to search_path(params.merge(search_code: nil)) do | 6 | = link_to search_path(params.merge(search_code: nil)) do |
| 7 | - Everything else | 7 | + Issues and Merge requests |
| 8 | 8 | ||
| 9 | .search_results | 9 | .search_results |
| 10 | - if params[:search_code].present? | 10 | - if params[:search_code].present? |
| 11 | .blob-results | 11 | .blob-results |
| 12 | - = render partial: "search/results/blob", collection: @blobs | ||
| 13 | - = paginate @blobs, theme: 'gitlab' | 12 | + = render partial: "search/results/blob", collection: @search_results[:blobs] |
| 13 | + = paginate @search_results[:blobs], theme: 'gitlab' | ||
| 14 | - else | 14 | - else |
| 15 | %ul.bordered-list | 15 | %ul.bordered-list |
| 16 | - = render partial: "search/results/merge_request", collection: @merge_requests | ||
| 17 | - = render partial: "search/results/issue", collection: @issues | 16 | + = render partial: "search/results/merge_request", collection: @search_results[:merge_requests] |
| 17 | + = render partial: "search/results/issue", collection: @search_results[:issues] |
app/views/search/_results.html.haml
| 1 | -%fieldset | ||
| 2 | - %legend | ||
| 3 | - Search results | ||
| 4 | - %span.cgray (#{@total_results}) | 1 | +%h4 |
| 2 | + #{@search_results[:total_results]} results found | ||
| 3 | + - if @project | ||
| 4 | + for #{link_to @project.name_with_namespace, @project} | ||
| 5 | + - elsif @group | ||
| 6 | + for #{link_to @group.name, @group} | ||
| 7 | + | ||
| 8 | +%hr | ||
| 5 | 9 | ||
| 6 | - if @project | 10 | - if @project |
| 7 | = render "project_results" | 11 | = render "project_results" |
spec/contexts/search_context_spec.rb
| 1 | require 'spec_helper' | 1 | require 'spec_helper' |
| 2 | 2 | ||
| 3 | -describe SearchContext do | 3 | +describe 'Search::GlobalContext' do |
| 4 | let(:found_namespace) { create(:namespace, name: 'searchable namespace', path:'another_thing') } | 4 | let(:found_namespace) { create(:namespace, name: 'searchable namespace', path:'another_thing') } |
| 5 | let(:user) { create(:user, namespace: found_namespace) } | 5 | let(:user) { create(:user, namespace: found_namespace) } |
| 6 | let!(:found_project) { create(:project, name: 'searchable_project', creator_id: user.id, namespace: found_namespace, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } | 6 | let!(:found_project) { create(:project, name: 'searchable_project', creator_id: user.id, namespace: found_namespace, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } |
| 7 | 7 | ||
| 8 | let(:unfound_namespace) { create(:namespace, name: 'unfound namespace', path: 'yet_something_else') } | 8 | let(:unfound_namespace) { create(:namespace, name: 'unfound namespace', path: 'yet_something_else') } |
| 9 | let!(:unfound_project) { create(:project, name: 'unfound_project', creator_id: user.id, namespace: unfound_namespace, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } | 9 | let!(:unfound_project) { create(:project, name: 'unfound_project', creator_id: user.id, namespace: unfound_namespace, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } |
| 10 | - | 10 | + |
| 11 | let(:internal_namespace) { create(:namespace, path: 'something_internal',name: 'searchable internal namespace') } | 11 | let(:internal_namespace) { create(:namespace, path: 'something_internal',name: 'searchable internal namespace') } |
| 12 | let(:internal_user) { create(:user, namespace: internal_namespace) } | 12 | let(:internal_user) { create(:user, namespace: internal_namespace) } |
| 13 | let!(:internal_project) { create(:project, name: 'searchable_internal_project', creator_id: internal_user.id, namespace: internal_namespace, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } | 13 | let!(:internal_project) { create(:project, name: 'searchable_internal_project', creator_id: internal_user.id, namespace: internal_namespace, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } |
| 14 | - | 14 | + |
| 15 | let(:public_namespace) { create(:namespace, path: 'something_public',name: 'searchable public namespace') } | 15 | let(:public_namespace) { create(:namespace, path: 'something_public',name: 'searchable public namespace') } |
| 16 | let(:public_user) { create(:user, namespace: public_namespace) } | 16 | let(:public_user) { create(:user, namespace: public_namespace) } |
| 17 | let!(:public_project) { create(:project, name: 'searchable_public_project', creator_id: public_user.id, namespace: public_namespace, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } | 17 | let!(:public_project) { create(:project, name: 'searchable_public_project', creator_id: public_user.id, namespace: public_namespace, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } |
| 18 | 18 | ||
| 19 | describe '#execute' do | 19 | describe '#execute' do |
| 20 | - it 'public projects should be searchable' do | ||
| 21 | - context = SearchContext.new([found_project.id], nil, {search_code: false, search: "searchable"}) | ||
| 22 | - results = context.execute | ||
| 23 | - results[:projects].should == [found_project, public_project] | 20 | + context 'unauthenticated' do |
| 21 | + it 'should return public projects only' do | ||
| 22 | + context = Search::GlobalContext.new(nil, search: "searchable") | ||
| 23 | + results = context.execute | ||
| 24 | + results[:projects].should have(1).items | ||
| 25 | + results[:projects].should include(public_project) | ||
| 26 | + end | ||
| 24 | end | 27 | end |
| 25 | 28 | ||
| 26 | - it 'internal projects should be searchable' do | ||
| 27 | - context = SearchContext.new([found_project.id], user, {search_code: false, search: "searchable"}) | ||
| 28 | - results = context.execute | ||
| 29 | - # can't seem to rely on the return order, so check this way | ||
| 30 | - #subject { results[:projects] } | ||
| 31 | - results[:projects].should have(3).items | ||
| 32 | - results[:projects].should include(found_project) | ||
| 33 | - results[:projects].should include(internal_project) | ||
| 34 | - results[:projects].should include(public_project) | ||
| 35 | - end | 29 | + context 'authenticated' do |
| 30 | + it 'should return public, internal and private projects' do | ||
| 31 | + context = Search::GlobalContext.new(user, search: "searchable") | ||
| 32 | + results = context.execute | ||
| 33 | + results[:projects].should have(3).items | ||
| 34 | + results[:projects].should include(public_project) | ||
| 35 | + results[:projects].should include(found_project) | ||
| 36 | + results[:projects].should include(internal_project) | ||
| 37 | + end | ||
| 38 | + | ||
| 39 | + it 'should return only public & internal projects' do | ||
| 40 | + context = Search::GlobalContext.new(internal_user, search: "searchable") | ||
| 41 | + results = context.execute | ||
| 42 | + results[:projects].should have(2).items | ||
| 43 | + results[:projects].should include(internal_project) | ||
| 44 | + results[:projects].should include(public_project) | ||
| 45 | + end | ||
| 36 | 46 | ||
| 37 | - it 'namespace name should be searchable' do | ||
| 38 | - context = SearchContext.new([found_project.id], user, {search_code: false, search: "searchable namespace"}) | ||
| 39 | - results = context.execute | ||
| 40 | - results[:projects].should == [found_project] | 47 | + it 'namespace name should be searchable' do |
| 48 | + context = Search::GlobalContext.new(user, search: "searchable namespace") | ||
| 49 | + results = context.execute | ||
| 50 | + results[:projects].should == [found_project] | ||
| 51 | + end | ||
| 41 | end | 52 | end |
| 42 | end | 53 | end |
| 43 | end | 54 | end |
spec/controllers/search_controller_spec.rb
| @@ -1,18 +0,0 @@ | @@ -1,18 +0,0 @@ | ||
| 1 | -require 'spec_helper' | ||
| 2 | - | ||
| 3 | -describe SearchController do | ||
| 4 | - let(:project) { create(:project, public: true) } | ||
| 5 | - let(:user) { create(:user) } | ||
| 6 | - | ||
| 7 | - before do | ||
| 8 | - sign_in(user) | ||
| 9 | - end | ||
| 10 | - | ||
| 11 | - describe '#find_project_ids' do | ||
| 12 | - it 'should include public projects ids when searching within a single project' do | ||
| 13 | - project_ids = controller.send(:find_project_ids,nil, project.id) | ||
| 14 | - project_ids.size.should == 1 | ||
| 15 | - project_ids[0].should == project.id | ||
| 16 | - end | ||
| 17 | - end | ||
| 18 | -end |