diff --git a/app/assets/javascripts/discourse/components/group-notifications-button.js.es6 b/app/assets/javascripts/discourse/components/group-notifications-button.js.es6 index 6010cf02a25..8f70ca102a3 100644 --- a/app/assets/javascripts/discourse/components/group-notifications-button.js.es6 +++ b/app/assets/javascripts/discourse/components/group-notifications-button.js.es6 @@ -2,7 +2,7 @@ import NotificationsButton from 'discourse/components/notifications-button'; export default NotificationsButton.extend({ classNames: ['notification-options', 'group-notification-menu'], - notificationLevel: Em.computed.alias('group.notification_level'), + notificationLevel: Em.computed.alias('group.group_user.notification_level'), i18nPrefix: 'groups.notifications', clicked(id) { diff --git a/app/assets/javascripts/discourse/controllers/group.js.es6 b/app/assets/javascripts/discourse/controllers/group.js.es6 index ac16e4342e0..56a5325d2a2 100644 --- a/app/assets/javascripts/discourse/controllers/group.js.es6 +++ b/app/assets/javascripts/discourse/controllers/group.js.es6 @@ -41,8 +41,16 @@ export default Ember.Controller.extend({ }); }, - @computed('model.is_member') - getTabs(isMember) { - return this.get('tabs').filter(t => isMember || !t.get('requiresMembership')); + @computed('model.is_group_user') + getTabs(isGroupUser) { + return this.get('tabs').filter(t => { + let isMember = false; + + if (this.currentUser) { + isMember = this.currentUser.admin || isGroupUser; + } + + return isMember || !t.get('requiresMembership'); + }); } }); diff --git a/app/assets/javascripts/discourse/models/group.js.es6 b/app/assets/javascripts/discourse/models/group.js.es6 index 9980acc81ea..4566c154abc 100644 --- a/app/assets/javascripts/discourse/models/group.js.es6 +++ b/app/assets/javascripts/discourse/models/group.js.es6 @@ -151,7 +151,7 @@ const Group = Discourse.Model.extend({ }, setNotification(notification_level) { - this.set("notification_level", notification_level); + this.set("group_user.notification_level", notification_level); return ajax(`/groups/${this.get("name")}/notifications`, { data: { notification_level }, type: "POST" @@ -181,7 +181,11 @@ Group.reopenClass({ offset: offset || 0 } }); - } + }, + + mentionable(name) { + return ajax(`/groups/${name}/mentionable`, { data: { name } }); + }, }); export default Group; diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 83223e79725..3e84d0c5501 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -341,7 +341,15 @@ const User = RestModel.extend({ } if (!Em.isEmpty(json.user.groups)) { - json.user.groups = json.user.groups.map(g => Group.create(g)); + const groups = []; + + for(let i = 0; i < json.user.groups.length; i++) { + const group = Group.create(json.user.groups[i]); + group.group_user = json.user.group_users[i]; + groups.push(group); + } + + json.user.groups = groups; } if (json.user.invited_by) { diff --git a/app/assets/javascripts/discourse/routes/new-message.js.es6 b/app/assets/javascripts/discourse/routes/new-message.js.es6 index d8e8c577511..85731a8e827 100644 --- a/app/assets/javascripts/discourse/routes/new-message.js.es6 +++ b/app/assets/javascripts/discourse/routes/new-message.js.es6 @@ -21,11 +21,11 @@ export default Discourse.Route.extend({ }); } else if (params.groupname) { // send a message to a group - Group.find(params.groupname).then(group => { - if (group.mentionable) { - Ember.run.next(() => e.send("createNewMessageViaParams", group.name, params.title, params.body)); + Group.mentionable(params.groupname).then(result => { + if (result.mentionable) { + Ember.run.next(() => e.send("createNewMessageViaParams", params.groupname, params.title, params.body)); } else { - bootbox.alert(I18n.t("composer.cant_send_pm", { username: group.name })); + bootbox.alert(I18n.t("composer.cant_send_pm", { username: params.groupname })); } }).catch(function() { bootbox.alert(I18n.t("generic_error")); diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 94ef113634f..daca3101247 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -1,7 +1,7 @@ class Admin::GroupsController < Admin::AdminController def index - groups = Group.order(:name).where("id <> ?", Group::AUTO_GROUPS[:everyone]) + groups = Group.order(:name).where("groups.id <> ?", Group::AUTO_GROUPS[:everyone]) if search = params[:search].to_s groups = groups.where("name ILIKE ?", "%#{search}%") diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index b4024a280ad..a39bdb3c98b 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -1,10 +1,10 @@ class GroupsController < ApplicationController - before_filter :ensure_logged_in, only: [:set_notifications] + before_filter :ensure_logged_in, only: [:set_notifications, :mentionable] skip_before_filter :preload_json, :check_xhr, only: [:posts_feed, :mentions_feed] def show - render_serialized(find_group(:id), BasicGroupSerializer) + render_serialized(find_group(:id), GroupShowSerializer, root: 'basic_group') end def counts @@ -120,6 +120,16 @@ class GroupsController < ApplicationController end end + def mentionable + group = find_group(:name) + + if group + render json: { mentionable: Group.mentionable(current_user).where(id: group.id).present? } + else + raise Discourse::InvalidAccess.new + end + end + def remove_member group = Group.find(params[:id]) guardian.ensure_can_edit!(group) diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 463c830e3ab..a73541a3fb9 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -169,7 +169,6 @@ class SessionController < ApplicationController login = params[:login].strip login = login[1..-1] if login[0] == "@" - if user = User.find_by_username_or_email(login) # If their password is correct diff --git a/app/models/group.rb b/app/models/group.rb index dcb2b6e9e6f..9d488ac7622 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -381,10 +381,6 @@ class Group < ActiveRecord::Base true end - def mentionable?(user, group_id) - Group.mentionable(user).where(id: group_id).exists? - end - def staff? STAFF_GROUPS.include?(self.name.to_sym) end diff --git a/app/serializers/basic_group_serializer.rb b/app/serializers/basic_group_serializer.rb index 7844e4f5121..783fdafb941 100644 --- a/app/serializers/basic_group_serializer.rb +++ b/app/serializers/basic_group_serializer.rb @@ -11,10 +11,7 @@ class BasicGroupSerializer < ApplicationSerializer :title, :grant_trust_level, :incoming_email, - :notification_level, :has_messages, - :is_member, - :mentionable, :flair_url, :flair_bg_color, :flair_color @@ -22,30 +19,4 @@ class BasicGroupSerializer < ApplicationSerializer def include_incoming_email? scope.is_staff? end - - def notification_level - fetch_group_user&.notification_level - end - - def include_notification_level? - scope.authenticated? - end - - def mentionable - object.mentionable?(scope.user, object.id) - end - - def is_member - scope.is_admin? || fetch_group_user.present? - end - - def include_is_member? - scope.authenticated? - end - - private - - def fetch_group_user - @group_user ||= object.group_users.find_by(user_id: scope.user.id) - end end diff --git a/app/serializers/basic_group_user_serializer.rb b/app/serializers/basic_group_user_serializer.rb new file mode 100644 index 00000000000..fa2015b8c4f --- /dev/null +++ b/app/serializers/basic_group_user_serializer.rb @@ -0,0 +1,3 @@ +class BasicGroupUserSerializer < ApplicationSerializer + attributes :group_id, :user_id, :notification_level +end diff --git a/app/serializers/group_show_serializer.rb b/app/serializers/group_show_serializer.rb new file mode 100644 index 00000000000..58358c1bac1 --- /dev/null +++ b/app/serializers/group_show_serializer.rb @@ -0,0 +1,11 @@ +class GroupShowSerializer < BasicGroupSerializer + attributes :is_group_user + + def include_is_group_user? + scope.authenticated? + end + + def is_group_user + object.users.include?(scope.user) + end +end diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 4017061a46a..e7f6073ea3a 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -73,6 +73,7 @@ class UserSerializer < BasicUserSerializer has_one :invited_by, embed: :object, serializer: BasicUserSerializer has_many :groups, embed: :object, serializer: BasicGroupSerializer + has_many :group_users, embed: :object, serializer: BasicGroupUserSerializer has_many :featured_user_badges, embed: :ids, serializer: UserBadgeSerializer, root: :user_badges has_one :card_badge, embed: :object, serializer: BadgeSerializer has_one :user_option, embed: :object, serializer: UserOptionSerializer @@ -127,13 +128,19 @@ class UserSerializer < BasicUserSerializer end def groups + groups = object.groups.order(:id) + if scope.is_admin? || object.id == scope.user.try(:id) - object.groups + groups else - object.groups.where(visible: true) + groups.where(visible: true) end end + def group_users + object.group_users.order(:group_id) + end + def include_email? object.id && object.id == scope.user.try(:id) end diff --git a/config/routes.rb b/config/routes.rb index 06f799640bc..f465e56a380 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -398,7 +398,7 @@ Discourse::Application.routes.draw do get "posts/:username/flagged" => "posts#flagged_posts", constraints: {username: USERNAME_ROUTE_FORMAT} get "groups/:id.json" => 'groups#show', constraints: {id: USERNAME_ROUTE_FORMAT}, defaults: {format: 'json'} - + resources :groups, id: USERNAME_ROUTE_FORMAT do get "posts.rss" => "groups#posts_feed", format: :rss get "mentions.rss" => "groups#mentions_feed", format: :rss @@ -409,6 +409,7 @@ Discourse::Application.routes.draw do get 'mentions' get 'messages' get 'counts' + get 'mentionable' member do put "members" => "groups#add_members" diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 46f428400a3..6971184dd4a 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -34,10 +34,7 @@ describe Admin::GroupsController do "primary_group"=>false, "grant_trust_level"=>nil, "incoming_email"=>nil, - "notification_level"=>2, "has_messages"=>false, - "is_member"=>true, - "mentionable"=>false, "flair_url"=>nil, "flair_bg_color"=>nil, "flair_color"=>nil diff --git a/spec/fabricators/email_token_fabricator.rb b/spec/fabricators/email_token_fabricator.rb new file mode 100644 index 00000000000..95738b8cfb4 --- /dev/null +++ b/spec/fabricators/email_token_fabricator.rb @@ -0,0 +1,4 @@ +Fabricator(:email_token) do + user + email { |attrs| attrs[:user].email } +end diff --git a/spec/integration/groups_spec.rb b/spec/integration/groups_spec.rb new file mode 100644 index 00000000000..396d7df9f41 --- /dev/null +++ b/spec/integration/groups_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +describe "Groups" do + describe "checking if a group can be mentioned" do + let(:password) { 'somecomplicatedpassword' } + let(:email_token) { Fabricate(:email_token, confirmed: true) } + let(:user) { email_token.user } + let(:group) { Fabricate(:group, name: 'test', users: [user]) } + + before do + user.update_attributes!(password: password) + end + + it "should return the right response" do + group + + post "/session.json", { login: user.username, password: password } + expect(response).to be_success + + get "/groups/test/mentionable.json", { name: group.name } + + expect(response).to be_success + + response_body = JSON.parse(response.body) + expect(response_body["mentionable"]).to eq(false) + + group.update_attributes!(alias_level: Group::ALIAS_LEVELS[:everyone]) + + get "/groups/test/mentionable.json", { name: group.name } + expect(response).to be_success + + response_body = JSON.parse(response.body) + expect(response_body["mentionable"]).to eq(true) + end + end +end diff --git a/test/javascripts/acceptance/groups-test.js.es6 b/test/javascripts/acceptance/groups-test.js.es6 index 1e4ecf08cf4..0fcc909999d 100644 --- a/test/javascripts/acceptance/groups-test.js.es6 +++ b/test/javascripts/acceptance/groups-test.js.es6 @@ -1,4 +1,5 @@ -import { acceptance } from "helpers/qunit-helpers"; +import { acceptance, logIn } from "helpers/qunit-helpers"; + acceptance("Groups"); test("Browsing Groups", () => { @@ -24,6 +25,18 @@ test("Browsing Groups", () => { visit("/groups/discourse/messages"); andThen(() => { + ok($('.action-list li').length === 4, 'it should not show messages tab'); ok(count('.user-stream .item') > 0, "it lists stream items"); }); }); + +test("Messages tab", () => { + logIn(); + Discourse.reset(); + + visit("/groups/discourse"); + + andThen(() => { + ok($('.action-list li').length === 5, 'it should show messages tab if user is admin'); + }); +});