Skip to content

Commit

Permalink
pkp#9253 Use null to associate announcements with the site
Browse files Browse the repository at this point in the history
  • Loading branch information
NateWr committed Aug 28, 2023
1 parent 90dba7b commit 9b5bb0a
Show file tree
Hide file tree
Showing 12 changed files with 105 additions and 112 deletions.
30 changes: 12 additions & 18 deletions api/v1/announcements/PKPAnnouncementHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
}

Expand Down Expand Up @@ -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]);

Expand All @@ -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();
Expand Down Expand Up @@ -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']);

Expand All @@ -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();

Expand Down Expand Up @@ -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');
}

Expand All @@ -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
Expand Down
30 changes: 19 additions & 11 deletions classes/announcement/AnnouncementTypeDAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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);
Expand All @@ -131,7 +135,7 @@ public function updateObject($announcementType)
SET context_id = ?
WHERE type_id = ?',
[
(int) $announcementType->getContextId(),
$announcementType->getContextId(),
(int) $announcementType->getId()
]
);
Expand Down Expand Up @@ -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);
}
Expand Down
20 changes: 19 additions & 1 deletion classes/announcement/Collector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -103,6 +106,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
*/
Expand Down Expand Up @@ -154,9 +166,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)) {
Expand Down
11 changes: 3 additions & 8 deletions classes/components/forms/announcement/PKPAnnouncementForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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;
Expand Down Expand Up @@ -98,11 +97,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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
28 changes: 9 additions & 19 deletions classes/migration/upgrade/v3_5_0/I9253_SiteAnnouncements.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
});
}

Expand All @@ -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();
});
}
}
33 changes: 9 additions & 24 deletions controllers/grid/announcements/AnnouncementTypeGridHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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(
Expand Down Expand Up @@ -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());
}

/**
Expand Down Expand Up @@ -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));
Expand All @@ -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()) {
Expand Down Expand Up @@ -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);

Expand All @@ -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;
}
}
Loading

0 comments on commit 9b5bb0a

Please sign in to comment.