diff --git a/app/assets/javascripts/discourse/components/group-navigation.js.es6 b/app/assets/javascripts/discourse/components/group-navigation.js.es6 index 35f5f238e47..e164f8c987a 100644 --- a/app/assets/javascripts/discourse/components/group-navigation.js.es6 +++ b/app/assets/javascripts/discourse/components/group-navigation.js.es6 @@ -1,15 +1,3 @@ -import computed from 'ember-addons/ember-computed-decorators'; - export default Ember.Component.extend({ tagName: '', - - @computed('group') - availableTabs(group) { - return this.get('tabs').filter(t => { - if (t.admin) { - return this.currentUser ? this.currentUser.canManageGroup(group) : false; - } - return true; - }); - } }); diff --git a/app/assets/javascripts/discourse/controllers/group-activity.js.es6 b/app/assets/javascripts/discourse/controllers/group-activity.js.es6 index 2722bebdd30..2e85014b8e3 100644 --- a/app/assets/javascripts/discourse/controllers/group-activity.js.es6 +++ b/app/assets/javascripts/discourse/controllers/group-activity.js.es6 @@ -1,14 +1,4 @@ -import computed from 'ember-addons/ember-computed-decorators'; - export default Ember.Controller.extend({ application: Ember.inject.controller(), queryParams: ['category_id'], - - @computed('model.is_group_user') - showGroupMessages(isGroupUser) { - if (!this.siteSettings.enable_personal_messages) { - return false; - } - return isGroupUser || (this.currentUser && this.currentUser.admin); - } }); diff --git a/app/assets/javascripts/discourse/controllers/group.js.es6 b/app/assets/javascripts/discourse/controllers/group.js.es6 index 98e4687f893..3cd976b59a9 100644 --- a/app/assets/javascripts/discourse/controllers/group.js.es6 +++ b/app/assets/javascripts/discourse/controllers/group.js.es6 @@ -1,4 +1,4 @@ -import { default as computed, observes } from 'ember-addons/ember-computed-decorators'; +import { default as computed } from 'ember-addons/ember-computed-decorators'; const Tab = Ember.Object.extend({ init() { @@ -14,16 +14,49 @@ export default Ember.Controller.extend({ counts: null, showing: 'members', - tabs: [ - Tab.create({ name: 'members', route: 'group.index', icon: 'users' }), - Tab.create({ name: 'activity' }), - Tab.create({ - name: 'edit', i18nKey: 'edit.title', icon: 'pencil', admin: true - }), - Tab.create({ - name: 'logs', i18nKey: 'logs.title', icon: 'list-alt', admin: true - }) - ], + @computed('showMessages', 'model.user_count') + tabs(showMessages, userCount) { + const membersTab = Tab.create({ + name: 'members', + route: 'group.index', + icon: 'users' + }); + + membersTab.set('count', userCount); + + const defaultTabs = [ + membersTab, + Tab.create({ name: 'activity' }) + ]; + + if (showMessages) { + defaultTabs.push(Tab.create({ + name: 'messages.inbox', i18nKey: 'messages' + })); + } + + if (this.currentUser && this.currentUser.canManageGroup(this.model)) { + defaultTabs.push(...[ + Tab.create({ + name: 'edit', i18nKey: 'edit.title', icon: 'pencil' + }), + Tab.create({ + name: 'logs', i18nKey: 'logs.title', icon: 'list-alt' + }) + ]); + } + + return defaultTabs; + }, + + @computed('model.is_group_user') + showMessages(isGroupUser) { + if (!this.siteSettings.enable_personal_messages) { + return false; + } + + return isGroupUser || (this.currentUser && this.currentUser.admin); + }, @computed('model.is_group_owner', 'model.automatic') canEditGroup(isGroupOwner, automatic) { @@ -50,11 +83,6 @@ export default Ember.Controller.extend({ return this.currentUser && messageable; }, - @observes('model.user_count') - _setMembersTabCount() { - this.get('tabs')[0].set('count', this.get('model.user_count')); - }, - actions: { messageGroup() { this.send('createNewMessageViaParams', this.get('model.name')); diff --git a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 index ee447923aa5..740952f0f94 100644 --- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 +++ b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 @@ -56,11 +56,15 @@ export default function() { this.route('posts'); this.route('topics'); this.route('mentions'); - this.route('messages'); }); this.route('logs'); this.route('edit'); + + this.route('messages', function() { + this.route('inbox'); + this.route('archive'); + }); }); // User routes diff --git a/app/assets/javascripts/discourse/routes/build-group-messages-route.js.es6 b/app/assets/javascripts/discourse/routes/build-group-messages-route.js.es6 new file mode 100644 index 00000000000..460134ee8de --- /dev/null +++ b/app/assets/javascripts/discourse/routes/build-group-messages-route.js.es6 @@ -0,0 +1,35 @@ +import UserTopicListRoute from "discourse/routes/user-topic-list"; + +export default (type) => { + return UserTopicListRoute.extend({ + titleToken() { + return I18n.t(`user.messages.${type}`); + }, + + model() { + const groupName = this.modelFor('group').get('name'); + const username = this.currentUser.get("username_lower"); + let filter = `topics/private-messages-group/${username}/${groupName}`; + if (this._isArchive()) filter = `${filter}/archive`; + return this.store.findFiltered("topicList", { filter }); + }, + + setupController() { + this._super.apply(this, arguments); + + const groupName = this.modelFor('group').get('name'); + let channel = `/private-messages/group/${groupName}`; + if (this._isArchive()) channel = `${channel}/archive`; + this.controllerFor("user-topics-list").subscribe(channel); + + this.controllerFor("user-topics-list").setProperties({ + hideCategory: true, + showPosters: true + }); + }, + + _isArchive() { + return type === 'archive'; + }, + }); +}; diff --git a/app/assets/javascripts/discourse/routes/group-activity-messages.js.es6 b/app/assets/javascripts/discourse/routes/group-activity-messages.js.es6 deleted file mode 100644 index 660a5c7cd5f..00000000000 --- a/app/assets/javascripts/discourse/routes/group-activity-messages.js.es6 +++ /dev/null @@ -1,3 +0,0 @@ -import { buildGroupPage } from 'discourse/routes/group-activity-posts'; - -export default buildGroupPage('messages'); diff --git a/app/assets/javascripts/discourse/routes/group-edit.js.es6 b/app/assets/javascripts/discourse/routes/group-edit.js.es6 index 19b8ec1caee..7f3dc02cec5 100644 --- a/app/assets/javascripts/discourse/routes/group-edit.js.es6 +++ b/app/assets/javascripts/discourse/routes/group-edit.js.es6 @@ -1,4 +1,4 @@ -export default Ember.Route.extend({ +export default Discourse.Route.extend({ titleToken() { return I18n.t('groups.edit.title'); }, diff --git a/app/assets/javascripts/discourse/routes/group-logs.js.es6 b/app/assets/javascripts/discourse/routes/group-logs.js.es6 index 0d365fab188..9658160fee3 100644 --- a/app/assets/javascripts/discourse/routes/group-logs.js.es6 +++ b/app/assets/javascripts/discourse/routes/group-logs.js.es6 @@ -1,6 +1,6 @@ -export default Ember.Route.extend({ +export default Discourse.Route.extend({ titleToken() { - return I18n.t('groups.logs'); + return I18n.t('groups.logs.title'); }, model() { diff --git a/app/assets/javascripts/discourse/routes/group-messages-archive.js.es6 b/app/assets/javascripts/discourse/routes/group-messages-archive.js.es6 new file mode 100644 index 00000000000..4920aa20005 --- /dev/null +++ b/app/assets/javascripts/discourse/routes/group-messages-archive.js.es6 @@ -0,0 +1,3 @@ +import buildGroupMessagesRoute from 'discourse/routes/build-group-messages-route'; + +export default buildGroupMessagesRoute('archive'); diff --git a/app/assets/javascripts/discourse/routes/group-messages-inbox.js.es6 b/app/assets/javascripts/discourse/routes/group-messages-inbox.js.es6 new file mode 100644 index 00000000000..03ad513b5d2 --- /dev/null +++ b/app/assets/javascripts/discourse/routes/group-messages-inbox.js.es6 @@ -0,0 +1,3 @@ +import buildGroupMessagesRoute from 'discourse/routes/build-group-messages-route'; + +export default buildGroupMessagesRoute('inbox'); diff --git a/app/assets/javascripts/discourse/routes/group-messages.js.es6 b/app/assets/javascripts/discourse/routes/group-messages.js.es6 new file mode 100644 index 00000000000..b93bb52d201 --- /dev/null +++ b/app/assets/javascripts/discourse/routes/group-messages.js.es6 @@ -0,0 +1,5 @@ +export default Discourse.Route.extend({ + titleToken() { + return I18n.t('groups.messages'); + }, +}); diff --git a/app/assets/javascripts/discourse/templates/components/group-navigation.hbs b/app/assets/javascripts/discourse/templates/components/group-navigation.hbs index 8501e9e2f54..b97cdfa040a 100644 --- a/app/assets/javascripts/discourse/templates/components/group-navigation.hbs +++ b/app/assets/javascripts/discourse/templates/components/group-navigation.hbs @@ -1,5 +1,5 @@ {{#mobile-nav class='group-nav' desktopClass="nav nav-pills" currentPath=currentPath}} - {{#each availableTabs as |tab|}} + {{#each tabs as |tab|}}
  • {{#link-to tab.route group title=tab.message class=tab.name}} {{#if tab.icon}}{{d-icon tab.icon}}{{/if}} diff --git a/app/assets/javascripts/discourse/templates/group/activity.hbs b/app/assets/javascripts/discourse/templates/group/activity.hbs index 50798a89d7c..957c08e9398 100644 --- a/app/assets/javascripts/discourse/templates/group/activity.hbs +++ b/app/assets/javascripts/discourse/templates/group/activity.hbs @@ -5,9 +5,6 @@ {{#if siteSettings.enable_mentions}} {{group-activity-filter filter="mentions" categoryId=category_id}} {{/if}} - {{#if showGroupMessages}} - {{group-activity-filter filter="messages"}} - {{/if}} {{plugin-outlet name="below-group-activity-nav" tagName='' connectorTagName='li'}} {{/mobile-nav}} diff --git a/app/assets/javascripts/discourse/templates/group/messages.hbs b/app/assets/javascripts/discourse/templates/group/messages.hbs new file mode 100644 index 00000000000..658ee7a67bb --- /dev/null +++ b/app/assets/javascripts/discourse/templates/group/messages.hbs @@ -0,0 +1,22 @@ +
    +
    + {{#d-section class="group-messages-navigation" pageClass="user-messages"}} + {{#mobile-nav class='messages-nav' desktopClass='nav-stacked action-list' currentPath=currentPath}} +
  • + {{#link-to 'group.messages.inbox' model.name}} + {{i18n 'user.messages.inbox'}} + {{/link-to}} +
  • +
  • + {{#link-to 'group.messages.archive' model.name}} + {{i18n 'user.messages.archive'}} + {{/link-to}} +
  • + {{/mobile-nav}} + {{/d-section}} + +
    + {{outlet}} +
    + + diff --git a/app/assets/stylesheets/desktop/user.scss b/app/assets/stylesheets/desktop/user.scss index a7ac7803c32..78edd4385eb 100644 --- a/app/assets/stylesheets/desktop/user.scss +++ b/app/assets/stylesheets/desktop/user.scss @@ -31,7 +31,7 @@ margin-top: 18px; } -.user-table { +.user-table, .group-table { width: 100%; display: table; table-layout: fixed; diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index f4881391381..9f48cc843ad 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -5,7 +5,6 @@ class GroupsController < ApplicationController :mentionable, :messageable, :update, - :messages, :histories, :request_membership, :search @@ -124,19 +123,6 @@ class GroupsController < ApplicationController render 'posts/latest', formats: [:rss] end - def messages - group = find_group(:group_id) - posts = if guardian.can_see_group_messages?(group) - group.messages_for( - guardian, - params.permit(:before_post_id, :category_id) - ).where(post_number: 1).limit(20).to_a - else - [] - end - render_serialized posts, GroupPostSerializer - end - def members group = find_group(:group_id) diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb index 96e7d749d69..61b3fab8710 100644 --- a/app/models/topic_tracking_state.rb +++ b/app/models/topic_tracking_state.rb @@ -257,7 +257,9 @@ SQL if post && allowed_user_ids.include?(post.user_id) channels["/private-messages/sent"] = [post.user_id] - elsif archive_user_id + end + + if archive_user_id user_ids = [archive_user_id] [ @@ -267,19 +269,21 @@ SQL ].each do |channel| channels[channel] = user_ids end - else - topic.allowed_groups.each do |group| - group_channels = [] - group_channels << "/private-messages/group/#{group.name.downcase}" - group_channels << "#{group_channels.first}/archive" if group_archive - group_channels.each { |channel| channels[channel] = group.users.pluck(:id) } - end end if channels.except("/private-messages/sent").blank? channels["/private-messages/inbox"] = allowed_user_ids end + topic.allowed_groups.each do |group| + group_user_ids = group.users.pluck(:id) + next if group_user_ids.blank? + group_channels = [] + group_channels << "/private-messages/group/#{group.name.downcase}" + group_channels << "#{group_channels.first}/archive" if group_archive + group_channels.each { |channel| channels[channel] = group_user_ids } + end + message = { topic_id: topic.id } diff --git a/config/routes.rb b/config/routes.rb index bc091d4a9f5..f1dfc801bb6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -456,7 +456,6 @@ Discourse::Application.routes.draw do get 'members' get 'posts' get 'mentions' - get 'messages' get 'counts' get 'mentionable' get 'messageable' @@ -467,8 +466,16 @@ Discourse::Application.routes.draw do end member do - get 'activity' => "groups#show" - get 'activity/:filter' => "groups#show" + %w{ + activity + activity/:filter + messages + messages/inbox + messages/archive + }.each do |path| + get path => 'groups#show' + end + put "members" => "groups#add_members" delete "members" => "groups#remove_member" post "request_membership" => "groups#request_membership" diff --git a/spec/models/topic_tracking_state_spec.rb b/spec/models/topic_tracking_state_spec.rb index 720657a77b7..6b35ff55a69 100644 --- a/spec/models/topic_tracking_state_spec.rb +++ b/spec/models/topic_tracking_state_spec.rb @@ -104,11 +104,16 @@ describe TopicTrackingState do it "should publish the right message" do messages = MessageBus.track_publish do TopicTrackingState.publish_private_message( - private_message_topic, + private_message_topic ) end - expect(messages.count).to eq(2) + expect(messages.count).to eq(3) + + message = messages.first + expect(message.channel).to eq('/private-messages/inbox') + expect(message.data["topic_id"]).to eq(private_message_topic.id) + expect(message.user_ids).to eq(private_message_topic.allowed_users.map(&:id)) [group1, group2].each do |group| message = messages.find do |message| @@ -129,7 +134,12 @@ describe TopicTrackingState do ) end - expect(messages.count).to eq(4) + expect(messages.count).to eq(5) + + message = messages.first + expect(message.channel).to eq('/private-messages/inbox') + expect(message.data["topic_id"]).to eq(private_message_topic.id) + expect(message.user_ids).to eq(private_message_topic.allowed_users.map(&:id)) [group1, group2].each do |group| group_channel = "/private-messages/group/#{group.name}" @@ -160,6 +170,12 @@ describe TopicTrackingState do ) end + let!(:group) do + group = Fabricate(:group, users: [Fabricate(:user)]) + private_message_topic.allowed_groups << group + group + end + it 'should publish the right message' do messages = MessageBus.track_publish do TopicTrackingState.publish_private_message( @@ -168,11 +184,12 @@ describe TopicTrackingState do ) end - expect(messages.count).to eq(2) + expect(messages.count).to eq(3) [ ['/private-messages/inbox', private_message_topic.allowed_users.map(&:id)], - ['/private-messages/sent', [user.id]] + ['/private-messages/sent', [user.id]], + ["/private-messages/group/#{group.name}", [group.users.first.id]] ].each do |channel, user_ids| message = messages.find do |message| message.channel == channel diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index f4862827deb..288d49cd293 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' RSpec.describe ListController do let(:topic) { Fabricate(:topic) } + let(:group) { Fabricate(:group) } describe '#index' do it "doesn't throw an error with a negative page" do @@ -109,9 +110,33 @@ RSpec.describe ListController do end end - describe '#group_topics' do - let(:group) { Fabricate(:group) } + describe '#private_messages_group' do + let(:user) do + user = Fabricate(:user) + group.add(user) + sign_in(user) + user + end + let!(:topic) do + Fabricate(:private_message_topic, + allowed_groups: [group], + ) + end + + let(:private_post) { Fabricate(:post, topic: topic) } + + it 'should return the right response' do + get "/topics/private-messages-group/#{user.username}/#{group.name}.json" + + expect(response.status).to eq(200) + + expect(JSON.parse(response.body)["topic_list"]["topics"].first["id"]) + .to eq(topic.id) + end + end + + describe '#group_topics' do %i{user user2}.each do |user| let(user) do user = Fabricate(:user) diff --git a/test/javascripts/acceptance/groups-test.js.es6 b/test/javascripts/acceptance/groups-test.js.es6 index 12298b6882e..19106a1ace6 100644 --- a/test/javascripts/acceptance/groups-test.js.es6 +++ b/test/javascripts/acceptance/groups-test.js.es6 @@ -1,6 +1,29 @@ import { acceptance, logIn } from "helpers/qunit-helpers"; -acceptance("Groups"); +const response = object => { + return [ + 200, + { "Content-Type": "application/json" }, + object + ]; +}; + +acceptance("Groups", { + beforeEach() { + server.get('/groups/snorlax.json', () => { // eslint-disable-line no-undef + return response({"basic_group":{"id":41,"automatic":false,"name":"snorlax","user_count":1,"alias_level":0,"visible":true,"automatic_membership_email_domains":"","automatic_membership_retroactive":false,"primary_group":true,"title":"Team Snorlax","grant_trust_level":null,"incoming_email":null,"has_messages":false,"flair_url":"","flair_bg_color":"","flair_color":"","bio_raw":"","bio_cooked":null,"public":true,"is_group_user":true,"is_group_owner":true}}); + }); + + // Workaround while awaiting https://github.com/tildeio/route-recognizer/issues/53 + server.get('/groups/snorlax/logs.json', request => { // eslint-disable-line no-undef + if (request.queryParams["filters[action]"]) { + return response({"logs":[{"action":"change_group_setting","subject":"title","prev_value":null,"new_value":"Team Snorlax","created_at":"2016-12-12T08:27:46.408Z","acting_user":{"id":1,"username":"tgx","avatar_template":"/images/avatar.png"},"target_user":null}],"all_loaded":true}); + } else { + return response({"logs":[{"action":"change_group_setting","subject":"title","prev_value":null,"new_value":"Team Snorlax","created_at":"2016-12-12T08:27:46.408Z","acting_user":{"id":1,"username":"tgx","avatar_template":"/images/avatar.png"},"target_user":null},{"action":"add_user_to_group","subject":null,"prev_value":null,"new_value":null,"created_at":"2016-12-12T08:27:27.725Z","acting_user":{"id":1,"username":"tgx","avatar_template":"/images/avatar.png"},"target_user":{"id":1,"username":"tgx","avatar_template":"/images/avatar.png"}}],"all_loaded":true}); + } + }); + } +}); QUnit.test("Browsing Groups", assert => { visit("/groups"); @@ -49,6 +72,12 @@ QUnit.test("Anonymous Viewing Group", assert => { assert.ok(count('.avatar-flair .fa-adjust') === 1, "it displays the group's avatar flair"); assert.ok(count('.group-members tr') > 0, "it lists group members"); assert.ok(count('.group-message-button') === 0, 'it does not show group message button'); + + assert.equal( + count(".nav-pills li a[title='Messages']"), + 0, + 'it deos not show group messages navigation link' + ); }); click(".nav-pills li a[title='Activity']"); @@ -71,11 +100,6 @@ QUnit.test("Anonymous Viewing Group", assert => { }); andThen(() => { - assert.equal( - find(".group-activity li a[href='/groups/discourse/activity/messages']").length, - 0, - 'it should not show messages tab if user is not a group user or admin' - ); assert.ok(find(".nav-pills li a[title='Edit Group']").length === 0, 'it should not show messages tab if user is not admin'); assert.ok(find(".nav-pills li a[title='Logs']").length === 0, 'it should not show Logs tab if user is not admin'); assert.ok(count('.group-post') > 0, "it lists stream items"); @@ -124,6 +148,48 @@ QUnit.test("User Viewing Group", assert => { }); }); +QUnit.test("Admin viewing group messages when there are no messages", assert => { + server.get('/topics/private-messages-group/eviltrout/discourse.json', () => { // eslint-disable-line no-undef + return response({ topic_list: { topics: [] } }); + }); + + logIn(); + Discourse.reset(); + + visit("/groups/discourse"); + + click(".nav-pills li a[title='Messages']"); + + andThen(() => { + assert.equal( + find(".alert").text().trim(), + I18n.t('choose_topic.none_found'), + 'it should display the right alert' + ); + }); +}); + +QUnit.test("Admin viewing group messages", assert => { + server.get('/topics/private-messages-group/eviltrout/discourse.json', () => { // eslint-disable-line no-undef + return response({"users":[{"id":2, "username":"bruce1", "avatar_template":"/letter_avatar_proxy/v2/letter/b/9de053/{size}.png"}, {"id":3, "username":"CodingHorror", "avatar_template":"/letter_avatar_proxy/v2/letter/c/e8c25b/{size}.png"}], "primary_groups":[], "topic_list":{"can_create_topic":true, "draft":null, "draft_key":"new_topic", "draft_sequence":0, "per_page":30, "topics":[{"id":12199, "title":"This is a private message 1", "fancy_title":"This is a private message 1", "slug":"this-is-a-private-message-1", "posts_count":0, "reply_count":0, "highest_post_number":0, "image_url":null, "created_at":"2018-03-16T03:38:45.583Z", "last_posted_at":null, "bumped":true, "bumped_at":"2018-03-16T03:38:45.583Z", "unseen":false, "pinned":false, "unpinned":null, "visible":true, "closed":false, "archived":false, "bookmarked":null, "liked":null, "views":0, "like_count":0, "has_summary":false, "archetype":"private_message", "last_poster_username":"bruce1", "category_id":null, "pinned_globally":false, "featured_link":null, "posters":[{"extras":"latest single", "description":"Original Poster, Most Recent Poster", "user_id":2, "primary_group_id":null}], "participants":[{"extras":"latest", "description":null, "user_id":2, "primary_group_id":null}, {"extras":null, "description":null, "user_id":3, "primary_group_id":null}]}]}}); + }); + + logIn(); + Discourse.reset(); + + visit("/groups/discourse"); + + click(".nav-pills li a[title='Messages']"); + + andThen(() => { + assert.equal( + find(".topic-list-item .link-top-line").text().trim(), + "This is a private message 1", + 'it should display the list of group topics' + ); + }); +}); + QUnit.test("Admin Viewing Group", assert => { logIn(); Discourse.reset(); @@ -136,14 +202,4 @@ QUnit.test("Admin Viewing Group", assert => { assert.equal(count('.group-message-button'), 1, 'it displays show group message button'); assert.equal(find('.group-info-name').text(), 'Awesome Team', 'it should display the group name'); }); - - click(".nav-pills li a[title='Activity']"); - - andThen(() => { - assert.equal( - find(".group-activity li a[href='/groups/discourse/activity/messages']").length, - 1, - 'it should show messages tab if user is admin' - ); - }); });