-
-
Notifications
You must be signed in to change notification settings - Fork 125
feat(manage/media): let some roles wipe all game screenshots #4848
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| use App\Models\GameScreenshot; | ||
| use App\Models\User; | ||
| use App\Platform\Actions\AddGameScreenshotAction; | ||
| use App\Platform\Actions\ClearGameScreenshotsFromGamePageAction; | ||
| use App\Platform\Enums\GameScreenshotStatus; | ||
| use App\Platform\Enums\ScreenshotType; | ||
| use App\Rules\DisallowAnimatedImageRule; | ||
|
|
@@ -16,6 +17,7 @@ | |
| use Filament\Actions\Action; | ||
| use Filament\Actions\ActionGroup; | ||
| use Filament\Actions\DeleteAction; | ||
| use Filament\Actions\RestoreAction; | ||
| use Filament\Forms; | ||
| use Filament\Notifications\Notification; | ||
| use Filament\Resources\RelationManagers\RelationManager; | ||
|
|
@@ -64,6 +66,9 @@ public function table(Table $table): Table | |
| /** @var Game $game */ | ||
| $game = $this->getOwnerRecord(); | ||
|
|
||
| /** @var User $user */ | ||
| $user = Auth::user(); | ||
|
|
||
| return $table | ||
| ->headerActions([ | ||
| Action::make('upload_screenshot') | ||
|
|
@@ -139,14 +144,42 @@ public function table(Table $table): Table | |
| ->title("{$label} uploaded successfully") | ||
| ->send(); | ||
| }), | ||
|
|
||
| ActionGroup::make([ | ||
| Action::make('clear_screenshots_from_game_page') | ||
| ->label('Clear Screenshots') | ||
| ->icon('heroicon-o-trash') | ||
| ->color('danger') | ||
| ->requiresConfirmation() | ||
| ->modalHeading("Clear this game's screenshots?") | ||
| ->modalDescription('All screenshots will be moved to the trash and the game will use placeholder images. You can restore them later.') | ||
| ->modalSubmitActionLabel('Clear Screenshots') | ||
| ->action(function () use ($game): void { | ||
| $clearedCount = (new ClearGameScreenshotsFromGamePageAction())->execute($game); | ||
|
|
||
| $this->logScreenshotActivity($game) | ||
| ->event('clearedScreenshots') | ||
| ->log("Cleared {$clearedCount} screenshot(s)"); | ||
|
|
||
| Notification::make() | ||
| ->success() | ||
| ->title('Screenshots cleared') | ||
| ->send(); | ||
| }), | ||
| ]) | ||
| ->label('More') | ||
| ->icon('heroicon-m-ellipsis-vertical') | ||
| ->iconButton() | ||
| ->tooltip('More actions') | ||
| ->visible(fn (): bool => $user->can('clearScreenshots', $game) && $game->gameScreenshots()->exists()), | ||
| ]) | ||
| ->modifyQueryUsing(function (Builder $query) { | ||
| /** @var Builder<GameScreenshot> $query */ | ||
| $query->with(['media', 'game.system']) | ||
| ->orderByType() | ||
| ->orderBy('order_column'); | ||
|
|
||
| if (!$this->shouldShowArchivedScreenshots()) { | ||
| if (!$this->shouldShowRejectedOrReplacedScreenshots()) { | ||
| $query->whereNotIn('status', [ | ||
| GameScreenshotStatus::Rejected->value, | ||
| GameScreenshotStatus::Replaced->value, | ||
|
|
@@ -212,6 +245,11 @@ public function table(Table $table): Table | |
| ) | ||
| ->color(fn (GameScreenshot $record): ?string => $record->has_wrong_resolution ? 'danger' : null) | ||
| ->icon(fn (GameScreenshot $record): ?string => $record->has_wrong_resolution ? 'heroicon-o-exclamation-triangle' : null), | ||
|
|
||
| Tables\Columns\TextColumn::make('deleted_at') | ||
| ->dateTime() | ||
| ->sortable() | ||
| ->toggleable(isToggledHiddenByDefault: true), | ||
| ]) | ||
| ->filters([ | ||
| Tables\Filters\SelectFilter::make('type') | ||
|
|
@@ -223,6 +261,11 @@ public function table(Table $table): Table | |
| ->placeholder('Published + Pending') | ||
| ->options(fn (): array => $this->getStatusFilterOptions()), | ||
|
|
||
| Tables\Filters\TrashedFilter::make() | ||
| ->label('Deleted screenshots') | ||
| ->placeholder('Current screenshots') | ||
| ->trueLabel('All screenshots') | ||
| ->falseLabel('Deleted screenshots'), | ||
| ]) | ||
| ->emptyStateHeading('No screenshots yet') | ||
| ->emptyStateDescription('Upload screenshots using the button above.') | ||
|
|
@@ -234,7 +277,7 @@ public function table(Table $table): Table | |
| ->iconButton() | ||
| ->tooltip('Set as Primary') | ||
| ->requiresConfirmation() | ||
| ->hidden(fn (GameScreenshot $record): bool => $record->is_primary) | ||
| ->hidden(fn (GameScreenshot $record): bool => $record->is_primary || $record->trashed()) | ||
| ->action(function (GameScreenshot $record) use ($game): void { | ||
| DB::transaction(function () use ($record) { | ||
| // Demote the current primary of the same type via Eloquent | ||
|
|
@@ -269,7 +312,7 @@ public function table(Table $table): Table | |
| ->label('Approve') | ||
| ->icon('heroicon-o-check-circle') | ||
| ->color('success') | ||
| ->visible(fn (GameScreenshot $record): bool => $record->status !== GameScreenshotStatus::Approved) | ||
| ->visible(fn (GameScreenshot $record): bool => !$record->trashed() && $record->status !== GameScreenshotStatus::Approved) | ||
| ->action(function (GameScreenshot $record) use ($game): void { | ||
| $oldStatus = $record->status; | ||
|
|
||
|
|
@@ -288,6 +331,7 @@ public function table(Table $table): Table | |
| Action::make('change_type') | ||
| ->label('Change Type') | ||
| ->icon('heroicon-o-tag') | ||
| ->visible(fn (GameScreenshot $record): bool => !$record->trashed()) | ||
| ->schema([ | ||
| Forms\Components\Select::make('type') | ||
| ->label('Type') | ||
|
|
@@ -320,7 +364,7 @@ public function table(Table $table): Table | |
| ->icon('heroicon-o-x-circle') | ||
| ->color('danger') | ||
| ->requiresConfirmation() | ||
| ->visible(fn (GameScreenshot $record): bool => !$record->is_primary && $record->status !== GameScreenshotStatus::Rejected) | ||
| ->visible(fn (GameScreenshot $record): bool => !$record->trashed() && !$record->is_primary && $record->status !== GameScreenshotStatus::Rejected) | ||
| ->action(function (GameScreenshot $record) use ($game): void { | ||
| $oldStatus = $record->status; | ||
|
|
||
|
|
@@ -344,14 +388,15 @@ public function table(Table $table): Table | |
| ->requiresConfirmation() | ||
| ->modalDescription(fn (GameScreenshot $record): string => $record->is_primary | ||
| ? 'This is a primary screenshot. The next published screenshot of this type will be promoted automatically, or the placeholder will be restored.' | ||
| : 'Are you sure you want to delete this screenshot?') | ||
| : 'Are you sure you want to permanently delete this screenshot?') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the previous diff, it authorized implicitly against the deletion policy (original uploader + still Pending). I think we should remove it altogether TBH, which I've done in latest. |
||
| ->using(function (GameScreenshot $record) use ($game): void { | ||
| $screenshotUrl = $record->media?->getUrl(); | ||
| $media = $record->media; | ||
| $type = $record->type->label(); | ||
| $wasPrimary = $record->is_primary; | ||
|
|
||
| $record->media?->delete(); | ||
| $record->delete(); | ||
| $record->forceDelete(); | ||
| $media?->delete(); | ||
|
|
||
| $this->logScreenshotActivity($game) | ||
| ->withProperty('attributes', [ | ||
|
|
@@ -362,6 +407,8 @@ public function table(Table $table): Table | |
| ->event('deletedScreenshot') | ||
| ->log('Deleted screenshot'); | ||
| }), | ||
|
|
||
| RestoreAction::make(), | ||
| ]), | ||
| ]); | ||
| } | ||
|
|
@@ -382,8 +429,11 @@ private function getStatusFilterOptions(): array | |
| $game = $this->getOwnerRecord(); | ||
|
|
||
| $selectedType = data_get($this->getTableFilterState('type'), 'value'); | ||
| $selectedTrashed = $this->getSelectedTrashedFilter(); | ||
|
|
||
| $counts = $game->gameScreenshots() | ||
| ->when($selectedTrashed === true, fn (Builder $query) => $query->withTrashed()) | ||
| ->when($selectedTrashed === false, fn (Builder $query) => $query->onlyTrashed()) | ||
| ->when($selectedType, fn (Builder $query) => $query->where('type', $selectedType)) | ||
| ->whereIn('status', [ | ||
| GameScreenshotStatus::Approved->value, | ||
|
|
@@ -475,13 +525,29 @@ private function getScreenshotHelperText(): ?string | |
| return $text; | ||
| } | ||
|
|
||
| private function shouldShowArchivedScreenshots(): bool | ||
| private function shouldShowRejectedOrReplacedScreenshots(): bool | ||
| { | ||
| if ($this->getSelectedTrashedFilter() === false) { | ||
| return true; | ||
| } | ||
|
|
||
| $selectedStatus = data_get($this->getTableFilterState('status'), 'value'); | ||
|
|
||
| return in_array($selectedStatus, [ | ||
| GameScreenshotStatus::Rejected->value, | ||
| GameScreenshotStatus::Replaced->value, | ||
| ], true); | ||
| } | ||
|
|
||
| /** | ||
| * Normalize the trashed filter state to a tri-state value. | ||
| * Filament serializes booleans as '1'/'0' through the form layer, | ||
| * so === true / === false comparisons against the raw value silently fail. | ||
| */ | ||
| private function getSelectedTrashedFilter(): ?bool | ||
| { | ||
| $value = data_get($this->getTableFilterState('trashed'), 'value'); | ||
|
|
||
| return filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace App\Platform\Actions; | ||
|
|
||
| use App\Models\Game; | ||
| use App\Models\GameScreenshot; | ||
| use App\Platform\Enums\ScreenshotType; | ||
| use Illuminate\Support\Facades\DB; | ||
|
|
||
| class ClearGameScreenshotsFromGamePageAction | ||
| { | ||
| public function execute(Game $game): int | ||
| { | ||
| return DB::transaction(function () use ($game): int { | ||
| $screenshotIds = $game->gameScreenshots() | ||
| ->pluck('id'); | ||
|
|
||
| if ($screenshotIds->isEmpty()) { | ||
| return 0; | ||
| } | ||
|
|
||
| GameScreenshot::whereKey($screenshotIds) | ||
| ->update(['is_primary' => false]); | ||
|
|
||
| $deletedCount = GameScreenshot::whereKey($screenshotIds) | ||
| ->delete(); | ||
|
|
||
| $game->syncLegacyScreenshotFields(ScreenshotType::Title); | ||
| $game->syncLegacyScreenshotFields(ScreenshotType::Ingame); | ||
|
|
||
| return $deletedCount; | ||
| }); | ||
| } | ||
| } |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right. This is intentional, especially through the player-facing UI. Reasoning: if some back office action deleted the image, it shouldn't be reuploaded by a player again for back office review. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?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('game_screenshots', function (Blueprint $table) { | ||
| $table->softDeletes()->after('updated_at'); | ||
| }); | ||
| } | ||
|
|
||
| public function down(): void | ||
| { | ||
| Schema::table('game_screenshots', function (Blueprint $table) { | ||
| $table->dropSoftDeletes(); | ||
| }); | ||
| } | ||
| }; |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This filter feels wrong.
I'm not viewing "Currently Deleted screenshots", I'm viewing non-deleted screenshots.
And I'm not sure that I like the way it combines with the other filter:
I feel like Deleted should just be its own Status. What does it mean to have a Deleted Published screenshot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrestled with this too when figuring out what solution I wanted to build for this functionality. My original idea was to have an "Archived" status, but that felt awfully ambiguous with "Rejected" also in the mix.
A deleted screenshot having a status of published means, on restore, the screenshot will be published again. That makes cleanly undoing the delete possible.
Here's where I've landed in latest - no more trashed entity filter. It's disguised as a Status to hopefully be more intuitive even though the underlying model is unchanged:
