#10328 Fix sql for postgres

pkp · Sep 16, 2024 · 5253484 · 5253484
1 parent 62adea1
commit 5253484
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 63 deletions.
diff --git a/classes/announcement/Announcement.php b/classes/announcement/Announcement.php
@@ -170,8 +170,8 @@ protected function scopeWithTypeIds(EloquentBuilder $builder, array $typeIds): E
      */
     protected function scopeWithActiveByDate(EloquentBuilder $builder, string $date = ''): EloquentBuilder
     {
-        return $builder->where('a.date_expire', '>', empty($date) ? Core::getCurrentDate() : $date)
-            ->orWhereNull('a.date_expire');
+        return $builder->where('date_expire', '>', empty($date) ? Core::getCurrentDate() : $date)
+            ->orWhereNull('date_expire');
     }
 
     /**

diff --git a/classes/core/SettingsBuilder.php b/classes/core/SettingsBuilder.php
@@ -58,35 +58,31 @@ public function update(array $values)
 
         // Don't update settings if they aren't set
         if ($settingValues->isEmpty()) {
-            return parent::update($primaryValues);
+            return parent::update($primaryValues->toArray());
+        }
+
+        $newQuery = clone $this->query;
+
+        if ($primaryValues->isNotEmpty()) {
+            $count = parent::update($primaryValues->toArray());
         }
 
         // TODO Eloquent transforms attributes to snake case, find and override instead of transforming here
         $settingValues = $settingValues->mapWithKeys(
             fn (mixed $value, string $key) => [Str::camel($key) => $value]
         );
 
-        $u = $this->model->getTable();
         $us = $this->model->getSettingsTable();
         $primaryKey = $this->model->getKeyName();
-        $query = $this->toBase();
 
-        // Add table name to specify the right columns in the already existing WHERE statements
-        $query->wheres = collect($query->wheres)->map(function (array $item) use ($u) {
-            $item['column'] = $u . '.' . $item['column'];
-            return $item;
-        })->toArray();
 
-        $sql = $this->buildUpdateSql($settingValues, $us, $query);
+        $sql = $this->buildUpdateSql($settingValues, $us, $newQuery);
 
         // Build a query for update
-        $count = $query->fromRaw($u . ', ' . $us)
-            ->whereColumn($u . '.' . $primaryKey, '=', $us . '.' . $primaryKey)
-            ->update(array_merge($primaryValues->toArray(), [
-                $us . '.setting_value' => DB::raw($sql),
-            ]));
+        $settingCount = DB::table($us)->whereIn($us . '.' . $primaryKey, $newQuery->select($primaryKey))
+            ->update([$us . '.setting_value' => DB::raw($sql)]);
 
-        return $count;
+        return $count ? $count + $settingCount : $settingCount; // TODO Return the count of updated setting rows?
     }
 
     /**
@@ -251,50 +247,32 @@ public function whereIn($column, $values, $boolean = 'and', $not = false)
     protected function getModelWithSettings(array|string $columns = ['*']): Collection
     {
         // First, get all Model columns from the main table
-        $rows = $this->query->get();
+        $primaryKey = $this->model->getKeyName();
+        $rows = $this->query->get()->keyBy($primaryKey);
         if ($rows->isEmpty()) {
             return $rows;
         }
 
         // Retrieve records from the settings table associated with the primary Model IDs
-        $primaryKey = $this->model->getKeyName();
         $ids = $rows->pluck($primaryKey)->toArray();
-        $settingsChunks = DB::table($this->model->getSettingsTable())
+        $settings = DB::table($this->model->getSettingsTable())
             ->whereIn($primaryKey, $ids)
-            // Order data by original primary Model's IDs
-            ->orderByRaw(
-                'FIELD(' .
-                $primaryKey .
-                ',' .
-                implode(',', $ids) .
-                ')'
-            )
-            ->get()
-            // Chunk records by Model IDs
-            ->chunkWhile(
-                fn (\stdClass $value, int $key, Collection $chunk) =>
-                    $value->{$primaryKey} === $chunk->last()->{$primaryKey}
-            );
-
-        // Associate settings with correspondent Model data
-        $rows = $rows->map(function (stdClass $row) use ($settingsChunks, $primaryKey, $columns) {
-            if ($settingsChunks->isNotEmpty()) {
-                // Don't iterate through all setting rows to avoid Big O(n^2) complexity, chunks are ordered by Model's IDs
-                // If Model's ID doesn't much it means it doesn't have any settings
-                if ($row->{$primaryKey} === $settingsChunks->first()->first()->{$primaryKey}) {
-                    $settingsChunk = $settingsChunks->shift();
-                    $settingsChunk->each(function (\stdClass $settingsRow) use ($row) {
-                        if ($settingsRow->locale) {
-                            $row->{$settingsRow->setting_name}[$settingsRow->locale] = $settingsRow->setting_value;
-                        } else {
-                            $row->{$settingsRow->setting_name} = $settingsRow->setting_value;
-                        }
-                    });
-                }
-                $row = $this->filterRow($row, $columns);
+            ->get();
+
+        $settings->each(function (\stdClass $setting) use ($rows, $primaryKey, $columns) {
+            $settingModelId = $setting->{$primaryKey};
+
+            // Retract the row and fill it with data from a settings table
+            $exactRow = $rows->pull($settingModelId);
+            if ($setting->locale) {
+                $exactRow->{$setting->setting_name}[$setting->locale] = $setting->setting_value;
+            } else {
+                $exactRow->{$setting->setting_name} = $setting->setting_value;
             }
 
-            return $row;
+            // Include only specified columns
+            $exactRow = $this->filterRow($exactRow, $columns);
+            $rows->put($settingModelId, $exactRow);
         });
 
         return $rows;

diff --git a/pages/announcement/AnnouncementHandler.php b/pages/announcement/AnnouncementHandler.php
@@ -53,7 +53,7 @@ public function index($args, $request)
         $request->getContext() ? $contextIds[] = $request->getContext()->getId() : $contextIds[] = PKPApplication::SITE_CONTEXT_ID;
         $announcements->withContextIds($contextIds);
 
-        $templateMgr->assign('announcements', $announcements->get()->toArray());
+        $templateMgr->assign('announcements', $announcements->get());
         $templateMgr->display('frontend/pages/announcements.tpl');
     }
 

diff --git a/templates/frontend/objects/announcement_summary.tpl b/templates/frontend/objects/announcement_summary.tpl
@@ -14,31 +14,31 @@
 	{assign var="heading" value="h2"}
 {/if}
 
-<article class="obj_announcement_summary{if $announcement->getImage()} obj_announcement_summary_has_image{/if}">
-	{if $announcement->getImage()}
+<article class="obj_announcement_summary{if $announcement->image} obj_announcement_summary_has_image{/if}">
+	{if $announcement->image}
 		<img
 			class="obj_announcement_summary_image"
-			src="{$announcement->getImageUrl()}"
-			alt="{$announcement->getImageAltText()}"
+			src="{$announcement->imageUrl}"
+			alt="{$announcement->imageAltText}"
 		/>
 	{/if}
 	<div class="obj_announcement_summary_details">
 		<{$heading}>
-			<a href="{url router=PKP\core\PKPApplication::ROUTE_PAGE page="announcement" op="view" path=$announcement->getId()}">
-				{$announcement->getLocalizedTitle()|escape}
+			<a href="{url router=PKP\core\PKPApplication::ROUTE_PAGE page="announcement" op="view" path=$announcement->getKey()}">
+				{$announcement->getLocalizedData('title')|escape}
 			</a>
 		</{$heading}>
 		<div class="date">
-			{$announcement->getDatePosted()|date_format:$dateFormatShort}
+			{$announcement->datePosted|date_format:$dateFormatShort}
 		</div>
 		<div class="summary">
-			{$announcement->getLocalizedDescriptionShort()|strip_unsafe_html}
-			<a href="{url router=PKP\core\PKPApplication::ROUTE_PAGE page="announcement" op="view" path=$announcement->getId()}" class="read_more">
+			{$announcement->getLocalizedData('descriptionShort')|strip_unsafe_html}
+			<a href="{url router=PKP\core\PKPApplication::ROUTE_PAGE page="announcement" op="view" path=$announcement->getKey()}" class="read_more">
 				<span aria-hidden="true" role="presentation">
 					{translate key="common.readMore"}
 				</span>
 				<span class="pkp_screen_reader">
-					{translate key="common.readMoreWithTitle" title=$announcement->getLocalizedTitle()|escape}
+					{translate key="common.readMoreWithTitle" title=$announcement->getLocalizedData('title')|escape}
 				</span>
 			</a>
 		</div>