Code improvements and add Create group service

13 files changed, 56 insertions, 79 deletions

diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss
index 18044b25b87..0931090b840 100644
--- a/app/assets/stylesheets/framework/common.scss
+++ b/app/assets/stylesheets/framework/common.scss
@@ -383,35 +383,25 @@ table {
   margin-right: -$gl-padding;
   border-top: 1px solid $border-color;
 }
-.message {
-  border: 1px solid #ccc;
-  padding: 10px;
-  color: #333;
-}
-.message {
-  border: 1px solid #ccc;
-  padding: 10px;
-  color: #333;
-}
 
-.group-projects-show-title{
-    h1 {
-      color: #313236;
-      margin: 0;
-      margin-bottom: 6px;
-      font-size: 23px;
-      font-weight: normal;
-    }
+.cover-title{
+  h1 {
+    color: #313236;
+    margin: 0;
+    margin-bottom: 6px;
+    font-size: 23px;
+    font-weight: normal;
+  }
 
-    .visibility-icon {
-      display: inline-block;
-      margin-left: 5px;
-      font-size: 18px;
-      color: $gray;
-    }
+  .visibility-icon {
+    display: inline-block;
+    margin-left: 5px;
+    font-size: 18px;
+    color: $gray;
+  }
 
-    p {
-      padding: 0 $gl-padding;
-      color: #5c5d5e;
-    }
+  p {
+    padding: 0 $gl-padding;
+    color: #5c5d5e;
   }
+}
diff --git a/app/assets/stylesheets/framework/mobile.scss b/app/assets/stylesheets/framework/mobile.scss
index 3bfac2ad9b5..d088228fe4c 100644
--- a/app/assets/stylesheets/framework/mobile.scss
+++ b/app/assets/stylesheets/framework/mobile.scss
@@ -48,7 +48,7 @@
       display: block;
     }
 
-    .project-home-desc {
+    #project-home-desc {
       font-size: 21px;
     }
 
diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss
index b1b76edfb32..1a7b0c1e278 100644
--- a/app/assets/stylesheets/pages/projects.scss
+++ b/app/assets/stylesheets/pages/projects.scss
@@ -61,28 +61,6 @@
     }
   }
 
-  .project-home-desc {
-    h1 {
-      color: #313236;
-      margin: 0;
-      margin-bottom: 6px;
-      font-size: 23px;
-      font-weight: normal;
-    }
-
-    .visibility-icon {
-      display: inline-block;
-      margin-left: 5px;
-      font-size: 18px;
-      color: $gray;
-    }
-
-    p {
-      padding: 0 $gl-padding;
-      color: #5c5d5e;
-    }
-  }
-
   .project-repo-buttons {
     margin-top: 20px;
     margin-bottom: 0px;
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 54f14e62ead..5baeb3def08 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -29,10 +29,8 @@ class GroupsController < Groups::ApplicationController
 
   def create
     @group = Group.new(group_params)
-    @group.name = @group.path.dup unless @group.name
 
-    if @group.save
-      @group.add_owner(current_user)
+    if Groups::CreateService.new(@group, current_user, group_params).execute
       redirect_to @group, notice: "Group '#{@group.name}' was successfully created."
     else
       render action: "new"
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 7b32572f822..481d00d6aae 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -3,13 +3,6 @@ class UsersController < ApplicationController
   before_action :set_user
 
   def show
-    @contributed_projects = contributed_projects.joined(@user).reject(&:forked?)
-
-    @projects = PersonalProjectsFinder.new(@user).execute(current_user)
-    @projects = @projects.page(params[:page]).per(PER_PAGE)
-
-    @groups =  JoinedGroupsFinder.new(@user).execute(current_user)
-
     respond_to do |format|
       format.html
 
@@ -115,7 +108,7 @@ class UsersController < ApplicationController
   end
 
   def load_groups
-    @groups = @user.groups.order_id_desc
+    @groups = JoinedGroupsFinder.new(@user).execute(current_user)
   end
 
   def projects_for_current_user
diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb
index 131b518563e..fbdf492c965 100644
--- a/app/finders/joined_groups_finder.rb
+++ b/app/finders/joined_groups_finder.rb
@@ -1,6 +1,6 @@
 #Shows only authorized groups of a user
 class JoinedGroupsFinder
-  def initialize(user = nil)
+  def initialize(user)
     @user = user
   end
 
diff --git a/app/models/ability.rb b/app/models/ability.rb
index fe460ccdaca..bd001ef1545 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -295,8 +295,8 @@ class Ability
     end
 
     def can_read_group?(user, group)
-      is_project_member = ProjectsFinder.new.execute(user, group: group).any?
-      user.admin? || group.public? || group.internal? || is_project_member || group.users.include?(user)
+      user.admin? || group.public? || group.internal? || group.users.include?(user) ||
+      ProjectsFinder.new.execute(user, group: group).any?
     end
 
     def namespace_abilities(user, namespace)
diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb
index 5becd475d3a..644ec7c013e 100644
--- a/app/services/groups/base_service.rb
+++ b/app/services/groups/base_service.rb
@@ -5,5 +5,9 @@ module Groups
     def initialize(group, user, params = {})
       @group, @current_user, @params = group, user, params.dup
     end
+
+    def add_error_message(message)
+      group.errors.add(:visibility_level, message)
+    end
   end
 end
diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb
new file mode 100644
index 00000000000..e2875aafb94
--- /dev/null
+++ b/app/services/groups/create_service.rb
@@ -0,0 +1,17 @@
+module Groups
+  class CreateService < Groups::BaseService
+    def execute
+      return false unless visibility_level_allowed?(params[:visibility_level])
+      @group.name = @group.path.dup unless @group.name
+      @group.save(params) && @group.add_owner(current_user)
+    end
+
+    private
+
+    def visibility_level_allowed?(level)
+      allowed = Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level])
+      add_error_message("Visibility level restricted by admin.") unless allowed
+      allowed
+    end
+  end
+end
diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb
index acb6c529c17..a7382c1e07c 100644
--- a/app/services/groups/update_service.rb
+++ b/app/services/groups/update_service.rb
@@ -5,7 +5,8 @@
 module Groups
   class UpdateService < Groups::BaseService
     def execute
-      visibility_level_allowed?(params[:visibility_level]) ? group.update_attributes(params) : false
+      return false unless visibility_level_allowed?(params[:visibility_level])
+      group.update_attributes(params)
     end
 
     private
@@ -22,7 +23,7 @@ module Groups
     def visibility_by_project(level)
       projects_visibility = group.projects.pluck(:visibility_level)
 
-      allowed_by_projects = !projects_visibility.any?{|project_visibility| level.to_i < project_visibility }
+      allowed_by_projects = !projects_visibility.any?{ |project_visibility| level.to_i < project_visibility }
       add_error_message("Cannot be changed. There are projects with higher visibility permissions.") unless allowed_by_projects
       allowed_by_projects
     end
@@ -32,13 +33,5 @@ module Groups
       add_error_message("You are not authorized to set this permission level.") unless allowed_by_user
       allowed_by_user
     end
-
-    def add_error_message(message)
-      level_name = Gitlab::VisibilityLevel.level_name(params[:visibility_level])
-      group.errors.add(:visibility_level, message)
-    end
   end
 end
-
-
-
diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb
index 522fae79503..4c121106bda 100644
--- a/app/services/projects/create_service.rb
+++ b/app/services/projects/create_service.rb
@@ -12,7 +12,7 @@ module Projects
       # Make sure that the user is allowed to use the specified visibility
       # level
 
-      unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) && @project.visibility_level_allowed?(@project.visibility_level)
+      unless visibility_level_allowed?
         deny_visibility_level(@project)
         return @project
       end
@@ -100,5 +100,9 @@ module Projects
 
       @project.import_start if @project.import?
     end
+
+    def visibility_level_allowed?
+      Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) && @project.visibility_level_allowed?(@project.visibility_level)
+    end
   end
 end
diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index 0a8c2da7207..641191edf05 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -14,11 +14,11 @@
   .avatar-holder
     = link_to group_icon(@group), target: '_blank' do
       = image_tag group_icon(@group), class: "avatar group-avatar s90"
-  .group-projects-show-title
+  .cover-title
     %h1
       = @group.name
 
-      %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: "#{visibility_level_label(@group.visibility_level)} - #{project_visibility_level_description(@group.visibility_level)}"}
+      %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: "#{visibility_level_label(@group.visibility_level)} - #{group_visibility_description(@group)}"}
         = visibility_level_icon(@group.visibility_level, fw: false)
 
   .cover-desc.username
diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml
index dd16f504c92..e8434b5292c 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/app/views/projects/_home_panel.html.haml
@@ -2,7 +2,7 @@
 .project-home-panel.cover-block.clearfix{:class => ("empty-project" if empty_repo)}
   .project-identicon-holder
     = project_icon(@project, alt: '', class: 'project-avatar avatar s90')
-  .group-projects-show-title
+  .cover-title#project-home-desc
     %h1
       = @project.name
       %span.visibility-icon.has_tooltip{data: { container: 'body' },