Skip to content
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
13 changes: 11 additions & 2 deletions app/Community/Actions/BuildShowForumTopicPagePropsAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use App\Data\UserData;
use App\Data\UserPermissionsData;
use App\Models\ForumTopic;
use App\Models\ForumTopicComment;
use App\Models\User;
use App\Policies\ForumTopicCommentPolicy;
use App\Support\Shortcode\Shortcode;
Expand Down Expand Up @@ -62,14 +63,21 @@ public function execute(
hubIds: $entities['hubIds'],
);

// Get accessible team accounts for the current user.
$accessibleTeamAccounts = null;
$replyableTeamAccounts = null;
$accessibleTeamIds = [];
if ($user) {
$accessibleTeamIds = (new ForumTopicCommentPolicy())->getAccessibleTeamIds($user);
if (!empty($accessibleTeamIds)) {
$teamUsers = User::whereIn('id', $accessibleTeamIds)->get();
$teamUsers = User::whereIn('id', $accessibleTeamIds)->with('roles')->get();
$accessibleTeamAccounts = $teamUsers->map(fn ($teamUser) => UserData::fromUser($teamUser)->include('id'));

$replyEligibleTeamUsers = $teamUsers->filter(
fn (User $teamUser) => $user->can('create', [ForumTopicComment::class, $topic, $teamUser]),
);
$replyableTeamAccounts = $replyEligibleTeamUsers->isNotEmpty()
? $replyEligibleTeamUsers->values()->map(fn ($teamUser) => UserData::fromUser($teamUser)->include('id'))
: null;
}
}

Expand Down Expand Up @@ -108,6 +116,7 @@ function ($comment, $index) use ($updatedBodies, $user, $accessibleTeamIds) {

$props = new ShowForumTopicPagePropsData(
accessibleTeamAccounts: $accessibleTeamAccounts,
replyableTeamAccounts: $replyableTeamAccounts,
can: UserPermissionsData::fromUser($user, forumTopic: $topic)->include(
'authorizeForumTopicComments',
'createForumTopicComments',
Expand Down
2 changes: 2 additions & 0 deletions app/Data/ShowForumTopicPagePropsData.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class ShowForumTopicPagePropsData extends Data
{
/**
* @param Collection<int, UserData>|null $accessibleTeamAccounts
* @param Collection<int, UserData>|null $replyableTeamAccounts
*/
public function __construct(
public UserPermissionsData $can,
Expand All @@ -23,6 +24,7 @@ public function __construct(
public PaginatedData $paginatedForumTopicComments,
public string $metaDescription,
public ?Collection $accessibleTeamAccounts = null,
public ?Collection $replyableTeamAccounts = null,
) {
}
}
14 changes: 10 additions & 4 deletions app/Helpers/database/forum.php
Comment thread
Jamiras marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use App\Models\ForumTopicComment;
use App\Models\Game;
use App\Models\User;
use App\Policies\ForumTopicCommentPolicy;
use App\Support\Shortcode\Shortcode;
use Illuminate\Support\Collection;

Expand Down Expand Up @@ -205,20 +206,25 @@ function submitTopicComment(
return $latestPost;
}

$topic = ForumTopic::findOrFail($topicId);

// Comments by an effective author who satisfies the topic's role
// whitelist are trusted & auto-authorized.
$isAuthorized = ($user->ManuallyVerified ?? false)
|| (new ForumTopicCommentPolicy())->matchesCommentRoleAllowlist($user, $topic);

$newComment = new ForumTopicComment([
'forum_topic_id' => $topicId,
'body' => $commentPayload,
'author_id' => $user->id,
'sent_by_id' => $sentByUser?->id,
'is_authorized' => $user->ManuallyVerified ?? false,
'is_authorized' => $isAuthorized,
]);
$newComment->save();

$topic = ForumTopic::find($topicId);

setLatestCommentInForumTopic($topic, $newComment->id);

if ($user->ManuallyVerified ?? false) {
if ($isAuthorized) {
// if user has any notifications pending for this post, assume they're no longer needed
$notificationService = new SubscriptionNotificationService();
$notificationService->resetNotification($user->id, SubscriptionSubjectType::ForumTopic, $topic->id);
Expand Down
1 change: 1 addition & 0 deletions app/Models/ForumTopic.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ protected static function boot(): void
'author_id',
'latest_comment_id',
'required_permissions',
'comment_role_id',
'pinned_at',
'locked_at',
];
Expand Down
59 changes: 45 additions & 14 deletions app/Policies/ForumTopicCommentPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,43 @@ public function view(?User $user, ForumTopicComment $comment): bool
return false;
}

public function viewUserPosts(User $currentUser, User $targetUser): bool
{
return !$targetUser->isBlocking($currentUser);
}

public function create(User $user, ForumTopic $topic, ?User $teamAccount = null): bool
{
if ($user->isMuted()) {
return false;
}

// Users are able to create forum topic replies on behalf of team accounts,
// assuming the correct role is attached to the user.
if ($teamAccount) {
return $this->canActAsTeamAccount($user, $teamAccount);
}
if (!$this->canActAsTeamAccount($user, $teamAccount)) {
return false;
}

/*
* verified and unverified users may comment
* muted, suspended, banned may not comment
*/
if ($user->isMuted()) {
return false;
// A topic lock still applies, even when posting on behalf of a team account.
if ($topic->is_locked && !$user->can('lock', $topic)) {
return false;
}

// When a comment role allowlist exists for the topic, it must be satisfied
// by the effective author (the team account, since that's whose name will
// appear on the comment).
if ($topic->comment_role_id !== null
&& !$this->matchesCommentRoleAllowlist($teamAccount, $topic)
) {
return false;
}

return true;
}

/*
* users may not reply to locked topics, unless they have
* the ability to lock/unlock topics themselves.
*/
// Users may not reply to locked topics, unless they have
// the ability to lock/unlock topics themselves.
if ($topic->is_locked && !$user->can('lock', $topic)) {
return false;
}
Expand All @@ -77,6 +94,16 @@ public function create(User $user, ForumTopic $topic, ?User $teamAccount = null)
return false;
}

// A topic with a comment-role allowlist only accepts
// replies from users in those roles.
if ($topic->comment_role_id !== null) {
if ($this->manage($user)) {
return true;
}

return $this->matchesCommentRoleAllowlist($user, $topic);
}

return true;
}

Expand Down Expand Up @@ -135,8 +162,12 @@ public function forceDelete(User $user, ForumTopicComment $comment): bool
return false;
}

public function viewUserPosts(User $currentUser, User $targetUser): bool
public function matchesCommentRoleAllowlist(User $user, ForumTopic $topic): bool
{
return !$targetUser->isBlocking($currentUser);
if ($topic->comment_role_id === null) {
return false;
}

return $user->roles->contains('id', $topic->comment_role_id);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

declare(strict_types=1);

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration {
public function up(): void
{
Schema::table('forum_topics', function (Blueprint $table) {
$table->unsignedBigInteger('comment_role_id')->nullable()->after('required_permissions');

$table->foreign('comment_role_id')
->references('id')
->on('auth_roles')
->onDelete('set null');
});
}

public function down(): void
{
Schema::table('forum_topics', function (Blueprint $table) {
$table->dropForeign(['comment_role_id']);
$table->dropColumn('comment_role_id');
});
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe('Component: QuickReplyForm', () => {
pageProps: {
auth: { user: createAuthenticatedUser() },
forumTopic: createForumTopic(),
accessibleTeamAccounts: [teamAccount1, teamAccount2], // !!
replyableTeamAccounts: [teamAccount1, teamAccount2], // !!
},
});

Expand All @@ -165,7 +165,7 @@ describe('Component: QuickReplyForm', () => {
pageProps: {
auth: { user: createAuthenticatedUser() },
forumTopic: createForumTopic(),
accessibleTeamAccounts: null, // !!
replyableTeamAccounts: null, // !!
},
});

Expand All @@ -187,7 +187,7 @@ describe('Component: QuickReplyForm', () => {
pageProps: {
auth: { user },
forumTopic: createForumTopic(),
accessibleTeamAccounts: [teamAccount], // !!
replyableTeamAccounts: [teamAccount], // !!
},
});

Expand Down Expand Up @@ -218,7 +218,7 @@ describe('Component: QuickReplyForm', () => {
pageProps: {
auth: { user },
forumTopic: createForumTopic(),
accessibleTeamAccounts: [teamAccount], // !!
replyableTeamAccounts: [teamAccount], // !!
},
});

Expand All @@ -245,7 +245,7 @@ describe('Component: QuickReplyForm', () => {
pageProps: {
auth: { user },
forumTopic: createForumTopic(),
accessibleTeamAccounts: [teamAccount], // !!
replyableTeamAccounts: [teamAccount], // !!
},
});

Expand Down Expand Up @@ -274,7 +274,7 @@ describe('Component: QuickReplyForm', () => {
pageProps: {
auth: { user: createAuthenticatedUser() },
forumTopic: topic,
accessibleTeamAccounts: [teamAccount], // !!
replyableTeamAccounts: [teamAccount], // !!
},
});

Expand Down Expand Up @@ -312,7 +312,7 @@ describe('Component: QuickReplyForm', () => {
pageProps: {
auth: { user: createAuthenticatedUser() },
forumTopic: topic,
accessibleTeamAccounts: [teamAccount], // !!
replyableTeamAccounts: [teamAccount], // !!
},
});

Expand Down Expand Up @@ -355,7 +355,7 @@ describe('Component: QuickReplyForm', () => {
pageProps: {
auth: { user: createAuthenticatedUser({ displayName: 'Scott' }) },
forumTopic: createForumTopic(),
accessibleTeamAccounts: [teamAccount1, teamAccount2, teamAccount3], // !! unsorted
replyableTeamAccounts: [teamAccount1, teamAccount2, teamAccount3], // !! unsorted
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ interface QuickReplyFormProps {
}

export const QuickReplyForm: FC<QuickReplyFormProps> = ({ onPreview }) => {
const { auth, forumTopic, accessibleTeamAccounts } =
const { auth, forumTopic, replyableTeamAccounts } =
usePageProps<App.Data.ShowForumTopicPageProps>();

const { t } = useTranslation();
Expand All @@ -41,13 +41,13 @@ export const QuickReplyForm: FC<QuickReplyFormProps> = ({ onPreview }) => {

const watchedPostAsUser =
watchedPostAsUserId !== 'self'
? accessibleTeamAccounts?.find((ta) => ta.id === Number(watchedPostAsUserId))
? replyableTeamAccounts?.find((ta) => ta.id === Number(watchedPostAsUserId))
: null;

// Sort team accounts alphabetically by display name.
const sortedTeamAccounts = accessibleTeamAccounts
? [...accessibleTeamAccounts].sort((a, b) => a.displayName.localeCompare(b.displayName))
: null;
const sortedTeamAccounts = (replyableTeamAccounts ?? [])
.slice()
.sort((a, b) => a.displayName.localeCompare(b.displayName));

const formRef = useRef<HTMLFormElement>(null);
useSubmitOnMetaEnter({
Expand All @@ -72,7 +72,7 @@ export const QuickReplyForm: FC<QuickReplyFormProps> = ({ onPreview }) => {
<div className="flex w-full flex-col gap-2 lg:flex-row lg:justify-between lg:gap-5">
<ShortcodePanel className="p-0" />

{sortedTeamAccounts?.length ? (
{sortedTeamAccounts.length ? (
<BaseFormField
control={form.control}
name="postAsUserId"
Expand Down Expand Up @@ -152,7 +152,7 @@ export const QuickReplyForm: FC<QuickReplyFormProps> = ({ onPreview }) => {
/>
) : null}

{!watchedPostAsUser && accessibleTeamAccounts?.length ? (
{!watchedPostAsUser && replyableTeamAccounts?.length ? (
<img
src={auth.user.avatarUrl}
alt={auth.user.displayName}
Expand Down
1 change: 1 addition & 0 deletions resources/js/types/generated.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ isSubscribed: boolean;
paginatedForumTopicComments: App.Data.PaginatedData<TItems>;
metaDescription: string;
accessibleTeamAccounts: Array<App.Data.User> | null;
replyableTeamAccounts: Array<App.Data.User> | null;
};
export type StaticData = {
numGames: number;
Expand Down
Loading
Loading