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}