diff --git a/app/Events/ResourceReviewProcessed.php b/app/Events/ResourceReviewProcessed.php index bcd2e72e..12778a62 100644 --- a/app/Events/ResourceReviewProcessed.php +++ b/app/Events/ResourceReviewProcessed.php @@ -16,7 +16,7 @@ class ResourceReviewProcessed implements ShouldDispatchAfterCommit * Create a new event instance. */ public function __construct( - public int $resource, + public int $resource_id, public ?array $oldReview, public ?array $newReview, ) {} diff --git a/app/Filament/Admin/Resources/ComputerScienceResource.php b/app/Filament/Admin/Resources/ComputerScienceResource.php index 4adb083c..4d305e3c 100644 --- a/app/Filament/Admin/Resources/ComputerScienceResource.php +++ b/app/Filament/Admin/Resources/ComputerScienceResource.php @@ -110,8 +110,7 @@ public static function table(Table $table): Table // Custom delete action that uses model delete() method DeleteAction::make() ->action(function (ModelsComputerScienceResource $record) { - // This calls the model's delete() method, triggering all events - $record->delete(); + $record->forceDelete(); }), ]) ->bulkActions([ diff --git a/app/Http/Controllers/ResourceEditsController.php b/app/Http/Controllers/ResourceEditsController.php index 3f7cca63..ef23c0ed 100644 --- a/app/Http/Controllers/ResourceEditsController.php +++ b/app/Http/Controllers/ResourceEditsController.php @@ -123,14 +123,12 @@ public function merge(ResourceEditsService $editsService, ResourceEdits $resourc } if (array_key_exists('image_path', $changes)) { - // TODO: Remove the old photo resource photo, will be handled in a cron job, - // - // photo image_url history can be viewed via activity log. - // - + if ($resource->image_path) { + Storage::disk('public')->delete($resource->image_path); + } $destPath = null; if (isset($changes['image_path'])) { - // Copy the new file from 'resource-edits' to 'resource' (do not delete the old one) + // Move the new file from 'resource-edits' to 'resource' $sourcePath = $changes['image_path']; $fileExtension = pathinfo($sourcePath, PATHINFO_EXTENSION); $newFileName = Str::random(40).'.'.$fileExtension; @@ -139,7 +137,6 @@ public function merge(ResourceEditsService $editsService, ResourceEdits $resourc // TODO: FIGURE OUT WHAT TO DO IN CASE OF EXCEPTION IN CODE FROM LATER STEPS Storage::disk('public')->move($sourcePath, $destPath); } - // Update image_path in DB $resource->image_path = $destPath; } diff --git a/app/Listeners/UpdateResourceReviewSummary.php b/app/Listeners/UpdateResourceReviewSummary.php index 3b9e07a3..7cdcdbfb 100644 --- a/app/Listeners/UpdateResourceReviewSummary.php +++ b/app/Listeners/UpdateResourceReviewSummary.php @@ -22,23 +22,29 @@ public function __construct() public function handle(ResourceReviewProcessed $event): void { Log::debug('Handling ResourceReviewProcessed', [ - 'resource_id' => $event->resource, + 'resource_id' => $event->resource_id, 'old_review' => $event->oldReview, 'new_review' => $event->newReview, ]); if ($event->oldReview == null && $event->newReview == null) { Log::critical('Update Resource Review Summary Listener reached impossible condition', [ - 'resource_id' => $event->resource, + 'resource_id' => $event->resource_id, 'error' => 'Both oldReview and newReview are null', ]); return; } - $review_summary = ResourceReviewSummary::firstOrNew( - ['computer_science_resource_id' => $event->resource], - ); + $reviewSummary = ResourceReviewSummary::where( + 'computer_science_resource_id', + $event->resource_id + )->first(); + + if (! $reviewSummary) { + // The resource review summary is created by the resource observer + return; + } $fields = [ 'community', @@ -52,14 +58,16 @@ public function handle(ResourceReviewProcessed $event): void foreach ($fields as $field) { $old = $event->oldReview[$field] ?? 0; $new = $event->newReview[$field] ?? 0; - $review_summary->$field += ($new - $old); + $reviewSummary->$field += ($new - $old); } + $reviewSummary->review_count = $reviewSummary->review_count ?? 0; if ($event->oldReview === null) { - // It's a new review, so increase the count - $review_summary->review_count += 1; + $reviewSummary->review_count++; + } elseif ($event->newReview === null) { + $reviewSummary->review_count--; } - $review_summary->save(); + $reviewSummary->save(); } } diff --git a/app/Models/ComputerScienceResource.php b/app/Models/ComputerScienceResource.php index e729cac1..045e9e67 100644 --- a/app/Models/ComputerScienceResource.php +++ b/app/Models/ComputerScienceResource.php @@ -14,6 +14,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\HasOne; +use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Support\Facades\Storage; use ShiftOneLabs\LaravelCascadeDeletes\CascadesDeletes; use Spatie\Activitylog\LogOptions; @@ -35,9 +36,20 @@ class ComputerScienceResource extends Model use HasVotes; use LogsActivity; use Sluggable; - - // TODO: ADD A TEST FOR RESOURCE DELETION. DO NOT USE YET IN PRODUCTION - protected $cascadeDeletes = ['votes', 'upvoteSummary', 'comments', 'commentsCountRelationship', 'edits', 'reviewSummary', 'reviews', '']; + use SoftDeletes; + + protected $cascadeDeletes = [ + // Votes + 'votes', + 'upvoteSummary', + // Comments + 'comments', + 'commentsCountRelationship', + 'edits', + // Reviews + 'reviews', + 'reviewSummary', + ]; protected $table = 'computer_science_resources'; diff --git a/app/Observers/ComputerScienceResourceObserver.php b/app/Observers/ComputerScienceResourceObserver.php index 244be455..b2f4bcd5 100644 --- a/app/Observers/ComputerScienceResourceObserver.php +++ b/app/Observers/ComputerScienceResourceObserver.php @@ -2,8 +2,8 @@ namespace App\Observers; -use App\Events\TagFrequencyChanged; use App\Models\ComputerScienceResource; +use App\Models\ResourceReviewSummary; use Illuminate\Support\Facades\Storage; class ComputerScienceResourceObserver @@ -13,7 +13,9 @@ class ComputerScienceResourceObserver */ public function created(ComputerScienceResource $computerScienceResource): void { - // TagFrequencyChanged is in store ComputerScienceResource controller + ResourceReviewSummary::create( + ['computer_science_resource_id' => $computerScienceResource->id], + ); } /** @@ -27,13 +29,7 @@ public function updated(ComputerScienceResource $computerScienceResource): void /** * Handle the ComputerScienceResource "deleted" event. */ - public function deleted(ComputerScienceResource $computerScienceResource): void - { - // Delete the image - if ($computerScienceResource->image_path) { - Storage::disk('public')->delete($computerScienceResource->image_path); - } - } + public function deleted(ComputerScienceResource $computerScienceResource): void {} /** * Handle the ComputerScienceResource "restored" event. @@ -43,11 +39,20 @@ public function restored(ComputerScienceResource $computerScienceResource): void // } + public function deleting(ComputerScienceResource $computerScienceResource): void + { + $computerScienceResource->topic_tags = []; + $computerScienceResource->programming_language_tags = []; + $computerScienceResource->general_tags = []; + + // Delete the image + if ($computerScienceResource->image_path) { + Storage::disk('public')->delete($computerScienceResource->image_path); + } + } + /** * Handle the ComputerScienceResource "force deleted" event. */ - public function forceDeleted(ComputerScienceResource $computerScienceResource): void - { - // - } + public function forceDeleted(ComputerScienceResource $computerScienceResource): void {} } diff --git a/app/Observers/ResourceReviewObserver.php b/app/Observers/ResourceReviewObserver.php index c4f56438..ac5303cb 100644 --- a/app/Observers/ResourceReviewObserver.php +++ b/app/Observers/ResourceReviewObserver.php @@ -32,9 +32,9 @@ public function updated(ResourceReview $resourceReview): void } /** - * Handle the ResourceReview "deleted" event. + * Handle the ResourceReview "deleting" event. */ - public function deleted(ResourceReview $resourceReview): void + public function deleting(ResourceReview $resourceReview): void { ResourceReviewProcessed::dispatch( $resourceReview->computer_science_resource_id, diff --git a/app/Services/ComputerScienceResourceService.php b/app/Services/ComputerScienceResourceService.php index 13e7cc69..d6ea772b 100644 --- a/app/Services/ComputerScienceResourceService.php +++ b/app/Services/ComputerScienceResourceService.php @@ -157,10 +157,6 @@ function () use ($computerScienceResource, $sortBy, $request) { */ private function existingConflictingResource(array $data): ?ComputerScienceResource { - // If there is a resource with these matching properties, it is safe to say - // it is the same resource - return ComputerScienceResource::where('name', $data['name']) - ->where('page_url', $data['page_url']) - ->first(); + return ComputerScienceResource::where('page_url', $data['page_url'])->first(); } } diff --git a/database/migrations/2025_08_29_170217_add_soft_delete_and_indexing_to_computer_science_resources.php b/database/migrations/2025_08_29_170217_add_soft_delete_and_indexing_to_computer_science_resources.php new file mode 100644 index 00000000..9ab6f2d3 --- /dev/null +++ b/database/migrations/2025_08_29_170217_add_soft_delete_and_indexing_to_computer_science_resources.php @@ -0,0 +1,30 @@ +softDeletes(); + $table->index('page_url'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('computer_science_resources', function (Blueprint $table) { + $table->dropSoftDeletes(); + $table->dropIndex(['page_url']); + }); + } +}; diff --git a/resources/js/Components/Resources/Reviews/CreateResourceReview.vue b/resources/js/Components/Resources/Reviews/CreateResourceReview.vue index 043a3375..b289c34c 100644 --- a/resources/js/Components/Resources/Reviews/CreateResourceReview.vue +++ b/resources/js/Components/Resources/Reviews/CreateResourceReview.vue @@ -10,7 +10,6 @@ import ListInput from "@/Components/ListInput.vue"; import Button from "primevue/button"; import FormSaverChip from "@/Components/Form/FormSaverChip.vue"; -import { yupResolver } from "@primevue/forms/resolvers/yup"; import { resourceReviewFields } from "@/Helpers/validation"; import { router } from "@inertiajs/vue3"; import axios from "axios"; @@ -71,44 +70,47 @@ const { clearLocalStorage, } = useLocalStorageSaver(form, props.resourceId, formFields, "review-draft"); -const resolver = ref(yupResolver(resourceReviewFields)); const isSubmitting = ref(false); const error = ref(null); -const submitReview = async (event) => { - if (!event.valid) { - console.error("Validation errors"); - return; - } +const errors = ref({}); +const submitReview = async () => { isSubmitting.value = true; - - const url = props.isEditingMode - ? route("reviews.update", { computerScienceResource: props.resourceId }) - : route("reviews.store", { computerScienceResource: props.resourceId }); - - const method = props.isEditingMode ? "put" : "post"; - - axios[method](url, form) - .then(() => { - // Clear localStorage on successful submission - clearLocalStorage(); - - const routeParams = { slug: props.resourceSlug }; - if (props.isEditingMode) { - routeParams.tab = "reviews"; - routeParams.sort_by = "recently_updated"; - } else { - routeParams.sort_by = "latest"; - } - router.visit(route('resources.show', routeParams)); - }) - .catch((err) => { - isSubmitting.value = false; - error.value = - "Something went wrong with submitting your review, please refresh or try again."; - console.error(err); - }); + try { + await resourceReviewFields.validate(form, { abortEarly: false }); + errors.value = {}; + + const url = props.isEditingMode + ? route("reviews.update", { computerScienceResource: props.resourceId }) + : route("reviews.store", { computerScienceResource: props.resourceId }); + + const method = props.isEditingMode ? "put" : "post"; + + await axios[method](url, form); + clearLocalStorage(); + + const routeParams = { slug: props.resourceSlug }; + if (props.isEditingMode) { + routeParams.tab = "reviews"; + routeParams.sort_by = "recently_updated"; + } else { + routeParams.sort_by = "latest"; + } + router.visit(route('resources.show', routeParams)); + } catch (e) { + isSubmitting.value = false; + if (e.inner) { + const yupErrors = {}; + e.inner.forEach((error) => { + yupErrors[error.path] = error.errors; + }); + errors.value = yupErrors; + } else { + error.value = "Something went wrong with submitting your review, please refresh or try again."; + console.error(e); + } + } }; @@ -126,7 +128,6 @@ const submitReview = async (event) => {
actingAs(User::factory()->create()); diff --git a/tests/Feature/ComputerScienceResourceTest.php b/tests/Feature/ComputerScienceResourceTest.php index bd342f9e..1bb5967f 100644 --- a/tests/Feature/ComputerScienceResourceTest.php +++ b/tests/Feature/ComputerScienceResourceTest.php @@ -3,6 +3,7 @@ namespace Tests\Feature; use App\Models\ComputerScienceResource; +use App\Models\TagFrequency; use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Http\UploadedFile; @@ -10,12 +11,14 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; +use Tests\Feature\Utils\TestingUtils; use Tests\RequestFactories\ComputerScienceResource\StoreResourceRequestFactory; use Tests\TestCase; class ComputerScienceResourceTest extends TestCase { use RefreshDatabase; + use TestingUtils; protected $user; @@ -138,7 +141,7 @@ public function test_model_removes_image_upon_deletion() Storage::disk('public')->assertExists($imagePath); - $resource->delete(); + $resource->forceDelete(); Storage::disk('public')->assertMissing($imagePath); } @@ -160,4 +163,52 @@ public function test_posting_duplicate_resource_returns_existing_resource() $resources = ComputerScienceResource::where('name', $formData['name'])->get(); $this->assertCount(1, $resources); } + + public function test_cleans_up_when_deleted() + { + $this->actingAs($this->user); + + Storage::fake('public'); + $resource = $this->createResource([ + 'topics_tags' => ['test1'], + 'programming_languages_tags' => ['test2'], + 'general_tags' => ['test3'], + ]); + + $commentData = $this->createComment('resource', $resource->id); + $commentId = $commentData['id']; + + $resourceReview = $this->createReview($resource->id); + + // Add a fake image to the resource + $imageFile = UploadedFile::fake()->image('test_image.jpg'); + $imagePath = 'resource/'.$imageFile->hashName(); + Storage::disk('public')->put($imagePath, $imageFile->getContent()); + $resource->image_path = $imagePath; + $resource->save(); + + // Assert resource, comment, and image exist before deletion + $this->assertDatabaseHas('computer_science_resources', ['id' => $resource->id]); + $this->assertDatabaseHas('comments', ['id' => $commentId]); + Storage::disk('public')->assertExists($imagePath); + + $resource->forceDelete(); + + // Assert resource and comment are deleted + $this->assertDatabaseMissing('computer_science_resources', ['id' => $resource->id]); + $this->assertDatabaseMissing('comments', ['id' => $commentId]); + $this->assertDatabaseMissing('comments_counts', ['commentable_id' => $resource->id, 'commentable_type' => ComputerScienceResource::class]); + + // Assert Reviews are removed + $this->assertDatabaseMissing('resource_reviews', ['id' => $resourceReview->id]); + $this->assertDatabaseMissing('resource_review_summaries', ['computer_science_resource_id' => $resource->id]); + + // Assert tags are removed + $this->assertNull(TagFrequency::where('type', 'topics_tags')->where('tag', 'test1')->first()); + $this->assertNull(TagFrequency::where('type', 'programming_languages_tags')->where('tag', 'test2')->first()); + $this->assertNull(TagFrequency::where('type', 'general_tags')->where('tag', 'test3')->first()); + + // Assert image is deleted + Storage::disk('public')->assertMissing($imagePath); + } } diff --git a/tests/Feature/ResourceEditsTest.php b/tests/Feature/ResourceEditsTest.php index 17558938..52cdd038 100644 --- a/tests/Feature/ResourceEditsTest.php +++ b/tests/Feature/ResourceEditsTest.php @@ -150,6 +150,7 @@ public function test_merged_edit_reflects_changes_on_original_resource(): void $mergeAttempts = 5; + $oldImagePath = $resource->image_path; for ($i = 0; $i < $mergeAttempts; $i++) { $changes = [ 'name' => "Resource Name Edited {$i}", @@ -175,6 +176,8 @@ public function test_merged_edit_reflects_changes_on_original_resource(): void Storage::disk('public')->assertExists($resource->image_path); Storage::disk('public')->assertMissing($edit->image_path); + Storage::disk('public')->assertMissing($oldImagePath); + $oldImagePath = $resource->image_path; $this->assertEquals($changes['page_url'], $resource->page_url); $this->assertEquals($changes['difficulty'], $resource->difficulty); @@ -243,9 +246,9 @@ public function test_merge_edits_deletes_all_previous_relationships(): void $this->assertNotEmpty(ResourceEdits::withTrashed()->find($edit->id)); } - public function test_merge_edits_keep_previous_resource_image(): void + public function test_merge_edits_deletes_resource_image(): void { - // We want the image to remain for a period of time + // We want the old image to be deleted after merging Storage::fake('public'); $this->actingAs($this->user); @@ -269,8 +272,8 @@ public function test_merge_edits_keep_previous_resource_image(): void // Refresh the resource model from the database $resource->refresh(); - // Assert the old image is still there and the new one exists - Storage::disk('public')->assertExists($oldImagePath); + // Assert the old image is deleted and the new one exists + Storage::disk('public')->assertMissing($oldImagePath); Storage::disk('public')->assertExists($resource->image_path); $this->assertNotEquals($oldImagePath, $resource->image_path); }