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

Clear allocation notes on server deletion #5157

Open
wants to merge 2 commits into
base: 1.0-develop
Choose a base branch
from

Conversation

giomxx
Copy link

@giomxx giomxx commented Jul 11, 2024

Currently when a user sets notes for an allocation, it's saved to the DB, then never touched again. When the server is deleted, the notes on that allocation are preserved.

This means that the DB can get filled with allocations with note entries, and a NULL server ID, and when the allocation eventually get's reused for another server, the note is displayed for the allocations on the new server.

My PR fixes this by ensuring the notes are removed from the allocations during the server deletion process.

@RMartinOscar
Copy link

RMartinOscar commented Jul 12, 2024

Hey that would work for server deleted since the change but older allocations will still have their notes we need to use a migration as well, something like this should do the job

database/migrations/2024_07_12_170235_clear_notes_on_unused_allocations.php

<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Support\Facades\DB;

return new class extends Migration
{
    /**
     * Run the migrations.
     */
    public function up(): void
    {
        DB::table('allocations')
            ->where('server_id', null)
            ->whereNot('notes', null)
            ->update(['notes' => null]);
    }

    /**
     * Reverse the migrations.
     */
    public function down(): void
    {
        // Reverse not needed
    }
};

@giomxx
Copy link
Author

giomxx commented Jul 13, 2024

@RMartinOscar Thanks! I had completely forgot about that. I've added the migration script now. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants