Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Put back announcement channel user listing #11518

Merged
merged 3 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions app/Http/Controllers/Chat/Channels/UsersController.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the GNU Affero General Public License v3.0.
// See the LICENCE file in the repository root for full licence text.

namespace App\Http\Controllers\Chat\Channels;

use App\Http\Controllers\Chat\Controller;
use App\Models\Chat\Channel;
use App\Models\Chat\UserChannel;
use App\Models\User;
use App\Transformers\UserCompactTransformer;

class UsersController extends Controller
{
public function index($channelId)
{
$channel = Channel::findOrFail($channelId);

if (!priv_check('ChatChannelListUsers', $channel)->can()) {
return [
'users' => [],
...cursor_for_response(null),
];
}

$channel = Channel::findOrFail($channelId);
$cursorHelper = UserChannel::makeDbCursorHelper();
[$userChannels, $hasMore] = $channel
->userChannels()
->select('user_id')
->limit(UserChannel::PER_PAGE)
->cursorSort($cursorHelper, cursor_from_params(\Request::all()))
->getWithHasMore();
$users = User
::with(UserCompactTransformer::CARD_INCLUDES_PRELOAD)
->find($userChannels->pluck('user_id'));

return [
'users' => json_collection(
$users,
new UserCompactTransformer(),
UserCompactTransformer::CARD_INCLUDES,
),
...cursor_for_response($cursorHelper->next($userChannels, $hasMore)),
];
}
}
9 changes: 9 additions & 0 deletions app/Models/Chat/UserChannel.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
namespace App\Models\Chat;

use App\Libraries\Notification\BatchIdentities;
use App\Models\Traits\WithDbCursorHelper;
use App\Models\User;
use App\Models\UserNotification;
use DB;
Expand All @@ -20,6 +21,14 @@
*/
class UserChannel extends Model
{
use WithDbCursorHelper;

const DEFAULT_SORT = 'user_id_asc';

const SORTS = [
'user_id_asc' => [['column' => 'user_id', 'order' => 'ASC']],
];

public $incrementing = false;
public $timestamps = false;

Expand Down
9 changes: 9 additions & 0 deletions app/Singletons/OsuAuthorize.php
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,15 @@ public function checkChatChannelCanMessage(?User $user, Channel $channel): strin
return 'ok';
}

public function checkChatChannelListUsers(?User $user, Channel $channel): ?string
{
if ($channel->isAnnouncement() && $this->doCheckUser($user, 'ChatAnnounce', $channel)->can()) {
return 'ok';
}

return null;
}

/**
* TODO: always use a channel for this check?
*
Expand Down
1 change: 1 addition & 0 deletions app/Transformers/Chat/ChannelTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public function includeCurrentUserAttributes(Channel $channel)
$result = $channel->checkCanMessage($this->user);

return $this->primitive([
'can_list_users' => priv_check_user($this->user, 'ChatChannelListUsers', $channel)->can(),
'can_message' => $result->can(),
'can_message_error' => $result->message(),
'last_read_id' => $channel->lastReadIdFor($this->user),
Expand Down
16 changes: 15 additions & 1 deletion resources/css/bem/chat-conversation.less
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@
margin-top: 10px;
}

&__more-users {
width: 100%;
display: flex;
justify-content: center;
}

&__unread-marker {
border-bottom: 1px solid @osu-colour-h1;
color: @osu-colour-h1;
Expand Down Expand Up @@ -106,10 +112,18 @@
gap: 2px;
align-content: center;
justify-content: center;
margin: 10px;

&--loading {
gap: 5px;
}
}

&__users-container {
.default-border-radius();
background-color: hsl(var(--hsl-b5));
padding: 5px;
display: grid;
gap: 5px;
margin: 10px;
}
}
12 changes: 12 additions & 0 deletions resources/js/chat/chat-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ interface GetChannelResponse {
users: UserJson[];
}

interface GetChannelUsersResponse {
cursor_string: null | string;
users: UserJson[];
}

interface GetMessagesResponse {
messages: MessageJson[];
users: UserJson[];
Expand Down Expand Up @@ -56,6 +61,13 @@ export function getChannel(channelId: number) {
}));
}

export function getChannelUsers(channelId: number, cursor: string) {
return $.get(route('chat.channels.users.index', {
channel: channelId,
cursor_string: cursor,
})) as JQuery.jqXHR<GetChannelUsersResponse>;
}

export function getMessages(channelId: number, params?: { until?: number }) {
const request = $.get(route('chat.channels.messages.index', { channel: channelId, return_object: 1, ...params })) as JQuery.jqXHR<GetMessagesResponse>;

Expand Down
35 changes: 26 additions & 9 deletions resources/js/chat/conversation-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -259,16 +259,33 @@ export default class ConversationView extends React.Component<Props> {
renderUsers() {
if (this.currentChannel?.type !== 'ANNOUNCE') return null;

const users = this.currentChannel.users;

if (users != null && users.length === 0) {
return null;
}

return (
<div className={classWithModifiers('chat-conversation__users', { loading: this.currentChannel.announcementUsers == null })}>
{this.currentChannel.announcementUsers == null ? (
<>
<Spinner modifiers='self-center' /><span>{trans('chat.loading_users')}</span>
</>
) : (
this.currentChannel.announcementUsers.map((user) => (
<UserCardBrick key={user.id} user={user.toJson()} />
))
<div className='chat-conversation__users-container'>
<div className={classWithModifiers('chat-conversation__users', { loading: users == null })}>
{users == null ? (
<>
<Spinner modifiers='self-center' /><span>{trans('chat.loading_users')}</span>
</>
Comment on lines +271 to +274
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just skip trying to load users if they don't have permission to view it anyway.
Or maybe return users: [] with the channel if they have no permission to load users and null otherwise 🤔 (or both)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into that... eventually...

probably not changing existing json fields though.

) : (
users.map((user) => (
<UserCardBrick key={user.id} user={user} />
))
)}
</div>
{users != null && this.currentChannel.usersCursor != null && (
<div className='chat-conversation__more-users'>
<ShowMoreLink
callback={this.currentChannel.loadUsers}
hasMore
loading={this.currentChannel.loadUsersXhr != null}
/>
</div>
)}
</div>
);
Expand Down
1 change: 1 addition & 0 deletions resources/js/interfaces/chat/channel-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export function filterSupportedChannelTypes(json: ChannelJson[]) {
export default interface ChannelJson {
channel_id: number;
current_user_attributes?: {
can_list_users: boolean;
can_message: boolean;
can_message_error: string | null;
last_read_id: number | null;
Expand Down
48 changes: 30 additions & 18 deletions resources/js/models/chat/channel.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the GNU Affero General Public License v3.0.
// See the LICENCE file in the repository root for full licence text.

import { markAsRead, getChannel, getMessages } from 'chat/chat-api';
import { markAsRead, getChannel, getChannelUsers, getMessages } from 'chat/chat-api';
import ChannelJson, { ChannelType, SupportedChannelType, supportedTypeLookup } from 'interfaces/chat/channel-json';
import MessageJson from 'interfaces/chat/message-json';
import UserJson from 'interfaces/user-json';
import { sortBy, throttle } from 'lodash';
import { action, computed, makeObservable, observable, runInAction } from 'mobx';
import User, { usernameSortAscending } from 'models/user';
import User from 'models/user';
import core from 'osu-core-singleton';
import Message from './message';

Expand All @@ -29,6 +30,7 @@ function getMinMessageIdFrom(messages: Message[]) {
export default class Channel {
private static readonly defaultIcon = '/images/layout/chat/channel-default.png'; // TODO: update with channel-specific icons?

@observable canListUsers: boolean = false;
@observable canMessageError: string | null = null;
@observable channelId: number;
@observable description?: string;
Expand All @@ -38,6 +40,7 @@ export default class Channel {
@observable lastReadId?: number;
@observable loadingEarlierMessages = false;
@observable loadingMessages = false;
@observable loadUsersXhr: ReturnType<typeof getChannelUsers> | undefined;
@observable messageLengthLimit = maxMessageLength;
@observable name = '';
needsRefresh = true;
Expand All @@ -49,21 +52,12 @@ export default class Channel {
scrollY: 0,
};
@observable userIds: number[] = [];
@observable users: null | UserJson[] = null;
@observable usersCursor: null | string = '';

private markAsReadLastSent = 0;
@observable private readonly messagesMap = new Map<number | string, Message>();
private serverLastMessageId?: number;
@observable private usersLoaded = false;

@computed
get announcementUsers() {
return this.usersLoaded
? this.userIds
.map((userId) => core.dataStore.userStore.get(userId))
.filter((u): u is User => u != null)
.sort(usernameSortAscending)
: null;
}

@computed
get canMessage() {
Expand Down Expand Up @@ -204,10 +198,7 @@ export default class Channel {
// nothing to load
if (this.newPmChannel) return;

if (this.type === 'ANNOUNCE' && !this.usersLoaded) {
this.loadMetadata();
}

this.loadUsers();
this.loadRecentMessages();
}

Expand Down Expand Up @@ -246,11 +237,31 @@ export default class Channel {
getChannel(this.channelId).done((json) => {
runInAction(() => {
this.updateWithJson(json);
this.usersLoaded = true;
});
});
}

@action
readonly loadUsers = () => {
if (!this.canListUsers) {
this.users = [];
this.usersCursor = null;
return;
}

if (this.usersCursor == null || this.loadUsersXhr != null) {
return;
}

this.loadUsersXhr = getChannelUsers(this.channelId, this.usersCursor)
.done((json) => runInAction(() => {
this.users = [...(this.users ?? []), ...json.users];
this.usersCursor = json.cursor_string;
})).always(action(() => {
this.loadUsersXhr = undefined;
}));
};

@action
moveMarkAsReadMarker() {
this.setLastReadId(this.lastMessageId);
Expand Down Expand Up @@ -282,6 +293,7 @@ export default class Channel {
this.serverLastMessageId = json.last_message_id;

if (json.current_user_attributes != null) {
this.canListUsers = json.current_user_attributes.can_list_users;
this.canMessageError = json.current_user_attributes.can_message_error;
const lastReadId = json.current_user_attributes.last_read_id ?? 0;
this.setLastReadId(lastReadId);
Expand Down
1 change: 1 addition & 0 deletions routes/web.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@
Route::get('updates', 'ChatController@updates')->name('updates');
Route::group(['as' => 'channels.', 'prefix' => 'channels'], function () {
Route::apiResource('{channel}/messages', 'Channels\MessagesController', ['only' => ['index', 'store']]);
Route::apiResource('{channel}/users', 'Channels\UsersController', ['only' => ['index']]);
Route::put('{channel}/users/{user}', 'ChannelsController@join')->name('join');
Route::delete('{channel}/users/{user}', 'ChannelsController@part')->name('part');
Route::put('{channel}/mark-as-read/{message}', 'ChannelsController@markAsRead')->name('mark-as-read');
Expand Down