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

refactor(Subscription): rename columns and adjust primary/foreign keys #2298

Merged
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
3 changes: 1 addition & 2 deletions app/Actions/ClearAccountDataAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ public function execute(User $user): void
DB::statement('DELETE FROM SetRequest WHERE User = :username', ['username' => $user->User]);
// TODO $user->badges()->delete();
DB::statement('DELETE FROM SiteAwards WHERE User = :username', ['username' => $user->User]);
// TODO $user->subscriptions()->delete();
DB::statement('DELETE FROM Subscription WHERE UserID = :userId', ['userId' => $user->ID]);
$user->subscriptions()->delete();

// use action to delete each participation so threads with no remaing active participants get cleaned up
$deleteMessageThreadAction = new DeleteMessageThreadAction();
Expand Down
9 changes: 9 additions & 0 deletions app/Community/Concerns/ActsAsCommunityMember.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use App\Community\Enums\UserRelationship;
use App\Models\ForumTopicComment;
use App\Models\MessageThreadParticipant;
use App\Models\Subscription;
use App\Models\User;
use App\Models\UserActivity;
use App\Models\UserComment;
Expand Down Expand Up @@ -124,4 +125,12 @@ public function forumPosts(): HasMany
{
return $this->hasMany(ForumTopicComment::class, 'AuthorID', 'ID');
}

/**
* @return HasMany<Subscription>
*/
public function subscriptions(): HasMany
{
return $this->hasMany(Subscription::class, 'user_id', 'ID');
}
}
81 changes: 43 additions & 38 deletions app/Helpers/database/subscription.php
Original file line number Diff line number Diff line change
@@ -1,29 +1,32 @@
<?php

use App\Community\Enums\SubscriptionSubjectType;
use App\Models\Subscription;

/**
* Update a subscription, i.e, either subscribe or unsubscribe a given user to or from a subject.
*
* @param bool $state whether the user is to be subscribed (true) or unsubscribed (false)
*/
function updateSubscription(string $subjectType, int $subjectID, int $userID, bool $state): bool
function updateSubscription(string $subjectType, int $subjectId, int $userId, bool $state): bool
{
sanitize_sql_inputs($subjectType);
$state = (int) $state;

$query = "
INSERT INTO Subscription(SubjectType, SubjectID, UserID, State)
VALUES ('$subjectType', $subjectID, $userID, '$state')
ON DUPLICATE KEY UPDATE State = '$state'
";

$dbResult = s_mysql_query($query);
Subscription::updateOrCreate(
[
'subject_type' => $subjectType,
'subject_id' => $subjectId,
'user_id' => $userId,
],
[
'state' => $state,
]
);

return (bool) $dbResult;
return true;
}

/**
* @deprecated $implicitSubscriptionQry considered harmful. Use Eloquent ORM.
*
* Checks whether a given user is subscribed to a subject, whether implicitly or explicitly.
*
* @param string|null $implicitSubscriptionQry optional sql query capable of identifying the existence of an implicit
Expand All @@ -39,12 +42,12 @@ function isUserSubscribedTo(string $subjectType, int $subjectID, int $userID, ?s
if ($implicitSubscriptionQry === null) {
$query = "
SELECT 1
FROM Subscription
FROM subscriptions
WHERE
SubjectType = '$subjectType'
AND SubjectID = $subjectID
AND UserID = $userID
AND State = 1
subject_type = '$subjectType'
AND subject_id = $subjectID
AND user_id = $userID
AND state = 1
";
} else {
// either there's an explicit subscription...
Expand All @@ -53,29 +56,29 @@ function isUserSubscribedTo(string $subjectType, int $subjectID, int $userID, ?s
// subscription to the subject (must be usable inside an EXISTS clause)
$query = "
SELECT 1
FROM Subscription
FROM subscriptions
WHERE
EXISTS (
SELECT 1
FROM Subscription
FROM subscriptions
WHERE
SubjectType = '$subjectType'
AND SubjectID = $subjectID
AND UserID = $userID
AND State = 1
subject_type = '$subjectType'
AND subject_id = $subjectID
AND user_id = $userID
AND state = 1
)
OR (
EXISTS (
$implicitSubscriptionQry
)
AND NOT EXISTS (
SELECT 1
FROM Subscription
FROM subscriptions
WHERE
SubjectType = '$subjectType'
AND SubjectID = $subjectID
AND UserID = $userID
AND State = 0
subject_type = '$subjectType'
AND subject_id = $subjectID
AND user_id = $userID
AND state = 0
)
)
";
Expand All @@ -93,6 +96,8 @@ function isUserSubscribedTo(string $subjectType, int $subjectID, int $userID, ?s
}

/**
* @deprecated $implicitSubscriptionQry considered harmful. Use Eloquent ORM.
*
* Retrieves the list of users that are subscribed to a given subject either implicitly or explicitly.
*
* @param ?int $reqWebsitePrefs optional required website preferences for a user to be considered a subscriber
Expand All @@ -112,13 +117,13 @@ function getSubscribersOf(string $subjectType, int $subjectID, ?int $reqWebsiteP
_ua.User,
_ua.EmailAddress
FROM
Subscription AS _sub
subscriptions AS _sub
INNER JOIN UserAccounts AS _ua
ON _ua.ID = _sub.UserID
ON _ua.ID = _sub.user_id
WHERE
_sub.SubjectType = '$subjectType'
AND _sub.SubjectID = $subjectID
AND _sub.State = 1
_sub.subject_type = '$subjectType'
AND _sub.subject_id = $subjectID
AND _sub.state = 1
$websitePrefsFilter
";

Expand All @@ -133,12 +138,12 @@ function getSubscribersOf(string $subjectType, int $subjectID, ?int $reqWebsiteP
(
$implicitSubscriptionQry
) as _ua
LEFT JOIN Subscription AS _sub
ON (_sub.SubjectType = '$subjectType'
AND _sub.SubjectID = $subjectID
AND _sub.UserID = _ua.ID)
LEFT JOIN subscriptions AS _sub
ON (_sub.subject_type = '$subjectType'
AND _sub.subject_id = $subjectID
AND _sub.user_id = _ua.ID)
WHERE
COALESCE(_sub.State, 1) = 1
COALESCE(_sub.state, 1) = 1
$websitePrefsFilter
UNION
$explicitSubscriptionQry
Expand Down
32 changes: 28 additions & 4 deletions app/Models/Subscription.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,36 @@
namespace App\Models;

use App\Support\Database\Eloquent\BaseModel;
use Illuminate\Database\Eloquent\Relations\BelongsTo;

class Subscription extends BaseModel
{
// TODO rename Subscription to subscriptions
protected $table = 'Subscription';
protected $table = 'subscriptions';

public const CREATED_AT = 'Created';
public const UPDATED_AT = 'Updated';
protected $fillable = [
'subject_type',
'subject_id',
'user_id',
'state',
];

protected $casts = [
'state' => 'boolean',
];

// == accessors

// == mutators

// == relations

/**
* @return BelongsTo<User, Subscription>
*/
public function user(): BelongsTo
{
return $this->belongsTo(User::class, 'user_id', 'ID');
}

// == scopes
}
3 changes: 2 additions & 1 deletion database/migrations/2012_10_03_133633_create_base_tables.php
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,7 @@ public function up(): void

if (!Schema::hasTable('Subscription')) {
Schema::create('Subscription', function (Blueprint $table) {
$table->bigIncrements('id');
$table->enum('SubjectType', [
'ForumTopic',
'UserWall',
Expand All @@ -645,7 +646,7 @@ public function up(): void
$table->unsignedInteger('UserID');
$table->unsignedTinyInteger('State')->comment('Whether UserID is subscribed (1) or unsubscribed (0)');

$table->primary(['SubjectType', 'SubjectID', 'UserID']);
$table->unique(['SubjectType', 'SubjectID', 'UserID']);
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
<?php

declare(strict_types=1);

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

return new class() extends Migration {
public function up(): void
{
// [1] Create `id` column as the new primary key.
// SQLite doesn't let us change a primary key after the initial migration.
/** @see 2012_10_03_133633_create_base_tables.php */
Schema::table('Subscription', function (Blueprint $table) {
if (DB::connection()->getDriverName() !== 'sqlite') {
$table->dropPrimary(['SubjectType', 'SubjectID', 'UserID']);
}
});
Schema::table('Subscription', function (Blueprint $table) {
if (DB::connection()->getDriverName() !== 'sqlite') {
$table->bigIncrements('id')->first();
}
});

// [2] Rename columns to align with Laravel conventions.
Schema::table('Subscription', function (Blueprint $table) {
$table->renameColumn('SubjectType', 'subject_type');
});
Schema::table('Subscription', function (Blueprint $table) {
$table->renameColumn('SubjectID', 'subject_id');
});
Schema::table('Subscription', function (Blueprint $table) {
$table->renameColumn('UserID', 'user_id');
});
Schema::table('Subscription', function (Blueprint $table) {
$table->renameColumn('State', 'state');
});
Schema::table('Subscription', function (Blueprint $table) {
$table->renameColumn('Created', 'created_at');
});
Schema::table('Subscription', function (Blueprint $table) {
$table->renameColumn('Updated', 'updated_at');
});

// [3] Change the datatype of `user_id` to match `UserAccounts.ID`.
Schema::table('Subscription', function (Blueprint $table) {
$table->unsignedBigInteger('user_id')->change();
});

// [4] Enforce a unique constraint on type, subject_id, and user_id combos.
Schema::table('Subscription', function (Blueprint $table) {
$table->unique(['subject_type', 'subject_id', 'user_id']);
});

// [5] Add a foreign key to UserAccounts.
Schema::table('Subscription', function (Blueprint $table) {
$table->foreign('user_id')->references('ID')->on('UserAccounts')->onDelete('cascade');
});

// [6] Rename the table from `Subscription` to `subscriptions`.
Schema::rename('Subscription', 'subscriptions');
}

public function down(): void
{
// [6] Rename the table from `Subscription` to `subscriptions`.
Schema::rename('subscriptions', 'Subscription');

// [5] Add a foreign key to UserAccounts.
Schema::table('Subscription', function (Blueprint $table) {
$table->dropForeign(['user_id']);
});

// [4] Enforce a unique constraint on type, subject_id, and user_id combos.
Schema::table('Subscription', function (Blueprint $table) {
$table->dropUnique(['subject_type', 'subject_id', 'user_id']);
});

// [3] Change the datatype of `user_id` to match `UserAccounts.ID`.
Schema::table('Subscription', function (Blueprint $table) {
$table->unsignedInteger('user_id')->change();
});

// [2] Rename columns to align with Laravel conventions.
Schema::table('Subscription', function (Blueprint $table) {
$table->renameColumn('subject_type', 'SubjectType');
$table->renameColumn('subject_id', 'SubjectID');
$table->renameColumn('user_id', 'UserID');
$table->renameColumn('state', 'State');
$table->renameColumn('created_at', 'Created');
$table->renameColumn('updated_at', 'Updated');
});

// [1] Create `id` column as the new primary key.
Schema::table('Subscription', function (Blueprint $table) {
$table->dropColumn('id');

$table->primary(['SubjectType', 'SubjectID', 'UserID']);
});
}
};
Loading