Allow admin to view logs of automatic groups.

app/assets/javascripts/discourse/controllers/group-manage.js.es6

@@ -7,16 +7,13 @@ export default Ember.Controller.extend({
   tabs(automatic) {
     const defaultTabs = [
       { route: 'group.manage.profile', title: 'groups.manage.profile.title' },
+      { route: 'group.manage.logs', title: 'groups.manage.logs.title' },
     ];
 
     if (!automatic) {
-      defaultTabs.push(
+      defaultTabs.splice(1, 0,
         { route: 'group.manage.members', title: 'groups.manage.members.title' }
       );
-
-      defaultTabs.push(
-        { route: 'group.manage.logs', title: 'groups.manage.logs.title' },
-      );
     }
 
     return defaultTabs;

app/assets/javascripts/discourse/templates/components/group-form.hbs

@@ -1,4 +1,4 @@
-<form class="groups-new-form form-horizontal">
+<form class="groups-form form-horizontal">
   {{#if model.automatic}}
     <div class="control-group">
       <label for="visiblity">{{i18n 'groups.visibility_levels.title'}}</label>

app/controllers/groups_controller.rb

@@ -1,5 +1,4 @@
 class GroupsController < ApplicationController
-
   requires_login only: [
     :set_notifications,
     :mentionable,
@@ -389,7 +388,7 @@ class GroupsController < ApplicationController
 
   def histories
     group = find_group(:group_id)
-    guardian.ensure_can_edit!(group)
+    guardian.ensure_can_edit!(group) unless current_user.admin
 
     page_size = 25
     offset = (params[:offset] && params[:offset].to_i) || 0

spec/requests/groups_controller_spec.rb

@@ -454,6 +454,18 @@ describe GroupsController do
         expect(group.incoming_email).to eq("test@mail.org")
         expect(group.grant_trust_level).to eq(1)
       end
+
+      it 'should not be allowed to update automatic groups' do
+        group = Group.find(Group::AUTO_GROUPS[:admins])
+
+        put "/groups/#{group.id}.json", params: {
+          group: {
+            messageable_level: 1
+          }
+        }
+
+        expect(response.status).to eq(403)
+      end
     end
 
     context "when user is group admin" do
@@ -947,7 +959,7 @@ describe GroupsController do
     end
   end
 
-  describe "group histories" do
+  describe "#histories" do
     context 'when user is not signed in' do
       it 'should raise the right error' do
         get "/groups/#{group.name}/logs.json"
@@ -967,18 +979,20 @@ describe GroupsController do
       end
     end
 
-    describe 'viewing history' do
+    describe 'when user is a group owner' do
-      context 'public group' do
       before do
         group.add_owner(user)
+        sign_in(user)
+      end
 
+      describe 'when viewing a public group' do
+        before do
           group.update_attributes!(
             public_admission: true,
             public_exit: true
           )
 
           GroupActionLogger.new(user, group).log_change_group_settings
-          sign_in(user)
         end
 
         it 'should allow group owner to view history' do
@@ -995,7 +1009,16 @@ describe GroupsController do
         end
       end
 
-      context 'admin' do
+      it 'should not be allowed to view history of an automatic group' do
+        group = Group.find_by(id: Group::AUTO_GROUPS[:admins])
+
+        get "/groups/#{group.name}/logs.json"
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'when user is an admin' do
       let(:admin) { Fabricate(:admin) }
 
       before do
@@ -1014,6 +1037,14 @@ describe GroupsController do
         expect(result["action"]).to eq(GroupHistory.actions[3].to_s)
       end
 
+      it 'should be able to view history of automatic groups' do
+        group = Group.find_by(id: Group::AUTO_GROUPS[:admins])
+
+        get "/groups/#{group.name}/logs.json"
+
+        expect(response.status).to eq(200)
+      end
+
       it 'should be able to filter through the history' do
         GroupActionLogger.new(admin, group).log_add_user_to_group(user)
         GroupActionLogger.new(admin, group).log_remove_user_from_group(user)
@@ -1031,7 +1062,6 @@ describe GroupsController do
       end
     end
   end
-  end
 
   describe '#request_membership' do
     let(:new_user) { Fabricate(:user) }

test/javascripts/acceptance/group-test.js.es6

@@ -56,6 +56,18 @@ QUnit.test("Anonymous Viewing Group", assert => {
   });
 });
 
+QUnit.test("Anonymous Viewing Automatic Group", assert => {
+  visit("/groups/moderators");
+
+  andThen(() => {
+    assert.equal(
+      count(".nav-pills li a[title='Manage']"),
+      0,
+      'it deos not show group messages navigation link'
+    );
+  });
+});
+
 QUnit.test("User Viewing Group", assert => {
   logIn();
   Discourse.reset();
@@ -128,7 +140,6 @@ QUnit.test("Admin viewing group messages", assert => {
   Discourse.reset();
 
   visit("/groups/discourse");
-
   click(".nav-pills li a[title='Messages']");
 
   andThen(() => {
@@ -156,3 +167,18 @@ QUnit.test("Admin Viewing Group", assert => {
     assert.equal(find('.group-info-name').text(), 'Awesome Team', 'it should display the group name');
   });
 });
+
+QUnit.test("Admin Viewing Automatic Group", assert => {
+  logIn();
+  Discourse.reset();
+
+  visit("/groups/moderators");
+  click(".nav-pills li a[title='Manage']");
+
+  andThen(() => {
+    assert.equal(
+      count('.groups-form .control-group'), 5,
+      'it should display the right fields'
+    );
+  });
+});

test/javascripts/fixtures/group-fixtures.js.es6

@@ -1,4 +1,37 @@
 export default {
+  "/groups/moderators":{
+    "group": {
+      "id": 50,
+      "automatic": true,
+      "name": "moderators",
+      "display_name": "moderators",
+      "mentionable_level": 0,
+      "messageable_level": 99,
+      "visibility_level": 0,
+      "automatic_membership_email_domains": null,
+      "automatic_membership_retroactive": false,
+      "primary_group": false,
+      "title": null,
+      "grant_trust_level": null,
+      "incoming_email": null,
+      "has_messages": true,
+      "flair_url": null,
+      "flair_bg_color": null,
+      "flair_color": null,
+      "bio_raw": null,
+      "bio_cooked": null,
+      "public_admission": false,
+      "public_exit": false,
+      "allow_membership_requests": false,
+      "full_name": null,
+      "default_notification_level": 2,
+      "membership_request_template": null,
+      "is_group_user": true,
+      "is_group_owner": true,
+      "mentionable": false,
+      "messageable": true
+    },
+  },
   "/groups/discourse":{
     "group":{
       "id":47,

test/javascripts/helpers/create-pretender.js.es6

@@ -308,6 +308,10 @@ export default function() {
       return response(200, fixturesByUrl['/groups/discourse/posts.json']);
     });
 
+    this.get("/groups/moderators/members.json", () => {
+      return response(200, fixturesByUrl['/groups/discourse/members.json']);
+    });
+
     this.get('/t/:topic_id/posts.json', request => {
       const postIds = request.queryParams.post_ids;
       const posts = postIds.map(p => ({id: parseInt(p), post_number: parseInt(p) }));