From 2b21e5ea7e36fcd25b1539477ad3c99391125b3f Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Fri, 5 May 2017 14:34:47 +0800
Subject: [PATCH] UX: Display translated group name for automatic groups.

---
 .../discourse/controllers/group.js.es6        |  6 ++---
 .../javascripts/discourse/models/group.js.es6 |  4 ++++
 .../discourse/templates/groups.hbs            |  2 +-
 app/models/group.rb                           |  2 +-
 app/serializers/basic_group_serializer.rb     | 11 +++++++++
 spec/serializers/basic_group_serializer.rb    | 23 +++++++++++++++++++
 test/javascripts/models/group-test.js.es6     | 13 +++++++++++
 7 files changed, 56 insertions(+), 5 deletions(-)
 create mode 100644 spec/serializers/basic_group_serializer.rb
 create mode 100644 test/javascripts/models/group-test.js.es6

diff --git a/app/assets/javascripts/discourse/controllers/group.js.es6 b/app/assets/javascripts/discourse/controllers/group.js.es6
index 07fc0ab8c15..09466b4c4f9 100644
--- a/app/assets/javascripts/discourse/controllers/group.js.es6
+++ b/app/assets/javascripts/discourse/controllers/group.js.es6
@@ -33,9 +33,9 @@ export default Ember.Controller.extend({
     return !automatic && isGroupOwner;
   },
 
-  @computed('model.name', 'model.full_name')
-  groupName(name, fullName) {
-    return (fullName || name).capitalize();
+  @computed('model.displayName', 'model.full_name')
+  groupName(displayName, fullName) {
+    return (fullName || displayName).capitalize();
   },
 
   @computed('model.name', 'model.flair_url', 'model.flair_bg_color', 'model.flair_color')
diff --git a/app/assets/javascripts/discourse/models/group.js.es6 b/app/assets/javascripts/discourse/models/group.js.es6
index 2af528f8187..a85025e410e 100644
--- a/app/assets/javascripts/discourse/models/group.js.es6
+++ b/app/assets/javascripts/discourse/models/group.js.es6
@@ -94,6 +94,10 @@ const Group = RestModel.extend({
     });
   },
 
+  @computed("display_name", "name")
+  displayName(groupDisplayName, name) {
+    return groupDisplayName || name;
+  },
 
   @computed('flair_bg_color')
   flairBackgroundHexColor() {
diff --git a/app/assets/javascripts/discourse/templates/groups.hbs b/app/assets/javascripts/discourse/templates/groups.hbs
index 2699777f5a6..98faa7e4174 100644
--- a/app/assets/javascripts/discourse/templates/groups.hbs
+++ b/app/assets/javascripts/discourse/templates/groups.hbs
@@ -26,7 +26,7 @@
                     {{/if}}
 
                     <span>
-                      <span class='groups-info-name'>{{group.name}}</span>
+                      <span class='groups-info-name'>{{group.displayName}}</span>
 
                       {{#if group.full_name}}
                         <span class='groups-info-full-name'>{{group.full_name}}</span>
diff --git a/app/models/group.rb b/app/models/group.rb
index 1c185cc7912..ddbe8ebb225 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -114,8 +114,8 @@ class Group < ActiveRecord::Base
 
     incoming_email.split("|").each do |email|
       escaped = Rack::Utils.escape_html(email)
+      self.errors.add(:base, I18n.t('groups.errors.invalid_incoming_email', email: escaped))
       if !Email.is_valid?(email)
-        self.errors.add(:base, I18n.t('groups.errors.invalid_incoming_email', email: escaped))
       elsif group = Group.where.not(id: self.id).find_by_email(email)
         self.errors.add(:base, I18n.t('groups.errors.email_already_used_in_group', email: escaped, group_name: Rack::Utils.escape_html(group.name)))
       elsif category = Category.find_by_email(email)
diff --git a/app/serializers/basic_group_serializer.rb b/app/serializers/basic_group_serializer.rb
index ea3df8e08ae..d86d0606fce 100644
--- a/app/serializers/basic_group_serializer.rb
+++ b/app/serializers/basic_group_serializer.rb
@@ -2,6 +2,7 @@ class BasicGroupSerializer < ApplicationSerializer
   attributes :id,
              :automatic,
              :name,
+             :display_name,
              :user_count,
              :alias_level,
              :visible,
@@ -22,6 +23,16 @@ class BasicGroupSerializer < ApplicationSerializer
              :full_name,
              :default_notification_level
 
+  def include_display_name?
+    object.automatic
+  end
+
+  def display_name
+    if auto_group_name = Group::AUTO_GROUP_IDS[object.id]
+      I18n.t("groups.default_names.#{auto_group_name}")
+    end
+  end
+
   def include_incoming_email?
     staff?
   end
diff --git a/spec/serializers/basic_group_serializer.rb b/spec/serializers/basic_group_serializer.rb
new file mode 100644
index 00000000000..712f06fa7f3
--- /dev/null
+++ b/spec/serializers/basic_group_serializer.rb
@@ -0,0 +1,23 @@
+require 'rails_helper'
+
+describe BasicGroupSerializer do
+  subject { described_class.new(group, scope: Guardian.new, root: false) }
+
+  describe '#display_name' do
+    describe 'automatic group' do
+      let(:group) { Group.find(1) }
+
+      it 'should include the display name' do
+        expect(subject.display_name).to eq(I18n.t('groups.default_names.admins'))
+      end
+    end
+
+    describe 'normal group' do
+      let(:group) { Fabricate(:group) }
+
+      it 'should not include the display name' do
+        expect(subject.display_name).to eq(nil)
+      end
+    end
+  end
+end
diff --git a/test/javascripts/models/group-test.js.es6 b/test/javascripts/models/group-test.js.es6
new file mode 100644
index 00000000000..60f04ed1bbb
--- /dev/null
+++ b/test/javascripts/models/group-test.js.es6
@@ -0,0 +1,13 @@
+import Group from 'discourse/models/group';
+
+module("model:group");
+
+test('displayName', function() {
+  const group = Group.create({ name: "test", display_name: 'donkey'  });
+
+  ok(group.get('displayName'), "donkey", 'it should return the display name');
+
+  group.set('display_name', null);
+
+  ok(group.get('displayName'), "test", "it should return the group's name");
+});