mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-16 16:45:07 +00:00
denkfabrik-li
5242169bb0
approve() refuses a request that is not pending:
abort_unless($group !== null && $client !== null
&& $membershipRequest->status === MembershipRequest::STATUS_PENDING, 404);
deny(), one method below, checks nothing. Denying is not idempotent, so
repeating it is not a no-op:
- denied_at is stamped again, and that is what the client's re-request
cooldown counts from (MyGroupsController::inDenyCooldown). Repeating
the request keeps one client out of one group for as long as somebody
cares to keep asking, without a single new decision being made.
- a second GroupMembershipDenied entry goes into the activity log, for
a denial that did not happen.
- a second "your request was declined" mail goes to the client.
The queue lists only pending requests, so nothing on the screen offers
this; it takes asking for the route directly. It needs
approve_groups_memberships_requests, so it is not a stranger's move.
The guard is the same one, answering the same 404, placed where deny()
can reach it. deny() keeps tolerating a vanished group or client -- that
tolerance is deliberate and separate: the denied row persists for the
cooldown even when the group it named is gone, and index() already
filters those rows out with whereHas.
Not in this change: deny() writes the status, the log entry and the
notification without a shared transaction. approve() has exactly the same
shape, so fixing one alone would replace a symmetry with a difference,
and doing both means also deciding where the mail sits relative to the
commit -- which is the question #1691 answers for file bytes, and worth
answering on its own rather than inside a state-machine fix.
85 lines
3.3 KiB
PHP
85 lines
3.3 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
use App\Models\User;
|
|
use App\Modules\Audit\Action;
|
|
use App\Modules\Audit\ActivityLog;
|
|
use App\Modules\Groups\Models\Group;
|
|
use App\Modules\Groups\Models\MembershipRequest;
|
|
use App\Modules\Groups\Notifications\GroupMembershipDeniedNotification;
|
|
use App\Modules\Platform\Settings\Setting;
|
|
use App\Modules\Platform\Settings\Settings;
|
|
use Illuminate\Support\Facades\Notification;
|
|
|
|
/**
|
|
* approve() refuses a request that is not pending. deny() did not, and a
|
|
* denial is not idempotent: it re-stamps denied_at, which is what the
|
|
* client's re-request cooldown counts from.
|
|
*/
|
|
beforeEach(function () {
|
|
$this->admin = User::factory()->create();
|
|
$this->client = User::factory()->client()->create();
|
|
$this->group = Group::query()->create(['name' => 'Guarded', 'slug' => 'guarded', 'public' => true]);
|
|
|
|
$this->request = MembershipRequest::query()->create([
|
|
'group_id' => $this->group->id,
|
|
'user_id' => $this->client->id,
|
|
'status' => MembershipRequest::STATUS_PENDING,
|
|
]);
|
|
});
|
|
|
|
test('denying a pending request works, once', function () {
|
|
$this->actingAs($this->admin)->delete("/membership-requests/{$this->request->id}")->assertRedirect();
|
|
|
|
expect($this->request->fresh()->status)->toBe(MembershipRequest::STATUS_DENIED);
|
|
|
|
$this->actingAs($this->admin)->delete("/membership-requests/{$this->request->id}")->assertNotFound();
|
|
});
|
|
|
|
test('a second denial does not move the stamp, or the log, or the mail', function () {
|
|
Notification::fake();
|
|
app(Settings::class)->set(Setting::EmailNotificationsEnabled, true);
|
|
|
|
$this->actingAs($this->admin)->delete("/membership-requests/{$this->request->id}");
|
|
$stamped = $this->request->fresh()->denied_at;
|
|
|
|
$this->travel(5)->days();
|
|
$this->actingAs($this->admin)->delete("/membership-requests/{$this->request->id}")->assertNotFound();
|
|
|
|
expect($this->request->fresh()->denied_at?->toIso8601String())->toBe($stamped?->toIso8601String())
|
|
->and(ActivityLog::query()->where('action', Action::GroupMembershipDenied->value)->count())->toBe(1);
|
|
|
|
Notification::assertSentToTimes($this->client, GroupMembershipDeniedNotification::class, 1);
|
|
});
|
|
|
|
test('a replayed denial cannot hold a client out of a group past the cooldown', function () {
|
|
app(Settings::class)->set(Setting::ClientsCanSelectGroup, 'public');
|
|
app(Settings::class)->set(Setting::ClientsMembershipDenyCooldownDays, 30);
|
|
|
|
$this->actingAs($this->admin)->delete("/membership-requests/{$this->request->id}");
|
|
|
|
// Day 29: still inside the cooldown, and a denial arriving now used
|
|
// to restart it.
|
|
$this->travel(29)->days();
|
|
$this->actingAs($this->admin)->delete("/membership-requests/{$this->request->id}")->assertNotFound();
|
|
|
|
$this->travel(2)->days();
|
|
|
|
$this->actingAs($this->client)
|
|
->post('/my-groups', ['group_id' => $this->group->id])
|
|
->assertSessionHasNoErrors();
|
|
|
|
expect($this->request->fresh()->status)->toBe(MembershipRequest::STATUS_PENDING);
|
|
});
|
|
|
|
test('approve already refused a request that is not pending', function () {
|
|
$this->actingAs($this->admin)->delete("/membership-requests/{$this->request->id}");
|
|
|
|
$this->actingAs($this->admin)
|
|
->post("/membership-requests/{$this->request->id}/approve")
|
|
->assertNotFound();
|
|
|
|
expect($this->group->members()->count())->toBe(0);
|
|
});
|