chore: review comments

flarum · Jun 21, 2024 · 942d5c3 · 942d5c3
1 parent abc545f
commit 942d5c3
Show file tree

Hide file tree

Showing 11 changed files with 80 additions and 35 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<Attrs extends ExtensionPageAttrs = ExtensionP
   }
 
   body(vnode: Mithril.VnodeDOM<Attrs, this>) {
+    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() ? (
-      <div className="ExtensionPage-body">{this.sections(vnode).toArray()}</div>
+      <div className="ExtensionPage-body">
+        {!supportsDbDriver && (
+          <Alert type="error" dismissible={false}>
+            {app.translator.trans('core.admin.extension.database_driver_mismatch')}
+          </Alert>
+        )}
+        {this.sections(vnode).toArray()}
+      </div>
     ) : (
       <div className="container">
         <h3 className="ExtensionPage-subHeader">{app.translator.trans('core.admin.extension.enable_to_see')}</h3>
@@ -187,7 +199,6 @@ export default class ExtensionPage<Attrs extends ExtensionPageAttrs = ExtensionP
         }
       };
 
-      // TODO v2.0: rename `uninstall` to `purge`
       items.add(
         'uninstall',
         <Button icon="fas fa-trash-alt" className="Button Button--primary" onclick={purge.bind(this)}>

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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