From c9cda06979e415401d2b420fcdf86957349d7dba Mon Sep 17 00:00:00 2001 From: Vitalii Bezsheiko Date: Mon, 2 Sep 2024 23:46:20 +0300 Subject: [PATCH 1/9] pkp/pkp-lib#10328 Refactor Announcements --- .../PKPAnnouncementController.php | 69 ++- classes/announcement/Announcement.php | 505 ++++++++++-------- classes/announcement/AnnouncementTypeDAO.php | 4 +- classes/announcement/Repository.php | 285 +--------- classes/announcement/maps/Schema.php | 9 +- classes/core/SettingsBuilder.php | 363 +++++++++++++ .../StoreTemporaryFileException.php | 17 +- classes/core/maps/Schema.php | 13 + classes/core/traits/ModelWithSettings.php | 151 ++++++ classes/mail/mailables/AnnouncementNotify.php | 6 +- .../notification/PKPNotificationManager.php | 5 +- .../AnnouncementNotificationManager.php | 10 +- classes/services/PKPContextService.php | 8 +- classes/services/PKPSchemaService.php | 55 +- .../NotificationsGridCellProvider.php | 5 +- .../NewAnnouncementNotifyUsers.php | 8 +- pages/admin/AdminHandler.php | 15 +- pages/announcement/AnnouncementHandler.php | 28 +- pages/index/PKPIndexHandler.php | 21 +- pages/management/ManagementHandler.php | 9 +- pages/sitemap/PKPSitemapHandler.php | 7 +- schemas/announcement.json | 12 + .../frontend/objects/announcement_full.tpl | 16 +- .../frontend/objects/announcements_list.tpl | 6 +- templates/frontend/pages/announcement.tpl | 4 +- 25 files changed, 1026 insertions(+), 605 deletions(-) create mode 100644 classes/core/SettingsBuilder.php create mode 100644 classes/core/traits/ModelWithSettings.php diff --git a/api/v1/announcements/PKPAnnouncementController.php b/api/v1/announcements/PKPAnnouncementController.php index 9398fb3756d..7132c1e89e3 100644 --- a/api/v1/announcements/PKPAnnouncementController.php +++ b/api/v1/announcements/PKPAnnouncementController.php @@ -25,9 +25,10 @@ use Illuminate\Http\Response; use Illuminate\Support\Facades\Bus; use Illuminate\Support\Facades\Route; -use PKP\announcement\Collector; +use PKP\announcement\Announcement; use PKP\context\Context; use PKP\core\exceptions\StoreTemporaryFileException; +use PKP\core\PKPApplication; use PKP\core\PKPBaseController; use PKP\core\PKPRequest; use PKP\db\DAORegistry; @@ -124,7 +125,7 @@ public function authorize(PKPRequest $request, array &$args, array $roleAssignme */ public function get(Request $illuminateRequest): JsonResponse { - $announcement = Repo::announcement()->get((int) $illuminateRequest->route('announcementId')); + $announcement = Announcement::find((int) $illuminateRequest->route('announcementId')); if (!$announcement) { return response()->json([ @@ -133,7 +134,7 @@ public function get(Request $illuminateRequest): JsonResponse } // The assocId in announcements should always point to the contextId - if ($announcement->getData('assocId') !== $this->getRequest()->getContext()?->getId()) { + if ($announcement->getAttribute('assocId') !== $this->getRequest()->getContext()?->getId()) { return response()->json([ 'error' => __('api.announcements.400.contextsNotMatched') ], Response::HTTP_BAD_REQUEST); @@ -149,43 +150,38 @@ public function get(Request $illuminateRequest): JsonResponse */ public function getMany(Request $illuminateRequest): JsonResponse { - $collector = Repo::announcement()->getCollector() - ->limit(self::DEFAULT_COUNT) - ->offset(0); + $announcements = Announcement::limit(self::DEFAULT_COUNT)->offset(0); foreach ($illuminateRequest->query() as $param => $val) { switch ($param) { case 'typeIds': - $collector->filterByTypeIds( + $announcements->withTypeIds( array_map('intval', paramToArray($val)) ); break; case 'count': - $collector->limit(min((int) $val, self::MAX_COUNT)); + $announcements->limit(min((int) $val, self::MAX_COUNT)); break; case 'offset': - $collector->offset((int) $val); + $announcements->offset((int) $val); break; case 'searchPhrase': - $collector->searchPhrase($val); + $announcements->withSearchPhrase($val); break; } } if ($this->getRequest()->getContext()) { - $collector->filterByContextIds([$this->getRequest()->getContext()->getId()]); + $announcements->withContextIds([$this->getRequest()->getContext()->getId()]); } else { - $collector->withSiteAnnouncements(Collector::SITE_ONLY); + $announcements->withContextIds([PKPApplication::SITE_CONTEXT_ID]); } - - Hook::run('API::announcements::params', [$collector, $illuminateRequest]); - - $announcements = $collector->getMany(); + Hook::run('API::announcements::params', [$announcements, $illuminateRequest]); return response()->json([ - 'itemsMax' => $collector->getCount(), - 'items' => Repo::announcement()->getSchemaMap()->summarizeMany($announcements)->values(), + 'itemsMax' => $announcements->count(), + 'items' => Repo::announcement()->getSchemaMap()->summarizeMany($announcements->get())->values(), ], Response::HTTP_OK); } @@ -209,27 +205,22 @@ public function add(Request $illuminateRequest): JsonResponse return response()->json($errors, Response::HTTP_BAD_REQUEST); } - $announcement = Repo::announcement()->newDataObject($params); - try { - $announcementId = Repo::announcement()->add($announcement); + $announcement = Announcement::create($params); } catch (StoreTemporaryFileException $e) { - $announcementId = $e->dataObject->getId(); + $announcementId = $e->getDataObjectId(); if ($announcementId) { - $announcement = Repo::announcement()->get($announcementId); - Repo::announcement()->delete($announcement); + Announcement::destroy([$announcementId]); } return response()->json([ 'image' => [__('api.400.errorUploadingImage')] ], Response::HTTP_BAD_REQUEST); } - $announcement = Repo::announcement()->get($announcementId); - $sendEmail = (bool) filter_var($params['sendEmail'], FILTER_VALIDATE_BOOLEAN); if ($context) { - $this->notifyUsers($request, $context, $announcementId, $sendEmail); + $this->notifyUsers($request, $context, $announcement->getKey(), $sendEmail); } return response()->json(Repo::announcement()->getSchemaMap()->map($announcement), Response::HTTP_OK); @@ -243,7 +234,8 @@ public function edit(Request $illuminateRequest): JsonResponse $request = $this->getRequest(); $context = $request->getContext(); - $announcement = Repo::announcement()->get((int) $illuminateRequest->route('announcementId')); + /** @var Announcement $announcement */ + $announcement = Announcement::find((int) $illuminateRequest->route('announcementId')); if (!$announcement) { return response()->json([ @@ -251,19 +243,19 @@ public function edit(Request $illuminateRequest): JsonResponse ], Response::HTTP_NOT_FOUND); } - if ($announcement->getData('assocType') !== Application::get()->getContextAssocType()) { + if ($announcement->getAttribute('assocType') !== Application::get()->getContextAssocType()) { throw new Exception('Announcement has an assocType that did not match the context.'); } // Don't allow to edit an announcement from one context from a different context's endpoint - if ($request->getContext()?->getId() !== $announcement->getData('assocId')) { + if ($request->getContext()?->getId() !== $announcement->getAttribute('assocId')) { return response()->json([ 'error' => __('api.announcements.400.contextsNotMatched') ], Response::HTTP_FORBIDDEN); } $params = $this->convertStringsToSchema(PKPSchemaService::SCHEMA_ANNOUNCEMENT, $illuminateRequest->input()); - $params['id'] = $announcement->getId(); + $params['id'] = $announcement->getKey(); $params['typeId'] ??= null; $primaryLocale = $context ? $context->getPrimaryLocale() : $request->getSite()->getPrimaryLocale(); @@ -275,15 +267,15 @@ public function edit(Request $illuminateRequest): JsonResponse } try { - Repo::announcement()->edit($announcement, $params); + $announcement->update($params); } catch (StoreTemporaryFileException $e) { - Repo::announcement()->delete($announcement); + $announcement->delete(); // TODO do we really need to delete an announcement if the image upload fails? return response()->json([ 'image' => [__('api.400.errorUploadingImage')] ], Response::HTTP_BAD_REQUEST); } - $announcement = Repo::announcement()->get($announcement->getId()); + $announcement = Announcement::find($announcement->getKey()); return response()->json(Repo::announcement()->getSchemaMap()->map($announcement), Response::HTTP_OK); } @@ -295,7 +287,8 @@ public function delete(Request $illuminateRequest): JsonResponse { $request = $this->getRequest(); - $announcement = Repo::announcement()->get((int) $illuminateRequest->route('announcementId')); + /** @var Announcement $announcement */ + $announcement = Announcement::find((int) $illuminateRequest->route('announcementId')); if (!$announcement) { return response()->json([ @@ -303,12 +296,12 @@ public function delete(Request $illuminateRequest): JsonResponse ], Response::HTTP_NOT_FOUND); } - if ($announcement->getData('assocType') !== Application::get()->getContextAssocType()) { + if ($announcement->getAttribute('assocType') !== Application::get()->getContextAssocType()) { throw new Exception('Announcement has an assocType that did not match the context.'); } // Don't allow to delete an announcement from one context from a different context's endpoint - if ($request->getContext()?->getId() !== $announcement->getData('assocId')) { + if ($request->getContext()?->getId() !== $announcement->getAttribute('assocId')) { return response()->json([ 'error' => __('api.announcements.400.contextsNotMatched') ], Response::HTTP_FORBIDDEN); @@ -316,7 +309,7 @@ public function delete(Request $illuminateRequest): JsonResponse $announcementProps = Repo::announcement()->getSchemaMap()->map($announcement); - Repo::announcement()->delete($announcement); + $announcement->delete(); return response()->json($announcementProps, Response::HTTP_OK); } diff --git a/classes/announcement/Announcement.php b/classes/announcement/Announcement.php index f3760ad3381..81a58866c4c 100644 --- a/classes/announcement/Announcement.php +++ b/classes/announcement/Announcement.php @@ -1,345 +1,430 @@ getData('assocId'); - } + use HasCamelCasing; + use ModelWithSettings; - /** - * Set assoc ID for this announcement. - * - * @param int $assocId - */ - public function setAssocId($assocId) - { - $this->setData('assocId', $assocId); - } + // The subdirectory where announcement images are stored + public const IMAGE_SUBDIRECTORY = 'announcements'; - /** - * Get assoc type for this announcement. - * - * @return int - */ - public function getAssocType() - { - return $this->getData('assocType'); - } + protected $table = 'announcements'; + protected $primaryKey = 'announcement_id'; + public const CREATED_AT = 'date_posted'; + public const UPDATED_AT = null; + protected string $settingsTable = 'announcement_settings'; /** - * Set assoc type for this announcement. + * The attributes that are mass assignable. * - * @param int $assocType + * @var array */ - public function setAssocType($assocType) - { - $this->setData('assocType', $assocType); - } + protected $fillable = ['assocType', 'assocId', 'typeId', 'title', 'image', 'description', 'descriptionShort']; /** - * Get the announcement type of the announcement. - * - * @return int + * @inheritDoc */ - public function getTypeId() + public static function getSchemaName(): string { - return $this->getData('typeId'); + return PKPSchemaService::SCHEMA_ANNOUNCEMENT; } /** - * Set the announcement type of the announcement. - * - * @param int $typeId + * @inheritDoc */ - public function setTypeId($typeId) + public function getSettingsTable(): string { - $this->setData('typeId', $typeId); + return $this->settingsTable; } /** - * Get the announcement type name of the announcement. - * - * @return string|null + * Add Model-level defined multilingual properties */ - public function getAnnouncementTypeName() + public function getMultilingualProps(): array { - $announcementTypeDao = DAORegistry::getDAO('AnnouncementTypeDAO'); /** @var AnnouncementTypeDAO $announcementTypeDao */ - return $this->getData('typeId') ? $announcementTypeDao->getById($this->getData('typeId'))?->getLocalizedTypeName() : null; + return array_merge( + $this->multilingualProps, + [ + 'fullTitle', + ] + ); } /** - * Get localized announcement title + * Delete announcement, also allows to delete multiple announcements by IDs at once with destroy() method * - * @return string - */ - public function getLocalizedTitle() - { - return $this->getLocalizedData('title'); - } - - /** - * Get full localized announcement title including type name + * @return bool|null * - * @return string + * @hook Announcement::delete::before [[$this]] */ - public function getLocalizedTitleFull() + public function delete() { - $typeName = $this->getAnnouncementTypeName(); - if (!empty($typeName)) { - return $typeName . ': ' . $this->getLocalizedTitle(); - } else { - return $this->getLocalizedTitle(); + Hook::call('Announcement::delete::before', [$this]); + + $deleted = parent::delete(); + if ($deleted) { + $this->deleteImage(); } - } - /** - * Get announcement title. - * - * @param string $locale - * - * @return string - */ - public function getTitle($locale) - { - return $this->getData('title', $locale); - } + Hook::call('Announcement::delete', [$this]); - /** - * Set announcement title. - * - * @param string $title - * @param string $locale - */ - public function setTitle($title, $locale) - { - $this->setData('title', $title, $locale); + return $deleted; } /** - * Get localized short description + * @return bool * - * @return string + * @hook Announcement::add [[$this]] */ - public function getLocalizedDescriptionShort() + public function save(array $options = []) { - return $this->getLocalizedData('descriptionShort'); + $newlyCreated = !$this->exists; + $saved = parent::save($options); + + // If it's a new model with an image attribute, upload an image + if ($saved && $newlyCreated && $this->hasAttribute('image')) { + $this->handleImageUpload(); + } + + // If it's updated model and a new image is uploaded, first, delete an old one + $hasNewImage = $this->hasAttribute('temporaryFileId'); + if ($saved && !$newlyCreated && $hasNewImage) { + $this->deleteImage(); + $this->handleImageUpload(); + } + + Hook::call('Announcement::add', [$this]); + + return $saved; } /** - * Get announcement brief description. - * - * @param string $locale - * - * @return string + * Filter by context IDs, accepts PKPApplication::SITE_CONTEXT_ID as a parameter array value if include site wide */ - public function getDescriptionShort($locale) + protected function scopeWithContextIds(EloquentBuilder $builder, array $contextIds): EloquentBuilder { - return $this->getData('descriptionShort', $locale); + $filteredIds = []; + $siteWide = false; + foreach ($contextIds as $contextId) { + if ($contextId == PKPApplication::SITE_CONTEXT_ID) { + $siteWide = true; + continue; + } + + $filteredIds[] = $contextId; + } + + return $builder->where('assoc_type', Application::get()->getContextAssocType()) + ->whereIn('assoc_id', $filteredIds) + ->when($siteWide, fn (EloquentBuilder $builder) => $builder->orWhereNull('assoc_id')); } /** - * Set announcement brief description. - * - * @param string $descriptionShort - * @param string $locale + * Filter by announcement type IDs */ - public function setDescriptionShort($descriptionShort, $locale) + protected function scopeWithTypeIds(EloquentBuilder $builder, array $typeIds): EloquentBuilder { - $this->setData('descriptionShort', $descriptionShort, $locale); + return $builder->whereIn('type_id', $typeIds); } /** - * Get localized full description * - * @return string + * @param string $date Optionally filter announcements by those + * not expired until $date (YYYY-MM-DD) */ - public function getLocalizedDescription() + protected function scopeWithActiveByDate(EloquentBuilder $builder, string $date = ''): EloquentBuilder { - return $this->getLocalizedData('description'); + return $builder->where('a.date_expire', '>', empty($date) ? Core::getCurrentDate() : $date) + ->orWhereNull('a.date_expire'); } /** - * Get announcement description. - * - * @param string $locale - * - * @return string + * Filter announcements by those matching a search query */ - public function getDescription($locale) + protected function scopeWithSearchPhrase(EloquentBuilder $builder, ?string $searchPhrase): EloquentBuilder { - return $this->getData('description', $locale); + if (is_null($searchPhrase)) { + return $builder; + } + + $words = explode(' ', $searchPhrase); + if (!count($words)) { + return $builder; + } + + return $builder->whereIn('announcement_id', function ($builder) use ($words) { + $builder->select('announcement_id')->from($this->getSettingsTable()); + foreach ($words as $word) { + $word = strtolower(addcslashes($word, '%_')); + $builder->where(function ($builder) use ($word) { + $builder->where(function ($builder) use ($word) { + $builder->where('setting_name', 'title'); + $builder->where(DB::raw('lower(setting_value)'), 'LIKE', "%{$word}%"); + }) + ->orWhere(function ($builder) use ($word) { + $builder->where('setting_name', 'descriptionShort'); + $builder->where(DB::raw('lower(setting_value)'), 'LIKE', "%{$word}%"); + }) + ->orWhere(function ($builder) use ($word) { + $builder->where('setting_name', 'description'); + $builder->where(DB::raw('lower(setting_value)'), 'LIKE', "%{$word}%"); + }); + }); + } + }); } /** - * Set announcement description. - * - * @param string $description - * @param string $locale + * Get the full title + * TODO temporary measure while AnnouncementType isn't refactored as Eloquent Model */ - public function setDescription($description, $locale) + protected function fullTitle(): Attribute { - $this->setData('description', $description, $locale); + return Attribute::make( + get: function (mixed $value, array $attributes) { + $announcementTypeDao = DAORegistry::getDAO('AnnouncementTypeDAO'); /** @var AnnouncementTypeDAO $announcementTypeDao */ + $multilingualTitle = $attributes['title']; + if (!isset($attributes['typeId'])) { + return $multilingualTitle; + } + + $type = $announcementTypeDao->getById($attributes['typeId']); + $typeName = $type->getData('name'); + + $multilingualFullTitle = $multilingualTitle; + foreach ($multilingualTitle as $locale => $title) { + if (isset($typeName[$locale])) { + $multilingualFullTitle[$locale] = $typeName . ': ' . $title; + } + } + + return $multilingualFullTitle; + } + ); } /** - * Get announcement expiration date. - * - * @return string (YYYY-MM-DD) + * Get announcement's image URL */ - public function getDateExpire() + protected function imageUrl(bool $withTimestamp = true): Attribute { - return $this->getData('dateExpire'); + return Attribute::make( + get: function () use ($withTimestamp) { + if (!$this->hasAttribute('image')) { + return ''; + } + $image = $this->getAttribute('image'); + + $filename = $image->uploadName; + if ($withTimestamp) { + $filename .= '?' . strtotime($image->dateUploaded); + } + + $publicFileManager = new PublicFileManager(); + + return join('/', [ + Application::get()->getRequest()->getBaseUrl(), + $this->hasAttribute('assocId') + ? $publicFileManager->getContextFilesPath($this->getAttribute('assocId')) + : $publicFileManager->getSiteFilesPath(), + static::IMAGE_SUBDIRECTORY, + $filename + ]); + } + ); } /** - * Set announcement expiration date. - * - * @param string $dateExpire (YYYY-MM-DD) + * Get alternative text of the image */ - public function setDateExpire($dateExpire) + protected function imageAltText(): Attribute { - $this->setData('dateExpire', $dateExpire); + return Attribute::make( + get: fn () => $this->image->altText ?? '' + ); } /** - * Get announcement posted date. - * - * @return string (YYYY-MM-DD) + * Get the date announcement was posted */ - public function getDatePosted() + protected function datePosted(): Attribute { - return date('Y-m-d', strtotime($this->getData('datePosted'))); + return Attribute::make( + get: fn (string $value) => date('Y-m-d', strtotime($value)) + ); } /** - * Get announcement posted datetime. - * - * @return string (YYYY-MM-DD HH:MM:SS) + * Delete the image related to announcement */ - public function getDatetimePosted() + protected function deleteImage(): void { - return $this->getData('datePosted'); + $image = $this->getAttribute('image'); + if ($image?->uploadName) { + $publicFileManager = new PublicFileManager(); + $filesPath = $this->hasAttribute('assocId') + ? $publicFileManager->getContextFilesPath($this->getAttribute('assocId')) + : $publicFileManager->getSiteFilesPath(); + + $publicFileManager->deleteByPath( + join('/', [ + $filesPath, + static::IMAGE_SUBDIRECTORY, + $image->uploadName, + ]) + ); + } } /** - * Set announcement posted date. + * Handle image uploads * - * @param string $datePosted (YYYY-MM-DD) + * @throws StoreTemporaryFileException Unable to store temporary file upload */ - public function setDatePosted($datePosted) + protected function handleImageUpload(): void { - $this->setData('datePosted', $datePosted); + $image = $this->getAttribute('image'); + if (!$image?->temporaryFileId) { + return; + } + + $user = Application::get()->getRequest()->getUser(); + $temporaryFileManager = new TemporaryFileManager(); + $temporaryFile = $temporaryFileManager->getFile((int) $image->temporaryFileId, $user?->getId()); + $filePath = static::IMAGE_SUBDIRECTORY . '/' . $this->getImageFilename($temporaryFile); + if (!$this->isValidImage($temporaryFile)) { + throw new StoreTemporaryFileException($temporaryFile, $filePath, $user, $this); + } + + if ($this->storeTemporaryFile($temporaryFile, $filePath, $user->getId())) { + $this->setAttribute( + 'image', + $this->getImageData($temporaryFile) + ); + $this->save(); + } else { + $this->delete(); + throw new StoreTemporaryFileException($temporaryFile, $filePath, $user, $this); + } } /** - * Set announcement posted datetime. + * Get the data array for a temporary file that has just been stored * - * @param string $datetimePosted (YYYY-MM-DD HH:MM:SS) + * @return array Data about the image, like the upload name, alt text, and date uploaded */ - public function setDatetimePosted($datetimePosted) + protected function getImageData(TemporaryFile $temporaryFile): array { - $this->setData('datePosted', $datetimePosted); + $image = $this->image; + + return [ + 'name' => $temporaryFile->getOriginalFileName(), + 'uploadName' => $this->getImageFilename($temporaryFile), + 'dateUploaded' => Core::getCurrentDate(), + 'altText' => $image->altText ?? '', + ]; } /** - * Get the featured image data + * Get the filename of the image upload */ - public function getImage(): ?array + protected function getImageFilename(TemporaryFile $temporaryFile): string { - return $this->getData('image'); - } + $fileManager = new FileManager(); - /** - * Set the featured image data - */ - public function setImage(array $image): void - { - $this->setData('image', $image); + return $this->getAttribute('announcementId') + . $fileManager->getImageExtension($temporaryFile->getFileType()); } /** - * Get the full URL to the image - * - * @param bool $withTimestamp Pass true to include a query argument with a timestamp - * of the date the image was uploaded in order to workaround cache bugs in browsers + * Check that temporary file is an image */ - public function getImageUrl(bool $withTimestamp = true): string + protected function isValidImage(TemporaryFile $temporaryFile): bool { - $image = $this->getImage(); - - if (!$image) { - return ''; + if (getimagesize($temporaryFile->getFilePath()) === false) { + return false; } - - $filename = $image['uploadName']; - if ($withTimestamp) { - $filename .= '?'. strtotime($image['dateUploaded']); + $extension = pathinfo($temporaryFile->getOriginalFileName(), PATHINFO_EXTENSION); + $fileManager = new FileManager(); + $extensionFromMimeType = $fileManager->getImageExtension( + PKPString::mime_content_type($temporaryFile->getFilePath()) + ); + if ($extensionFromMimeType !== '.' . $extension) { + return false; } - $publicFileManager = new PublicFileManager(); - - return join('/', [ - Application::get()->getRequest()->getBaseUrl(), - $this->getAssocId() - ? $publicFileManager->getContextFilesPath((int) $this->getAssocId()) - : $publicFileManager->getSiteFilesPath(), - Repo::announcement()->getImageSubdirectory(), - $filename - ]); + return true; } /** - * Get the alt text for the image + * Store a temporary file upload in the public files directory + * + * @param string $newPath The new filename with the path relative to the public files directoruy + * + * @return bool Whether or not the operation was successful */ - public function getImageAltText(): string + protected function storeTemporaryFile(TemporaryFile $temporaryFile, string $newPath, int $userId): bool { - $image = $this->getImage(); + $publicFileManager = new PublicFileManager(); + $temporaryFileManager = new TemporaryFileManager(); + + if ($assocId = $this->assocId) { + $result = $publicFileManager->copyContextFile( + $assocId, + $temporaryFile->getFilePath(), + $newPath + ); + } else { + $result = $publicFileManager->copySiteFile( + $temporaryFile->getFilePath(), + $newPath + ); + } - if (!$image || !$image['altText']) { - return ''; + if (!$result) { + return false; } - return $image['altText']; - } -} + $temporaryFileManager->deleteById($temporaryFile->getId(), $userId); -if (!PKP_STRICT_MODE) { - class_alias('\PKP\announcement\Announcement', '\Announcement'); + return $result; + } } diff --git a/classes/announcement/AnnouncementTypeDAO.php b/classes/announcement/AnnouncementTypeDAO.php index 2ce56dd2f69..8cf329a99f0 100644 --- a/classes/announcement/AnnouncementTypeDAO.php +++ b/classes/announcement/AnnouncementTypeDAO.php @@ -19,7 +19,6 @@ namespace PKP\announcement; use APP\core\Application; -use APP\facades\Repo; use Generator; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Facades\DB; @@ -116,8 +115,7 @@ public function deleteObject(AnnouncementType $announcementType): void */ public function deleteById(int $typeId): int { - $collector = Repo::announcement()->getCollector()->filterByTypeIds([$typeId]); - Repo::announcement()->deleteMany($collector); + Announcement::withTypeIds([$typeId])->delete(); return DB::table('announcement_types') ->where('type_id', '=', $typeId) diff --git a/classes/announcement/Repository.php b/classes/announcement/Repository.php index 021e0f58151..9cce7bfe8dc 100644 --- a/classes/announcement/Repository.php +++ b/classes/announcement/Repository.php @@ -2,90 +2,41 @@ /** * @file classes/announcement/Repository.php * - * Copyright (c) 2014-2020 Simon Fraser University - * Copyright (c) 2000-2020 John Willinsky + * Copyright (c) 2014-2024 Simon Fraser University + * Copyright (c) 2000-2024 John Willinsky * Distributed under the GNU GPL v3. For full terms see the file docs/COPYING. * * @class Repository * - * @brief A repository to find and manage announcements. + * @brief A helper class to handle operations with annoucnements */ namespace PKP\announcement; use APP\core\Application; use APP\core\Request; -use APP\file\PublicFileManager; use PKP\context\Context; -use PKP\core\Core; -use PKP\core\exceptions\StoreTemporaryFileException; -use PKP\core\PKPString; -use PKP\file\FileManager; -use PKP\file\TemporaryFile; -use PKP\file\TemporaryFileManager; use PKP\plugins\Hook; use PKP\services\PKPSchemaService; use PKP\validation\ValidatorFactory; class Repository { - /** @var DAO $dao */ - public $dao; - /** @var string $schemaMap The name of the class to map this entity to its schema */ - public $schemaMap = maps\Schema::class; + public string $schemaMap = maps\Schema::class; - /** @var Request $request */ - protected $request; + /** */ + protected Request $request; /** @var PKPSchemaService $schemaService */ - protected $schemaService; - + protected PKPSchemaService $schemaService; - public function __construct(DAO $dao, Request $request, PKPSchemaService $schemaService) + public function __construct(Request $request, PKPSchemaService $schemaService) { - $this->dao = $dao; $this->request = $request; $this->schemaService = $schemaService; } - /** @copydoc DAO::newDataObject() */ - public function newDataObject(array $params = []): Announcement - { - $object = $this->dao->newDataObject(); - if (!empty($params)) { - $object->setAllData($params); - } - return $object; - } - - /** @copydoc DAO::get() */ - public function get(int $id): ?Announcement - { - return $this->dao->get($id); - } - - /** @copydoc DAO::exists() */ - public function exists(int $id): bool - { - return $this->dao->exists($id); - } - - /** @copydoc DAO::getCollector() */ - public function getCollector(): Collector - { - return app(Collector::class); - } - - /** - * Get an instance of the map class for mapping - * announcements to their schema - */ - public function getSchemaMap(): maps\Schema - { - return app('maps')->withExtensions($this->schemaMap); - } - /** * Validate properties for an announcement * @@ -101,9 +52,11 @@ public function getSchemaMap(): maps\Schema */ public function validate(?Announcement $object, array $props, array $allowedLocales, string $primaryLocale): array { + $schema = Announcement::getSchemaName(); + $validator = ValidatorFactory::make( $props, - $this->schemaService->getValidationRules($this->dao->schema, $allowedLocales), + $this->schemaService->getValidationRules($schema, $allowedLocales), [ 'dateExpire.date_format' => __('stats.dateRange.invalidDate'), ] @@ -113,14 +66,14 @@ public function validate(?Announcement $object, array $props, array $allowedLoca ValidatorFactory::required( $validator, $object, - $this->schemaService->getRequiredProps($this->dao->schema), - $this->schemaService->getMultilingualProps($this->dao->schema), + $this->schemaService->getRequiredProps($schema), + $this->schemaService->getMultilingualProps($schema), $allowedLocales, $primaryLocale ); // Check for input from disallowed locales - ValidatorFactory::allowedLocales($validator, $this->schemaService->getMultilingualProps($this->dao->schema), $allowedLocales); + ValidatorFactory::allowedLocales($validator, $this->schemaService->getMultilingualProps($schema), $allowedLocales); $errors = []; @@ -133,221 +86,23 @@ public function validate(?Announcement $object, array $props, array $allowedLoca return $errors; } - /** @copydoc DAO::insert() */ - public function add(Announcement $announcement): int - { - $announcement->setData('datePosted', Core::getCurrentDate()); - $id = $this->dao->insert($announcement); - $announcement = $this->get($id); - - if ($announcement->getImage()) { - $this->handleImageUpload($announcement); - } - - Hook::call('Announcement::add', [$announcement]); - - return $id; - } - - /** - * Update an object in the database - * - * Deletes the old image if it has been removed, or a new image has - * been uploaded. - */ - public function edit(Announcement $announcement, array $params) - { - $newAnnouncement = clone $announcement; - $newAnnouncement->setAllData(array_merge($newAnnouncement->_data, $params)); - - Hook::call('Announcement::edit', [$newAnnouncement, $announcement, $params]); - - $this->dao->update($newAnnouncement); - - $image = $newAnnouncement->getImage(); - $hasNewImage = $image['temporaryFileId'] ?? null; - - if ((!$image || $hasNewImage) && $announcement->getImage()) { - $this->deleteImage($announcement); - } - - if ($hasNewImage) { - $this->handleImageUpload($newAnnouncement); - } - } - - /** @copydoc DAO::delete() */ - public function delete(Announcement $announcement) - { - Hook::call('Announcement::delete::before', [$announcement]); - - if ($announcement->getImage()) { - $this->deleteImage($announcement); - } - - $this->dao->delete($announcement); - - Hook::call('Announcement::delete', [$announcement]); - } - - /** - * Delete a collection of announcements - */ - public function deleteMany(Collector $collector) - { - foreach ($collector->getMany() as $announcement) { - $this->delete($announcement); - } - } - /** - * The subdirectory where announcement images are stored + * Get an instance of the map class for mapping + * announcements to their schema */ - public function getImageSubdirectory(): string + public function getSchemaMap(): maps\Schema { - return 'announcements'; + return app('maps')->withExtensions($this->schemaMap); } /** * Get the base URL for announcement file uploads */ - public function getFileUploadBaseUrl(?Context $context = null): string + public static function getFileUploadBaseUrl(?Context $context = null): string { return join('/', [ Application::get()->getRequest()->getPublicFilesUrl($context), - $this->getImageSubdirectory(), + Announcement::IMAGE_SUBDIRECTORY, ]); } - - /** - * Handle image uploads - * - * @throws StoreTemporaryFileException Unable to store temporary file upload - */ - protected function handleImageUpload(Announcement $announcement): void - { - $image = $announcement->getImage(); - if ($image['temporaryFileId'] ?? null) { - $user = Application::get()->getRequest()->getUser(); - $image = $announcement->getImage(); - $temporaryFileManager = new TemporaryFileManager(); - $temporaryFile = $temporaryFileManager->getFile((int) $image['temporaryFileId'], $user?->getId()); - $filePath = $this->getImageSubdirectory() . '/' . $this->getImageFilename($announcement, $temporaryFile); - if (!$this->isValidImage($temporaryFile, $filePath, $user, $announcement)) { - throw new StoreTemporaryFileException($temporaryFile, $filePath, $user, $announcement); - } - if ($this->storeTemporaryFile($temporaryFile, $filePath, $user->getId(), $announcement)) { - $announcement->setImage( - $this->getImageData($announcement, $temporaryFile) - ); - $this->dao->update($announcement); - } else { - $this->delete($announcement); - throw new StoreTemporaryFileException($temporaryFile, $filePath, $user, $announcement); - } - } - } - - /** - * Store a temporary file upload in the public files directory - * - * @param string $newPath The new filename with the path relative to the public files directoruy - * @return bool Whether or not the operation was successful - */ - protected function storeTemporaryFile(TemporaryFile $temporaryFile, string $newPath, int $userId, Announcement $announcement): bool - { - $publicFileManager = new PublicFileManager(); - $temporaryFileManager = new TemporaryFileManager(); - - if ($announcement->getAssocId()) { - $result = $publicFileManager->copyContextFile( - $announcement->getAssocId(), - $temporaryFile->getFilePath(), - $newPath - ); - } else { - $result = $publicFileManager->copySiteFile( - $temporaryFile->getFilePath(), - $newPath - ); - } - - if (!$result) { - return false; - } - - $temporaryFileManager->deleteById($temporaryFile->getId(), $userId); - - return $result; - } - - /** - * Get the data array for a temporary file that has just been stored - * - * @return array Data about the image, like the upload name, alt text, and date uploaded - */ - protected function getImageData(Announcement $announcement, TemporaryFile $temporaryFile): array - { - $image = $announcement->getImage(); - - return [ - 'name' => $temporaryFile->getOriginalFileName(), - 'uploadName' => $this->getImageFilename($announcement, $temporaryFile), - 'dateUploaded' => Core::getCurrentDate(), - 'altText' => !empty($image['altText']) ? $image['altText'] : '', - ]; - } - - /** - * Get the filename of the image upload - */ - protected function getImageFilename(Announcement $announcement, TemporaryFile $temporaryFile): string - { - $fileManager = new FileManager(); - - return $announcement->getId() - . $fileManager->getImageExtension($temporaryFile->getFileType()); - } - - /** - * Delete the image related to announcement - */ - protected function deleteImage(Announcement $announcement): void - { - $image = $announcement->getImage(); - if ($image['uploadName'] ?? null) { - $publicFileManager = new PublicFileManager(); - $filesPath = $announcement->getAssocId() - ? $publicFileManager->getContextFilesPath($announcement->getAssocId()) - : $publicFileManager->getSiteFilesPath(); - - $publicFileManager->deleteByPath( - join('/', [ - $filesPath, - $this->getImageSubdirectory(), - $image['uploadName'], - ]) - ); - } - } - - /** - * Check that temporary file is an image - */ - protected function isValidImage(TemporaryFile $temporaryFile): bool - { - if (getimagesize($temporaryFile->getFilePath()) === false) { - return false; - } - $extension = pathinfo($temporaryFile->getOriginalFileName(), PATHINFO_EXTENSION); - $fileManager = new FileManager(); - $extensionFromMimeType = $fileManager->getImageExtension( - PKPString::mime_content_type($temporaryFile->getFilePath()) - ); - if ($extensionFromMimeType !== '.' . $extension) { - return false; - } - - return true; - } } diff --git a/classes/announcement/maps/Schema.php b/classes/announcement/maps/Schema.php index 412fe84c28f..ee19b93253e 100644 --- a/classes/announcement/maps/Schema.php +++ b/classes/announcement/maps/Schema.php @@ -80,7 +80,7 @@ protected function mapByProperties(array $props, Announcement $item): array foreach ($props as $prop) { switch ($prop) { case '_href': - $output[$prop] = $this->getApiUrl('announcements/' . $item->getId()); + $output[$prop] = $this->getApiUrl('announcements/' . $item->getKey()); break; case 'url': $output[$prop] = $this->request->getDispatcher()->url( @@ -89,11 +89,14 @@ protected function mapByProperties(array $props, Announcement $item): array $this->getUrlPath(), 'announcement', 'view', - [$item->getId()] + [$item->getKey()] ); break; + case 'id': + $output[$prop] = $item->getKey(); + break; default: - $output[$prop] = $item->getData($prop); + $output[$prop] = $item->getAttribute($prop); break; } } diff --git a/classes/core/SettingsBuilder.php b/classes/core/SettingsBuilder.php new file mode 100644 index 00000000000..16d41034cc4 --- /dev/null +++ b/classes/core/SettingsBuilder.php @@ -0,0 +1,363 @@ +getModelWithSettings($columns); + $returner = $this->model->hydrate( + $rows->all() + )->all(); + + return $returner; + } + + /** + * Update records in the database, including settings + * + * @return int + */ + public function update(array $values) + { + // Separate Model's primary values from settings + [$settingValues, $primaryValues] = collect($values)->partition( + fn (array|string $value, string $key) => in_array(Str::camel($key), $this->model->getSettings()) + ); + + // Don't update settings if they aren't set + if ($settingValues->isEmpty()) { + return parent::update($primaryValues); + } + + // TODO Eloquent transforms attributes to snake case, find and override instead of transforming here + $settingValues = $settingValues->mapWithKeys( + fn (array|string $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); + + // 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), + ])); + + return $count; + } + + /** + * Insert the given attributes and set the ID on the model. + * Overrides Builder's method to insert setting values for a models with + * + * @param string|null $sequence + * + * @return int + */ + public function insertGetId(array $values, $sequence = null) + { + // Separate Model's primary values from settings + [$settingValues, $primaryValues] = collect($values)->partition( + fn (mixed $value, string $key) => in_array(Str::camel($key), $this->model->getSettings()) + ); + + $id = parent::insertGetId($primaryValues->toArray(), $sequence); + + if ($settingValues->isEmpty()) { + return $id; + } + + $rows = []; + $settingValues->each(function (mixed $settingValue, string $settingName) use ($id, &$rows) { + $settingName = Str::camel($settingName); + if ($this->isMultilingual($settingName)) { + foreach ($settingValue as $locale => $localizedValue) { + $rows[] = [ + $this->model->getKeyName() => $id, 'locale' => $locale, 'setting_name' => $settingName, 'setting_value' => $localizedValue + ]; + } + } else { + $rows[] = [ + $this->model->getKeyName() => $id, 'locale' => '', 'setting_name' => $settingName, 'setting_value' => $settingValue + ]; + } + }); + + DB::table($this->model->getSettingsTable())->insert($rows); + + return $id; + } + + /** + * Delete model with settings + */ + public function delete(): int + { + $id = parent::delete(); + if (!$id) { + return $id; + } + + DB::table($this->model->getSettingsTable())->where( + $this->model->getKeyName(), + $this->model->getRawOriginal($this->model->getKeyName()) ?? $this->model->getKey() + )->delete(); + + return $id; + } + + /** + * Add a basic where clause to the query. + * Overrides Eloquent Builder method to support settings table + * + * @param \Closure|string|array|\Illuminate\Contracts\Database\Query\Expression $column + * @param string $boolean + * @param null|mixed $operator + * @param null|mixed $value + * + * @return $this + */ + public function where($column, $operator = null, $value = null, $boolean = 'and') + { + if ($column instanceof ConditionExpression || $column instanceof Closure) { + return parent::where($column, $operator, $value, $boolean); + } + + $settings = []; + $primaryColumn = false; + + // See Illuminate\Database\Query\Builder::where() + [$value, $operator] = $this->query->prepareValueAndOperator( + $value, + $operator, + func_num_args() === 2 + ); + + $modelSettingsList = $this->model->getSettings(); + + if (is_string($column)) { + if (in_array($column, $modelSettingsList)) { + $settings[$column] = $value; + } else { + $primaryColumn = $column; + } + } + + if (is_array($column)) { + $settings = array_intersect($column, $modelSettingsList); + $primaryColumn = array_diff($column, $modelSettingsList); + } + + if (empty($settings)) { + return parent::where($column, $operator, $value, $boolean); + } + + $where = []; + foreach ($settings as $settingName => $settingValue) { + $where = array_merge($where, [ + 'setting_name' => $settingName, + 'setting_value' => $settingValue, + ]); + } + + $this->query->whereIn( + $this->model->getKeyName(), + fn (QueryBuilder $query) => + $query->select($this->model->getKeyName())->from($this->model->getSettingsTable())->where($where, null, null, $boolean) + ); + + if (!empty($primaryColumn)) { + parent::where($primaryColumn, $operator, $value, $boolean); + } + + return $this; + } + + /** + * Add a "where in" clause to the query. + * Overrides Illuminate\Database\Query\Builder to support settings in select queries + * + * @param \Illuminate\Contracts\Database\Query\Expression|string $column + * @param string $boolean + * @param bool $not + * + * @return $this + */ + public function whereIn($column, $values, $boolean = 'and', $not = false) + { + if ($column instanceof Expression || !in_array($column, $this->model->getSettings())) { + return parent::whereIn($column, $values, $boolean, $not); + } + + $this->query->whereIn( + $this->model->getKeyName(), + fn (QueryBuilder $query) => + $query + ->select($this->model->getKeyName()) + ->from($this->model->getSettingsTable()) + ->where('setting_name', $column) + ->whereIn('setting_value', $values, $boolean, $not) + ); + + return $this; + } + + /* + * Augment model with data from the settings table + */ + protected function getModelWithSettings(array|string $columns = ['*']): Collection + { + // First, get all Model columns from the main table + $rows = $this->query->get(); + 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()) + ->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); + } + + return $row; + }); + + return $rows; + } + + /** + * If specific columns are selected to fill the Model with, iterate and filter all, which aren't specified + * TODO Instead of iterating through all row properties, we can force to pass primary key as a mandatory column? + */ + protected function filterRow(stdClass $row, string|array $columns = ['*']): stdClass + { + if ($columns == ['*']) { + return $row; + } + + $columns = Arr::wrap($columns); + foreach ($row as $property) { + if (!in_array($property, $columns)) { + unset($row->{$property}); + } + } + + return $row; + } + + /** + * @param Collection $settingValues list of setting names as keys and setting values to be updated + * @param string $us name of the settings table + * @param QueryBuilder $query original query associated with the Model + * + * @return string raw SQL statement + * + * Helper method to build a query to update settings with a conditional statement: + * SET settings_value = CASE WHEN setting_name='' AND locale=''... + */ + protected function buildUpdateSql(Collection $settingValues, string $us, QueryBuilder $query): string + { + $sql = 'CASE '; + $bindings = []; + $settingValues->each(function (array|string $settingValue, string $settingName) use (&$sql, &$bindings, $us) { + if ($this->isMultilingual($settingName)) { + foreach ($settingValue as $locale => $localizedValue) { + $sql .= 'WHEN ' . $us . '.setting_name=? AND ' . $us . '.locale=? THEN ? '; + $bindings = array_merge($bindings, [$settingName, $locale, $localizedValue]); + } + } else { + $sql .= 'WHEN ' . $us . '.setting_name=? THEN ? '; + $bindings = array_merge($bindings, [$settingName, $settingValue]); + } + }); + $sql .= 'ELSE setting_value END'; + + // Fix the order of bindings in Laravel, user ID in the where statement should be the last + $query->bindings['where'] = array_merge($bindings, $query->bindings['where']); + + return $sql; + } + + /** + * Checks if setting is multilingual + */ + protected function isMultilingual(string $settingName): bool + { + return in_array($settingName, $this->model->getMultilingualProps()); + } +}; diff --git a/classes/core/exceptions/StoreTemporaryFileException.php b/classes/core/exceptions/StoreTemporaryFileException.php index c313fc22474..8c7ec2c0433 100644 --- a/classes/core/exceptions/StoreTemporaryFileException.php +++ b/classes/core/exceptions/StoreTemporaryFileException.php @@ -18,13 +18,14 @@ namespace PKP\core\exceptions; use Exception; +use Illuminate\Database\Eloquent\Model; use PKP\core\DataObject; use PKP\file\TemporaryFile; use PKP\user\User; class StoreTemporaryFileException extends Exception { - public function __construct(public TemporaryFile $temporaryFile, public string $targetPath, public ?User $user, public ?DataObject $dataObject) + public function __construct(public TemporaryFile $temporaryFile, public string $targetPath, public ?User $user, public DataObject|Model|null $dataObject) { $message = `Unable to store temporary file {$temporaryFile->getFilePath()} in {$targetPath}.`; if ($user) { @@ -32,8 +33,20 @@ public function __construct(public TemporaryFile $temporaryFile, public string $ } if ($dataObject) { $class = get_class($dataObject); - $message .= ` Handling {$class} id {$dataObject->getId()}.`; + $id = is_a($class, DataObject::class) ? $dataObject->getId() : $dataObject->id; + $message .= ` Handling {$class} id {$id}.`; } parent::__construct($message); } + + public function getDataObjectId(): ?int + { + if (!isset($this->dataObject)) { + return null; + } + + return is_a(get_class($this->dataObject), DataObject::class) ? + $this->dataObject->getId() : + $this->dataObject->getKey(); + } } diff --git a/classes/core/maps/Schema.php b/classes/core/maps/Schema.php index 2d2d3514157..3e312d53b9d 100644 --- a/classes/core/maps/Schema.php +++ b/classes/core/maps/Schema.php @@ -21,6 +21,19 @@ abstract class Schema extends Base { + /** + * ATTRIBUTE_* constants refer to type of attributes according to the Eloquent Model + * + * @var string Primary attribute of the Model derived from the main table + */ + public const ATTRIBUTE_ORIGIN_MAIN = 'primary'; + + /** @var string Model's attribute derived from settings table */ + public const ATTRIBUTE_ORIGIN_SETTINGS = 'setting'; + + /** @var string The value for this attribute is composed with Eloquent's Mutators */ + public const ATTRIBUTE_ORIGIN_COMPOSED = 'composed'; + public PKPRequest $request; public ?Context $context; diff --git a/classes/core/traits/ModelWithSettings.php b/classes/core/traits/ModelWithSettings.php new file mode 100644 index 00000000000..255b8c02298 --- /dev/null +++ b/classes/core/traits/ModelWithSettings.php @@ -0,0 +1,151 @@ +getSchemaName()) { + $this->setSchemaData(); + } + } + + /** + * Create a new Eloquent query builder for the model that supports settings table + * + * @param \Illuminate\Database\Query\Builder $query + * + * @return \Illuminate\Database\Eloquent\Builder|static + */ + public function newEloquentBuilder($query) + { + return new SettingsBuilder($query); + } + + /** + * Get a list of attributes from the settings table associated with the Model + */ + public function getSettings(): array + { + return $this->settings; + } + + /** + * Get multilingual attributes associated with the Model + */ + public function getMultilingualProps(): array + { + $modelProps = parent::getMultilingualProps(); + return array_merge($this->multilingualProps, $modelProps); + } + + /** + * @param string $data Model's localized attribute + * @param ?string $locale Locale to retrieve data for, default - current locale + * + * @throws Exception + * + * @return mixed Localized value + */ + public function getLocalizedData(string $data, ?string $locale = null): mixed + { + if (is_null($locale)) { + $locale = Locale::getLocale(); + } + + $multilingualProp = $this->getAttribute($data); + if (!$multilingualProp) { + throw new Exception('Attribute ' . $data . ' doesn\'t exist in the ' . static::class . ' model'); + } + + if (!in_array($data, $this->getMultilingualProps())) { + throw new Exception('Trying to retrieve localized data from a non-multilingual attribute ' . $data); + } + + // TODO What should the default behaviour be if localized value doesn't exist? + return $multilingualProp[$locale] ?? null; + } + + /** + * Sets the schema for current Model + */ + protected function setSchemaData(): void + { + $schemaService = app()->get('schema'); /** @var PKPSchemaService $schemaService */ + $schema = $schemaService->get($this->getSchemaName()); + $this->convertSchemaToCasts($schema); + $this->settings = array_merge($this->settings, $schemaService->groupPropsByOrigin($this->getSchemaName())[Schema::ATTRIBUTE_ORIGIN_SETTINGS]); + $this->multilingualProps = array_merge($this->settings, $schemaService->getMultilingualProps($this->getSchemaName())); + } + + /** + * Set casts by deriving proper types from schema + * TODO casts on multilingual properties. Keep in mind that overriding Model::attributesToArray() might conflict with HasCamelCasing trait + */ + protected function convertSchemaToCasts(stdClass $schema): void + { + $propCast = []; + foreach ($schema->properties as $propName => $propSchema) { + // Don't cast multilingual values as Eloquent tries to convert them from string to arrays with json_decode() + if (isset($propSchema->multilingual)) { + continue; + } + $propCast[$propName] = $propSchema->type; + } + + $this->mergeCasts($propCast); + } + +} diff --git a/classes/mail/mailables/AnnouncementNotify.php b/classes/mail/mailables/AnnouncementNotify.php index a171c1c004e..bd548f70860 100644 --- a/classes/mail/mailables/AnnouncementNotify.php +++ b/classes/mail/mailables/AnnouncementNotify.php @@ -81,15 +81,15 @@ public function setData(?string $locale = null): void $this->viewData = array_merge( $this->viewData, [ - static::$announcementTitle => $this->announcement->getData('title', $locale), - static::$announcementSummary => $this->announcement->getData('descriptionShort', $locale), + static::$announcementTitle => $this->announcement->getLocalizedData('title', $locale), + static::$announcementSummary => $this->announcement->getLocalizedData('descriptionShort', $locale), static::$announcementUrl => $dispatcher->url( $request, PKPApplication::ROUTE_PAGE, $this->context->getData('urlPath'), 'announcement', 'view', - $this->announcement->getId() + $this->announcement->getAttribute('announcementId') ), ] ); diff --git a/classes/notification/PKPNotificationManager.php b/classes/notification/PKPNotificationManager.php index fc5a6dc2186..7db3f044309 100644 --- a/classes/notification/PKPNotificationManager.php +++ b/classes/notification/PKPNotificationManager.php @@ -20,6 +20,7 @@ use APP\decision\Decision; use APP\facades\Repo; use APP\template\TemplateManager; +use PKP\announcement\Announcement; use PKP\core\PKPApplication; use PKP\core\PKPRequest; use PKP\db\DAORegistry; @@ -79,8 +80,8 @@ public function getNotificationUrl(PKPRequest $request, Notification $notificati if ($notification->assocType != Application::ASSOC_TYPE_ANNOUNCEMENT) { throw new \Exception('Unexpected assoc type!'); } - $announcement = Repo::announcement()->get($notification->assocId); - $context = $contextDao->getById($announcement->getAssocId()); + $announcement = Announcement::find($notification->assocId); + $context = $contextDao->getById($announcement->getAttribute('assocId')); return $dispatcher->url($request, PKPApplication::ROUTE_PAGE, $context->getPath(), 'announcement', 'view', [$notification->assocId]); case Notification::NOTIFICATION_TYPE_CONFIGURE_PAYMENT_METHOD: return __('notification.type.configurePaymentMethod'); diff --git a/classes/notification/managerDelegate/AnnouncementNotificationManager.php b/classes/notification/managerDelegate/AnnouncementNotificationManager.php index 84e4386dd9c..c55ca7ad2df 100644 --- a/classes/notification/managerDelegate/AnnouncementNotificationManager.php +++ b/classes/notification/managerDelegate/AnnouncementNotificationManager.php @@ -26,8 +26,8 @@ class AnnouncementNotificationManager extends NotificationManagerDelegate { - /** @var Announcement The announcement to send a notification about */ - public $_announcement; + /** The announcement to send a notification about */ + public Announcement $_announcement; /** * Initializes the class. @@ -66,7 +66,7 @@ public function getNotificationUrl(PKPRequest $request, Notification $notificati $request->getContext()->getData('urlPath'), 'announcement', 'view', - $this->_announcement->getId() + $this->_announcement->getAttribute('announcementId') ); } @@ -99,11 +99,11 @@ public function notify(User $user): ?Notification Application::get()->getRequest(), $user->getId(), Notification::NOTIFICATION_TYPE_NEW_ANNOUNCEMENT, - $this->_announcement->getAssocId(), + $this->_announcement->getAttribute('assocId'), null, null, Notification::NOTIFICATION_LEVEL_NORMAL, - ['contents' => $this->_announcement->getLocalizedTitle()] + ['contents' => $this->_announcement->getLocalizedData('title')] ); } } diff --git a/classes/services/PKPContextService.php b/classes/services/PKPContextService.php index 32638769831..fc8ab3fb4bf 100644 --- a/classes/services/PKPContextService.php +++ b/classes/services/PKPContextService.php @@ -21,6 +21,7 @@ use APP\facades\Repo; use APP\file\PublicFileManager; use APP\services\queryBuilders\ContextQueryBuilder; +use PKP\announcement\Announcement; use PKP\announcement\AnnouncementTypeDAO; use PKP\context\Context; use PKP\context\ContextDAO; @@ -627,11 +628,8 @@ public function delete($context) $genreDao = DAORegistry::getDAO('GenreDAO'); /** @var GenreDAO $genreDao */ $genreDao->deleteByContextId($context->getId()); - Repo::announcement()->deleteMany( - Repo::announcement() - ->getCollector() - ->filterByContextIds([$context->getId()]) - ); + // TODO is it OK to delete without listening Model's delete-associated events (not loading each Model)? + Announcement::withContextIds([$context->getId])->delete(); Repo::highlight() ->getCollector() diff --git a/classes/services/PKPSchemaService.php b/classes/services/PKPSchemaService.php index 327715bb284..0aabac27905 100644 --- a/classes/services/PKPSchemaService.php +++ b/classes/services/PKPSchemaService.php @@ -20,6 +20,7 @@ use Illuminate\Support\Arr; use Illuminate\Support\MessageBag; use PKP\core\DataObject; +use PKP\core\maps\Schema; use PKP\plugins\Hook; /** @@ -67,11 +68,12 @@ class PKPSchemaService * * @hook Schema::get::(schemaName) [[schema]] * @hook Schema::get:: + * @hook Schema::get::before:: */ public function get($schemaName, $forceReload = false) { Hook::run('Schema::get::before::' . $schemaName, [&$forceReload]); - + if (!$forceReload && array_key_exists($schemaName, $this->_schemas)) { return $this->_schemas[$schemaName]; } @@ -231,6 +233,57 @@ public function getMultilingualProps($schemaName) return $multilingualProps; } + /** + * Retrieves properties of the schema of certain origin + * + * @param string $schemaName One of the SCHEMA_... constants + * @param string $attributeOrigin one of the Schema::ATTRIBUTE_ORIGIN_* constants + * + * @return array List of property names + */ + public function getPropsByAttributeOrigin(string $schemaName, string $attributeOrigin): array + { + $schema = $this->get($schemaName); + + $propsByOrigin = []; + foreach ($schema->properies as $propName => $propSchema) { + if (!empty($propSchema->origin) && $propSchema->origin == $attributeOrigin) { + $propsByOrigin[] = $propName; + } + } + + return $propsByOrigin; + } + + /** + * Groups properties by their origin, see Schema::ATTRIBUTE_ORIGIN_* constants + * + * @return array>, e.g. ['primary' => ['assocId', 'assocType']] + */ + public function groupPropsByOrigin(string $schemaName): array + { + $schema = $this->get($schemaName); + $propsByOrigin = []; + foreach ($schema->properties as $propName => $propSchema) { + if (!empty($propSchema->origin)) { + switch($propSchema->origin) { + case Schema::ATTRIBUTE_ORIGIN_SETTINGS: + $propsByOrigin[Schema::ATTRIBUTE_ORIGIN_SETTINGS][] = $propName; + break; + case Schema::ATTRIBUTE_ORIGIN_COMPOSED: + $propsByOrigin[Schema::ATTRIBUTE_ORIGIN_COMPOSED][] = $propName; + break; + case Schema::ATTRIBUTE_ORIGIN_MAIN: + default: + $propsByOrigin[Schema::ATTRIBUTE_ORIGIN_MAIN][] = $propName; + break; + } + } + } + + return $propsByOrigin; + } + /** * Sanitize properties according to a schema * diff --git a/controllers/grid/notifications/NotificationsGridCellProvider.php b/controllers/grid/notifications/NotificationsGridCellProvider.php index bbe1e393fc5..de65b974b59 100644 --- a/controllers/grid/notifications/NotificationsGridCellProvider.php +++ b/controllers/grid/notifications/NotificationsGridCellProvider.php @@ -20,6 +20,7 @@ use APP\facades\Repo; use APP\notification\NotificationManager; use APP\template\TemplateManager; +use PKP\announcement\Announcement; use PKP\controllers\grid\GridCellProvider; use PKP\controllers\grid\GridColumn; use PKP\controllers\grid\GridHandler; @@ -129,9 +130,9 @@ public function _getTitle($notification) return '—'; case Application::ASSOC_TYPE_ANNOUNCEMENT: $announcementId = $notification->assocId; - $announcement = Repo::announcement()->get($announcementId); + $announcement = Announcement::find($announcementId); if ($announcement) { - return $announcement->getLocalizedTitle(); + return $announcement->getLocalizedData('title'); } return null; case Application::ASSOC_TYPE_SUBMISSION: diff --git a/jobs/notifications/NewAnnouncementNotifyUsers.php b/jobs/notifications/NewAnnouncementNotifyUsers.php index 586afe5f502..649b2955222 100644 --- a/jobs/notifications/NewAnnouncementNotifyUsers.php +++ b/jobs/notifications/NewAnnouncementNotifyUsers.php @@ -61,7 +61,7 @@ public function __construct( public function handle() { - $announcement = Repo::announcement()->get($this->announcementId); + $announcement = Announcement::find($this->announcementId); // Announcement was removed if (!$announcement) { throw new JobException(JobException::INVALID_PAYLOAD); @@ -97,9 +97,9 @@ public function handle() * Creates new announcement notification email */ protected function createMailable( - Context $context, - User $recipient, - Announcement $announcement, + Context $context, + User $recipient, + Announcement $announcement, EmailTemplate $template ): AnnouncementNotify { $mailable = new AnnouncementNotify($context, $announcement); diff --git a/pages/admin/AdminHandler.php b/pages/admin/AdminHandler.php index 912cad54523..c498c4f6d91 100644 --- a/pages/admin/AdminHandler.php +++ b/pages/admin/AdminHandler.php @@ -25,7 +25,7 @@ use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; use PDO; -use PKP\announcement\Collector; +use PKP\announcement\Announcement; use PKP\components\forms\announcement\PKPAnnouncementForm; use PKP\components\forms\context\PKPAnnouncementSettingsForm; use PKP\components\forms\context\PKPContextForm; @@ -42,6 +42,7 @@ use PKP\components\listPanels\PKPAnnouncementsListPanel; use PKP\config\Config; use PKP\core\JSONMessage; +use PKP\core\PKPApplication; use PKP\core\PKPContainer; use PKP\core\PKPRequest; use PKP\db\DAORegistry; @@ -214,7 +215,7 @@ public function settings($args, $request) $siteStatisticsForm = new PKPSiteStatisticsForm($apiUrl, $locales, $site); $highlightsListPanel = $this->getHighlightsListPanel(); $announcementSettingsForm = new PKPAnnouncementSettingsForm($apiUrl, $locales, $site); - $announcementsForm = new PKPAnnouncementForm($announcementsApiUrl, $locales, Repo::announcement()->getFileUploadBaseUrl(), $temporaryFileApiUrl); + $announcementsForm = new PKPAnnouncementForm($announcementsApiUrl, $locales, Repo::Announcement()->getFileUploadBaseUrl(), $temporaryFileApiUrl); $announcementsListPanel = $this->getAnnouncementsListPanel($announcementsApiUrl, $announcementsForm); $templateMgr = TemplateManager::getManager($request); @@ -757,13 +758,11 @@ protected function getHighlightsListPanel(): HighlightsListPanel */ protected function getAnnouncementsListPanel(string $apiUrl, PKPAnnouncementForm $form): PKPAnnouncementsListPanel { - $collector = Repo::announcement() - ->getCollector() - ->withSiteAnnouncements(Collector::SITE_ONLY); + $announcements = Announcement::withContextIds([PKPApplication::SITE_CONTEXT_ID]); - $itemsMax = $collector->getCount(); + $itemsMax = $announcements->count(); $items = Repo::announcement()->getSchemaMap()->summarizeMany( - $collector->limit(30)->getMany() + $announcements->limit(30)->get() ); return new PKPAnnouncementsListPanel( @@ -773,7 +772,7 @@ protected function getAnnouncementsListPanel(string $apiUrl, PKPAnnouncementForm 'apiUrl' => $apiUrl, 'form' => $form, 'getParams' => [ - 'contextIds' => [Application::SITE_CONTEXT_ID], + 'contextIds' => [PKPApplication::SITE_CONTEXT_ID], 'count' => 30, ], 'items' => $items->values(), diff --git a/pages/announcement/AnnouncementHandler.php b/pages/announcement/AnnouncementHandler.php index bdc8ca5f42f..3a242ba3d3a 100644 --- a/pages/announcement/AnnouncementHandler.php +++ b/pages/announcement/AnnouncementHandler.php @@ -18,10 +18,10 @@ use APP\core\Application; use APP\core\Request; -use APP\facades\Repo; use APP\handler\Handler; use APP\template\TemplateManager; -use PKP\announcement\Collector; +use PKP\announcement\Announcement; +use PKP\core\PKPApplication; use PKP\core\PKPRequest; class AnnouncementHandler extends Handler @@ -47,19 +47,13 @@ public function index($args, $request) $templateMgr->assign('announcementsIntroduction', $this->getAnnouncementsIntro($request)); // TODO the announcements list should support pagination - $collector = Repo::announcement() - ->getCollector() - ->filterByActive(); + $announcements = Announcement::withActiveByDate(); - if ($request->getContext()) { - $collector->filterByContextIds([$request->getContext()->getId()]); - } else { - $collector->withSiteAnnouncements(Collector::SITE_ONLY); - } - - $announcements = $collector->getMany(); + $contextIds = []; + $request->getContext() ? $contextIds[] = $request->getContext()->getId() : $contextIds[] = PKPApplication::SITE_CONTEXT_ID; + $announcements->withContextIds($contextIds); - $templateMgr->assign('announcements', $announcements->toArray()); + $templateMgr->assign('announcements', $announcements->get()->toArray()); $templateMgr->display('frontend/pages/announcements.tpl'); } @@ -78,13 +72,13 @@ public function view($args, $request) $this->setupTemplate($request); $announcementId = (int) array_shift($args); - $announcement = Repo::announcement()->get($announcementId); + $announcement = Announcement::find($announcementId); if ( $announcement - && $announcement->getAssocType() == Application::getContextAssocType() - && $announcement->getAssocId() == $request->getContext()?->getId() + && $announcement->getAttribute('assocType') == Application::getContextAssocType() + && $announcement->getAttribute('assocId') == $request->getContext()?->getId() && ( - $announcement->getDateExpire() == null || strtotime($announcement->getDateExpire()) > time() + $announcement->getAttribute('dateExpire') == null || strtotime($announcement->getAttribute('dateExpire')) > time() ) ) { $templateMgr = TemplateManager::getManager($request); diff --git a/pages/index/PKPIndexHandler.php b/pages/index/PKPIndexHandler.php index fff01001e12..9a19b58f2db 100644 --- a/pages/index/PKPIndexHandler.php +++ b/pages/index/PKPIndexHandler.php @@ -19,8 +19,9 @@ use APP\facades\Repo; use APP\handler\Handler; use Illuminate\Support\LazyCollection; -use PKP\announcement\Collector; +use PKP\announcement\Announcement; use PKP\context\Context; +use PKP\core\PKPApplication; use PKP\site\Site; use PKP\template\PKPTemplateManager; @@ -31,7 +32,6 @@ class PKPIndexHandler extends Handler * * @protected * - * @param Context $context * @param PKPTemplateManager $templateMgr */ protected function _setupAnnouncements(Context|Site $contextOrSite, $templateMgr) @@ -39,21 +39,14 @@ protected function _setupAnnouncements(Context|Site $contextOrSite, $templateMgr $enableAnnouncements = $contextOrSite->getData('enableAnnouncements'); $numAnnouncementsHomepage = $contextOrSite->getData('numAnnouncementsHomepage'); if ($enableAnnouncements && $numAnnouncementsHomepage) { - $collector = Repo::announcement() - ->getCollector() - ->filterByActive() - ->limit((int) $numAnnouncementsHomepage); + $announcements = Announcement::withActiveByDate()->limit((int) $numAnnouncementsHomepage); - if (is_a($contextOrSite, Context::class)) { - $collector->filterByContextIds([$contextOrSite->getId()]); - } else { - $collector->withSiteAnnouncements(Collector::SITE_ONLY); - } - - $announcements = $collector->getMany(); + $contextIds = []; + is_a($contextOrSite, Context::class) ? $contextIds[] = $contextOrSite->getId() : $contextIds[] = PKPApplication::SITE_CONTEXT_ID; + $announcements->withContextIds($contextIds); $templateMgr->assign([ - 'announcements' => $announcements->toArray(), + 'announcements' => $announcements->get()->toArray(), 'numAnnouncementsHomepage' => $numAnnouncementsHomepage, ]); } diff --git a/pages/management/ManagementHandler.php b/pages/management/ManagementHandler.php index 776082bdaf3..ea30a86287e 100644 --- a/pages/management/ManagementHandler.php +++ b/pages/management/ManagementHandler.php @@ -28,6 +28,7 @@ use APP\file\PublicFileManager; use APP\handler\Handler; use APP\template\TemplateManager; +use PKP\announcement\Announcement; use PKP\components\forms\announcement\PKPAnnouncementForm; use PKP\components\forms\context\PKPAnnouncementSettingsForm; use PKP\components\forms\context\PKPAppearanceMastheadForm; @@ -398,13 +399,11 @@ public function announcements($args, $request) $request->getContext() ); - $collector = Repo::announcement() - ->getCollector() - ->filterByContextIds([$request->getContext()->getId()]); + $announcements = Announcement::withContextIds([$request->getContext()->getId()]); - $itemsMax = $collector->getCount(); + $itemsMax = $announcements->count(); $items = Repo::announcement()->getSchemaMap()->summarizeMany( - $collector->limit(30)->getMany() + $announcements->limit(30)->get() ); $announcementsListPanel = new \PKP\components\listPanels\PKPAnnouncementsListPanel( diff --git a/pages/sitemap/PKPSitemapHandler.php b/pages/sitemap/PKPSitemapHandler.php index 1d1ad9504a9..d2a10fb4081 100644 --- a/pages/sitemap/PKPSitemapHandler.php +++ b/pages/sitemap/PKPSitemapHandler.php @@ -18,10 +18,10 @@ use APP\core\Application; use APP\core\Request; -use APP\facades\Repo; use APP\handler\Handler; use DOMDocument; use DOMNode; +use PKP\announcement\Announcement; use PKP\db\DAORegistry; use PKP\navigationMenu\NavigationMenuItem; use PKP\navigationMenu\NavigationMenuItemDAO; @@ -109,10 +109,7 @@ public function _createContextSitemap($request) // Announcements if ($context->getData('enableAnnouncements') == 1) { $root->appendChild($this->_createUrlTree($doc, $request->url($context->getPath(), 'announcement'))); - $announcementIds = Repo::announcement() - ->getCollector() - ->filterByContextIds([$context->getId()]) - ->getIds(); + $announcementIds = Announcement::withContextIds([$context->getId()])->pluck('announcementId')->toArray(); foreach ($announcementIds as $announcementId) { $root->appendChild($this->_createUrlTree($doc, $request->url($context->getPath(), 'announcement', 'view', $announcementId))); diff --git a/schemas/announcement.json b/schemas/announcement.json index 349b136d680..6edfa98bb41 100644 --- a/schemas/announcement.json +++ b/schemas/announcement.json @@ -8,6 +8,7 @@ "properties": { "_href": { "type": "string", + "origin": "composed", "description": "The URL to this announcement in the REST API.", "format": "uri", "readOnly": true, @@ -15,6 +16,7 @@ }, "assocId": { "type": "integer", + "origin": "primary", "description": "The journal, press or preprint server ID. Null for site-level announcements.", "apiSummary": true, "validation": [ @@ -23,11 +25,13 @@ }, "assocType": { "type": "integer", + "origin": "primary", "description": "The assoc object. This should always be `ASSOC_TYPE_JOURNAL` (OJS), `ASSOC_TYPE_PRESS` (OMP) or `ASSOC_TYPE_SERVER` (OPS).", "apiSummary": true }, "dateExpire": { "type": "string", + "origin": "primary", "description": "(Optional) The date that this announcement expires, if one is set. This is typically used to express closing dates for calls for papers.", "apiSummary": true, "validation": [ @@ -37,6 +41,7 @@ }, "datePosted": { "type": "string", + "origin": "primary", "description": "The date this announcement was posted.", "apiSummary": true, "writeDisabledInApi": true, @@ -47,6 +52,7 @@ }, "description": { "type": "string", + "origin": "setting", "description": "The full text of the announcement.", "multilingual": true, "apiSummary": true, @@ -56,6 +62,7 @@ }, "descriptionShort": { "type": "string", + "origin": "setting", "description": "A summary of this announcement.", "multilingual": true, "apiSummary": true, @@ -65,11 +72,13 @@ }, "id": { "type": "integer", + "origin": "primary", "readOnly": true, "apiSummary": true }, "image": { "type": "object", + "origin": "setting", "description": "The image to show with this announcement.", "apiSummary": true, "validation": [ @@ -96,6 +105,7 @@ }, "title": { "type": "string", + "origin": "setting", "multilingual": true, "apiSummary": true, "validation": [ @@ -104,6 +114,7 @@ }, "typeId": { "type": "integer", + "origin": "primary", "description": "(Optional) One of the announcement type ids.", "apiSummary": true, "validation": [ @@ -112,6 +123,7 @@ }, "url": { "type": "string", + "origin": "composed", "format": "uri", "readOnly": true, "apiSummary": true, diff --git a/templates/frontend/objects/announcement_full.tpl b/templates/frontend/objects/announcement_full.tpl index 8d9de6fbca6..425147f0e95 100644 --- a/templates/frontend/objects/announcement_full.tpl +++ b/templates/frontend/objects/announcement_full.tpl @@ -13,23 +13,23 @@

- {$announcement->getLocalizedTitle()|escape} + {$announcement->getLocalizedData('title')|escape}

- {$announcement->getDatePosted()|date_format:$dateFormatShort} + {$announcement->getAttribute('datePosted')|date_format:$dateFormatShort}
- {if $announcement->getImage()} + {if $announcement->getData('image')} {$announcement->getImageAltText()} {/if}
- {if $announcement->getLocalizedDescription()} - {$announcement->getLocalizedDescription()|strip_unsafe_html} + {if $announcement->getLocalizedData('description')} + {$announcement->getLocalizedData('description')|strip_unsafe_html} {else} - {$announcement->getLocalizedDescriptionShort()|strip_unsafe_html} + {$announcement->getLocalizedData('descriptionShort')|strip_unsafe_html} {/if}
diff --git a/templates/frontend/objects/announcements_list.tpl b/templates/frontend/objects/announcements_list.tpl index 65e3ed96f52..f188f4b869b 100644 --- a/templates/frontend/objects/announcements_list.tpl +++ b/templates/frontend/objects/announcements_list.tpl @@ -27,12 +27,12 @@ {else} {/if} diff --git a/templates/frontend/pages/announcement.tpl b/templates/frontend/pages/announcement.tpl index 2dc36eae4fe..e72b145d7b7 100644 --- a/templates/frontend/pages/announcement.tpl +++ b/templates/frontend/pages/announcement.tpl @@ -9,11 +9,11 @@ * * @uses $announcement Announcement The announcement to display *} -{include file="frontend/components/header.tpl" pageTitleTranslated=$announcement->getLocalizedTitle()|escape} +{include file="frontend/components/header.tpl" pageTitleTranslated=$announcement->getLocalizedData('title')|escape}
- {include file="frontend/components/breadcrumbs_announcement.tpl" currentTitle=$announcement->getLocalizedTitle()|escape} + {include file="frontend/components/breadcrumbs_announcement.tpl" currentTitle=$announcement->getLocalizedData('title')|escape} {* Display book details *} {include file="frontend/objects/announcement_full.tpl"} From cc6b601362aab77f5fb7ad84fe9d2f96954817c4 Mon Sep 17 00:00:00 2001 From: Vitalii Bezsheiko Date: Wed, 11 Sep 2024 00:04:13 +0300 Subject: [PATCH 2/9] pkp/pkp-lib#10328 Fix multilingual attributes retrieval --- classes/core/traits/ModelWithSettings.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/core/traits/ModelWithSettings.php b/classes/core/traits/ModelWithSettings.php index 255b8c02298..5c820f2d8d0 100644 --- a/classes/core/traits/ModelWithSettings.php +++ b/classes/core/traits/ModelWithSettings.php @@ -127,7 +127,7 @@ protected function setSchemaData(): void $schema = $schemaService->get($this->getSchemaName()); $this->convertSchemaToCasts($schema); $this->settings = array_merge($this->settings, $schemaService->groupPropsByOrigin($this->getSchemaName())[Schema::ATTRIBUTE_ORIGIN_SETTINGS]); - $this->multilingualProps = array_merge($this->settings, $schemaService->getMultilingualProps($this->getSchemaName())); + $this->multilingualProps = array_merge($this->multilingualProps, $schemaService->getMultilingualProps($this->getSchemaName())); } /** From a978805c2baff274deb486e0f772cf4efa7e6318 Mon Sep 17 00:00:00 2001 From: Vitalii Bezsheiko Date: Wed, 11 Sep 2024 10:08:42 +0300 Subject: [PATCH 3/9] pkp/pkp-lib#10328 Fix method calls --- pages/announcement/AnnouncementHandler.php | 2 +- templates/frontend/objects/announcement_full.tpl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pages/announcement/AnnouncementHandler.php b/pages/announcement/AnnouncementHandler.php index 3a242ba3d3a..f9e2fda141e 100644 --- a/pages/announcement/AnnouncementHandler.php +++ b/pages/announcement/AnnouncementHandler.php @@ -83,7 +83,7 @@ public function view($args, $request) ) { $templateMgr = TemplateManager::getManager($request); $templateMgr->assign('announcement', $announcement); - $templateMgr->assign('announcementTitle', $announcement->getLocalizedTitleFull()); + $templateMgr->assign('announcementTitle', $announcement->getLocalizedData('fullTitle')); return $templateMgr->display('frontend/pages/announcement.tpl'); } $request->redirect(null, 'announcement'); diff --git a/templates/frontend/objects/announcement_full.tpl b/templates/frontend/objects/announcement_full.tpl index 425147f0e95..62ecb461d7b 100644 --- a/templates/frontend/objects/announcement_full.tpl +++ b/templates/frontend/objects/announcement_full.tpl @@ -18,7 +18,7 @@
{$announcement->getAttribute('datePosted')|date_format:$dateFormatShort}
- {if $announcement->getData('image')} + {if $announcement->getAttribute('image')} Date: Thu, 12 Sep 2024 14:21:11 +0300 Subject: [PATCH 4/9] pkp/pkp-lib#10328 Resolve conflict with HasCamelCasing --- classes/announcement/Announcement.php | 2 -- classes/core/SettingsBuilder.php | 6 +++--- classes/core/traits/ModelWithSettings.php | 23 ++++++++++++++++++----- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/classes/announcement/Announcement.php b/classes/announcement/Announcement.php index 81a58866c4c..a7c57236809 100644 --- a/classes/announcement/Announcement.php +++ b/classes/announcement/Announcement.php @@ -15,7 +15,6 @@ use APP\core\Application; use APP\file\PublicFileManager; -use Eloquence\Behaviours\HasCamelCasing; use Illuminate\Database\Eloquent\Builder as EloquentBuilder; use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Model; @@ -40,7 +39,6 @@ */ class Announcement extends Model { - use HasCamelCasing; use ModelWithSettings; // The subdirectory where announcement images are stored diff --git a/classes/core/SettingsBuilder.php b/classes/core/SettingsBuilder.php index 16d41034cc4..5a1a2152b42 100644 --- a/classes/core/SettingsBuilder.php +++ b/classes/core/SettingsBuilder.php @@ -53,7 +53,7 @@ public function update(array $values) { // Separate Model's primary values from settings [$settingValues, $primaryValues] = collect($values)->partition( - fn (array|string $value, string $key) => in_array(Str::camel($key), $this->model->getSettings()) + fn (mixed $value, string $key) => in_array(Str::camel($key), $this->model->getSettings()) ); // Don't update settings if they aren't set @@ -63,7 +63,7 @@ public function update(array $values) // TODO Eloquent transforms attributes to snake case, find and override instead of transforming here $settingValues = $settingValues->mapWithKeys( - fn (array|string $value, string $key) => [Str::camel($key) => $value] + fn (mixed $value, string $key) => [Str::camel($key) => $value] ); $u = $this->model->getTable(); @@ -334,7 +334,7 @@ protected function buildUpdateSql(Collection $settingValues, string $us, QueryBu { $sql = 'CASE '; $bindings = []; - $settingValues->each(function (array|string $settingValue, string $settingName) use (&$sql, &$bindings, $us) { + $settingValues->each(function (mixed $settingValue, string $settingName) use (&$sql, &$bindings, $us) { if ($this->isMultilingual($settingName)) { foreach ($settingValue as $locale => $localizedValue) { $sql .= 'WHEN ' . $us . '.setting_name=? AND ' . $us . '.locale=? THEN ? '; diff --git a/classes/core/traits/ModelWithSettings.php b/classes/core/traits/ModelWithSettings.php index 5c820f2d8d0..fa56c94a989 100644 --- a/classes/core/traits/ModelWithSettings.php +++ b/classes/core/traits/ModelWithSettings.php @@ -17,6 +17,7 @@ namespace PKP\core\traits; +use Eloquence\Behaviours\HasCamelCasing; use Exception; use PKP\core\maps\Schema; use PKP\core\SettingsBuilder; @@ -26,6 +27,8 @@ trait ModelWithSettings { + use HasCamelCasing; + // The list of attributes associated with the model settings protected array $settings = []; @@ -87,8 +90,7 @@ public function getSettings(): array */ public function getMultilingualProps(): array { - $modelProps = parent::getMultilingualProps(); - return array_merge($this->multilingualProps, $modelProps); + return $this->multilingualProps; } /** @@ -110,7 +112,7 @@ public function getLocalizedData(string $data, ?string $locale = null): mixed throw new Exception('Attribute ' . $data . ' doesn\'t exist in the ' . static::class . ' model'); } - if (!in_array($data, $this->getMultilingualProps())) { + if (!in_array($data, $this->multilingualProps)) { throw new Exception('Trying to retrieve localized data from a non-multilingual attribute ' . $data); } @@ -126,8 +128,8 @@ protected function setSchemaData(): void $schemaService = app()->get('schema'); /** @var PKPSchemaService $schemaService */ $schema = $schemaService->get($this->getSchemaName()); $this->convertSchemaToCasts($schema); - $this->settings = array_merge($this->settings, $schemaService->groupPropsByOrigin($this->getSchemaName())[Schema::ATTRIBUTE_ORIGIN_SETTINGS]); - $this->multilingualProps = array_merge($this->multilingualProps, $schemaService->getMultilingualProps($this->getSchemaName())); + $this->settings = array_merge($this->getSettings(), $schemaService->groupPropsByOrigin($this->getSchemaName())[Schema::ATTRIBUTE_ORIGIN_SETTINGS]); + $this->multilingualProps = array_merge($this->getMultilingualProps(), $schemaService->getMultilingualProps($this->getSchemaName())); } /** @@ -148,4 +150,15 @@ protected function convertSchemaToCasts(stdClass $schema): void $this->mergeCasts($propCast); } + /** + * Override method from HasCamelCasing to retrieve values from setting attributes as it leads to the conflict + */ + public function getAttribute($key): mixed + { + if (in_array($key, $this->getSettings())) { + return parent::getAttribute($key); + } + + return $this->isRelation($key) ? parent::getAttribute($key) : parent::getAttribute($this->getSnakeKey($key)); + } } From c4c3219ea6da1d8d450e672d741426724c6826b7 Mon Sep 17 00:00:00 2001 From: Vitalii Bezsheiko Date: Mon, 16 Sep 2024 12:53:28 +0300 Subject: [PATCH 5/9] pkp/pkp-lib#10328 Fix sql for postgres --- classes/announcement/Announcement.php | 4 +- classes/core/SettingsBuilder.php | 78 +++++++------------ pages/announcement/AnnouncementHandler.php | 2 +- .../frontend/objects/announcement_summary.tpl | 20 ++--- 4 files changed, 41 insertions(+), 63 deletions(-) diff --git a/classes/announcement/Announcement.php b/classes/announcement/Announcement.php index a7c57236809..8391db37a46 100644 --- 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 index 5a1a2152b42..df221394f0e 100644 --- a/classes/core/SettingsBuilder.php +++ b/classes/core/SettingsBuilder.php @@ -58,7 +58,13 @@ 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 @@ -66,27 +72,17 @@ public function update(array $values) 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 index f9e2fda141e..6375df73ea2 100644 --- 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 index 48a5b57074a..ee5674bcb10 100644 --- a/templates/frontend/objects/announcement_summary.tpl +++ b/templates/frontend/objects/announcement_summary.tpl @@ -14,31 +14,31 @@ {assign var="heading" value="h2"} {/if} -
- {if $announcement->getImage()} +
+ {if $announcement->image} {$announcement->getImageAltText()} {/if}
<{$heading}> - getId()}"> - {$announcement->getLocalizedTitle()|escape} + getKey()}"> + {$announcement->getLocalizedData('title')|escape}
- {$announcement->getDatePosted()|date_format:$dateFormatShort} + {$announcement->datePosted|date_format:$dateFormatShort}
- {$announcement->getLocalizedDescriptionShort()|strip_unsafe_html} - getId()}" class="read_more"> + {$announcement->getLocalizedData('descriptionShort')|strip_unsafe_html} + getKey()}" class="read_more"> - {translate key="common.readMoreWithTitle" title=$announcement->getLocalizedTitle()|escape} + {translate key="common.readMoreWithTitle" title=$announcement->getLocalizedData('title')|escape}
From d841f7001686bd214891b130419730736a7a8e31 Mon Sep 17 00:00:00 2001 From: Vitalii Bezsheiko Date: Mon, 23 Sep 2024 17:40:31 +0300 Subject: [PATCH 6/9] pkp/pkp-lib#10328 Remove announcement delete hooks --- classes/announcement/Announcement.php | 6 ------ classes/core/PKPApplication.php | 1 + 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/classes/announcement/Announcement.php b/classes/announcement/Announcement.php index 8391db37a46..3c2747a4ca3 100644 --- a/classes/announcement/Announcement.php +++ b/classes/announcement/Announcement.php @@ -90,20 +90,14 @@ public function getMultilingualProps(): array * Delete announcement, also allows to delete multiple announcements by IDs at once with destroy() method * * @return bool|null - * - * @hook Announcement::delete::before [[$this]] */ public function delete() { - Hook::call('Announcement::delete::before', [$this]); - $deleted = parent::delete(); if ($deleted) { $this->deleteImage(); } - Hook::call('Announcement::delete', [$this]); - return $deleted; } diff --git a/classes/core/PKPApplication.php b/classes/core/PKPApplication.php index 47b9ac1d545..b14d39f2846 100644 --- a/classes/core/PKPApplication.php +++ b/classes/core/PKPApplication.php @@ -153,6 +153,7 @@ class_alias('\PKP\payment\QueuedPayment', '\QueuedPayment'); // QueuedPayment in Hook::addUnsupportedHooks('APIHandler::endpoints'); // pkp/pkp-lib#9434 Unavailable since stable-3_4_0; remove for 3.6.0 development branch Hook::addUnsupportedHooks('Mail::send', 'EditorAction::modifyDecisionOptions', 'EditorAction::recordDecision', 'Announcement::getProperties', 'Author::getProperties::values', 'EmailTemplate::getProperties', 'Galley::getProperties::values', 'Issue::getProperties::fullProperties', 'Issue::getProperties::summaryProperties', 'Issue::getProperties::values', 'Publication::getProperties', 'Section::getProperties::fullProperties', 'Section::getProperties::summaryProperties', 'Section::getProperties::values', 'Submission::getProperties::values', 'SubmissionFile::getProperties', 'User::getProperties::fullProperties', 'User::getProperties::reviewerSummaryProperties', 'User::getProperties::summaryProperties', 'User::getProperties::values', 'Announcement::getMany::queryBuilder', 'Announcement::getMany::queryObject', 'Author::getMany::queryBuilder', 'Author::getMany::queryObject', 'EmailTemplate::getMany::queryBuilder', 'EmailTemplate::getMany::queryObject::custom', 'EmailTemplate::getMany::queryObject::default', 'Galley::getMany::queryBuilder', 'Issue::getMany::queryBuilder', 'Publication::getMany::queryBuilder', 'Publication::getMany::queryObject', 'Stats::getOrderedObjects::queryBuilder', 'Stats::getRecords::queryBuilder', 'Stats::queryBuilder', 'Stats::queryObject', 'Submission::getMany::queryBuilder', 'Submission::getMany::queryObject', 'SubmissionFile::getMany::queryBuilder', 'SubmissionFile::getMany::queryObject', 'User::getMany::queryBuilder', 'User::getMany::queryObject', 'User::getReviewers::queryBuilder', 'CategoryDAO::_fromRow', 'IssueDAO::_fromRow', 'IssueDAO::_returnIssueFromRow', 'SectionDAO::_fromRow', 'UserDAO::_returnUserFromRow', 'UserDAO::_returnUserFromRowWithData', 'UserDAO::_returnUserFromRowWithReviewerStats', 'UserGroupDAO::_returnFromRow', 'ReviewerSubmissionDAO::_fromRow', 'API::stats::publication::abstract::params', 'API::stats::publication::galley::params', 'API::stats::publications::abstract::params', 'API::stats::publications::galley::params', 'PKPLocale::installLocale', 'PKPLocale::registerLocaleFile', 'PKPLocale::registerLocaleFile::isValidLocaleFile', 'PKPLocale::translate', 'API::submissions::files::params', 'ArticleGalleyDAO::getLocalizedGalleysByArticle', 'PluginGridHandler::plugin', 'PluginGridHandler::plugin', 'SubmissionFile::assignedFileStages', 'SubmissionHandler::saveSubmit'); // From the 3.4.0 Release Notebook; remove for 3.6.0 development branch Hook::addUnsupportedHooks('AcronPlugin::parseCronTab'); // pkp/pkp-lib#9678 Unavailable since stable-3_5_0; + Hook::addUnsupportedHooks('Announcement::delete::before', 'Announcement::delete'); // pkp/pkp-lib#10328 Unavailable since stable-3_5_0, use Eloquent Model events instead // If not in strict mode, globally expose constants on this class. if (!PKP_STRICT_MODE) { From 0f5c2aaa15d79893f4feb819c0e588f5a6400386 Mon Sep 17 00:00:00 2001 From: Vitalii Bezsheiko Date: Mon, 23 Sep 2024 21:47:43 +0300 Subject: [PATCH 7/9] pkp/pkp-lib#10328 Replace getAttribute method with property calls --- api/v1/announcements/PKPAnnouncementController.php | 10 +++++----- classes/notification/PKPNotificationManager.php | 2 +- pages/announcement/AnnouncementHandler.php | 6 +++--- templates/frontend/objects/announcement_full.tpl | 8 ++++---- templates/frontend/objects/announcements_list.tpl | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/api/v1/announcements/PKPAnnouncementController.php b/api/v1/announcements/PKPAnnouncementController.php index 7132c1e89e3..3282a280505 100644 --- a/api/v1/announcements/PKPAnnouncementController.php +++ b/api/v1/announcements/PKPAnnouncementController.php @@ -134,7 +134,7 @@ public function get(Request $illuminateRequest): JsonResponse } // The assocId in announcements should always point to the contextId - if ($announcement->getAttribute('assocId') !== $this->getRequest()->getContext()?->getId()) { + if ($announcement->assocId !== $this->getRequest()->getContext()?->getId()) { return response()->json([ 'error' => __('api.announcements.400.contextsNotMatched') ], Response::HTTP_BAD_REQUEST); @@ -243,12 +243,12 @@ public function edit(Request $illuminateRequest): JsonResponse ], Response::HTTP_NOT_FOUND); } - if ($announcement->getAttribute('assocType') !== Application::get()->getContextAssocType()) { + if ($announcement->assocType !== Application::get()->getContextAssocType()) { throw new Exception('Announcement has an assocType that did not match the context.'); } // Don't allow to edit an announcement from one context from a different context's endpoint - if ($request->getContext()?->getId() !== $announcement->getAttribute('assocId')) { + if ($request->getContext()?->getId() !== $announcement->assocId) { return response()->json([ 'error' => __('api.announcements.400.contextsNotMatched') ], Response::HTTP_FORBIDDEN); @@ -296,12 +296,12 @@ public function delete(Request $illuminateRequest): JsonResponse ], Response::HTTP_NOT_FOUND); } - if ($announcement->getAttribute('assocType') !== Application::get()->getContextAssocType()) { + if ($announcement->assocType !== Application::get()->getContextAssocType()) { throw new Exception('Announcement has an assocType that did not match the context.'); } // Don't allow to delete an announcement from one context from a different context's endpoint - if ($request->getContext()?->getId() !== $announcement->getAttribute('assocId')) { + if ($request->getContext()?->getId() !== $announcement->assocId) { return response()->json([ 'error' => __('api.announcements.400.contextsNotMatched') ], Response::HTTP_FORBIDDEN); diff --git a/classes/notification/PKPNotificationManager.php b/classes/notification/PKPNotificationManager.php index 7db3f044309..6d6ec555fcf 100644 --- a/classes/notification/PKPNotificationManager.php +++ b/classes/notification/PKPNotificationManager.php @@ -81,7 +81,7 @@ public function getNotificationUrl(PKPRequest $request, Notification $notificati throw new \Exception('Unexpected assoc type!'); } $announcement = Announcement::find($notification->assocId); - $context = $contextDao->getById($announcement->getAttribute('assocId')); + $context = $contextDao->getById($announcement->assocId); return $dispatcher->url($request, PKPApplication::ROUTE_PAGE, $context->getPath(), 'announcement', 'view', [$notification->assocId]); case Notification::NOTIFICATION_TYPE_CONFIGURE_PAYMENT_METHOD: return __('notification.type.configurePaymentMethod'); diff --git a/pages/announcement/AnnouncementHandler.php b/pages/announcement/AnnouncementHandler.php index 6375df73ea2..261aa9e2040 100644 --- a/pages/announcement/AnnouncementHandler.php +++ b/pages/announcement/AnnouncementHandler.php @@ -75,10 +75,10 @@ public function view($args, $request) $announcement = Announcement::find($announcementId); if ( $announcement - && $announcement->getAttribute('assocType') == Application::getContextAssocType() - && $announcement->getAttribute('assocId') == $request->getContext()?->getId() + && $announcement->assocType == Application::getContextAssocType() + && $announcement->assocId == $request->getContext()?->getId() && ( - $announcement->getAttribute('dateExpire') == null || strtotime($announcement->getAttribute('dateExpire')) > time() + $announcement->dateExpire == null || strtotime($announcement->dateExpire) > time() ) ) { $templateMgr = TemplateManager::getManager($request); diff --git a/templates/frontend/objects/announcement_full.tpl b/templates/frontend/objects/announcement_full.tpl index 62ecb461d7b..5830e1d4321 100644 --- a/templates/frontend/objects/announcement_full.tpl +++ b/templates/frontend/objects/announcement_full.tpl @@ -16,13 +16,13 @@ {$announcement->getLocalizedData('title')|escape}
- {$announcement->getAttribute('datePosted')|date_format:$dateFormatShort} + {$announcement->datePosted|date_format:$dateFormatShort}
- {if $announcement->getAttribute('image')} + {if $announcement->image} {$announcement->getAttribute('imageAltText')} {/if}
diff --git a/templates/frontend/objects/announcements_list.tpl b/templates/frontend/objects/announcements_list.tpl index f188f4b869b..fc4cef21774 100644 --- a/templates/frontend/objects/announcements_list.tpl +++ b/templates/frontend/objects/announcements_list.tpl @@ -32,7 +32,7 @@
- {$announcement->getAttribute('datePosted')|date_format:$dateFormatShort} + {$announcement->datePosted|date_format:$dateFormatShort}
{/if} From 070aa6f77b2b14e31b9089b5f82a48e8ba11c329 Mon Sep 17 00:00:00 2001 From: Vitalii Bezsheiko Date: Mon, 30 Sep 2024 19:45:02 +0300 Subject: [PATCH 8/9] pkp/pkp-lib#10328 Fix dates as Carbon, fillables and garded attributes --- .../PKPAnnouncementController.php | 6 +- classes/announcement/Announcement.php | 33 +-- classes/announcement/Collector.php | 242 ------------------ classes/core/PKPApplication.php | 2 +- classes/core/SettingsBuilder.php | 2 +- classes/core/traits/ModelWithSettings.php | 48 +++- classes/services/PKPSchemaService.php | 36 ++- pages/announcement/AnnouncementHandler.php | 3 +- .../frontend/objects/announcement_full.tpl | 2 +- .../frontend/objects/announcement_summary.tpl | 6 +- .../frontend/objects/announcements_list.tpl | 4 +- 11 files changed, 88 insertions(+), 296 deletions(-) delete mode 100644 classes/announcement/Collector.php diff --git a/api/v1/announcements/PKPAnnouncementController.php b/api/v1/announcements/PKPAnnouncementController.php index 3282a280505..f2d8a8dcd3e 100644 --- a/api/v1/announcements/PKPAnnouncementController.php +++ b/api/v1/announcements/PKPAnnouncementController.php @@ -220,7 +220,7 @@ public function add(Request $illuminateRequest): JsonResponse $sendEmail = (bool) filter_var($params['sendEmail'], FILTER_VALIDATE_BOOLEAN); if ($context) { - $this->notifyUsers($request, $context, $announcement->getKey(), $sendEmail); + $this->notifyUsers($request, $context, $announcement->id, $sendEmail); } return response()->json(Repo::announcement()->getSchemaMap()->map($announcement), Response::HTTP_OK); @@ -255,7 +255,7 @@ public function edit(Request $illuminateRequest): JsonResponse } $params = $this->convertStringsToSchema(PKPSchemaService::SCHEMA_ANNOUNCEMENT, $illuminateRequest->input()); - $params['id'] = $announcement->getKey(); + $params['id'] = $announcement->id; $params['typeId'] ??= null; $primaryLocale = $context ? $context->getPrimaryLocale() : $request->getSite()->getPrimaryLocale(); @@ -275,7 +275,7 @@ public function edit(Request $illuminateRequest): JsonResponse ], Response::HTTP_BAD_REQUEST); } - $announcement = Announcement::find($announcement->getKey()); + $announcement = Announcement::find($announcement->id); return response()->json(Repo::announcement()->getSchemaMap()->map($announcement), Response::HTTP_OK); } diff --git a/classes/announcement/Announcement.php b/classes/announcement/Announcement.php index 3c2747a4ca3..b075928fe23 100644 --- a/classes/announcement/Announcement.php +++ b/classes/announcement/Announcement.php @@ -55,12 +55,12 @@ class Announcement extends Model * * @var array */ - protected $fillable = ['assocType', 'assocId', 'typeId', 'title', 'image', 'description', 'descriptionShort']; + protected $guarded = ['announcementId', 'datePosted']; /** * @inheritDoc */ - public static function getSchemaName(): string + public static function getSchemaName(): ?string { return PKPSchemaService::SCHEMA_ANNOUNCEMENT; } @@ -117,7 +117,7 @@ public function save(array $options = []) } // If it's updated model and a new image is uploaded, first, delete an old one - $hasNewImage = $this->hasAttribute('temporaryFileId'); + $hasNewImage = $this?->image?->temporaryFileId; if ($saved && !$newlyCreated && $hasNewImage) { $this->deleteImage(); $this->handleImageUpload(); @@ -270,17 +270,7 @@ protected function imageUrl(bool $withTimestamp = true): Attribute protected function imageAltText(): Attribute { return Attribute::make( - get: fn () => $this->image->altText ?? '' - ); - } - - /** - * Get the date announcement was posted - */ - protected function datePosted(): Attribute - { - return Attribute::make( - get: fn (string $value) => date('Y-m-d', strtotime($value)) + get: fn () => $this->image?->altText ?? '' ); } @@ -362,7 +352,7 @@ protected function getImageFilename(TemporaryFile $temporaryFile): string { $fileManager = new FileManager(); - return $this->getAttribute('announcementId') + return $this->id . $fileManager->getImageExtension($temporaryFile->getFileType()); } @@ -419,4 +409,17 @@ protected function storeTemporaryFile(TemporaryFile $temporaryFile, string $newP return $result; } + + /** + * Get the attributes that should be cast. + * + * @return array + */ + protected function casts(): array + { + return [ + 'dateExpire' => 'datetime', + 'datePosted' => 'datetime', + ]; + } } diff --git a/classes/announcement/Collector.php b/classes/announcement/Collector.php deleted file mode 100644 index 72b63d82bdd..00000000000 --- a/classes/announcement/Collector.php +++ /dev/null @@ -1,242 +0,0 @@ -dao = $dao; - } - - /** @copydoc DAO::getCount() */ - public function getCount(): int - { - return $this->dao->getCount($this); - } - - /** - * @copydoc DAO::getIds() - * - * @return Collection - */ - public function getIds(): Collection - { - return $this->dao->getIds($this); - } - - /** - * @copydoc DAO::getMany() - * - * @return LazyCollection - */ - public function getMany(): LazyCollection - { - return $this->dao->getMany($this); - } - - /** - * Filter announcements by one or more contexts - */ - public function filterByContextIds(?array $contextIds): self - { - $this->contextIds = $contextIds; - return $this; - } - - /** - * Filter announcements by those that have not expired - * - * @param string $date Optionally filter announcements by those - * not expired until $date (YYYY-MM-DD). - */ - public function filterByActive(string $date = ''): self - { - $this->isActive = empty($date) - ? Core::getCurrentDate() - : $date; - return $this; - } - - /** - * Filter announcements by one or more announcement types - */ - public function filterByTypeIds(array $typeIds): self - { - $this->typeIds = $typeIds; - return $this; - } - - /** - * Include site-level announcements in the results - */ - public function withSiteAnnouncements(?string $includeMethod = self::SITE_AND_CONTEXTS): self - { - $this->includeSite = $includeMethod; - return $this; - } - - /** - * Filter announcements by those matching a search query - */ - public function searchPhrase(?string $phrase): self - { - $this->searchPhrase = $phrase; - return $this; - } - - /** - * Limit the number of objects retrieved - */ - public function limit(?int $count): self - { - $this->count = $count; - return $this; - } - - /** - * Offset the number of objects retrieved, for example to - * retrieve the second page of contents - */ - public function offset(?int $offset): self - { - $this->offset = $offset; - return $this; - } - - /** - * Order the results - * - * Results are ordered by the date posted by default. - * - * @param string $sorter One of the self::ORDERBY_ constants - * @param string $direction One of the self::ORDER_DIR_ constants - */ - public function orderBy(?string $sorter, string $direction = self::ORDER_DIR_DESC): self - { - $this->orderBy = $sorter; - $this->orderDirection = $direction; - return $this; - } - - /** - * @copydoc CollectorInterface::getQueryBuilder() - * - * @hook Announcement::Collector [[&$qb, $this]] - */ - public function getQueryBuilder(): Builder - { - $qb = DB::table($this->dao->table . ' as a') - ->select(['a.*']); - - if (isset($this->contextIds) && $this->includeSite !== self::SITE_ONLY) { - $qb->where('a.assoc_type', Application::get()->getContextAssocType()); - $qb->whereIn('a.assoc_id', $this->contextIds); - if ($this->includeSite === self::SITE_AND_CONTEXTS) { - $qb->orWhereNull('a.assoc_id'); - } - } elseif ($this->includeSite === self::SITE_ONLY) { - $qb->where('a.assoc_type', Application::get()->getContextAssocType()); - $qb->whereNull('a.assoc_id'); - } - - if (isset($this->typeIds)) { - $qb->whereIn('a.type_id', $this->typeIds); - } - - $qb->when($this->isActive, fn ($qb) => $qb->where(function ($qb) { - $qb->where('a.date_expire', '>', $this->isActive) - ->orWhereNull('a.date_expire'); - })); - - if ($this->searchPhrase !== null) { - $words = explode(' ', $this->searchPhrase); - if (count($words)) { - $qb->whereIn('a.announcement_id', function ($query) use ($words) { - $query->select('announcement_id')->from($this->dao->settingsTable); - foreach ($words as $word) { - $word = strtolower(addcslashes($word, '%_')); - $query->where(function ($query) use ($word) { - $query->where(function ($query) use ($word) { - $query->where('setting_name', 'title'); - $query->where(DB::raw('lower(setting_value)'), 'LIKE', "%{$word}%"); - }) - ->orWhere(function ($query) use ($word) { - $query->where('setting_name', 'descriptionShort'); - $query->where(DB::raw('lower(setting_value)'), 'LIKE', "%{$word}%"); - }) - ->orWhere(function ($query) use ($word) { - $query->where('setting_name', 'description'); - $query->where(DB::raw('lower(setting_value)'), 'LIKE', "%{$word}%"); - }); - }); - } - }); - } - } - - $qb->orderByDesc('a.date_posted'); - - if (isset($this->count)) { - $qb->limit($this->count); - } - - if (isset($this->offset)) { - $qb->offset($this->offset); - } - - if (isset($this->orderBy)) { - $qb->orderBy('a.' . $this->orderBy, $this->orderDirection); - // Add a secondary sort by id to catch cases where two - // announcements share the same date - if (in_array($this->orderBy, [SELF::ORDERBY_DATE_EXPIRE, SELF::ORDERBY_DATE_POSTED])) { - $qb->orderBy('a.announcement_id', $this->orderDirection); - } - } - - Hook::call('Announcement::Collector', [&$qb, $this]); - - return $qb; - } -} diff --git a/classes/core/PKPApplication.php b/classes/core/PKPApplication.php index b14d39f2846..5e32022c74e 100644 --- a/classes/core/PKPApplication.php +++ b/classes/core/PKPApplication.php @@ -153,7 +153,7 @@ class_alias('\PKP\payment\QueuedPayment', '\QueuedPayment'); // QueuedPayment in Hook::addUnsupportedHooks('APIHandler::endpoints'); // pkp/pkp-lib#9434 Unavailable since stable-3_4_0; remove for 3.6.0 development branch Hook::addUnsupportedHooks('Mail::send', 'EditorAction::modifyDecisionOptions', 'EditorAction::recordDecision', 'Announcement::getProperties', 'Author::getProperties::values', 'EmailTemplate::getProperties', 'Galley::getProperties::values', 'Issue::getProperties::fullProperties', 'Issue::getProperties::summaryProperties', 'Issue::getProperties::values', 'Publication::getProperties', 'Section::getProperties::fullProperties', 'Section::getProperties::summaryProperties', 'Section::getProperties::values', 'Submission::getProperties::values', 'SubmissionFile::getProperties', 'User::getProperties::fullProperties', 'User::getProperties::reviewerSummaryProperties', 'User::getProperties::summaryProperties', 'User::getProperties::values', 'Announcement::getMany::queryBuilder', 'Announcement::getMany::queryObject', 'Author::getMany::queryBuilder', 'Author::getMany::queryObject', 'EmailTemplate::getMany::queryBuilder', 'EmailTemplate::getMany::queryObject::custom', 'EmailTemplate::getMany::queryObject::default', 'Galley::getMany::queryBuilder', 'Issue::getMany::queryBuilder', 'Publication::getMany::queryBuilder', 'Publication::getMany::queryObject', 'Stats::getOrderedObjects::queryBuilder', 'Stats::getRecords::queryBuilder', 'Stats::queryBuilder', 'Stats::queryObject', 'Submission::getMany::queryBuilder', 'Submission::getMany::queryObject', 'SubmissionFile::getMany::queryBuilder', 'SubmissionFile::getMany::queryObject', 'User::getMany::queryBuilder', 'User::getMany::queryObject', 'User::getReviewers::queryBuilder', 'CategoryDAO::_fromRow', 'IssueDAO::_fromRow', 'IssueDAO::_returnIssueFromRow', 'SectionDAO::_fromRow', 'UserDAO::_returnUserFromRow', 'UserDAO::_returnUserFromRowWithData', 'UserDAO::_returnUserFromRowWithReviewerStats', 'UserGroupDAO::_returnFromRow', 'ReviewerSubmissionDAO::_fromRow', 'API::stats::publication::abstract::params', 'API::stats::publication::galley::params', 'API::stats::publications::abstract::params', 'API::stats::publications::galley::params', 'PKPLocale::installLocale', 'PKPLocale::registerLocaleFile', 'PKPLocale::registerLocaleFile::isValidLocaleFile', 'PKPLocale::translate', 'API::submissions::files::params', 'ArticleGalleyDAO::getLocalizedGalleysByArticle', 'PluginGridHandler::plugin', 'PluginGridHandler::plugin', 'SubmissionFile::assignedFileStages', 'SubmissionHandler::saveSubmit'); // From the 3.4.0 Release Notebook; remove for 3.6.0 development branch Hook::addUnsupportedHooks('AcronPlugin::parseCronTab'); // pkp/pkp-lib#9678 Unavailable since stable-3_5_0; - Hook::addUnsupportedHooks('Announcement::delete::before', 'Announcement::delete'); // pkp/pkp-lib#10328 Unavailable since stable-3_5_0, use Eloquent Model events instead + Hook::addUnsupportedHooks('Announcement::delete::before', 'Announcement::delete', 'Announcement::Collector'); // pkp/pkp-lib#10328 Unavailable since stable-3_5_0, use Eloquent Model events instead // If not in strict mode, globally expose constants on this class. if (!PKP_STRICT_MODE) { diff --git a/classes/core/SettingsBuilder.php b/classes/core/SettingsBuilder.php index df221394f0e..f10466b65e4 100644 --- a/classes/core/SettingsBuilder.php +++ b/classes/core/SettingsBuilder.php @@ -82,7 +82,7 @@ public function update(array $values) $settingCount = DB::table($us)->whereIn($us . '.' . $primaryKey, $newQuery->select($primaryKey)) ->update([$us . '.setting_value' => DB::raw($sql)]); - return $count ? $count + $settingCount : $settingCount; // TODO Return the count of updated setting rows? + return ($count ?? 0) + $settingCount; } /** diff --git a/classes/core/traits/ModelWithSettings.php b/classes/core/traits/ModelWithSettings.php index fa56c94a989..d077cf73bb3 100644 --- a/classes/core/traits/ModelWithSettings.php +++ b/classes/core/traits/ModelWithSettings.php @@ -19,6 +19,7 @@ use Eloquence\Behaviours\HasCamelCasing; use Exception; +use Illuminate\Database\Eloquent\Casts\Attribute; use PKP\core\maps\Schema; use PKP\core\SettingsBuilder; use PKP\facades\Locale; @@ -54,13 +55,17 @@ abstract public static function getSchemaName(): ?string; /** * See Illuminate\Database\Eloquent\Concerns\HasAttributes::mergeCasts() + * + * @param array $casts + * + * @return array */ - abstract public function mergeCasts(array $casts); + abstract protected function ensureCastsAreStringValues($casts); public function __construct(array $attributes = []) { parent::__construct($attributes); - if ($this->getSchemaName()) { + if (static::getSchemaName()) { $this->setSchemaData(); } } @@ -107,34 +112,39 @@ public function getLocalizedData(string $data, ?string $locale = null): mixed $locale = Locale::getLocale(); } - $multilingualProp = $this->getAttribute($data); - if (!$multilingualProp) { - throw new Exception('Attribute ' . $data . ' doesn\'t exist in the ' . static::class . ' model'); + if (!in_array($data, $this->getMultilingualProps())) { + throw new Exception( + sprintf('Given localized property %s does not exist in %s model', $data, static::class) + ); } - if (!in_array($data, $this->multilingualProps)) { - throw new Exception('Trying to retrieve localized data from a non-multilingual attribute ' . $data); - } + $multilingualProp = $this->getAttribute($data); - // TODO What should the default behaviour be if localized value doesn't exist? return $multilingualProp[$locale] ?? null; } /** - * Sets the schema for current Model + * Sets the schema for the current Model */ protected function setSchemaData(): void { $schemaService = app()->get('schema'); /** @var PKPSchemaService $schemaService */ $schema = $schemaService->get($this->getSchemaName()); $this->convertSchemaToCasts($schema); - $this->settings = array_merge($this->getSettings(), $schemaService->groupPropsByOrigin($this->getSchemaName())[Schema::ATTRIBUTE_ORIGIN_SETTINGS]); + $this->settings = array_merge($this->getSettings(), $schemaService->groupPropsByOrigin($this->getSchemaName())[Schema::ATTRIBUTE_ORIGIN_SETTINGS] ?? []); $this->multilingualProps = array_merge($this->getMultilingualProps(), $schemaService->getMultilingualProps($this->getSchemaName())); + + $writableProps = $schemaService->groupPropsByOrigin($this->getSchemaName(), true); + $this->fillable = array_merge( + $writableProps[Schema::ATTRIBUTE_ORIGIN_SETTINGS], + $writableProps[Schema::ATTRIBUTE_ORIGIN_MAIN], + $this->fillable, + ); } /** * Set casts by deriving proper types from schema - * TODO casts on multilingual properties. Keep in mind that overriding Model::attributesToArray() might conflict with HasCamelCasing trait + * FIXME pkp/pkp-lib#10476 casts on multilingual properties. Keep in mind that overriding Model::attributesToArray() might conflict with HasCamelCasing trait */ protected function convertSchemaToCasts(stdClass $schema): void { @@ -147,7 +157,8 @@ protected function convertSchemaToCasts(stdClass $schema): void $propCast[$propName] = $propSchema->type; } - $this->mergeCasts($propCast); + $propCasts = $this->ensureCastsAreStringValues($propCast); + $this->casts = array_merge($propCasts, $this->casts); } /** @@ -161,4 +172,15 @@ public function getAttribute($key): mixed return $this->isRelation($key) ? parent::getAttribute($key) : parent::getAttribute($this->getSnakeKey($key)); } + + /** + * Create an id attribute for the models + */ + protected function id(): Attribute + { + return Attribute::make( + get: fn ($value, $attributes) => $attributes[$this->primaryKey] ?? null, + set: fn ($value) => [$this->primaryKey => $value], + ); + } } diff --git a/classes/services/PKPSchemaService.php b/classes/services/PKPSchemaService.php index 0aabac27905..ab9a1862a7a 100644 --- a/classes/services/PKPSchemaService.php +++ b/classes/services/PKPSchemaService.php @@ -69,6 +69,7 @@ class PKPSchemaService * @hook Schema::get::(schemaName) [[schema]] * @hook Schema::get:: * @hook Schema::get::before:: + * @hook Schema::get::before:: */ public function get($schemaName, $forceReload = false) { @@ -260,24 +261,31 @@ public function getPropsByAttributeOrigin(string $schemaName, string $attributeO * * @return array>, e.g. ['primary' => ['assocId', 'assocType']] */ - public function groupPropsByOrigin(string $schemaName): array + public function groupPropsByOrigin(string $schemaName, bool $excludeReadOnly = false): array { $schema = $this->get($schemaName); $propsByOrigin = []; foreach ($schema->properties as $propName => $propSchema) { - if (!empty($propSchema->origin)) { - switch($propSchema->origin) { - case Schema::ATTRIBUTE_ORIGIN_SETTINGS: - $propsByOrigin[Schema::ATTRIBUTE_ORIGIN_SETTINGS][] = $propName; - break; - case Schema::ATTRIBUTE_ORIGIN_COMPOSED: - $propsByOrigin[Schema::ATTRIBUTE_ORIGIN_COMPOSED][] = $propName; - break; - case Schema::ATTRIBUTE_ORIGIN_MAIN: - default: - $propsByOrigin[Schema::ATTRIBUTE_ORIGIN_MAIN][] = $propName; - break; - } + if (empty($propSchema->origin)) { + continue; + } + + // Exclude readonly if specified + if ($excludeReadOnly && $propSchema->origin->readOnly) { + continue; + } + + switch($propSchema->origin) { + case Schema::ATTRIBUTE_ORIGIN_SETTINGS: + $propsByOrigin[Schema::ATTRIBUTE_ORIGIN_SETTINGS][] = $propName; + break; + case Schema::ATTRIBUTE_ORIGIN_COMPOSED: + $propsByOrigin[Schema::ATTRIBUTE_ORIGIN_COMPOSED][] = $propName; + break; + case Schema::ATTRIBUTE_ORIGIN_MAIN: + default: + $propsByOrigin[Schema::ATTRIBUTE_ORIGIN_MAIN][] = $propName; + break; } } diff --git a/pages/announcement/AnnouncementHandler.php b/pages/announcement/AnnouncementHandler.php index 261aa9e2040..2ca7619d86a 100644 --- a/pages/announcement/AnnouncementHandler.php +++ b/pages/announcement/AnnouncementHandler.php @@ -20,6 +20,7 @@ use APP\core\Request; use APP\handler\Handler; use APP\template\TemplateManager; +use Carbon\Carbon; use PKP\announcement\Announcement; use PKP\core\PKPApplication; use PKP\core\PKPRequest; @@ -78,7 +79,7 @@ public function view($args, $request) && $announcement->assocType == Application::getContextAssocType() && $announcement->assocId == $request->getContext()?->getId() && ( - $announcement->dateExpire == null || strtotime($announcement->dateExpire) > time() + $announcement->dateExpire == null || Carbon::now()->lte($announcement->dateExpire) ) ) { $templateMgr = TemplateManager::getManager($request); diff --git a/templates/frontend/objects/announcement_full.tpl b/templates/frontend/objects/announcement_full.tpl index 5830e1d4321..83e0d4c58f2 100644 --- a/templates/frontend/objects/announcement_full.tpl +++ b/templates/frontend/objects/announcement_full.tpl @@ -16,7 +16,7 @@ {$announcement->getLocalizedData('title')|escape}
- {$announcement->datePosted|date_format:$dateFormatShort} + {$announcement->datePosted->format($dateFormatShort)}
{if $announcement->image} <{$heading}> - getKey()}"> + id}"> {$announcement->getLocalizedData('title')|escape}
- {$announcement->datePosted|date_format:$dateFormatShort} + {$announcement->datePosted->format($dateFormatShort)}
{$announcement->getLocalizedData('descriptionShort')|strip_unsafe_html} - getKey()}" class="read_more"> + id}" class="read_more"> diff --git a/templates/frontend/objects/announcements_list.tpl b/templates/frontend/objects/announcements_list.tpl index fc4cef21774..e89a8eca8a0 100644 --- a/templates/frontend/objects/announcements_list.tpl +++ b/templates/frontend/objects/announcements_list.tpl @@ -27,12 +27,12 @@ {else} {/if} From 4e0933e792a63a01d2654c5132bedb9e4423eb37 Mon Sep 17 00:00:00 2001 From: Vitalii Bezsheiko Date: Tue, 1 Oct 2024 16:50:27 +0300 Subject: [PATCH 9/9] pkp/pkp-lib#10328 Replace TODOs with issues --- classes/announcement/Announcement.php | 1 - classes/core/SettingsBuilder.php | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/classes/announcement/Announcement.php b/classes/announcement/Announcement.php index b075928fe23..9f865880b81 100644 --- a/classes/announcement/Announcement.php +++ b/classes/announcement/Announcement.php @@ -206,7 +206,6 @@ protected function scopeWithSearchPhrase(EloquentBuilder $builder, ?string $sear /** * Get the full title - * TODO temporary measure while AnnouncementType isn't refactored as Eloquent Model */ protected function fullTitle(): Attribute { diff --git a/classes/core/SettingsBuilder.php b/classes/core/SettingsBuilder.php index f10466b65e4..b7b1a408375 100644 --- a/classes/core/SettingsBuilder.php +++ b/classes/core/SettingsBuilder.php @@ -67,7 +67,7 @@ public function update(array $values) $count = parent::update($primaryValues->toArray()); } - // TODO Eloquent transforms attributes to snake case, find and override instead of transforming here + // FIXME pkp/pkp-lib#10485 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] ); @@ -280,7 +280,6 @@ protected function getModelWithSettings(array|string $columns = ['*']): Collecti /** * If specific columns are selected to fill the Model with, iterate and filter all, which aren't specified - * TODO Instead of iterating through all row properties, we can force to pass primary key as a mandatory column? */ protected function filterRow(stdClass $row, string|array $columns = ['*']): stdClass {