From 942d5c357b120b9c7ccaf1ce7c6aba7d47e85378 Mon Sep 17 00:00:00 2001 From: Sami Mazouz Date: Fri, 21 Jun 2024 08:37:40 +0100 Subject: [PATCH] chore: review comments --- .../Api/Controller/ListFlagsController.php | 2 +- .../tests/integration/api/flags/ListTest.php | 8 +++++++- .../src/Api/Controller/ShowStatisticsData.php | 4 +++- .../src/PinStickiedDiscussionsToTop.php | 5 ++++- .../core/js/src/admin/AdminApplication.tsx | 7 +++++++ .../js/src/admin/components/ExtensionPage.tsx | 15 ++++++++++++-- framework/core/locale/core.yml | 1 + ...11_093900_change_access_tokens_columns.php | 14 ++++++------- ...1_convert_preferences_to_json_in_users.php | 20 +++++++++---------- ..._convert_data_to_json_in_notifications.php | 20 +++++++++---------- .../Install/Steps/EnableBundledExtensions.php | 19 ++++++++++++++++-- 11 files changed, 80 insertions(+), 35 deletions(-) diff --git a/extensions/flags/src/Api/Controller/ListFlagsController.php b/extensions/flags/src/Api/Controller/ListFlagsController.php index 21a64c34a4..50931e29f9 100644 --- a/extensions/flags/src/Api/Controller/ListFlagsController.php +++ b/extensions/flags/src/Api/Controller/ListFlagsController.php @@ -60,7 +60,7 @@ protected function data(ServerRequestInterface $request, Document $document): it fn (Builder $query) => $query->distinct('post_id')->orderBy('post_id'), else: fn (Builder $query) => $query->groupBy('post_id') ) - ->latest('flags.id') + ->latest('flags.created_at') ->get(); $this->loadRelations($flags, $include, $request); diff --git a/extensions/flags/tests/integration/api/flags/ListTest.php b/extensions/flags/tests/integration/api/flags/ListTest.php index c934f23d82..92f36e1627 100644 --- a/extensions/flags/tests/integration/api/flags/ListTest.php +++ b/extensions/flags/tests/integration/api/flags/ListTest.php @@ -17,6 +17,7 @@ use Flarum\Testing\integration\RetrievesAuthorizedUsers; use Flarum\Testing\integration\TestCase; use Flarum\User\User; +use Illuminate\Database\PostgresConnection; use Illuminate\Support\Arr; class ListTest extends TestCase @@ -85,7 +86,12 @@ public function admin_can_see_one_flag_per_post() $data = json_decode($body, true)['data']; $ids = Arr::pluck($data, 'id'); - $this->assertCount(3, $data); + + if ($this->database() instanceof PostgresConnection) { + $this->assertEqualsCanonicalizing(['3', '4', '5'], $ids); + } else { + $this->assertEqualsCanonicalizing(['1', '4', '5'], $ids); + } } /** diff --git a/extensions/statistics/src/Api/Controller/ShowStatisticsData.php b/extensions/statistics/src/Api/Controller/ShowStatisticsData.php index 285bfcb5c7..82dc3cad4d 100644 --- a/extensions/statistics/src/Api/Controller/ShowStatisticsData.php +++ b/extensions/statistics/src/Api/Controller/ShowStatisticsData.php @@ -11,6 +11,7 @@ use Carbon\Carbon; use DateTime; +use Exception; use Flarum\Discussion\Discussion; use Flarum\Http\RequestUtil; use Flarum\Post\Post; @@ -141,7 +142,8 @@ private function getTimedCounts(Builder $query, string $column, ?DateTime $start $dbFormattedDatetime = match ($query->getConnection()->getDriverName()) { 'sqlite' => "strftime($format, $column)", 'pgsql' => "TO_CHAR($column, $format)", - default => "DATE_FORMAT($column, $format)", + 'mysql' => "DATE_FORMAT($column, $format)", + default => throw new Exception('Unsupported database driver'), }; $results = $query diff --git a/extensions/sticky/src/PinStickiedDiscussionsToTop.php b/extensions/sticky/src/PinStickiedDiscussionsToTop.php index c0dd58bde8..dcb53e3550 100755 --- a/extensions/sticky/src/PinStickiedDiscussionsToTop.php +++ b/extensions/sticky/src/PinStickiedDiscussionsToTop.php @@ -9,6 +9,7 @@ namespace Flarum\Sticky; +use DateTime; use Flarum\Search\Database\DatabaseSearchState; use Flarum\Search\SearchCriteria; use Flarum\Tags\Search\Filter\TagFilter; @@ -46,6 +47,8 @@ public function __invoke(DatabaseSearchState $state, SearchCriteria $criteria): $sticky->where('is_sticky', true); unset($sticky->orders); + $epochTime = (new DateTime('@0'))->format('Y-m-d H:i:s'); + /** @var Builder $q */ foreach ([$sticky, $query] as $q) { $read = $q->newQuery() @@ -58,7 +61,7 @@ public function __invoke(DatabaseSearchState $state, SearchCriteria $criteria): // Add the bindings manually (rather than as the second // argument in orderByRaw) for now due to a bug in Laravel which // would add the bindings in the wrong order. - $q->selectRaw('(is_sticky and not exists ('.$read->toSql().') and last_posted_at > ?) as is_unread_sticky', array_merge($read->getBindings(), [$state->getActor()->marked_all_as_read_at ?: '1970-01-01 00:00:00'])); + $q->selectRaw('(is_sticky and not exists ('.$read->toSql().') and last_posted_at > ?) as is_unread_sticky', array_merge($read->getBindings(), [$state->getActor()->marked_all_as_read_at ?: $epochTime])); } $query->union($sticky); diff --git a/framework/core/js/src/admin/AdminApplication.tsx b/framework/core/js/src/admin/AdminApplication.tsx index 978f92e158..512390aabe 100644 --- a/framework/core/js/src/admin/AdminApplication.tsx +++ b/framework/core/js/src/admin/AdminApplication.tsx @@ -49,6 +49,13 @@ export interface AdminApplicationData extends ApplicationData { maintenanceByConfig: boolean; safeModeExtensions?: string[] | null; safeModeExtensionsConfig?: string[] | null; + + dbDriver: string; + dbVersion: string; + phpVersion: string; + queueDriver: string; + schedulerStatus: string; + sessionDriver: string; } export default class AdminApplication extends Application { diff --git a/framework/core/js/src/admin/components/ExtensionPage.tsx b/framework/core/js/src/admin/components/ExtensionPage.tsx index 8d1a42fa4d..abf6166581 100644 --- a/framework/core/js/src/admin/components/ExtensionPage.tsx +++ b/framework/core/js/src/admin/components/ExtensionPage.tsx @@ -20,6 +20,7 @@ import Form from '../../common/components/Form'; import Icon from '../../common/components/Icon'; import { MaintenanceMode } from '../../common/Application'; import InfoTile from '../../common/components/InfoTile'; +import Alert from '../../common/components/Alert'; export interface ExtensionPageAttrs extends IPageAttrs { id: string; @@ -79,8 +80,19 @@ export default class ExtensionPage) { + const supportsDbDriver = + !this.extension.extra['flarum-extension']['database-support'] || + this.extension.extra['flarum-extension']['database-support'].map((driver) => driver.toLowerCase()).includes(app.data.dbDriver.toLowerCase()); + return this.isEnabled() ? ( -
{this.sections(vnode).toArray()}
+
+ {!supportsDbDriver && ( + + {app.translator.trans('core.admin.extension.database_driver_mismatch')} + + )} + {this.sections(vnode).toArray()} +
) : (

{app.translator.trans('core.admin.extension.enable_to_see')}

@@ -187,7 +199,6 @@ export default class ExtensionPage diff --git a/framework/core/locale/core.yml b/framework/core/locale/core.yml index 1848f3807a..459a2e1976 100644 --- a/framework/core/locale/core.yml +++ b/framework/core/locale/core.yml @@ -211,6 +211,7 @@ core: extension: configure_scopes: Configure Scopes confirm_purge: Purging will remove all database entries and assets related to the extension. It will not uninstall the extension; that must be done via Composer. Are you sure you want to continue? + database_driver_mismatch: This extension does not support your configured database driver. disabled: Disabled enable_to_see: Enable the extension to view and change settings. enabled: Enabled diff --git a/framework/core/migrations/2018_01_11_093900_change_access_tokens_columns.php b/framework/core/migrations/2018_01_11_093900_change_access_tokens_columns.php index a60269e4ad..09b392e0c9 100644 --- a/framework/core/migrations/2018_01_11_093900_change_access_tokens_columns.php +++ b/framework/core/migrations/2018_01_11_093900_change_access_tokens_columns.php @@ -26,13 +26,7 @@ $table->integer('user_id')->unsigned()->change(); }); - if ($schema->getConnection()->getDriverName() !== 'pgsql') { - // Use a separate schema instance because this column gets renamed - // in the previous one. - $schema->table('access_tokens', function (Blueprint $table) { - $table->dateTime('last_activity_at')->change(); - }); - } else { + if ($schema->getConnection()->getDriverName() === 'pgsql') { $prefix = $schema->getConnection()->getTablePrefix(); // Changing an integer col to datetime is an unusual operation in PostgreSQL. @@ -41,6 +35,12 @@ ALTER COLUMN last_activity_at TYPE TIMESTAMP(0) WITHOUT TIME ZONE USING to_timestamp(last_activity_at) SQL); + } else { + // Use a separate schema instance because this column gets renamed + // in the previous one. + $schema->table('access_tokens', function (Blueprint $table) { + $table->dateTime('last_activity_at')->change(); + }); } }, diff --git a/framework/core/migrations/2024_05_05_000001_convert_preferences_to_json_in_users.php b/framework/core/migrations/2024_05_05_000001_convert_preferences_to_json_in_users.php index e63ede3b42..1129e4f03c 100644 --- a/framework/core/migrations/2024_05_05_000001_convert_preferences_to_json_in_users.php +++ b/framework/core/migrations/2024_05_05_000001_convert_preferences_to_json_in_users.php @@ -12,26 +12,26 @@ return [ 'up' => function (Builder $schema) { - if ($schema->getConnection()->getDriverName() !== 'pgsql') { - $schema->table('users', function (Blueprint $table) { - $table->json('preferences')->nullable()->change(); - }); - } else { + if ($schema->getConnection()->getDriverName() === 'pgsql') { $users = $schema->getConnection()->getSchemaGrammar()->wrapTable('users'); $preferences = $schema->getConnection()->getSchemaGrammar()->wrap('preferences'); $schema->getConnection()->statement("ALTER TABLE $users ALTER COLUMN $preferences TYPE JSON USING preferences::TEXT::JSON"); + } else { + $schema->table('users', function (Blueprint $table) { + $table->json('preferences')->nullable()->change(); + }); } }, 'down' => function (Builder $schema) { - if ($schema->getConnection()->getDriverName() !== 'pgsql') { - $schema->table('users', function (Blueprint $table) { - $table->binary('preferences')->nullable()->change(); - }); - } else { + if ($schema->getConnection()->getDriverName() === 'pgsql') { $users = $schema->getConnection()->getSchemaGrammar()->wrapTable('users'); $preferences = $schema->getConnection()->getSchemaGrammar()->wrap('preferences'); $schema->getConnection()->statement("ALTER TABLE $users ALTER COLUMN $preferences TYPE BYTEA USING preferences::TEXT::BYTEA"); + } else { + $schema->table('users', function (Blueprint $table) { + $table->binary('preferences')->nullable()->change(); + }); } } ]; diff --git a/framework/core/migrations/2024_05_07_000001_convert_data_to_json_in_notifications.php b/framework/core/migrations/2024_05_07_000001_convert_data_to_json_in_notifications.php index e915775ee6..c433d0c087 100644 --- a/framework/core/migrations/2024_05_07_000001_convert_data_to_json_in_notifications.php +++ b/framework/core/migrations/2024_05_07_000001_convert_data_to_json_in_notifications.php @@ -12,26 +12,26 @@ return [ 'up' => function (Builder $schema) { - if ($schema->getConnection()->getDriverName() !== 'pgsql') { - $schema->table('notifications', function (Blueprint $table) { - $table->json('data')->nullable()->change(); - }); - } else { + if ($schema->getConnection()->getDriverName() === 'pgsql') { $notifications = $schema->getConnection()->getSchemaGrammar()->wrapTable('notifications'); $data = $schema->getConnection()->getSchemaGrammar()->wrap('data'); $schema->getConnection()->statement("ALTER TABLE $notifications ALTER COLUMN $data TYPE JSON USING data::TEXT::JSON"); + } else { + $schema->table('notifications', function (Blueprint $table) { + $table->json('data')->nullable()->change(); + }); } }, 'down' => function (Builder $schema) { - if ($schema->getConnection()->getDriverName() !== 'pgsql') { - $schema->table('notifications', function (Blueprint $table) { - $table->binary('data')->nullable()->change(); - }); - } else { + if ($schema->getConnection()->getDriverName() === 'pgsql') { $notifications = $schema->getConnection()->getSchemaGrammar()->wrapTable('notifications'); $data = $schema->getConnection()->getSchemaGrammar()->wrap('data'); $schema->getConnection()->statement("ALTER TABLE $notifications ALTER COLUMN $data TYPE BYTEA USING data::TEXT::BYTEA"); + } else { + $schema->table('notifications', function (Blueprint $table) { + $table->binary('data')->nullable()->change(); + }); } } ]; diff --git a/framework/core/src/Install/Steps/EnableBundledExtensions.php b/framework/core/src/Install/Steps/EnableBundledExtensions.php index b39c7954d1..2029347b79 100644 --- a/framework/core/src/Install/Steps/EnableBundledExtensions.php +++ b/framework/core/src/Install/Steps/EnableBundledExtensions.php @@ -24,7 +24,22 @@ class EnableBundledExtensions implements Step { - public const EXTENSION_WHITELIST = []; + public const DEFAULT_ENABLED_EXTENSIONS = [ + 'flarum-approval', + 'flarum-bbcode', + 'flarum-emoji', + 'flarum-lang-english', + 'flarum-flags', + 'flarum-likes', + 'flarum-lock', + 'flarum-markdown', + 'flarum-mentions', + 'flarum-statistics', + 'flarum-sticky', + 'flarum-subscriptions', + 'flarum-suspend', + 'flarum-tags', + ]; /** * @var string[] @@ -39,7 +54,7 @@ public function __construct( private readonly string $assetPath, ?array $enabledExtensions = null ) { - $this->enabledExtensions = $enabledExtensions ?? self::EXTENSION_WHITELIST; + $this->enabledExtensions = $enabledExtensions ?? self::DEFAULT_ENABLED_EXTENSIONS; } public function getMessage(): string