From c788c66a85ba4682f5e27a12b5fa73489af23b5f Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Mon, 5 Sep 2016 13:14:49 -0500
Subject: [PATCH] Improved helper methods, better flow for
 `project.lfs_enabled?`, and UI fixes.

---
 app/helpers/groups_helper.rb                  | 23 +++++++++++++++++--
 app/helpers/projects_helper.rb                |  4 ++--
 app/models/project.rb                         |  4 ----
 app/views/admin/groups/show.html.haml         |  3 +--
 app/views/admin/projects/show.html.haml       |  2 +-
 app/views/projects/edit.html.haml             | 17 +++++++-------
 ...901213340_add_lfs_enabled_to_namespaces.rb | 17 --------------
 7 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index e87197d2056..0352a48e050 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -25,7 +25,26 @@ module GroupsHelper
   end
 
   def projects_with_lfs_enabled(group)
-    total = group.projects.size
-    "#{total - group.projects.select{ |p| !p.lfs_enabled? }.size}/#{total} projects have it enabled"
+    lfs_enabled = group.projects.select(&:lfs_enabled?).size
+    size = group.projects.size
+
+    if lfs_enabled == size || lfs_enabled == 0
+      ' on all projects'
+    else
+      " on #{lfs_enabled}/#{size} projects"
+    end
+  end
+
+  def group_lfs_status(group)
+    if group.lfs_enabled?
+      output = content_tag(:span, class: 'lfs-enabled') do
+        'Enabled'
+      end
+    else
+      output = content_tag(:span, class: 'lfs-disabled') do
+        'Disabled'
+      end
+    end
+    output << projects_with_lfs_enabled(group)
   end
 end
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index b4be679c72d..16a8e52a4ca 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -202,8 +202,8 @@ module ProjectsHelper
     nav_tabs.flatten
   end
 
-  def lfs_status_helper(subject)
-    if subject.lfs_enabled?
+  def project_lfs_status(project)
+    if project.lfs_enabled?
       content_tag(:span, class: 'lfs-enabled') do
         'Enabled'
       end
diff --git a/app/models/project.rb b/app/models/project.rb
index 8e3bedffe1f..8b5a6f167bd 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -393,10 +393,6 @@ class Project < ActiveRecord::Base
   end
 
   def lfs_enabled?
-    # Specifically check is lfs_enabled is false
-    return false if self[:lfs_enabled] == false
-
-    # Should only fallback to the namespace value if no value is set for the project
     return namespace.lfs_enabled? if self[:lfs_enabled].nil?
 
     self[:lfs_enabled] && Gitlab.config.lfs.enabled
diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml
index b8e83075a72..0188ed448ce 100644
--- a/app/views/admin/groups/show.html.haml
+++ b/app/views/admin/groups/show.html.haml
@@ -40,8 +40,7 @@
         %li
           %span.light Group Git LFS status:
           %strong
-            = lfs_status_helper(@group)
-            = projects_with_lfs_enabled(@group)
+            = group_lfs_status(@group)
             = link_to icon('question-circle'), help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs')
 
     .panel.panel-default
diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml
index eecc69b125c..6c7c3c48604 100644
--- a/app/views/admin/projects/show.html.haml
+++ b/app/views/admin/projects/show.html.haml
@@ -77,7 +77,7 @@
           %li
             %span.light Git LFS status:
             %strong
-              = lfs_status_helper(@project)
+              = project_lfs_status(@project)
               = link_to icon('question-circle'), help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs')
         - else
           %li
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index f6d751a343e..a04d53e02bf 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -84,15 +84,14 @@
                     = project_feature_access_select(:snippets_access_level)
 
             - if Gitlab.config.lfs.enabled && current_user.admin?
-              .form-group
-                .checkbox
-                  = f.label :lfs_enabled do
-                    = f.check_box :lfs_enabled, checked: @project.lfs_enabled?
-                    %strong LFS
-                    %br
-                    %span.descr
-                      Git Large File Storage
-                      = link_to icon('question-circle'), help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs')
+              .row
+                .col-md-9
+                  = f.label :lfs_enabled, 'LFS', class: 'label-light'
+                  %span.help-block
+                    Git Large File Storage
+                    = link_to icon('question-circle'), help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs')
+                .col-md-3
+                  = f.select :lfs_enabled, [%w(Enabled true), %w(Disabled false)], {}, selected: @project.lfs_enabled?, class: 'pull-right form-control'
 
           - if Gitlab.config.registry.enabled
             .form-group
diff --git a/db/migrate/20160901213340_add_lfs_enabled_to_namespaces.rb b/db/migrate/20160901213340_add_lfs_enabled_to_namespaces.rb
index 9b3ee915e21..fd413d1ca8c 100644
--- a/db/migrate/20160901213340_add_lfs_enabled_to_namespaces.rb
+++ b/db/migrate/20160901213340_add_lfs_enabled_to_namespaces.rb
@@ -4,25 +4,8 @@
 class AddLfsEnabledToNamespaces < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
-  # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  # When a migration requires downtime you **must** uncomment the following
-  # constant and define a short and easy to understand explanation as to why the
-  # migration requires downtime.
-  # DOWNTIME_REASON = ''
-
-  # When using the methods "add_concurrent_index" or "add_column_with_default"
-  # you must disable the use of transactions as these methods can not run in an
-  # existing transaction. When using "add_concurrent_index" make sure that this
-  # method is the _only_ method called in the migration, any other changes
-  # should go in a separate migration. This ensures that upon failure _only_ the
-  # index creation fails and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
-  # disable_ddl_transaction!
-
   def change
     add_column :namespaces, :lfs_enabled, :boolean
   end
-- 
GitLab