Merge branch 't1254-snippets-refactoring' into 'master'

Snippets need to be either public/internal/private

See merge request !1162

app/controllers/projects/snippets_controller.rb

@@ -17,7 +17,10 @@ class Projects::SnippetsController < Projects::ApplicationController
   respond_to :html
 
   def index
-    @snippets = @project.snippets.fresh.non_expired
+    @snippets = SnippetsFinder.new.execute(current_user, {
+      filter: :by_project,
+      project: @project
+    })
   end
 
   def new
@@ -88,6 +91,6 @@ class Projects::SnippetsController < Projects::ApplicationController
   end
 
   def snippet_params
-    params.require(:project_snippet).permit(:title, :content, :file_name, :private)
+    params.require(:project_snippet).permit(:title, :content, :file_name, :private, :visibility_level)
   end
 end

app/controllers/snippets_controller.rb

@@ -9,12 +9,14 @@ class SnippetsController < ApplicationController
 
   before_filter :set_title
 
+  skip_before_filter :authenticate_user!, only: [:index, :user_index]
+
   respond_to :html
 
-  layout 'navless'
+  layout :determine_layout
 
   def index
-    @snippets = Snippet.are_internal.fresh.non_expired.page(params[:page]).per(20)
+    @snippets = SnippetsFinder.new.execute(current_user, filter: :all).page(params[:page]).per(20)
   end
 
   def user_index
@@ -22,22 +24,11 @@ class SnippetsController < ApplicationController
 
     render_404 and return unless @user
 
-    @snippets = @user.snippets.fresh.non_expired
-
-    if @user == current_user
-      @snippets = case params[:scope]
-                  when 'are_internal' then
-                    @snippets.are_internal
-                  when 'are_private' then
-                    @snippets.are_private
-                  else
-                    @snippets
-                  end
-    else
-      @snippets = @snippets.are_internal
-    end
-
-    @snippets = @snippets.page(params[:page]).per(20)
+    @snippets = SnippetsFinder.new.execute(current_user, {
+      filter: :by_user,
+      user: @user,
+      scope: params[:scope]}).
+    page(params[:page]).per(20)
 
     if @user == current_user
       render 'current_user_index'
@@ -95,7 +86,14 @@ class SnippetsController < ApplicationController
   protected
 
   def snippet
-    @snippet ||= PersonalSnippet.where('author_id = :user_id or private is false', user_id: current_user.id).find(params[:id])
+    @snippet ||= if current_user
+                   PersonalSnippet.where("author_id = ? OR visibility_level IN (?)",
+                     current_user.id,
+                     [Snippet::PUBLIC, Snippet::INTERNAL]).
+                     find(params[:id])
+                 else
+                   PersonalSnippet.are_public.find(params[:id])
+                 end
   end
 
   def authorize_modify_snippet!
@@ -111,6 +109,10 @@ class SnippetsController < ApplicationController
   end
 
   def snippet_params
-    params.require(:personal_snippet).permit(:title, :content, :file_name, :private)
+    params.require(:personal_snippet).permit(:title, :content, :file_name, :private, :visibility_level)
+  end
+
+  def determine_layout
+    current_user ? 'navless' : 'public_users'
   end
 end

app/finders/snippets_finder.rb

+class SnippetsFinder
+  def execute(current_user, params = {})
+    filter = params[:filter]
+
+    case filter
+    when :all then
+      snippets(current_user).fresh.non_expired
+    when :by_user then
+      by_user(current_user, params[:user], params[:scope])
+    when :by_project
+      by_project(current_user, params[:project])
+    end
+  end
+
+  private
+
+  def snippets(current_user)
+    if current_user
+      Snippet.public_and_internal
+    else
+      # Not authenticated
+      #
+      # Return only:
+      #   public snippets
+      Snippet.are_public
+    end
+  end
+
+  def by_user(current_user, user, scope)
+    snippets = user.snippets.fresh.non_expired
+
+    if user == current_user
+      snippets = case scope
+                 when 'are_internal' then
+                   snippets.are_internal
+                 when 'are_private' then
+                   snippets.are_private
+                 when 'are_public' then
+                   snippets.are_public
+                 else
+                   snippets
+                 end
+    else
+      snippets = snippets.public_and_internal
+    end
+  end
+
+  def by_project(current_user, project)
+    snippets = project.snippets.fresh.non_expired
+
+    if current_user
+      if project.team.member?(current_user.id)
+        snippets
+      else
+        snippets.public_and_internal
+      end
+    else
+      snippets.are_public
+    end
+  end
+end

app/helpers/visibility_level_helper.rb

@@ -28,6 +28,23 @@ module VisibilityLevelHelper
     end
   end
 
+  def snippet_visibility_level_description(level)
+    capture_haml do
+      haml_tag :span do
+        case level
+        when Gitlab::VisibilityLevel::PRIVATE
+          haml_concat "The snippet is visible only for me"
+        when Gitlab::VisibilityLevel::INTERNAL
+          haml_concat "The snippet is visible for any logged in user."
+        when Gitlab::VisibilityLevel::PUBLIC
+          haml_concat "The snippet can be accessed"
+          haml_concat "without any"
+          haml_concat "authentication."
+        end
+      end
+    end
+  end
+
   def visibility_level_icon(level)
     case level
     when Gitlab::VisibilityLevel::PRIVATE

app/models/project_team.rb

@@ -133,6 +133,10 @@ class ProjectTeam
     max_tm_access(user.id) == Gitlab::Access::MASTER
   end
 
+  def member?(user_id)
+    !!find_tm(user_id)
+  end
+
   def max_tm_access(user_id)
     access = []
     access << project.project_members.find_by(user_id: user_id).try(:access_field)

app/models/snippet.rb

@@ -17,8 +17,9 @@
 
 class Snippet < ActiveRecord::Base
   include Linguist::BlobHelper
+  include Gitlab::VisibilityLevel
 
-  default_value_for :private, true
+  default_value_for :visibility_level, Snippet::PRIVATE
 
   belongs_to :author, class_name: "User"
 
@@ -30,10 +31,13 @@ class Snippet < ActiveRecord::Base
   validates :title, presence: true, length: { within: 0..255 }
   validates :file_name, presence: true, length: { within: 0..255 }
   validates :content, presence: true
+  validates :visibility_level, inclusion: { in: Gitlab::VisibilityLevel.values }
 
   # Scopes
-  scope :are_internal,  -> { where(private: false) }
-  scope :are_private, -> { where(private: true) }
+  scope :are_internal,  -> { where(visibility_level: Snippet::INTERNAL) }
+  scope :are_private, -> { where(visibility_level: Snippet::PRIVATE) }
+  scope :are_public, -> { where(visibility_level: Snippet::PUBLIC) }
+  scope :public_and_internal, -> { where(visibility_level: [Snippet::PUBLIC, Snippet::INTERNAL]) }
   scope :fresh,   -> { order("created_at DESC") }
   scope :expired, -> { where(["expires_at IS NOT NULL AND expires_at < ?", Time.current]) }
   scope :non_expired, -> { where(["expires_at IS NULL OR expires_at > ?", Time.current]) }
@@ -66,6 +70,10 @@ class Snippet < ActiveRecord::Base
     expires_at && expires_at < Time.current
   end
 
+  def visibility_level_field
+    visibility_level
+  end 
+
   class << self
     def search(query)
       where('(title LIKE :query OR file_name LIKE :query)', query: "%#{query}%")
@@ -76,7 +84,7 @@ class Snippet < ActiveRecord::Base
     end
 
     def accessible_to(user)
-      where('private = ? OR author_id = ?', false, user)
+      where('visibility_level IN (?) OR author_id = ?', [Snippet::INTERNAL, Snippet::PUBLIC], user)
     end
   end
 end

app/views/shared/snippets/_form.html.haml

@@ -10,22 +10,8 @@
       = f.label :title, class: 'control-label'
       .col-sm-10= f.text_field :title, placeholder: "Example Snippet", class: 'form-control', required: true
 
-    - unless @snippet.respond_to?(:project)
-      .form-group
-        = f.label "Access", class: 'control-label'
-        .col-sm-10
-          = f.label :private_true, class: 'radio-label' do
-            = f.radio_button :private, true
-            %span
-              %strong Private
-              (only you can see this snippet)
-          %br
-          = f.label :private_false, class: 'radio-label' do
-            = f.radio_button :private, false
-            %span
-              %strong Internal
-              (GitLab users can see this snippet)
-
+    = render "shared/snippets/visibility_level", f: f, visibility_level: gitlab_config.default_projects_features.visibility_level, can_change_visibility_level: true
+    
     .form-group
       .file-editor
         = f.label :file_name, "File", class: 'control-label'

app/views/shared/snippets/_visibility_level.html.haml

+.form-group.project-visibility-level-holder
+  = f.label :visibility_level, class: 'control-label' do
+    Visibility Level
+    = link_to "(?)", help_page_path("public_access", "public_access")
+  .col-sm-10
+    - if can_change_visibility_level
+      - Gitlab::VisibilityLevel.values.each do |level|
+        .radio
+          - restricted = restricted_visibility_levels.include?(level)
+          = f.radio_button :visibility_level, level, disabled: restricted
+          = label "#{dom_class(@snippet)}_visibility_level", level do
+            = visibility_level_icon(level)
+            .option-title
+              = visibility_level_label(level)
+            .option-descr
+              = snippet_visibility_level_description(level)
+      - unless restricted_visibility_levels.empty?
+        .col-sm-10
+          %span.info
+            Some visibility level settings have been restricted by the administrator.
+    - else
+      .col-sm-10
+        %span.info
+          = visibility_level_icon(visibility_level)
+          %strong
+            = visibility_level_label(visibility_level)
+          .light= visibility_level_description(visibility_level)

app/views/snippets/current_user_index.html.haml

@@ -28,6 +28,11 @@
           Internal
           %span.pull-right
             = @user.snippets.are_internal.count
+      = nav_tab :scope, 'are_public' do
+        = link_to user_snippets_path(@user, scope: 'are_public') do
+          Public
+          %span.pull-right
+            = @user.snippets.are_public.count
 
   .col-md-9.my-snippets
     = render 'snippets'

app/views/snippets/index.html.haml

@@ -2,10 +2,12 @@
   Public snippets
 
   .pull-right
-    = link_to new_snippet_path, class: "btn btn-new btn-grouped", title: "New Snippet" do
-      Add new snippet
-    = link_to user_snippets_path(current_user), class: "btn btn-grouped"  do
-      My snippets
+    
+    - if current_user
+      = link_to new_snippet_path, class: "btn btn-new btn-grouped", title: "New Snippet" do
+        Add new snippet
+      = link_to user_snippets_path(current_user), class: "btn btn-grouped"  do
+        My snippets
 
 %p.light
   Public snippets created by you and other users are listed here

app/views/snippets/user_index.html.haml

@@ -4,8 +4,9 @@
   %span
     \/
     Snippets
-  = link_to new_snippet_path, class: "btn btn-small add_new pull-right", title: "New Snippet" do
-    Add new snippet
+  - if current_user
+    = link_to new_snippet_path, class: "btn btn-small add_new pull-right", title: "New Snippet" do
+      Add new snippet
 
 %hr
 

db/migrate/20141007100818_add_visibility_level_to_snippet.rb

+class AddVisibilityLevelToSnippet < ActiveRecord::Migration
+  def up
+    add_column :snippets, :visibility_level, :integer, :default => 0, :null => false
+
+    Snippet.where(private: true).update_all(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+    Snippet.where(private: false).update_all(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+
+    add_index :snippets, :visibility_level
+
+    remove_column :snippets, :private
+  end
+
+  def down
+    add_column :snippets, :private, :boolean, :default => false, :null => false
+    
+    Snippet.where(visibility_level: Gitlab::VisibilityLevel::INTERNAL).update_all(private: false)
+    Snippet.where(visibility_level: Gitlab::VisibilityLevel::PRIVATE).update_all(private: true)
+    
+    remove_column :snippets, :visibility_level
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20141006143943) do
+ActiveRecord::Schema.define(version: 20141007100818) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -293,20 +293,21 @@ ActiveRecord::Schema.define(version: 20141006143943) do
   create_table "snippets", force: true do |t|
     t.string   "title"
     t.text     "content"
-    t.integer  "author_id",                 null: false
+    t.integer  "author_id",                    null: false
     t.integer  "project_id"
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string   "file_name"
     t.datetime "expires_at"
-    t.boolean  "private",    default: true, null: false
     t.string   "type"
+    t.integer  "visibility_level", default: 0, null: false
   end
 
   add_index "snippets", ["author_id"], name: "index_snippets_on_author_id", using: :btree
   add_index "snippets", ["created_at"], name: "index_snippets_on_created_at", using: :btree
   add_index "snippets", ["expires_at"], name: "index_snippets_on_expires_at", using: :btree
   add_index "snippets", ["project_id"], name: "index_snippets_on_project_id", using: :btree
+  add_index "snippets", ["visibility_level"], name: "index_snippets_on_visibility_level", using: :btree
 
   create_table "taggings", force: true do |t|
     t.integer  "tag_id"

features/snippets/discover.feature

@@ -4,8 +4,10 @@ Feature: Snippets Discover
     Given I sign in as a user
     And I have public "Personal snippet one" snippet
     And I have private "Personal snippet private" snippet
+    And I have internal "Personal snippet internal" snippet
 
   Scenario: I should see snippets
     Given I visit snippets page
     Then I should see "Personal snippet one" in snippets
+    And I should see "Personal snippet internal" in snippets
     And I should not see "Personal snippet private" in snippets

features/snippets/user.feature

@@ -4,20 +4,31 @@ Feature: Snippets User
     Given I sign in as a user
     And I have public "Personal snippet one" snippet
     And I have private "Personal snippet private" snippet
+    And I have internal "Personal snippet internal" snippet
 
   Scenario: I should see all my snippets
     Given I visit my snippets page
     Then I should see "Personal snippet one" in snippets
     And I should see "Personal snippet private" in snippets
+    And I should see "Personal snippet internal" in snippets
 
   Scenario: I can see only my private snippets
     Given I visit my snippets page
     And I click "Private" filter
     Then I should not see "Personal snippet one" in snippets
+    And I should not see "Personal snippet internal" in snippets
     And I should see "Personal snippet private" in snippets
 
   Scenario: I can see only my public snippets
     Given I visit my snippets page
-    And I click "Internal" filter
+    And I click "Public" filter
     Then I should see "Personal snippet one" in snippets
     And I should not see "Personal snippet private" in snippets
+    And I should not see "Personal snippet internal" in snippets
+
+  Scenario: I can see only my internal snippets
+    Given I visit my snippets page
+    And I click "Internal" filter
+    Then I should see "Personal snippet internal" in snippets
+    And I should not see "Personal snippet private" in snippets
+    And I should not see "Personal snippet one" in snippets

features/steps/shared/snippet.rb

@@ -6,7 +6,7 @@ module SharedSnippet
            title: "Personal snippet one",
            content: "Test content",
            file_name: "snippet.rb",
-           private: false,
+           visibility_level: Snippet::PUBLIC,
            author: current_user)
   end
 
@@ -15,9 +15,19 @@ module SharedSnippet
            title: "Personal snippet private",
            content: "Provate content",
            file_name: "private_snippet.rb",
-           private: true,
+           visibility_level: Snippet::PRIVATE,
            author: current_user)
   end
+  
+  step 'I have internal "Personal snippet internal" snippet' do
+    create(:personal_snippet,
+           title: "Personal snippet internal",
+           content: "Provate content",
+           file_name: "internal_snippet.rb",
+           visibility_level: Snippet::INTERNAL,
+           author: current_user)
+  end
+  
   step 'I have a public many lined snippet' do
     create(:personal_snippet,
            title: 'Many lined snippet',
@@ -38,7 +48,7 @@ module SharedSnippet
              |line fourteen
            END
            file_name: 'many_lined_snippet.rb',
-           private: true,
+           visibility_level: Snippet::PUBLIC,
            author: current_user)
   end
 end

features/steps/snippets/discover.rb

@@ -7,6 +7,10 @@ class Spinach::Features::SnippetsDiscover < Spinach::FeatureSteps
     page.should have_content "Personal snippet one"
   end
 
+  step 'I should see "Personal snippet internal" in snippets' do
+    page.should have_content "Personal snippet internal"
+  end
+
   step 'I should not see "Personal snippet private" in snippets' do
     page.should_not have_content "Personal snippet private"
   end

features/steps/snippets/user.rb

@@ -15,6 +15,10 @@ class Spinach::Features::SnippetsUser < Spinach::FeatureSteps
     page.should have_content "Personal snippet private"
   end
 
+  step 'I should see "Personal snippet internal" in snippets' do
+    page.should have_content "Personal snippet internal"
+  end
+
   step 'I should not see "Personal snippet one" in snippets' do
     page.should_not have_content "Personal snippet one"
   end
@@ -23,6 +27,10 @@ class Spinach::Features::SnippetsUser < Spinach::FeatureSteps
     page.should_not have_content "Personal snippet private"
   end
 
+  step 'I should not see "Personal snippet internal" in snippets' do
+    page.should_not have_content "Personal snippet internal"
+  end
+
   step 'I click "Internal" filter' do
     within('.nav-stacked') do
       click_link "Internal"
@@ -35,6 +43,12 @@ class Spinach::Features::SnippetsUser < Spinach::FeatureSteps
     end
   end
 
+  step 'I click "Public" filter' do
+    within('.nav-stacked') do
+      click_link "Public"
+    end
+  end
+
   def snippet
     @snippet ||= PersonalSnippet.find_by!(title: "Personal snippet one")
   end

spec/finders/snippets_finder_spec.rb

+require 'spec_helper'
+
+describe SnippetsFinder do
+  let(:user) { create :user }
+  let(:user1) { create :user }
+  let(:group) { create :group }
+
+  let(:project1) { create(:empty_project, :public,   group: group) }
+  let(:project2) { create(:empty_project, :private,  group: group) }
+  
+
+  context ':all filter' do
+    before do
+      @snippet1 = create(:personal_snippet, visibility_level: Snippet::PRIVATE)
+      @snippet2 = create(:personal_snippet, visibility_level: Snippet::INTERNAL)
+      @snippet3 = create(:personal_snippet, visibility_level: Snippet::PUBLIC)
+    end
+
+    it "returns all private and internal snippets" do
+      snippets = SnippetsFinder.new.execute(user, filter: :all)
+      snippets.should include(@snippet2, @snippet3)
+      snippets.should_not include(@snippet1)
+    end
+
+    it "returns all public snippets" do
+      snippets = SnippetsFinder.new.execute(nil, filter: :all)
+      snippets.should include(@snippet3)
+      snippets.should_not include(@snippet1, @snippet2)
+    end
+  end
+
+  context ':by_user filter' do
+    before do
+      @snippet1 = create(:personal_snippet, visibility_level: Snippet::PRIVATE, author: user)
+      @snippet2 = create(:personal_snippet, visibility_level: Snippet::INTERNAL, author: user)
+      @snippet3 = create(:personal_snippet, visibility_level: Snippet::PUBLIC, author: user)
+    end
+
+    it "returns all public and internal snippets" do
+      snippets = SnippetsFinder.new.execute(user1, filter: :by_user, user: user)
+      snippets.should include(@snippet2, @snippet3)
+      snippets.should_not include(@snippet1)
+    end
+
+    it "returns internal snippets" do
+      snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user, scope: "are_internal")
+      snippets.should include(@snippet2)
+      snippets.should_not include(@snippet1, @snippet3)
+    end
+
+    it "returns private snippets" do
+      snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user, scope: "are_private")
+      snippets.should include(@snippet1)
+      snippets.should_not include(@snippet2, @snippet3)
+    end
+
+    it "returns public snippets" do
+      snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user, scope: "are_public")
+      snippets.should include(@snippet3)
+      snippets.should_not include(@snippet1, @snippet2)
+    end
+
+    it "returns all snippets" do
+      snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user)
+      snippets.should include(@snippet1, @snippet2, @snippet3)
+    end
+  end
+
+  context 'by_project filter' do
+    before do
+      @snippet1 = create(:project_snippet, visibility_level: Snippet::PRIVATE, project: project1)
+      @snippet2 = create(:project_snippet, visibility_level: Snippet::INTERNAL, project: project1)
+      @snippet3 = create(:project_snippet, visibility_level: Snippet::PUBLIC, project: project1)
+    end
+
+    it "returns public snippets for unauthorized user" do
+      snippets = SnippetsFinder.new.execute(nil, filter: :by_project, project: project1)
+      snippets.should include(@snippet3)
+      snippets.should_not include(@snippet1, @snippet2)
+    end
+
+    it "returns public and internal snippets for none project members" do
+      snippets = SnippetsFinder.new.execute(user, filter: :by_project, project: project1)
+      snippets.should include(@snippet2, @snippet3)
+      snippets.should_not include(@snippet1)
+    end
+
+    it "returns all snippets for project members" do
+      project1.team << [user, :developer] 
+      snippets = SnippetsFinder.new.execute(user, filter: :by_project, project: project1)
+      snippets.should include(@snippet1, @snippet2, @snippet3)
+    end
+  end
+end

spec/models/project_team_spec.rb

@@ -27,6 +27,8 @@ describe ProjectTeam do
       it { project.team.master?(guest).should be_false }
       it { project.team.master?(reporter).should be_false }
       it { project.team.master?(nonmember).should be_false }
+      it { project.team.member?(nonmember).should be_false }
+      it { project.team.member?(guest).should be_true }
     end
   end
 
@@ -60,6 +62,8 @@ describe ProjectTeam do
       it { project.team.master?(guest).should be_true }
       it { project.team.master?(reporter).should be_false }
       it { project.team.master?(nonmember).should be_false }
+      it { project.team.member?(nonmember).should be_false }
+      it { project.team.member?(guest).should be_true }
     end
   end
 end