From a68a4382db74b002ba0c6e69f9bcb5f810d7ee6e Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Sun, 16 Jun 2024 01:50:46 +0300 Subject: [PATCH] pkp/pkp-lib#10060 Typed announcement code --- classes/announcement/Announcement.php | 120 ++++++------------ classes/announcement/AnnouncementType.php | 26 +--- classes/announcement/AnnouncementTypeDAO.php | 117 +++++------------ classes/announcement/Collector.php | 105 +++++++-------- classes/announcement/Repository.php | 31 ++--- classes/announcement/maps/Schema.php | 1 - classes/handler/PKPHandler.php | 2 +- .../AnnouncementTypeGridHandler.php | 46 ++----- pages/announcement/AnnouncementHandler.php | 1 - pages/announcement/index.php | 2 +- 10 files changed, 141 insertions(+), 310 deletions(-) diff --git a/classes/announcement/Announcement.php b/classes/announcement/Announcement.php index 48a82bedc79..c3962035d46 100644 --- a/classes/announcement/Announcement.php +++ b/classes/announcement/Announcement.php @@ -35,92 +35,76 @@ class Announcement extends \PKP\core\DataObject // /** * Get assoc ID for this announcement. - * - * @return int */ - public function getAssocId() + public function getAssocId(): ?int { return $this->getData('assocId'); } /** * Set assoc ID for this announcement. - * - * @param int $assocId */ - public function setAssocId($assocId) + public function setAssocId(?int $assocId): void { $this->setData('assocId', $assocId); } /** * Get assoc type for this announcement. - * - * @return int */ - public function getAssocType() + public function getAssocType(): ?int { return $this->getData('assocType'); } /** * Set assoc type for this announcement. - * - * @param int $assocType */ - public function setAssocType($assocType) + public function setAssocType(?int $assocType): void { $this->setData('assocType', $assocType); } /** * Get the announcement type of the announcement. - * - * @return int */ - public function getTypeId() + public function getTypeId(): ?int { return $this->getData('typeId'); } /** * Set the announcement type of the announcement. - * - * @param int $typeId */ - public function setTypeId($typeId) + public function setTypeId(?int $typeId): void { $this->setData('typeId', $typeId); } /** * Get the announcement type name of the announcement. - * - * @return string|null */ - public function getAnnouncementTypeName() + public function getAnnouncementTypeName(): ?string { + if (!$this->getData('typeId')) { + return null; + } $announcementTypeDao = DAORegistry::getDAO('AnnouncementTypeDAO'); /** @var AnnouncementTypeDAO $announcementTypeDao */ - $announcementType = $announcementTypeDao->getById($this->getData('typeId')); - return $announcementType ? $announcementType->getLocalizedTypeName() : null; + return $announcementTypeDao->getById($this->getData('typeId'))->getLocalizedTypeName(); } /** * Get localized announcement title - * - * @return string */ - public function getLocalizedTitle() + public function getLocalizedTitle(): ?string { return $this->getLocalizedData('title'); } /** * Get full localized announcement title including type name - * - * @return string */ - public function getLocalizedTitleFull() + public function getLocalizedTitleFull(): ?string { $typeName = $this->getAnnouncementTypeName(); if (!empty($typeName)) { @@ -132,89 +116,64 @@ public function getLocalizedTitleFull() /** * Get announcement title. - * - * @param string $locale - * - * @return string */ - public function getTitle($locale) + public function getTitle(?string $locale): array|string|null { - return $this->getData('title', $locale); + return $this->getData('title', $locale) ?? ''; } /** * Set announcement title. - * - * @param string $title - * @param string $locale */ - public function setTitle($title, $locale) + public function setTitle(array|string|null $title, ?string $locale): void { $this->setData('title', $title, $locale); } /** * Get localized short description - * - * @return string */ - public function getLocalizedDescriptionShort() + public function getLocalizedDescriptionShort(): ?string { return $this->getLocalizedData('descriptionShort'); } /** * Get announcement brief description. - * - * @param string $locale - * - * @return string */ - public function getDescriptionShort($locale) + public function getDescriptionShort(?string $locale): array|string|null { return $this->getData('descriptionShort', $locale); } /** * Set announcement brief description. - * - * @param string $descriptionShort - * @param string $locale */ - public function setDescriptionShort($descriptionShort, $locale) + public function setDescriptionShort(array|string|null $descriptionShort, ?string $locale): void { $this->setData('descriptionShort', $descriptionShort, $locale); } /** * Get localized full description - * - * @return string */ - public function getLocalizedDescription() + public function getLocalizedDescription(): ?string { return $this->getLocalizedData('description'); } /** * Get announcement description. - * - * @param string $locale - * - * @return string */ - public function getDescription($locale) + public function getDescription(?string $locale): array|string|null { return $this->getData('description', $locale); } /** * Set announcement description. - * - * @param string $description - * @param string $locale */ - public function setDescription($description, $locale) + public function setDescription(array|string|null $description, ?string $locale): void { $this->setData('description', $description, $locale); } @@ -222,9 +181,9 @@ public function setDescription($description, $locale) /** * Get announcement expiration date. * - * @return string (YYYY-MM-DD) + * @return ?string Format (YYYY-MM-DD) */ - public function getDateExpire() + public function getDateExpire(): ?string { return $this->getData('dateExpire'); } @@ -232,9 +191,9 @@ public function getDateExpire() /** * Set announcement expiration date. * - * @param string $dateExpire (YYYY-MM-DD) + * @param ?string $dateExpire Format YYYY-MM-DD HH:MM:SS */ - public function setDateExpire($dateExpire) + public function setDateExpire(?string $dateExpire): void { $this->setData('dateExpire', $dateExpire); } @@ -242,9 +201,9 @@ public function setDateExpire($dateExpire) /** * Get announcement posted date. * - * @return string (YYYY-MM-DD) + * @return ?string Format YYYY-MM-DD HH:MM:SS */ - public function getDatePosted() + public function getDatePosted(): ?string { return date('Y-m-d', strtotime($this->getData('datePosted'))); } @@ -252,9 +211,9 @@ public function getDatePosted() /** * Get announcement posted datetime. * - * @return string (YYYY-MM-DD HH:MM:SS) + * @return ?string Format YYYY-MM-DD HH:MM:SS */ - public function getDatetimePosted() + public function getDatetimePosted(): ?string { return $this->getData('datePosted'); } @@ -262,9 +221,9 @@ public function getDatetimePosted() /** * Set announcement posted date. * - * @param string $datePosted (YYYY-MM-DD) + * @param ?string $datePosted Format YYYY-MM-DD HH:MM:SS */ - public function setDatePosted($datePosted) + public function setDatePosted(?string $datePosted): void { $this->setData('datePosted', $datePosted); } @@ -272,9 +231,9 @@ public function setDatePosted($datePosted) /** * Set announcement posted datetime. * - * @param string $datetimePosted (YYYY-MM-DD HH:MM:SS) + * @param ?string $datetimePosted Format YYYY-MM-DD HH:MM:SS */ - public function setDatetimePosted($datetimePosted) + public function setDatetimePosted(?string $datetimePosted): void { $this->setData('datePosted', $datetimePosted); } @@ -304,7 +263,6 @@ public function setImage(array $image): void public function getImageUrl(bool $withTimestamp = true): string { $image = $this->getImage(); - if (!$image) { return ''; } @@ -319,7 +277,7 @@ public function getImageUrl(bool $withTimestamp = true): string return join('/', [ Application::get()->getRequest()->getBaseUrl(), $this->getAssocId() - ? $publicFileManager->getContextFilesPath((int) $this->getAssocId()) + ? $publicFileManager->getContextFilesPath($this->getAssocId()) : $publicFileManager->getSiteFilesPath(), Repo::announcement()->getImageSubdirectory(), $filename @@ -331,13 +289,7 @@ public function getImageUrl(bool $withTimestamp = true): string */ public function getImageAltText(): string { - $image = $this->getImage(); - - if (!$image || !$image['altText']) { - return ''; - } - - return $image['altText']; + return $this->getImage()['altText'] ?? ''; } } diff --git a/classes/announcement/AnnouncementType.php b/classes/announcement/AnnouncementType.php index ba8021ee2f1..0d97a42becf 100644 --- a/classes/announcement/AnnouncementType.php +++ b/classes/announcement/AnnouncementType.php @@ -20,58 +20,42 @@ class AnnouncementType extends \PKP\core\DataObject { - // - // Get/set methods - // /** * Get context ID for this announcement. - * - * @return int */ - public function getContextId() + public function getContextId(): ?int { return $this->getData('contextId'); } /** * Set context ID for this announcement. - * - * @param int $contextId */ - public function setContextId($contextId) + public function setContextId(?int $contextId): void { $this->setData('contextId', $contextId); } /** * Get the type of the announcement type. - * - * @return string */ - public function getLocalizedTypeName() + public function getLocalizedTypeName(): ?string { return $this->getLocalizedData('name'); } /** * Get the type of the announcement type. - * - * @param string $locale - * - * @return string */ - public function getName($locale) + public function getName(?string $locale): array|string|null { return $this->getData('name', $locale); } /** * Set the type of the announcement type. - * - * @param string $name - * @param string $locale */ - public function setName($name, $locale) + public function setName(array|string|null $name, ?string $locale): void { $this->setData('name', $name, $locale); } diff --git a/classes/announcement/AnnouncementTypeDAO.php b/classes/announcement/AnnouncementTypeDAO.php index 5d30f6a694f..ade57ad54be 100644 --- a/classes/announcement/AnnouncementTypeDAO.php +++ b/classes/announcement/AnnouncementTypeDAO.php @@ -19,40 +19,30 @@ namespace PKP\announcement; use APP\facades\Repo; +use Generator; +use Illuminate\Database\Query\Builder; +use Illuminate\Support\Facades\DB; class AnnouncementTypeDAO extends \PKP\db\DAO { /** * Generate a new data object. - * - * @return AnnouncementType */ - public function newDataObject() + public function newDataObject(): AnnouncementType { return new AnnouncementType(); } /** * Retrieve an announcement type by announcement type ID. - * - * @param ?int $typeId Announcement type ID - * @param ?int $contextId Optional context ID - * - * @return AnnouncementType */ - public function getById($typeId, $contextId = null) + public function getById(int $typeId, ?int $contextId = null): ?AnnouncementType { - $params = [(int) $typeId]; - if ($contextId !== null) { - $params[] = (int) $contextId; - } - $result = $this->retrieve( - 'SELECT * FROM announcement_types WHERE type_id = ?' . - ($contextId !== null ? ' AND context_id = ?' : ''), - $params - ); - $row = $result->current(); - return $row ? $this->_fromRow((array) $row) : null; + $announcement = DB::table('announcement_types') + ->where('type_id', '=', $typeId) + ->when($contextId !== null, fn (Builder $q) => $q->where(DB::raw('COALESCE(context_id, 0)'), '=', $contextId)) + ->first(); + return $announcement ? $this->_fromRow((array) $announcement) : null; } /** @@ -60,19 +50,15 @@ public function getById($typeId, $contextId = null) * * @return array */ - public function getLocaleFieldNames() + public function getLocaleFieldNames(): array { return ['name']; } /** * Internal function to return an AnnouncementType object from a row. - * - * @param array $row - * - * @return AnnouncementType */ - public function _fromRow($row) + public function _fromRow(array $row): AnnouncementType { $announcementType = $this->newDataObject(); $announcementType->setId($row['type_id']); @@ -84,10 +70,8 @@ public function _fromRow($row) /** * Update the localized settings for this object - * - * @param AnnouncementType $announcementType */ - public function updateLocaleFields($announcementType) + public function updateLocaleFields(AnnouncementType $announcementType): void { $this->updateDataObjectSettings( 'announcement_type_settings', @@ -98,61 +82,34 @@ public function updateLocaleFields($announcementType) /** * Insert a new AnnouncementType. - * - * @param AnnouncementType $announcementType - * - * @return int */ - public function insertObject($announcementType) + public function insertObject(AnnouncementType $announcementType): int { - $this->update( - sprintf('INSERT INTO announcement_types - (context_id) - VALUES - (?)'), - [ - $announcementType->getContextId() - ? (int) $announcementType->getContextId() - : null - ] - ); - $announcementType->setId($this->getInsertId()); + $id = DB::table('announcement_types')->insertGetId(['context_id' => $announcementType->getContextId()], 'type_id'); + $announcementType->setId($id); $this->updateLocaleFields($announcementType); - return $announcementType->getId(); + return $id; } /** * Update an existing announcement type. - * - * @param AnnouncementType $announcementType - * - * @return bool */ - public function updateObject($announcementType) + public function updateObject(AnnouncementType $announcementType): bool { - $returner = $this->update( - 'UPDATE announcement_types - SET context_id = ? - WHERE type_id = ?', - [ - $announcementType->getContextId(), - (int) $announcementType->getId() - ] - ); + $affected = DB::table('announcement_types') + ->where('type_id', '=', $announcementType->getId()) + ->update(['context_id' => $announcementType->getContextId()]); $this->updateLocaleFields($announcementType); - return $returner; + return $affected > 0; } /** - * Delete an announcement type. Note that all announcements with this type are also - * deleted. - * - * @param AnnouncementType $announcementType + * Delete an announcement type. Note that all announcements with this type are also deleted. */ - public function deleteObject($announcementType) + public function deleteObject(AnnouncementType $announcementType): void { - return $this->deleteById($announcementType->getId()); + $this->deleteById($announcementType->getId()); } /** @@ -169,10 +126,8 @@ public function deleteById(int $typeId): void /** * Delete announcement types by context ID. - * - * @param int $contextId */ - public function deleteByContextId($contextId) + public function deleteByContextId(?int $contextId): void { foreach ($this->getByContextId($contextId) as $type) { $this->deleteObject($type); @@ -182,21 +137,15 @@ public function deleteByContextId($contextId) /** * Retrieve an array of announcement types matching a particular context ID. * - * @return \Generator Matching AnnouncementTypes + * @return Generator Matching AnnouncementTypes */ - public function getByContextId(?int $contextId) + public function getByContextId(?int $contextId): Generator { - if ($contextId) { - $result = $this->retrieve( - 'SELECT * FROM announcement_types WHERE context_id = ? ORDER BY type_id', - [$contextId] - ); - } else { - $result = $this->retrieve( - 'SELECT * FROM announcement_types WHERE context_id IS NULL ORDER BY type_id' - ); - } - foreach ($result as $row) { + $rows = DB::table('announcement_types') + ->where(DB::raw('COALESCE(context_id, 0)'), '=', (int) $contextId) + ->orderBy('type_id') + ->get(); + foreach ($rows as $row) { yield $row->type_id => $this->_fromRow((array) $row); } } diff --git a/classes/announcement/Collector.php b/classes/announcement/Collector.php index f0ebb64445e..895d477e9b1 100644 --- a/classes/announcement/Collector.php +++ b/classes/announcement/Collector.php @@ -20,6 +20,7 @@ use Illuminate\Support\LazyCollection; use PKP\core\Core; use PKP\core\interfaces\CollectorInterface; +use PKP\core\PKPString; use PKP\plugins\Hook; /** @@ -34,7 +35,6 @@ class Collector implements CollectorInterface public const SITE_ONLY = 'site'; public const SITE_AND_CONTEXTS = 'all'; - public DAO $dao; public ?array $contextIds = null; public ?string $isActive = null; public ?string $searchPhrase = null; @@ -45,9 +45,13 @@ class Collector implements CollectorInterface public string $orderBy = self::ORDERBY_DATE_POSTED; public string $orderDirection = self::ORDER_DIR_DESC; - public function __construct(DAO $dao) + /** + * Constructor + * + * @param DAO + */ + public function __construct(public DAO $dao) { - $this->dao = $dao; } /** @copydoc DAO::getCount() */ @@ -79,7 +83,7 @@ public function getMany(): LazyCollection /** * Filter announcements by one or more contexts */ - public function filterByContextIds(?array $contextIds): self + public function filterByContextIds(?array $contextIds): static { $this->contextIds = $contextIds; return $this; @@ -91,7 +95,7 @@ public function filterByContextIds(?array $contextIds): self * @param string $date Optionally filter announcements by those * not expired until $date (YYYY-MM-DD). */ - public function filterByActive(string $date = ''): self + public function filterByActive(?string $date = ''): static { $this->isActive = empty($date) ? Core::getCurrentDate() @@ -102,7 +106,7 @@ public function filterByActive(string $date = ''): self /** * Filter announcements by one or more announcement types */ - public function filterByTypeIds(array $typeIds): self + public function filterByTypeIds(?array $typeIds): static { $this->typeIds = $typeIds; return $this; @@ -111,7 +115,7 @@ public function filterByTypeIds(array $typeIds): self /** * Include site-level announcements in the results */ - public function withSiteAnnouncements(?string $includeMethod = self::SITE_AND_CONTEXTS): self + public function withSiteAnnouncements(?string $includeMethod = self::SITE_AND_CONTEXTS): static { $this->includeSite = $includeMethod; return $this; @@ -120,7 +124,7 @@ public function withSiteAnnouncements(?string $includeMethod = self::SITE_AND_CO /** * Filter announcements by those matching a search query */ - public function searchPhrase(?string $phrase): self + public function searchPhrase(?string $phrase): static { $this->searchPhrase = $phrase; return $this; @@ -129,7 +133,7 @@ public function searchPhrase(?string $phrase): self /** * Limit the number of objects retrieved */ - public function limit(?int $count): self + public function limit(?int $count): static { $this->count = $count; return $this; @@ -139,7 +143,7 @@ public function limit(?int $count): self * Offset the number of objects retrieved, for example to * retrieve the second page of contents */ - public function offset(?int $offset): self + public function offset(?int $offset): static { $this->offset = $offset; return $this; @@ -153,7 +157,7 @@ public function offset(?int $offset): self * @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 + public function orderBy(?string $sorter, string $direction = self::ORDER_DIR_DESC): static { $this->orderBy = $sorter; $this->orderDirection = $direction; @@ -165,53 +169,39 @@ public function orderBy(?string $sorter, string $direction = self::ORDER_DIR_DES */ 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'); + $qb = DB::table($this->dao->table, 'a') + ->select(['a.*']) + ->where('a.assoc_type', Application::get()->getContextAssocType()); + + $includeSite = in_array($this->includeSite, [static::SITE_AND_CONTEXTS, static::SITE_ONLY]); + if (isset($this->contextIds) || $includeSite) { + $qb->whereIn(DB::raw('COALESCE(a.assoc_id, 0)'), array_merge($this->contextIds ?? [], $includeSite ? [0] : [])); } 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->when( + $this->isActive, + fn (Builder $qb) => $qb->where(fn (Builder $qb) => $qb->where('a.date_expire', '>', $this->isActive)->orWhereNull('a.date_expire')) + ); + + $searchPhrase = trim($this->searchPhrase ?? ''); + if (strlen($searchPhrase)) { + $words = PKPString::regexp_split('/\s+/', $searchPhrase); + $qb->whereIn('a.announcement_id', function (Builder $q) use ($words) { + $q->select('announcement_id')->from($this->dao->settingsTable); + foreach ($words as $word) { + $word = strtolower(addcslashes($word, '%_')); + $q->where( + fn (Builder $q) => $q + ->where(fn (Builder $q) => $q->where('setting_name', 'title')->where(DB::raw('lower(setting_value)'), 'LIKE', "%{$word}%")) + ->orWhere(fn (Builder $q) => $q->where('setting_name', 'descriptionShort')->where(DB::raw('lower(setting_value)'), 'LIKE', "%{$word}%")) + ->orWhere(fn (Builder $q) => $q->where('setting_name', 'description')->where(DB::raw('lower(setting_value)'), 'LIKE', "%{$word}%")) + ); + } + }); } $qb->orderByDesc('a.date_posted'); @@ -224,14 +214,9 @@ public function getQueryBuilder(): Builder $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); - } - } + $qb->orderBy('a.' . $this->orderBy, $this->orderDirection); + // Add a secondary sort by id to catch cases where two announcements share the same date + $qb->orderBy('a.announcement_id', $this->orderDirection); Hook::call('Announcement::Collector', [&$qb, $this]); diff --git a/classes/announcement/Repository.php b/classes/announcement/Repository.php index 557d83e211d..5ef60252b77 100644 --- a/classes/announcement/Repository.php +++ b/classes/announcement/Repository.php @@ -29,24 +29,17 @@ 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; - /** @var Request $request */ - protected $request; - - /** @var PKPSchemaService $schemaService */ - protected $schemaService; - - - public function __construct(DAO $dao, Request $request, PKPSchemaService $schemaService) + /** + * Constructor + * + * @param DAO $dao + * @param PKPSchemaService $schemaService + */ + public function __construct(public DAO $dao, protected Request $request, protected PKPSchemaService $schemaService) { - $this->dao = $dao; - $this->request = $request; - $this->schemaService = $schemaService; } /** @copydoc DAO::newDataObject() */ @@ -153,7 +146,7 @@ public function add(Announcement $announcement): int * Deletes the old image if it has been removed, or a new image has * been uploaded. */ - public function edit(Announcement $announcement, array $params) + public function edit(Announcement $announcement, array $params): void { $newAnnouncement = clone $announcement; $newAnnouncement->setAllData(array_merge($newAnnouncement->_data, $params)); @@ -175,7 +168,7 @@ public function edit(Announcement $announcement, array $params) } /** @copydoc DAO::delete() */ - public function delete(Announcement $announcement) + public function delete(Announcement $announcement): void { Hook::call('Announcement::delete::before', [$announcement]); @@ -191,7 +184,7 @@ public function delete(Announcement $announcement) /** * Delete a collection of announcements */ - public function deleteMany(Collector $collector) + public function deleteMany(Collector $collector): void { foreach ($collector->getMany() as $announcement) { $this->delete($announcement); @@ -231,7 +224,7 @@ protected function handleImageUpload(Announcement $announcement): void $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)) { + if (!$this->isValidImage($temporaryFile)) { throw new StoreTemporaryFileException($temporaryFile, $filePath, $user, $announcement); } if ($this->storeTemporaryFile($temporaryFile, $filePath, $user->getId(), $announcement)) { @@ -249,7 +242,7 @@ protected function handleImageUpload(Announcement $announcement): void /** * 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 + * @param string $newPath The new filename with the path relative to the public files directory * @return bool Whether or not the operation was successful */ protected function storeTemporaryFile(TemporaryFile $temporaryFile, string $newPath, int $userId, Announcement $announcement): bool diff --git a/classes/announcement/maps/Schema.php b/classes/announcement/maps/Schema.php index d28f31d6d44..4f13cd00ece 100644 --- a/classes/announcement/maps/Schema.php +++ b/classes/announcement/maps/Schema.php @@ -14,7 +14,6 @@ namespace PKP\announcement\maps; use APP\core\Application; -use APP\core\Request; use Illuminate\Support\Enumerable; use PKP\announcement\Announcement; use PKP\core\PKPApplication; diff --git a/classes/handler/PKPHandler.php b/classes/handler/PKPHandler.php index 23cabdea72c..a2093665abc 100644 --- a/classes/handler/PKPHandler.php +++ b/classes/handler/PKPHandler.php @@ -550,7 +550,7 @@ public function setupTemplate($request) public static function hashPageContext($request, $contextData = []) { return md5( - implode(',', $request->getRouter()->getRequestedContextPath($request)) . ',' . + $request->getRouter()->getRequestedContextPath($request) . ',' . $request->getRequestedPage() . ',' . $request->getRequestedOp() . ',' . serialize($contextData) diff --git a/controllers/grid/announcements/AnnouncementTypeGridHandler.php b/controllers/grid/announcements/AnnouncementTypeGridHandler.php index 0d5c1df9f31..dcc02164963 100644 --- a/controllers/grid/announcements/AnnouncementTypeGridHandler.php +++ b/controllers/grid/announcements/AnnouncementTypeGridHandler.php @@ -16,7 +16,6 @@ namespace PKP\controllers\grid\announcements; -use APP\core\Application; use APP\notification\NotificationManager; use PKP\announcement\AnnouncementTypeDAO; use PKP\controllers\grid\announcements\form\AnnouncementTypeForm; @@ -30,7 +29,6 @@ use PKP\notification\PKPNotification; use PKP\security\authorization\ContextAccessPolicy; use PKP\security\authorization\PKPSiteAccessPolicy; -use PKP\security\authorization\UserRolesRequiredPolicy; use PKP\security\Role; class AnnouncementTypeGridHandler extends GridHandler @@ -52,21 +50,16 @@ public function __construct() ); } - // - // Overridden template methods - // /** * @copydoc GridHandler::authorize() */ public function authorize($request, &$args, $roleAssignments) { $context = $request->getContext(); - - if ($context) { - $this->addPolicy(new ContextAccessPolicy($request, $roleAssignments)); - } else { - $this->addPolicy(new PKPSiteAccessPolicy($request, null, $roleAssignments)); - } + $policy = $context + ? new ContextAccessPolicy($request, $roleAssignments) + : new PKPSiteAccessPolicy($request, null, $roleAssignments); + $this->addPolicy($policy); $announcementTypeId = $request->getUserVar('announcementTypeId'); if ($announcementTypeId) { @@ -143,31 +136,18 @@ protected function getRowInstance() return new AnnouncementTypeGridRow(); } - // - // Public grid actions. - // /** * Display form to add announcement type. - * - * @param array $args - * @param PKPRequest $request - * - * @return JSONMessage */ - public function addAnnouncementType($args, $request) + public function addAnnouncementType(array $args, PKPRequest $request): JSONMessage { return $this->editAnnouncementType($args, $request); } /** * Display form to edit an announcement type. - * - * @param array $args - * @param PKPRequest $request - * - * @return JSONMessage JSON object */ - public function editAnnouncementType($args, $request) + public function editAnnouncementType(array $args, PKPRequest $request): JSONMessage { $announcementTypeId = (int)$request->getUserVar('announcementTypeId'); $announcementTypeForm = new AnnouncementTypeForm($request->getContext()?->getId(), $announcementTypeId); @@ -178,13 +158,8 @@ public function editAnnouncementType($args, $request) /** * Save an edited/inserted announcement type. - * - * @param array $args - * @param PKPRequest $request - * - * @return JSONMessage JSON object */ - public function updateAnnouncementType($args, $request) + public function updateAnnouncementType(array $args, PKPRequest $request): JSONMessage { // Identify the announcement type id. $announcementTypeId = $request->getUserVar('announcementTypeId'); @@ -218,13 +193,8 @@ public function updateAnnouncementType($args, $request) /** * Delete an announcement type. - * - * @param array $args - * @param PKPRequest $request - * - * @return JSONMessage JSON object */ - public function deleteAnnouncementType($args, $request) + public function deleteAnnouncementType(array $args, PKPRequest $request): JSONMessage { $announcementTypeId = (int) $request->getUserVar('announcementTypeId'); diff --git a/pages/announcement/AnnouncementHandler.php b/pages/announcement/AnnouncementHandler.php index fe3f850ed5d..6ed02b425b6 100644 --- a/pages/announcement/AnnouncementHandler.php +++ b/pages/announcement/AnnouncementHandler.php @@ -24,7 +24,6 @@ use PKP\announcement\Collector; use PKP\config\Config; use PKP\core\PKPRequest; -use PKP\security\authorization\ContextRequiredPolicy; class AnnouncementHandler extends Handler { diff --git a/pages/announcement/index.php b/pages/announcement/index.php index 3f46f958af8..fe208280749 100644 --- a/pages/announcement/index.php +++ b/pages/announcement/index.php @@ -20,6 +20,6 @@ switch ($op) { case 'index': case 'view': - define('HANDLER_CLASS', 'PKP\pages\announcement\AnnouncementHandler'); + define('HANDLER_CLASS', \PKP\pages\announcement\AnnouncementHandler::class); break; }