From 81d4aafa77076a6f678af490a96914e6fad03364 Mon Sep 17 00:00:00 2001 From: luchaos Date: Fri, 3 Jun 2022 13:23:25 +0200 Subject: [PATCH] Refactor preferences & subscriptions - Add subscriptions created and updated timestamps migration - Simplify preferences bit shifting - Use constants for subscriptions --- ...603_000000_Add_Subscription_Timestamps.sql | 3 + lib/database/forum.php | 3 +- lib/database/message.php | 4 +- lib/database/ticket.php | 3 +- lib/database/user-activity.php | 1 - lib/database/user-preference.php | 15 ++ ...subscription.php => user-subscription.php} | 207 ++++++++++-------- lib/functions.php | 3 +- lib/util/bit.php | 4 +- lib/util/mail.php | 14 +- public/controlpanel.php | 75 ++----- public/gameInfo.php | 4 +- public/request/user/update-avatar.php | 2 +- public/request/user/update-notification.php | 27 --- public/request/user/update-preference.php | 28 +++ public/subscriptions.php | 51 +++++ public/test/subscriptions.php | 13 ++ src/Preference.php | 39 ++++ src/UserPreference.php | 36 --- tests/src/Util/BitTest.php | 20 ++ 20 files changed, 326 insertions(+), 226 deletions(-) create mode 100644 database/20220603_000000_Add_Subscription_Timestamps.sql create mode 100644 lib/database/user-preference.php rename lib/database/{subscription.php => user-subscription.php} (61%) delete mode 100644 public/request/user/update-notification.php create mode 100644 public/request/user/update-preference.php create mode 100644 public/subscriptions.php create mode 100644 public/test/subscriptions.php create mode 100644 src/Preference.php delete mode 100644 src/UserPreference.php create mode 100644 tests/src/Util/BitTest.php diff --git a/database/20220603_000000_Add_Subscription_Timestamps.sql b/database/20220603_000000_Add_Subscription_Timestamps.sql new file mode 100644 index 0000000000..6f989620eb --- /dev/null +++ b/database/20220603_000000_Add_Subscription_Timestamps.sql @@ -0,0 +1,3 @@ +ALTER TABLE `Subscription` + ADD COLUMN `Created` TIMESTAMP NULL DEFAULT CURRENT_TIMESTAMP COMMENT 'Timestamp for when this was created', + ADD COLUMN `Updated` TIMESTAMP NULL DEFAULT CURRENT_TIMESTAMP COMMENT 'Timestamp for when this was last modified'; diff --git a/lib/database/forum.php b/lib/database/forum.php index 0df6508b77..9783bdf678 100644 --- a/lib/database/forum.php +++ b/lib/database/forum.php @@ -3,6 +3,7 @@ use RA\ArticleType; use RA\ForumTopicAction; use RA\Permissions; +use RA\Preference; use RA\SubscriptionSubjectType; function getForumList($categoryID = 0): ?array @@ -345,7 +346,7 @@ function notifyUsersAboutForumActivity($topicID, $topicTitle, $author, $commentI $subscribers = getSubscribersOf( SubscriptionSubjectType::ForumTopic, $topicID, - (1 << 3), + Preference::EmailOn_ForumReply, " SELECT DISTINCT ua.* FROM diff --git a/lib/database/message.php b/lib/database/message.php index 14fcf76d1f..b5aa829374 100644 --- a/lib/database/message.php +++ b/lib/database/message.php @@ -1,6 +1,6 @@ 0; mysqli_free_result($dbResult); + return $isSubscribed; } /** * 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 - * @param ?string $implicitSubscriptionQry sql query that returns the set of users that are implicitly subscribed to + * @param ?int $requiredPreference optional required website preferences for a user to be considered a subscriber + * @param ?string $implicitSubscriptionQuery sql query that returns the set of users that are implicitly subscribed to * the subject (must return whole UserAccounts rows) */ -function getSubscribersOf(string $subjectType, int $subjectID, int $reqWebsitePrefs = null, string $implicitSubscriptionQry = null): array +function getSubscribersOf(string $subjectType, int $subjectID, int $requiredPreference = null, string $implicitSubscriptionQuery = null): array { - sanitize_sql_inputs($subjectType, $subjectID, $reqWebsitePrefs); + sanitize_sql_inputs($subjectType, $subjectID, $requiredPreference); - $websitePrefsFilter = ( - $reqWebsitePrefs === null ? "" : "AND (_ua.websitePrefs & $reqWebsitePrefs) != 0" - ); + $preferenceFilter = ''; + if ($requiredPreference !== null) { + $preferenceBit = 1 << $requiredPreference; + $preferenceFilter = "AND (_ua.websitePrefs & $preferenceBit) != 0"; + } - $explicitSubscriptionQry = " + $explicitSubscriptionQuery = " SELECT _ua.User, _ua.EmailAddress @@ -122,19 +126,18 @@ function getSubscribersOf(string $subjectType, int $subjectID, int $reqWebsitePr _sub.SubjectType = '$subjectType' AND _sub.SubjectID = $subjectID AND _sub.State = 1 - $websitePrefsFilter + $preferenceFilter "; - if ($implicitSubscriptionQry === null) { - $query = $explicitSubscriptionQry; - } else { + $query = $explicitSubscriptionQuery; + if ($implicitSubscriptionQuery !== null) { $query = " SELECT _ua.User, _ua.EmailAddress FROM ( - $implicitSubscriptionQry + $implicitSubscriptionQuery ) as _ua LEFT JOIN Subscription AS _sub ON (_sub.SubjectType = '$subjectType' @@ -142,9 +145,9 @@ function getSubscribersOf(string $subjectType, int $subjectID, int $reqWebsitePr AND _sub.UserID = _ua.ID) WHERE COALESCE(_sub.State, 1) = 1 - $websitePrefsFilter + $preferenceFilter UNION - $explicitSubscriptionQry + $explicitSubscriptionQuery "; } @@ -157,6 +160,34 @@ function getSubscribersOf(string $subjectType, int $subjectID, int $reqWebsitePr return mysqli_fetch_all($dbResult, MYSQLI_ASSOC); } +// article comments + +function isUserSubscribedToArticleComments($articleType, $articleID, $userID): bool +{ + sanitize_sql_inputs($articleType, $articleID, $userID); + + $subjectType = SubscriptionSubjectType::fromArticleType($articleType); + if ($subjectType === null) { + return false; + } + + return isUserSubscribedTo( + $subjectType, + $articleID, + $userID, + " + SELECT DISTINCT ua.* + FROM + Comment AS c + LEFT JOIN UserAccounts AS ua ON ua.ID = c.UserID + WHERE + c.ArticleType = $articleType + AND c.ArticleID = $articleID + AND c.UserID = $userID + " + ); +} + /** * Merges two lists of subscribers as returned by `getSubscribersOf`. */ @@ -181,75 +212,75 @@ function mergeSubscribers(array $subscribersA, array $subscribersB): array return $subscribersA; } -function getSubscribersOfGameWall($gameID): array +function getSubscribersOfGame($gameID): array { - return getSubscribersOfArticle(1, $gameID, (1 << 1)); + return getSubscribersOfArticle(ArticleType::Game, $gameID); } function getSubscribersOfAchievement($achievementID, $gameID, $achievementAuthor): array { + return mergeSubscribers( // users directly subscribed to the achievement - $achievementSubs = getSubscribersOfArticle(2, $achievementID, (1 << 1), $achievementAuthor); - - // devs subscribed to the achievement through the game - $gameAchievementsSubs = getSubscribersOf(SubscriptionSubjectType::GameAchievements, $gameID, (1 << 0) /* (1 << 1) */); - - return mergeSubscribers($achievementSubs, $gameAchievementsSubs); + getSubscribersOfArticle(ArticleType::Achievement, $achievementID, $achievementAuthor), + // devs subscribed to the achievement through the game + getSubscribersOf(SubscriptionSubjectType::GameAchievements, $gameID, Preference::EmailOn_ActivityComment) + ); } function getSubscribersOfUserWall($userID, $userName): array { - return getSubscribersOfArticle(3, $userID, (1 << 2), $userName); -} - -function getSubscribersOfFeedActivity($activityID, $author): array -{ - return getSubscribersOfArticle(5, $activityID, (1 << 0), $author, true); + return getSubscribersOfArticle(ArticleType::User, $userID, $userName); } function getSubscribersOfTicket($ticketID, $ticketAuthor, $gameID): array { + return mergeSubscribers( // users directly subscribed to the ticket - $ticketSubs = getSubscribersOfArticle(7, $ticketID, (1 << 1), $ticketAuthor, true); - - // devs subscribed to the ticket through the game - $gameTicketsSubs = getSubscribersOf(SubscriptionSubjectType::GameTickets, $gameID, (1 << 0) /* (1 << 1) */); - - return mergeSubscribers($ticketSubs, $gameTicketsSubs); + getSubscribersOfArticle(ArticleType::AchievementTicket, $ticketID, $ticketAuthor, false), + // devs subscribed to the ticket through the game + getSubscribersOf(SubscriptionSubjectType::GameTickets, $gameID, Preference::EmailOn_ActivityComment) + ); } function getSubscribersOfArticle( $articleType, $articleID, - $reqWebsitePrefs, $subjectAuthor = null, - $noExplicitSubscriptions = false + $byExplicitSubscriptions = true ): array { - $websitePrefsFilter = ($noExplicitSubscriptions !== true - ? "" : "AND (_ua.websitePrefs & $reqWebsitePrefs) != 0"); - - $authorQry = ($subjectAuthor === null ? "" : " - UNION - SELECT _ua.* - FROM UserAccounts as _ua - WHERE _ua.User = '$subjectAuthor' - $websitePrefsFilter - "); - - $qry = " + $query = " SELECT DISTINCT _ua.* FROM Comment AS _c INNER JOIN UserAccounts as _ua ON _ua.ID = _c.UserID WHERE _c.ArticleType = $articleType AND _c.ArticleID = $articleID - $websitePrefsFilter - $authorQry "; - if ($noExplicitSubscriptions) { - $dbResult = s_mysql_query($qry); + if (!$byExplicitSubscriptions) { + $preferenceBit = match ($articleType) { + ArticleType::AchievementTicket => 1 << Preference::EmailOn_AchievementComment, + // ArticleType::Activity => 1 << UserPreference::EmailOn_ActivityComment, + ArticleType::Game => 1 << Preference::EmailOn_AchievementComment, + ArticleType::Achievement => 1 << Preference::EmailOn_AchievementComment, + ArticleType::User => 1 << Preference::EmailOn_UserWallComment, + default => null, + }; + + $query .= " AND (_ua.websitePrefs & $preferenceBit) != 0"; + } + + if ($subjectAuthor !== null) { + $query .= " + UNION + SELECT _ua.* + FROM UserAccounts as _ua + WHERE _ua.User = '$subjectAuthor' + "; + } + + if (!$byExplicitSubscriptions) { + $dbResult = s_mysql_query($query); if (!$dbResult) { - log_sql_fail(); return []; } @@ -265,33 +296,21 @@ function getSubscribersOfArticle( $subjectType, $articleID, (1 << 0), // code suggests the value of $reqWebsitePrefs should be used, but the feature is disabled for now - $qry + $query ); } -function isUserSubscribedToArticleComments($articleType, $articleID, $userID): bool +function getSubscriptions(string $userID) { - sanitize_sql_inputs($articleType, $articleID, $userID); + $query = "SELECT * FROM Subscription WHERE UserID = $userID"; - $subjectType = SubscriptionSubjectType::fromArticleType($articleType); - - if ($subjectType === null) { - return false; + $dbResult = s_mysql_query($query); + if ($dbResult === false) { + global $db; + error_log(__FUNCTION__ . ": " . mysqli_error($db)); + error_log($query); + return []; } - return isUserSubscribedTo( - $subjectType, - $articleID, - $userID, - " - SELECT DISTINCT ua.* - FROM - Comment AS c - LEFT JOIN UserAccounts AS ua ON ua.ID = c.UserID - WHERE - c.ArticleType = $articleType - AND c.ArticleID = $articleID - AND c.UserID = $userID - " - ); + return mysqli_fetch_all($dbResult, MYSQLI_ASSOC); } diff --git a/lib/functions.php b/lib/functions.php index df27397f89..4c3c8925e9 100644 --- a/lib/functions.php +++ b/lib/functions.php @@ -23,7 +23,6 @@ require_once __DIR__ . '/database/set-request.php'; require_once __DIR__ . '/database/site-award.php'; require_once __DIR__ . '/database/static.php'; -require_once __DIR__ . '/database/subscription.php'; require_once __DIR__ . '/database/ticket.php'; require_once __DIR__ . '/database/user.php'; require_once __DIR__ . '/database/user-account-deletion.php'; @@ -32,6 +31,8 @@ require_once __DIR__ . '/database/user-email-verify.php'; require_once __DIR__ . '/database/user-password-reset.php'; require_once __DIR__ . '/database/user-permission.php'; +require_once __DIR__ . '/database/user-preference.php'; +require_once __DIR__ . '/database/user-subscription.php'; require_once __DIR__ . '/render/achievement.php'; require_once __DIR__ . '/render/code-note.php'; diff --git a/lib/util/bit.php b/lib/util/bit.php index f5a5a66728..344883f625 100644 --- a/lib/util/bit.php +++ b/lib/util/bit.php @@ -1,6 +1,6 @@ "; + echo << + EOL; } ?> @@ -170,27 +165,12 @@ function onResetComplete(data) { return false; } - function DoChangeUserPrefs() { - var newUserPrefs = 0; - for (i = 0; i < 7; ++i) { // 0-6 are set if checked - var checkbox = document.getElementById('UserPreference' + i); - if (checkbox != null && checkbox.checked) - newUserPrefs += (1 << i); - } - - for (i = 8; i < 15; ++i) { // 8-14 are set if checked - var checkbox = document.getElementById('UserPreference' + i); - if (checkbox != null && checkbox.checked) - newUserPrefs += (1 << i); - } - - // 7 is set if unchecked - var checkbox = document.getElementById('UserPref7'); - if (checkbox != null && !checkbox.checked) - newUserPrefs += (1 << 7); + function UpdatePreference(bitIndex) { + var checkbox = document.getElementById('UserPreference' + bitIndex); + console.log(bitIndex, (checkbox && checkbox.checked) ? 1 : 0) $('#loadingicon').attr('src', '').fadeTo(100, 1.0); - var posting = $.post('/request/user/update-notification.php', {u: '', p: newUserPrefs}); + var posting = $.post('/request/user/update-preference.php', { p: bitIndex, v: (checkbox && checkbox.checked) ? 1 : 0 }); posting.done(function () { $('#loadingicon').attr('src', '').delay(750).fadeTo('slow', 0.0); }); @@ -343,51 +323,44 @@ function validateEmail() { Event - Email Me - Site Msg + Email If someone comments on my activity: - - + If someone comments on an achievement I created: - - + If someone comments on my user wall: - - + If someone comments on a forum topic I'm involved in: - - + If someone adds me as a friend: - - + If someone sends me a private message: - - + With the weekly RA Newsletter: - - - - - When viewing a game with mature content: - - + +
+ +
' width='16' height='16' alt='loading icon'/>
diff --git a/public/gameInfo.php b/public/gameInfo.php index b3b1d110a4..dcdd53adb8 100644 --- a/public/gameInfo.php +++ b/public/gameInfo.php @@ -3,10 +3,10 @@ use RA\ArticleType; use RA\ImageType; use RA\Permissions; +use RA\Preference; use RA\RatingType; use RA\SubscriptionSubjectType; use RA\TicketFilters; -use RA\UserPreference; require_once __DIR__ . '/../vendor/autoload.php'; require_once __DIR__ . '/../lib/bootstrap.php'; @@ -78,7 +78,7 @@ if ($v != 1 && $isFullyFeaturedGame) { foreach ($gameHubs as $hub) { if ($hub['Title'] == '[Theme - Mature]') { - if ($userDetails && BitSet($userDetails['websitePrefs'], UserPreference::SiteMsgOff_MatureContent)) { + if ($userDetails && isBitSet($userDetails['websitePrefs'], Preference::HideMatureContent)) { break; } diff --git a/public/request/user/update-avatar.php b/public/request/user/update-avatar.php index f44ed63e2e..58af137af1 100644 --- a/public/request/user/update-avatar.php +++ b/public/request/user/update-avatar.php @@ -17,7 +17,7 @@ echo json_encode([ 'Success' => false, 'Error' => $exception->getMessage(), - ]); + ], JSON_THROW_ON_ERROR); exit; } diff --git a/public/request/user/update-notification.php b/public/request/user/update-notification.php deleted file mode 100644 index 0321cff760..0000000000 --- a/public/request/user/update-notification.php +++ /dev/null @@ -1,27 +0,0 @@ - false, 'error' => 'Unauthorized']); + exit; +} + +$preference = (int) requestInputPost('p'); +$value = (int) requestInputPost('v'); + +if (!Preference::valid($preference)) { + http_response_code(400); + echo json_encode(['success' => false, 'error' => 'Bad request: invalid preference']); + exit; +} + +if (updatePreference($user, $preference, (bool) $value)) { + echo json_encode(['success' => true]); + exit; +} + +echo json_encode(['success' => false, 'error' => 'Something went wrong']); diff --git a/public/subscriptions.php b/public/subscriptions.php new file mode 100644 index 0000000000..a19d6a4e32 --- /dev/null +++ b/public/subscriptions.php @@ -0,0 +1,51 @@ + + + + + + + + + +
+
+ +
+

Subsriptions

+ + + +
+
+
+ +
+ + +*/ diff --git a/public/test/subscriptions.php b/public/test/subscriptions.php new file mode 100644 index 0000000000..31e2023e15 --- /dev/null +++ b/public/test/subscriptions.php @@ -0,0 +1,13 @@ +assertTrue(isBitSet(7, 0)); + $this->assertTrue(isBitSet(7, 1)); + $this->assertTrue(isBitSet(7, 2)); + $this->assertFalse(isBitSet(7, 3)); + $this->assertFalse(isBitSet(7, 4)); + $this->assertTrue(isBitSet("7", 0)); + } +}