From 0052e78bfe87cb66e15cba6500ce8dd9ca84526a Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Mon, 17 Jun 2013 13:43:06 +1000
Subject: [PATCH] render error when people attempt to save an invalid group
 name hide controls when we showing an automatic group

---
 app/assets/javascripts/admin/models/group.js  | 19 +++++++++++++------
 .../admin/templates/groups.js.handlebars      |  6 ++++--
 app/controllers/admin/groups_controller.rb    |  9 ++++++---
 config/locales/client.en.yml                  |  2 ++
 spec/models/group_spec.rb                     |  5 +++++
 5 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/app/assets/javascripts/admin/models/group.js b/app/assets/javascripts/admin/models/group.js
index 1f7d0173b82..a67929e7798 100644
--- a/app/assets/javascripts/admin/models/group.js
+++ b/app/assets/javascripts/admin/models/group.js
@@ -65,13 +65,20 @@ Discourse.Group = Discourse.Model.extend({
     var group = this;
     group.set('disableSave', true);
 
-    return Discourse.ajax("/admin/groups/" + this.get('id'), {type: "PUT", data: {
-      group: {
-        name: this.get('name'),
-        usernames: this.get('usernames')
+    return Discourse.ajax("/admin/groups/" + this.get('id'), {
+      type: "PUT",
+      data: {
+        group: {
+          name: this.get('name'),
+          usernames: this.get('usernames')
+        }
+      },
+      complete: function(){
+        group.set('disableSave', false);
       }
-    }}).then(function(r){
-      group.set('disableSave', false);
+    }).then(null, function(e){
+      var message = $.parseJSON(e.responseText).errors;
+      bootbox.alert(message);
     });
   }
 
diff --git a/app/assets/javascripts/admin/templates/groups.js.handlebars b/app/assets/javascripts/admin/templates/groups.js.handlebars
index e5ef8efa777..cfff5e5a2ce 100644
--- a/app/assets/javascripts/admin/templates/groups.js.handlebars
+++ b/app/assets/javascripts/admin/templates/groups.js.handlebars
@@ -26,11 +26,13 @@
 
           {{userSelector usernames=usernames id="group-users" placeholderKey="admin.groups.selector_placeholder" tabindex="1" disabledBinding="automatic"}}
           <div class='controls'>
-            <button {{action save this}} {{bindAttr disabled="disableSave"}} class='btn'>{{i18n admin.customize.save}}</button>
             {{#unless automatic}}
+              <button {{action save this}} {{bindAttr disabled="disableSave"}} class='btn'>{{i18n admin.customize.save}}</button>
               {{#if id}}
                 <a {{action destroy this}} class='delete-link'>{{i18n admin.customize.delete}}</a>
               {{/if}}
+            {{else}}
+              {{i18n admin.groups.can_not_edit_automatic}}
             {{/unless}}
           </div>
         {{/with}}
@@ -38,7 +40,7 @@
         <div class='spinner'>{{i18n loading}}</div>
       {{/if}}
     {{else}}
-      nothing here yet
+      {{i18n admin.groups.about}}
     {{/if}}
   </div>
 </div>
diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb
index 06d9404c03e..40026b6d014 100644
--- a/app/controllers/admin/groups_controller.rb
+++ b/app/controllers/admin/groups_controller.rb
@@ -20,9 +20,12 @@ class Admin::GroupsController < Admin::AdminController
       can_not_modify_automatic
     else
       group.usernames = params[:group][:usernames]
-      group.name = params[:group][:name] if params[:name]
-      group.save!
-      render json: "ok"
+      group.name = params[:group][:name] if params[:group][:name]
+      if group.save
+        render json: "ok"
+      else
+        render_json_error group
+      end
     end
   end
 
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index cfcb40d5b44..54169cc20c1 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -1042,6 +1042,8 @@ en:
         edit: "Edit Groups"
         selector_placeholder: "add users"
         name_placeholder: "Group name, no spaces, same as username rule"
+        about: "Edit your group membership and names here"
+        can_not_edit_automatic: "Automatic group membership is determined automatically, administer users to assign roles and trust levels"
 
       api:
         title: "API"
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 7e2f663a01a..23741dd1bc1 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -14,6 +14,11 @@ describe Group do
       group.name = "this_is_a_name"
       group.valid?.should be_true
     end
+
+    it "is invalid for non names" do
+      group.name = "this is_a_name"
+      group.valid?.should be_false
+    end
   end
 
   it "Can update moderator/staff/admin groups correctly" do