#9253 Use null to associate announcements with the site

pkp · Nov 8, 2023 · b6771ef · b6771ef
1 parent f5e1ecf
commit b6771ef
Show file tree

Hide file tree

Showing 12 changed files with 105 additions and 112 deletions.
diff --git a/api/v1/announcements/PKPAnnouncementHandler.php b/api/v1/announcements/PKPAnnouncementHandler.php
@@ -22,6 +22,7 @@
 use APP\facades\Repo;
 use Exception;
 use Illuminate\Support\Facades\Bus;
+use PKP\announcement\Collector;
 use PKP\config\Config;
 use PKP\context\Context;
 use PKP\db\DAORegistry;
@@ -133,7 +134,7 @@ public function get($slimRequest, $response, $args)
         }
 
         // The assocId in announcements should always point to the contextId
-        if ($announcement->getData('assocId') !== $this->getRequest()->getContext()->getId()) {
+        if ($announcement->getData('assocId') !== $this->getRequest()->getContext()?->getId()) {
             return $response->withStatus(404)->withJsonError('api.announcements.400.contextsNotMatched');
         }
 
@@ -174,7 +175,12 @@ public function getMany($slimRequest, $response, $args)
             }
         }
 
-        $collector->filterByContextIds([$this->getContextId()]);
+        if ($this->getRequest()->getContext()) {
+            $collector->filterByContextIds([$this->getRequest()->getContext()->getId()]);
+        } else {
+            $collector->withSiteAnnouncements(Collector::SITE_ONLY);
+        }
+
 
         Hook::call('API::submissions::params', [$collector, $slimRequest]);
 
@@ -199,11 +205,10 @@ public function add($slimRequest, $response, $args)
     {
         $request = $this->getRequest();
         $context = $request->getContext();
-        $contextId = $this->getContextId();
 
         $params = $this->convertStringsToSchema(PKPSchemaService::SCHEMA_ANNOUNCEMENT, $slimRequest->getParsedBody());
         $params['assocType'] = Application::get()->getContextAssocType();
-        $params['assocId'] = $contextId;
+        $params['assocId'] = $context?->getId();
 
         $primaryLocale = $context ? $context->getPrimaryLocale() : $request->getSite()->getPrimaryLocale();
         $allowedLocales = $context ? $context->getSupportedFormLocales() : $request->getSite()->getSupportedLocales();
@@ -236,6 +241,7 @@ public function add($slimRequest, $response, $args)
     public function edit($slimRequest, $response, $args)
     {
         $request = $this->getRequest();
+        $context = $request->getContext();
 
         $announcement = Repo::announcement()->get((int) $args['announcementId']);
 
@@ -248,15 +254,14 @@ public function edit($slimRequest, $response, $args)
         }
 
         // Don't allow to edit an announcement from one context from a different context's endpoint
-        if ($this->getContextId() !== $announcement->getData('assocId')) {
+        if ($context?->getId() !== $announcement->getData('assocId')) {
             return $response->withStatus(403)->withJsonError('api.announcements.400.contextsNotMatched');
         }
 
         $params = $this->convertStringsToSchema(PKPSchemaService::SCHEMA_ANNOUNCEMENT, $slimRequest->getParsedBody());
         $params['id'] = $announcement->getId();
         $params['typeId'] ??= null;
 
-        $context = $request->getContext();
         $primaryLocale = $context ? $context->getPrimaryLocale() : $request->getSite()->getPrimaryLocale();
         $allowedLocales = $context ? $context->getSupportedFormLocales() : $request->getSite()->getSupportedLocales();
 
@@ -296,7 +301,7 @@ public function delete($slimRequest, $response, $args)
         }
 
         // Don't allow to delete an announcement from one context from a different context's endpoint
-        if ($this->getContextId() !== $announcement->getData('assocId')) {
+        if ($request->getContext()?->getId() !== $announcement->getData('assocId')) {
             return $response->withStatus(403)->withJsonError('api.announcements.400.contextsNotMatched');
         }
 
@@ -307,17 +312,6 @@ public function delete($slimRequest, $response, $args)
         return $response->withJson($announcementProps, 200);
     }
 
-    /**
-     * Get the context id or site-wide context id of the current request
-     */
-    protected function getContextId(): int
-    {
-        $context = $this->getRequest()->getContext();
-        return $context
-            ? $context->getId()
-            : Application::CONTEXT_ID_NONE;
-    }
-
     /**
      * Modify the role assignments so that only
      * site admins have access

diff --git a/classes/announcement/AnnouncementTypeDAO.php b/classes/announcement/AnnouncementTypeDAO.php
@@ -35,8 +35,8 @@ public function newDataObject()
     /**
      * Retrieve an announcement type by announcement type ID.
      *
-     * @param int $typeId Announcement type ID
-     * @param int $contextId Optional context ID
+     * @param ?int $typeId Announcement type ID
+     * @param ?int $contextId Optional context ID
      *
      * @return AnnouncementType
      */
@@ -110,7 +110,11 @@ public function insertObject($announcementType)
 				(context_id)
 				VALUES
 				(?)'),
-            [(int) $announcementType->getContextId()]
+            [
+                $announcementType->getContextId()
+                    ? (int) $announcementType->getContextId()
+                    : null
+            ]
         );
         $announcementType->setId($this->getInsertId());
         $this->updateLocaleFields($announcementType);
@@ -131,7 +135,7 @@ public function updateObject($announcementType)
                         SET	context_id = ?
 			WHERE	type_id = ?',
             [
-                (int) $announcementType->getContextId(),
+                $announcementType->getContextId(),
                 (int) $announcementType->getId()
             ]
         );
@@ -181,16 +185,20 @@ public function deleteByContextId($contextId)
     /**
      * Retrieve an array of announcement types matching a particular context ID.
      *
-     * @param int $contextId
-     *
      * @return \Generator<int,AnnouncementType> Matching AnnouncementTypes
      */
-    public function getByContextId($contextId)
+    public function getByContextId(?int $contextId)
     {
-        $result = $this->retrieve(
-            'SELECT * FROM announcement_types WHERE context_id = ? ORDER BY type_id',
-            [(int) $contextId]
-        );
+        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) {
             yield $row->type_id => $this->_fromRow((array) $row);
         }

diff --git a/classes/announcement/Collector.php b/classes/announcement/Collector.php
@@ -31,12 +31,15 @@ class Collector implements CollectorInterface
     public const ORDERBY_DATE_EXPIRE = 'date_expire';
     public const ORDER_DIR_ASC = 'ASC';
     public const ORDER_DIR_DESC = 'DESC';
+    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;
     public ?array $typeIds = null;
+    public ?string $includeSite = null;
     public ?int $count = null;
     public ?int $offset = null;
     public string $orderBy = self::ORDERBY_DATE_POSTED;
@@ -105,6 +108,15 @@ public function filterByTypeIds(array $typeIds): self
         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
      */
@@ -156,9 +168,15 @@ public function getQueryBuilder(): Builder
         $qb = DB::table($this->dao->table . ' as a')
             ->select(['a.*']);
 
-        if (isset($this->contextIds)) {
+        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)) {

diff --git a/classes/components/forms/announcement/PKPAnnouncementForm.php b/classes/components/forms/announcement/PKPAnnouncementForm.php
@@ -23,7 +23,6 @@
 use PKP\components\forms\FormComponent;
 use PKP\context\Context;
 use PKP\db\DAORegistry;
-use PKP\site\Site;
 
 define('FORM_ANNOUNCEMENT', 'announcement');
 
@@ -35,15 +34,15 @@ class PKPAnnouncementForm extends FormComponent
     /** @copydoc FormComponent::$method */
     public $method = 'POST';
 
-    public Context|Site $context;
+    public ?Context $context;
 
     /**
      * Constructor
      *
      * @param string $action URL to submit the form to
      * @param array $locales Supported locales
      */
-    public function __construct($action, $locales, Context|Site $context)
+    public function __construct($action, $locales, ?Context $context = null)
     {
         $this->action = $action;
         $this->locales = $locales;
@@ -99,11 +98,7 @@ protected function getAnnouncementTypeOptions(): array
         /** @var AnnouncementTypeDAO */
         $announcementTypeDao = DAORegistry::getDAO('AnnouncementTypeDAO');
 
-        $announcementTypes = $announcementTypeDao->getByContextId(
-            is_a($this->context, Context::class)
-                ? $this->context->getId()
-                : Application::CONTEXT_ID_NONE
-        );
+        $announcementTypes = $announcementTypeDao->getByContextId($this->context?->getId());
 
         $announcementTypeOptions = [];
         foreach ($announcementTypes as $announcementType) {

diff --git a/classes/components/listPanels/PKPAnnouncementsListPanel.php b/classes/components/listPanels/PKPAnnouncementsListPanel.php
@@ -61,7 +61,7 @@ protected function getUrlBase(): string
         return $request->getDispatcher()->url(
             $request,
             Application::ROUTE_PAGE,
-            is_a($this->form->context, Context::class)
+            $this->form->context
                 ? $request->getContext()->getPath()
                 : 'index',
             'announcement',

diff --git a/classes/migration/upgrade/v3_5_0/I9253_SiteAnnouncements.php b/classes/migration/upgrade/v3_5_0/I9253_SiteAnnouncements.php
@@ -12,7 +12,6 @@
 
 namespace PKP\migration\upgrade\v3_5_0;
 
-use APP\core\Application;
 use Illuminate\Database\Schema\Blueprint;
 use Illuminate\Support\Facades\Schema;
 use PKP\migration\Migration;
@@ -24,8 +23,11 @@ class I9253_SiteAnnouncements extends Migration
      */
     public function up(): void
     {
+        Schema::table('announcements', function (Blueprint $table) {
+            $table->bigInteger('assoc_id')->nullable()->change();
+        });
         Schema::table('announcement_types', function (Blueprint $table) {
-            $table->dropForeign(['context_id']);
+            $table->bigInteger('context_id')->nullable()->change();
         });
     }
 
@@ -34,23 +36,11 @@ public function up(): void
      */
     public function down(): void
     {
-        $app = Application::getName();
-
-        $contextIdColumn = 'journal_id';
-        $contextTable = 'journals';
-        if ($app === 'omp') {
-            $contextIdColumn = 'press_id';
-            $contextTable = 'presses';
-        } elseif ($app === 'ops') {
-            $contextIdColumn = 'server_id';
-            $contextTable = 'servers';
-        }
-
-        Schema::table('announcement_types', function (Blueprint $table) use ($contextIdColumn, $contextTable) {
-            $table
-                ->foreign('context_id')
-                ->references($contextIdColumn)
-                ->on($contextTable);
+        Schema::table('announcements', function (Blueprint $table) {
+            $table->bigInteger('assoc_id')->nullable(false)->change();
+        });
+        Schema::table('announcement_types', function (Blueprint $table) {
+            $table->bigInteger('context_id')->nullable(false)->change();
         });
     }
 }
diff --git a/controllers/grid/announcements/AnnouncementTypeGridHandler.php b/controllers/grid/announcements/AnnouncementTypeGridHandler.php
@@ -60,20 +60,20 @@ public function __construct()
      */
     public function authorize($request, &$args, $roleAssignments)
     {
-        $contextId = $this->getContextId();
+        $context = $request->getContext();
 
-        if ($contextId === Application::CONTEXT_ID_NONE) {
-            $this->addPolicy(new PKPSiteAccessPolicy($request, null, $roleAssignments));
-        } else {
+        if ($context) {
             $this->addPolicy(new ContextAccessPolicy($request, $roleAssignments));
+        } else {
+            $this->addPolicy(new PKPSiteAccessPolicy($request, null, $roleAssignments));
         }
 
         $announcementTypeId = $request->getUserVar('announcementTypeId');
         if ($announcementTypeId) {
             // Ensure announcement type is valid and for this context
             $announcementTypeDao = DAORegistry::getDAO('AnnouncementTypeDAO'); /** @var AnnouncementTypeDAO $announcementTypeDao */
             $announcementType = $announcementTypeDao->getById($announcementTypeId);
-            if (!$announcementType || $announcementType->getContextId() != $contextId) {
+            if (!$announcementType || $announcementType->getContextId() != $context?->getId()) {
                 return false;
             }
         }
@@ -95,8 +95,6 @@ public function initialize($request, $args = null)
         // Set the no items row text
         $this->setEmptyRowText('manager.announcementTypes.noneCreated');
 
-        $context = $request->getContext();
-
         // Columns
         $announcementTypeCellProvider = new AnnouncementTypeGridCellProvider();
         $this->addColumn(
@@ -134,7 +132,7 @@ public function initialize($request, $args = null)
     protected function loadData($request, $filter)
     {
         $announcementTypeDao = DAORegistry::getDAO('AnnouncementTypeDAO'); /** @var AnnouncementTypeDAO $announcementTypeDao */
-        return $announcementTypeDao->getByContextId($this->getContextId());
+        return $announcementTypeDao->getByContextId($request->getContext()?->getId());
     }
 
     /**
@@ -172,7 +170,7 @@ public function addAnnouncementType($args, $request)
     public function editAnnouncementType($args, $request)
     {
         $announcementTypeId = (int)$request->getUserVar('announcementTypeId');
-        $announcementTypeForm = new AnnouncementTypeForm($this->getContextId(), $announcementTypeId);
+        $announcementTypeForm = new AnnouncementTypeForm($request->getContext()?->getId(), $announcementTypeId);
         $announcementTypeForm->initData();
 
         return new JSONMessage(true, $announcementTypeForm->fetch($request));
@@ -192,7 +190,7 @@ public function updateAnnouncementType($args, $request)
         $announcementTypeId = $request->getUserVar('announcementTypeId');
 
         // Form handling.
-        $announcementTypeForm = new AnnouncementTypeForm($this->getContextId(), $announcementTypeId);
+        $announcementTypeForm = new AnnouncementTypeForm($request->getContext()?->getId(), $announcementTypeId);
         $announcementTypeForm->readInputData();
 
         if ($announcementTypeForm->validate()) {
@@ -231,7 +229,7 @@ public function deleteAnnouncementType($args, $request)
         $announcementTypeId = (int) $request->getUserVar('announcementTypeId');
 
         $announcementTypeDao = DAORegistry::getDAO('AnnouncementTypeDAO'); /** @var AnnouncementTypeDAO $announcementTypeDao */
-        $announcementType = $announcementTypeDao->getById($announcementTypeId, $this->getContextId());
+        $announcementType = $announcementTypeDao->getById($announcementTypeId, $request->getContext()?->getId());
         if ($announcementType && $request->checkCSRF()) {
             $announcementTypeDao->deleteObject($announcementType);
 
@@ -245,17 +243,4 @@ public function deleteAnnouncementType($args, $request)
 
         return new JSONMessage(false);
     }
-
-    /**
-     * Get the id of the request context
-     *
-     * Returns CONTEXT_ID_NONE for site-level requests
-     */
-    protected function getContextId(): int
-    {
-        $request = Application::get()->getRequest();
-        return $request->getContext()
-            ? $request->getContext()->getId()
-            : Application::CONTEXT_ID_NONE;
-    }
 }