diff --git a/core/modules/workspace/src/Annotation/RepositoryHandler.php b/core/modules/workspace/src/Annotation/RepositoryHandler.php index 4ac8eb6..516e690 100644 --- a/core/modules/workspace/src/Annotation/RepositoryHandler.php +++ b/core/modules/workspace/src/Annotation/RepositoryHandler.php @@ -7,6 +7,11 @@ /** * Defines a RepositoryHandler annotation object. * + * @see \Drupal\workspace\RepositoryHandlerInterface + * @see \Drupal\workspace\RepositoryHandlerBase + * @see \Drupal\workspace\RepositoryHandlerManager + * @see plugin_api + * * @Annotation */ class RepositoryHandler extends Plugin { diff --git a/core/modules/workspace/src/Entity/ContentWorkspace.php b/core/modules/workspace/src/Entity/ContentWorkspace.php index cfa1377..20203cd 100644 --- a/core/modules/workspace/src/Entity/ContentWorkspace.php +++ b/core/modules/workspace/src/Entity/ContentWorkspace.php @@ -5,6 +5,7 @@ use Drupal\Core\Entity\ContentEntityBase; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Field\BaseFieldDefinition; +use Drupal\Core\StringTranslation\TranslatableMarkup; /** * Defines the Content workspace entity. @@ -40,28 +41,29 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { $fields = parent::baseFieldDefinitions($entity_type); $fields['workspace'] = BaseFieldDefinition::create('entity_reference') - ->setLabel(t('workspace')) - ->setDescription(t('The workspace of the referenced content.')) + ->setLabel(new TranslatableMarkup('workspace')) + ->setDescription(new TranslatableMarkup('The workspace of the referenced content.')) ->setSetting('target_type', 'workspace') ->setRequired(TRUE) ->setRevisionable(TRUE) ->addConstraint('workspace', []); $fields['content_entity_type_id'] = BaseFieldDefinition::create('string') - ->setLabel(t('Content entity type ID')) - ->setDescription(t('The ID of the content entity type this workspace is for.')) + ->setLabel(new TranslatableMarkup('Content entity type ID')) + ->setDescription(new TranslatableMarkup('The ID of the content entity type this workspace is for.')) + ->setSetting('max_length', EntityTypeInterface::ID_MAX_LENGTH) ->setRequired(TRUE) ->setRevisionable(TRUE); $fields['content_entity_id'] = BaseFieldDefinition::create('integer') - ->setLabel(t('Content entity ID')) - ->setDescription(t('The ID of the content entity this workspace is for.')) + ->setLabel(new TranslatableMarkup('Content entity ID')) + ->setDescription(new TranslatableMarkup('The ID of the content entity this workspace is for.')) ->setRequired(TRUE) ->setRevisionable(TRUE); $fields['content_entity_revision_id'] = BaseFieldDefinition::create('integer') - ->setLabel(t('Content entity revision ID')) - ->setDescription(t('The revision ID of the content entity this workspace is for.')) + ->setLabel(new TranslatableMarkup('Content entity revision ID')) + ->setDescription(new TranslatableMarkup('The revision ID of the content entity this workspace is for.')) ->setRequired(TRUE) ->setRevisionable(TRUE); diff --git a/core/modules/workspace/src/Entity/ReplicationLog.php b/core/modules/workspace/src/Entity/ReplicationLog.php index ba640eb..5853820 100644 --- a/core/modules/workspace/src/Entity/ReplicationLog.php +++ b/core/modules/workspace/src/Entity/ReplicationLog.php @@ -44,7 +44,7 @@ public function getHistory() { /** * {@inheritdoc} */ - public function addHistory(array $history) { + public function appendHistory(array $history) { // We need to wrap the passed-in argument in another array in order for it // to be set as the first item (i.e. delta = 0) of the field item list. $histories = array_merge([$history], $this->getHistory()); @@ -111,24 +111,20 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { $fields['history'] = BaseFieldDefinition::create('replication_history') ->setLabel(new TranslatableMarkup('Replication log history')) ->setDescription(new TranslatableMarkup('The version id of the test entity.')) - ->setReadOnly(TRUE) ->setCardinality(FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED); - $fields['session_id'] = BaseFieldDefinition::create('string') + $fields['session_id'] = BaseFieldDefinition::create('uuid') ->setLabel(new TranslatableMarkup('Replication session ID')) - ->setDescription(new TranslatableMarkup('The unique session ID of the last replication. Shortcut to the session_id in the last history item.')) - ->setReadOnly(TRUE); + ->setDescription(new TranslatableMarkup('The unique session ID of the last replication. Shortcut to the session_id in the last history item.')); $fields['source_last_sequence'] = BaseFieldDefinition::create('string') ->setLabel(new TranslatableMarkup('Last processed checkpoint')) - ->setDescription(new TranslatableMarkup('The last processed checkpoint. Shortcut to the source_last_sequence in the last history item.')) - ->setReadOnly(TRUE); + ->setDescription(new TranslatableMarkup('The last processed checkpoint. Shortcut to the source_last_sequence in the last history item.')); $fields['status'] = BaseFieldDefinition::create('boolean') ->setLabel(new TranslatableMarkup('Status')) ->setDescription(new TranslatableMarkup('Replication status')) - ->setDefaultValue(TRUE) - ->setReadOnly(TRUE); + ->setDefaultValue(TRUE); return $fields; } diff --git a/core/modules/workspace/src/Entity/Workspace.php b/core/modules/workspace/src/Entity/Workspace.php index 7e1b914..f8bac92 100644 --- a/core/modules/workspace/src/Entity/Workspace.php +++ b/core/modules/workspace/src/Entity/Workspace.php @@ -72,14 +72,14 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { $fields = parent::baseFieldDefinitions($entity_type); $fields['id'] = BaseFieldDefinition::create('string') - ->setLabel(new TranslatableMarkup('Workaspace ID')) + ->setLabel(new TranslatableMarkup('Workspace ID')) ->setDescription(new TranslatableMarkup('The workspace ID.')) ->setSetting('max_length', 128) ->setRequired(TRUE) - ->addPropertyConstraints('value', ['Regex' => ['pattern' => '/^[\da-z_$()+-\/]*$/']]); + ->addPropertyConstraints('value', ['Regex' => ['pattern' => '/^[a-z0-9_]*$/']]); $fields['label'] = BaseFieldDefinition::create('string') - ->setLabel(new TranslatableMarkup('Workaspace name')) + ->setLabel(new TranslatableMarkup('Workspace name')) ->setDescription(new TranslatableMarkup('The workspace name.')) ->setRevisionable(TRUE) ->setSetting('max_length', 128) diff --git a/core/modules/workspace/src/EntityAccess.php b/core/modules/workspace/src/EntityAccess.php index 2ba84f2..d0d05bf 100644 --- a/core/modules/workspace/src/EntityAccess.php +++ b/core/modules/workspace/src/EntityAccess.php @@ -124,7 +124,7 @@ protected function bypassAccessResult(AccountInterface $account) { return AccessResult::allowedIfHasPermission($account, 'bypass entity access workspace ' . $active_workspace->id()) ->orIf( - AccessResult::allowedIf($active_workspace->getOwnerId() == $account->id()) + AccessResult::allowedIf($active_workspace->getOwnerId() == $account->id())->cachePerUser()->addCacheableDependency($active_workspace) ->andIf(AccessResult::allowedIfHasPermission($account, 'bypass entity access own workspace')) ); } diff --git a/core/modules/workspace/src/Form/WorkspaceDeployForm.php b/core/modules/workspace/src/Form/WorkspaceDeployForm.php index 34364e7..d17a99d 100644 --- a/core/modules/workspace/src/Form/WorkspaceDeployForm.php +++ b/core/modules/workspace/src/Form/WorkspaceDeployForm.php @@ -9,7 +9,7 @@ use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Messenger\MessengerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; -use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; +use Symfony\Component\HttpKernel\Exception\HttpException; /** * Provides the workspace deploy form. @@ -70,7 +70,7 @@ public function form(array $form, FormStateInterface $form_state) { // We can not deploy if we do not have an upstream workspace. if (!$workspace->getRepositoryHandlerPlugin()) { - throw new BadRequestHttpException(); + throw new HttpException(500, 'The specified repository handler plugin does not exist.'); } $form['help'] = [ diff --git a/core/modules/workspace/src/Form/WorkspaceForm.php b/core/modules/workspace/src/Form/WorkspaceForm.php index 93226f0..cc0b85c 100644 --- a/core/modules/workspace/src/Form/WorkspaceForm.php +++ b/core/modules/workspace/src/Form/WorkspaceForm.php @@ -128,8 +128,6 @@ public function save(array $form, FormStateInterface $form_state) { $workspace->setNewRevision(TRUE); $status = $workspace->save(); - \Drupal::service('plugin.manager.workspace.repository_handler')->clearCachedDefinitions(); - $info = ['%info' => $workspace->label()]; $context = ['@type' => $workspace->bundle(), '%info' => $workspace->label()]; $logger = $this->logger('workspace'); @@ -146,7 +144,9 @@ public function save(array $form, FormStateInterface $form_state) { if ($workspace->id()) { $form_state->setValue('id', $workspace->id()); $form_state->set('id', $workspace->id()); - $redirect = $this->currentUser()->hasPermission('administer workspaces') ? $workspace->toUrl('collection') : Url::fromRoute(''); + + $collection_url = $workspace->toUrl('collection'); + $redirect = $collection_url->access() ? $collection_url : Url::fromRoute(''); $form_state->setRedirectUrl($redirect); } else { diff --git a/core/modules/workspace/src/Form/WorkspaceSwitcherForm.php b/core/modules/workspace/src/Form/WorkspaceSwitcherForm.php index 8e7cf64..4e21390 100644 --- a/core/modules/workspace/src/Form/WorkspaceSwitcherForm.php +++ b/core/modules/workspace/src/Form/WorkspaceSwitcherForm.php @@ -22,11 +22,11 @@ class WorkspaceSwitcherForm extends FormBase { protected $workspaceManager; /** - * The entity type manager. + * The workspace entity storage handler. * - * @var \Drupal\Core\Entity\EntityTypeManagerInterface + * @var \Drupal\Core\Entity\EntityStorageInterface */ - protected $entityTypeManager; + protected $workspaceStorage; /** * The messenger service. @@ -47,7 +47,7 @@ class WorkspaceSwitcherForm extends FormBase { */ public function __construct(WorkspaceManagerInterface $workspace_manager, EntityTypeManagerInterface $entity_type_manager, MessengerInterface $messenger) { $this->workspaceManager = $workspace_manager; - $this->entityTypeManager = $entity_type_manager; + $this->workspaceStorage = $entity_type_manager->getStorage('workspace'); $this->messenger = $messenger; } @@ -73,7 +73,7 @@ public function getFormId() { * {@inheritdoc} */ public function buildForm(array $form, FormStateInterface $form_state) { - $workspaces = $this->entityTypeManager->getStorage('workspace')->loadMultiple(); + $workspaces = $this->workspaceStorage->loadMultiple(); $workspace_labels = []; foreach ($workspaces as $workspace) { $workspace_labels[$workspace->id()] = $workspace->label(); @@ -115,10 +115,8 @@ public function buildForm(array $form, FormStateInterface $form_state) { public function validateForm(array &$form, FormStateInterface $form_state) { $id = $form_state->getValue('workspace_id'); - // Ensure the workspace by that id exists. - /** @var \Drupal\workspace\WorkspaceInterface $workspace */ - $workspace = $this->entityTypeManager->getStorage('workspace')->load($id); - if (!$workspace) { + // Ensure the workspace by that ID exists. + if (!$this->workspaceStorage->load($id)) { $form_state->setErrorByName('workspace_id', $this->t('This workspace does not exist.')); } } @@ -130,7 +128,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) { $id = $form_state->getValue('workspace_id'); /** @var \Drupal\workspace\WorkspaceInterface $workspace */ - $workspace = $this->entityTypeManager->getStorage('workspace')->load($id); + $workspace = $this->workspaceStorage->load($id); try { $this->workspaceManager->setActiveWorkspace($workspace); diff --git a/core/modules/workspace/src/Negotiator/DefaultWorkspaceNegotiator.php b/core/modules/workspace/src/Negotiator/DefaultWorkspaceNegotiator.php index 6ab1349..e93d0c6 100644 --- a/core/modules/workspace/src/Negotiator/DefaultWorkspaceNegotiator.php +++ b/core/modules/workspace/src/Negotiator/DefaultWorkspaceNegotiator.php @@ -47,7 +47,7 @@ public function applies(Request $request) { /** * {@inheritdoc} */ - public function getWorkspace(Request $request) { + public function getActiveWorkspace(Request $request) { if (!$this->defaultWorkspace) { $default_workspace = $this->workspaceStorage->create([ 'id' => WorkspaceManager::DEFAULT_WORKSPACE, @@ -64,6 +64,6 @@ public function getWorkspace(Request $request) { /** * {@inheritdoc} */ - public function setWorkspace(WorkspaceInterface $workspace) {} + public function setActiveWorkspace(WorkspaceInterface $workspace) {} } diff --git a/core/modules/workspace/src/Negotiator/SessionWorkspaceNegotiator.php b/core/modules/workspace/src/Negotiator/SessionWorkspaceNegotiator.php index 1514ac0..618a022 100644 --- a/core/modules/workspace/src/Negotiator/SessionWorkspaceNegotiator.php +++ b/core/modules/workspace/src/Negotiator/SessionWorkspaceNegotiator.php @@ -54,15 +54,14 @@ public function __construct(AccountInterface $current_user, PrivateTempStoreFact * {@inheritdoc} */ public function applies(Request $request) { - // This negotiator only applies if the current user is authenticated, - // i.e. a session exists. + // This negotiator only applies if the current user is authenticated. return $this->currentUser->isAuthenticated(); } /** * {@inheritdoc} */ - public function getWorkspace(Request $request) { + public function getActiveWorkspace(Request $request) { $workspace_id = $this->tempstore->get('active_workspace_id'); if ($workspace_id && ($workspace = $this->workspaceStorage->load($workspace_id))) { @@ -75,7 +74,7 @@ public function getWorkspace(Request $request) { /** * {@inheritdoc} */ - public function setWorkspace(WorkspaceInterface $workspace) { + public function setActiveWorkspace(WorkspaceInterface $workspace) { $this->tempstore->set('active_workspace_id', $workspace->id()); } diff --git a/core/modules/workspace/src/Negotiator/WorkspaceNegotiatorInterface.php b/core/modules/workspace/src/Negotiator/WorkspaceNegotiatorInterface.php index 4ccee6a..5ec824e 100644 --- a/core/modules/workspace/src/Negotiator/WorkspaceNegotiatorInterface.php +++ b/core/modules/workspace/src/Negotiator/WorkspaceNegotiatorInterface.php @@ -37,7 +37,7 @@ public function applies(Request $request); * The negotiated workspace or NULL if the negotiator could not determine a * valid workspace. */ - public function getWorkspace(Request $request); + public function getActiveWorkspace(Request $request); /** * Sets the negotiated workspace. @@ -45,6 +45,6 @@ public function getWorkspace(Request $request); * @param \Drupal\workspace\WorkspaceInterface $workspace * The workspace entity. */ - public function setWorkspace(WorkspaceInterface $workspace); + public function setActiveWorkspace(WorkspaceInterface $workspace); } diff --git a/core/modules/workspace/src/Plugin/RepositoryHandler/LocalWorkspaceRepositoryHandler.php b/core/modules/workspace/src/Plugin/RepositoryHandler/LocalWorkspaceRepositoryHandler.php index 3e932a8..f468031 100644 --- a/core/modules/workspace/src/Plugin/RepositoryHandler/LocalWorkspaceRepositoryHandler.php +++ b/core/modules/workspace/src/Plugin/RepositoryHandler/LocalWorkspaceRepositoryHandler.php @@ -2,6 +2,7 @@ namespace Drupal\workspace\Plugin\RepositoryHandler; +use Drupal\Component\Uuid\UuidInterface; use Drupal\Core\Database\Connection; use Drupal\workspace\Entity\ReplicationLog; use Drupal\workspace\ReplicationLogInterface; @@ -9,7 +10,6 @@ use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\workspace\RepositoryHandlerInterface; -use Drupal\workspace\WorkspaceManager; use Drupal\workspace\WorkspaceManagerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -58,6 +58,13 @@ class LocalWorkspaceRepositoryHandler extends RepositoryHandlerBase implements R protected $database; /** + * The UUID service. + * + * @var \Drupal\Component\Uuid\UuidInterface + */ + protected $uuidService; + + /** * Constructs a new LocalWorkspaceRepositoryHandler. * * @param array $configuration @@ -73,11 +80,12 @@ class LocalWorkspaceRepositoryHandler extends RepositoryHandlerBase implements R * @param \Drupal\Core\Database\Connection $database * The database connection. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, WorkspaceManagerInterface $workspace_manager, Connection $database) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, WorkspaceManagerInterface $workspace_manager, Connection $database, UuidInterface $uuid_service) { parent::__construct($configuration, $plugin_id, $plugin_definition); $this->entityTypeManager = $entity_type_manager; $this->workspaceManager = $workspace_manager; $this->database = $database; + $this->uuidService = $uuid_service; $this->upstreamWorkspace = $this->entityTypeManager->getStorage('workspace')->load($this->getDerivativeId()); } @@ -91,7 +99,8 @@ public static function create(ContainerInterface $container, array $configuratio $plugin_definition, $container->get('entity_type.manager'), $container->get('workspace.manager'), - $container->get('database') + $container->get('database'), + $container->get('uuid') ); } @@ -125,7 +134,6 @@ public function replicate(RepositoryHandlerInterface $source, RepositoryHandlerI $target_workspace = $this->entityTypeManager->getStorage('workspace')->load($target->getDerivativeId()); $start_time = new \DateTime(); - $session_id = \md5((\microtime(TRUE) * 1000000)); // @todo Figure out if we want to include more information in the // replication log ID. // @see http://docs.couchdb.org/en/2.0.0/replication/protocol.html#generate-replication-id @@ -220,7 +228,7 @@ public function replicate(RepositoryHandlerInterface $source, RepositoryHandlerI return $this->updateReplicationLog($replication_log, [ 'entity_write_failures' => 0, 'end_time' => (new \DateTime())->format('D, d M Y H:i:s e'), - 'session_id' => $session_id, + 'session_id' => $this->uuidService->generate(), 'start_last_sequence' => $this->getLastSequenceId($replication_log), 'start_time' => $start_time->format('D, d M Y H:i:s e'), ]); @@ -252,7 +260,8 @@ protected function getLastSequenceId(ReplicationLogInterface $replication_log) { * The updated replication log entity. */ protected function updateReplicationLog(ReplicationLogInterface $replication_log, array $history) { - $replication_log->addHistory($history); + $replication_log->appendHistory($history); + $replication_log->setSessionId($history['session_id']); $replication_log->save(); return $replication_log; } diff --git a/core/modules/workspace/src/ReplicationLogInterface.php b/core/modules/workspace/src/ReplicationLogInterface.php index 6dd41dd..ebedeca 100644 --- a/core/modules/workspace/src/ReplicationLogInterface.php +++ b/core/modules/workspace/src/ReplicationLogInterface.php @@ -6,6 +6,13 @@ /** * Defines an interface for the Replication log entity type. + * + * This entity type holds the replication history (recorded checkpoints and a + * few more statistics) between a source and a target repository handler. + * + * Whenever a replication is performed, a new replication log is created or an + * existing one is updated, based on a ID that is uniquely identifiable from the + * given source and target. */ interface ReplicationLogInterface extends ContentEntityInterface { @@ -25,7 +32,7 @@ public function getHistory(); * * @return $this */ - public function addHistory(array $history); + public function appendHistory(array $history); /** * Gets the session ID. diff --git a/core/modules/workspace/src/RepositoryHandlerBase.php b/core/modules/workspace/src/RepositoryHandlerBase.php index 818d01a..172f26c 100644 --- a/core/modules/workspace/src/RepositoryHandlerBase.php +++ b/core/modules/workspace/src/RepositoryHandlerBase.php @@ -45,17 +45,4 @@ public function calculateDependencies() { return []; } - /** - * Replicates content from a source repository to a target repository. - * - * @param \Drupal\workspace\RepositoryHandlerInterface $source - * The repository handler to replicate from. - * @param \Drupal\workspace\RepositoryHandlerInterface $target - * The repository handler to replicate to. - * - * @return \Drupal\workspace\ReplicationLogInterface - * The replication log for the replication. - */ - abstract public function replicate(RepositoryHandlerInterface $source, RepositoryHandlerInterface $target); - } diff --git a/core/modules/workspace/src/RepositoryHandlerInterface.php b/core/modules/workspace/src/RepositoryHandlerInterface.php index 916263f..49f3dea 100644 --- a/core/modules/workspace/src/RepositoryHandlerInterface.php +++ b/core/modules/workspace/src/RepositoryHandlerInterface.php @@ -49,4 +49,17 @@ public function getDescription(); */ public function isRemote(); + /** + * Replicates content from a source repository to a target repository. + * + * @param \Drupal\workspace\RepositoryHandlerInterface $source + * The repository handler to replicate from. + * @param \Drupal\workspace\RepositoryHandlerInterface $target + * The repository handler to replicate to. + * + * @return \Drupal\workspace\ReplicationLogInterface + * The replication log for the replication. + */ + public function replicate(RepositoryHandlerInterface $source, RepositoryHandlerInterface $target); + } diff --git a/core/modules/workspace/src/WorkspaceAccessControlHandler.php b/core/modules/workspace/src/WorkspaceAccessControlHandler.php index 7b4fb35..e9ea77d 100644 --- a/core/modules/workspace/src/WorkspaceAccessControlHandler.php +++ b/core/modules/workspace/src/WorkspaceAccessControlHandler.php @@ -37,7 +37,7 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter // Or if it's their own workspace, and they have permission to access // their own workspace. ->orIf( - AccessResult::allowedIf($entity->getOwnerId() == $account->id())->addCacheableDependency($entity) + AccessResult::allowedIf($entity->getOwnerId() == $account->id())->cachePerUser()->addCacheableDependency($entity) ->andIf(AccessResult::allowedIfHasPermission($account, $operations[$operation]['own'])) ) ->orIf(AccessResult::allowedIfHasPermission($account, $permission_operation . ' workspace ' . $entity->id())); diff --git a/core/modules/workspace/src/WorkspaceListBuilder.php b/core/modules/workspace/src/WorkspaceListBuilder.php index fc4ef14..f4b2b21 100644 --- a/core/modules/workspace/src/WorkspaceListBuilder.php +++ b/core/modules/workspace/src/WorkspaceListBuilder.php @@ -86,7 +86,9 @@ public function getDefaultOperations(EntityInterface $entity) { if ($entity->id() != $active_workspace->id()) { $operations['activate'] = [ 'title' => $this->t('Set Active'), - 'weight' => 20, + // Use a weight lower than the one of the 'Edit' operation because we + // want the 'Activate' operation to be the primary operation. + 'weight' => 0, 'url' => $entity->toUrl('activate-form', ['query' => ['destination' => $entity->toUrl('collection')->toString()]]), ]; } @@ -94,7 +96,9 @@ public function getDefaultOperations(EntityInterface $entity) { if ($entity->getRepositoryHandlerPlugin()) { $operations['deploy'] = [ 'title' => $this->t('Deploy content'), - 'weight' => 20, + // The 'Deploy' operation should be the default one for the currently + // active workspace. + 'weight' => ($entity->id() == $active_workspace->id()) ? 0 : 20, 'url' => $entity->toUrl('deploy-form', ['query' => ['destination' => $entity->toUrl('collection')->toString()]]), ]; } diff --git a/core/modules/workspace/src/WorkspaceManager.php b/core/modules/workspace/src/WorkspaceManager.php index fbce585..516ca5b 100644 --- a/core/modules/workspace/src/WorkspaceManager.php +++ b/core/modules/workspace/src/WorkspaceManager.php @@ -125,7 +125,7 @@ public function __construct(RequestStack $request_stack, EntityTypeManagerInterf */ public function entityTypeCanBelongToWorkspaces(EntityTypeInterface $entity_type) { if (!in_array($entity_type->id(), $this->blacklist, TRUE) - && is_a($entity_type->getClass(), EntityPublishedInterface::class, TRUE) + && $entity_type->entityClassImplements(EntityPublishedInterface::class) && $entity_type->isRevisionable()) { return TRUE; } @@ -154,7 +154,7 @@ public function getActiveWorkspace() { foreach ($this->negotiatorIds as $negotiator_id) { $negotiator = $this->classResolver->getInstanceFromDefinition($negotiator_id); if ($negotiator->applies($request)) { - if ($active_workspace = $negotiator->getWorkspace($request)) { + if ($active_workspace = $negotiator->getActiveWorkspace($request)) { break; } } @@ -180,7 +180,7 @@ public function setActiveWorkspace(WorkspaceInterface $workspace) { foreach ($this->negotiatorIds as $negotiator_id) { $negotiator = $this->classResolver->getInstanceFromDefinition($negotiator_id); if ($negotiator->applies($request)) { - $negotiator->setWorkspace($workspace); + $negotiator->setActiveWorkspace($workspace); break; } } diff --git a/core/modules/workspace/tests/src/Functional/WorkspaceBypassTest.php b/core/modules/workspace/tests/src/Functional/WorkspaceBypassTest.php index a3b5342..f302e23 100644 --- a/core/modules/workspace/tests/src/Functional/WorkspaceBypassTest.php +++ b/core/modules/workspace/tests/src/Functional/WorkspaceBypassTest.php @@ -18,6 +18,9 @@ class WorkspaceBypassTest extends BrowserTestBase { placeBlock as drupalPlaceBlock; } + /** + * {@inheritdoc} + */ public static $modules = ['node', 'user', 'block', 'workspace']; /** @@ -38,7 +41,7 @@ public function testBypassSpecificWorkspace() { // Login as a limited-access user and create a workspace. $this->drupalLogin($ditka); - $vanilla_node = $this->createNodeThroughUi('Vanilla node', 'test'); + $this->createNodeThroughUi('Vanilla node', 'test'); $bears = $this->createWorkspaceThroughUi('Bears', 'bears'); $this->switchToWorkspace($bears); @@ -56,8 +59,7 @@ public function testBypassSpecificWorkspace() { // Because Lombardi has the bypass permission, he should be able to create // and edit any node. $this->drupalGet('/node/' . $ditka_bears_node_id . '/edit'); - $session = $this->getSession(); - $this->assertEquals(200, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(200); $lombardi_bears_node = $this->createNodeThroughUi('Lombardi Bears node', 'test'); $lombardi_bears_node_id = $lombardi_bears_node->id(); @@ -66,8 +68,7 @@ public function testBypassSpecificWorkspace() { $this->switchToWorkspace($bears); $this->drupalGet('/node/' . $lombardi_bears_node_id . '/edit'); - $session = $this->getSession(); - $this->assertEquals(403, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(403); // Create a new user that should NOT be able to edit anything in the Bears // workspace. @@ -76,9 +77,7 @@ public function testBypassSpecificWorkspace() { $this->switchToWorkspace($bears); $this->drupalGet('/node/' . $ditka_bears_node_id . '/edit'); - $session = $this->getSession(); - $this->assertEquals(403, $session->getStatusCode()); - + $this->assertSession()->statusCodeEquals(403); } /** @@ -108,8 +107,7 @@ public function testBypassOwnWorkspace() { // Editing both nodes should be possible. $this->drupalGet('/node/' . $ditka_bears_node_id . '/edit'); - $session = $this->getSession(); - $this->assertEquals(200, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(200); // Create a new user that should be able to edit anything in the Bears // workspace. @@ -120,8 +118,7 @@ public function testBypassOwnWorkspace() { // Because editor 2 has the bypass permission, he should be able to create // and edit any node. $this->drupalGet('/node/' . $ditka_bears_node_id . '/edit'); - $session = $this->getSession(); - $this->assertEquals(403, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(403); } } diff --git a/core/modules/workspace/tests/src/Functional/WorkspaceIndividualPermissionsTest.php b/core/modules/workspace/tests/src/Functional/WorkspaceIndividualPermissionsTest.php index 507342f..71a5ca6 100644 --- a/core/modules/workspace/tests/src/Functional/WorkspaceIndividualPermissionsTest.php +++ b/core/modules/workspace/tests/src/Functional/WorkspaceIndividualPermissionsTest.php @@ -48,10 +48,9 @@ public function testEditIndividualWorkspace() { $editor2 = $this->drupalCreateUser(array_merge($permissions, ['edit workspace ' . $bears->id()])); $this->drupalLogin($editor2); - $session = $this->getSession(); $this->drupalGet("/admin/config/workflow/workspace/{$bears->id()}/edit"); - $this->assertEquals(200, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(200); } /** @@ -69,7 +68,6 @@ public function testViewIndividualWorkspace() { // Login as a limited-access user and create a workspace. $this->drupalLogin($editor1); - $this->createWorkspaceThroughUi('Bears', 'bears'); $bears = Workspace::load('bears'); @@ -77,21 +75,18 @@ public function testViewIndividualWorkspace() { $editor2 = $this->drupalCreateUser(array_merge($permissions, ['view workspace ' . $bears->id()])); $this->drupalLogin($editor2); - $session = $this->getSession(); - $this->createWorkspaceThroughUi('Packers', 'packers'); - $packers = Workspace::load('packers'); // Load the activate form for the Bears workspace. It should work, because // the user has the permission specific to that workspace. $this->drupalGet("admin/config/workflow/workspace/{$bears->id()}/activate"); - $this->assertEquals(200, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(200); // But editor 1 cannot view the Packers workspace. $this->drupalLogin($editor1); $this->drupalGet("admin/config/workflow/workspace/{$packers->id()}/activate"); - $this->assertEquals(403, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(403); } } diff --git a/core/modules/workspace/tests/src/Functional/WorkspacePermissionsTest.php b/core/modules/workspace/tests/src/Functional/WorkspacePermissionsTest.php index 895f65b..ba61faf 100644 --- a/core/modules/workspace/tests/src/Functional/WorkspacePermissionsTest.php +++ b/core/modules/workspace/tests/src/Functional/WorkspacePermissionsTest.php @@ -65,39 +65,33 @@ public function testEditOwnWorkspace() { // Login as a limited-access user and create a workspace. $this->drupalLogin($editor1); - $this->createWorkspaceThroughUi('Bears', 'bears'); // Now edit that same workspace; We should be able to do so. $bears = Workspace::load('bears'); - $session = $this->getSession(); - $this->drupalGet("/admin/config/workflow/workspace/{$bears->id()}/edit"); - $this->assertEquals(200, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(200); - $page = $session->getPage(); + $page = $this->getSession()->getPage(); $page->fillField('label', 'Bears again'); $page->fillField('id', 'bears'); $page->findButton(t('Save'))->click(); - $session->getPage()->hasContent('Bears again (bears)'); + $page->hasContent('Bears again (bears)'); // Now login as a different user and ensure they don't have edit access, // and vice versa. $editor2 = $this->drupalCreateUser($permissions); $this->drupalLogin($editor2); - $session = $this->getSession(); - $this->createWorkspaceThroughUi('Packers', 'packers'); - $packers = Workspace::load('packers'); $this->drupalGet("/admin/config/workflow/workspace/{$packers->id()}/edit"); - $this->assertEquals(200, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(200); $this->drupalGet("/admin/config/workflow/workspace/{$bears->id()}/edit"); - $this->assertEquals(403, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(403); } /** @@ -115,39 +109,34 @@ public function testEditAnyWorkspace() { // Login as a limited-access user and create a workspace. $this->drupalLogin($editor1); - $this->createWorkspaceThroughUi('Bears', 'bears'); // Now edit that same workspace; We should be able to do so. $bears = Workspace::load('bears'); - $session = $this->getSession(); - $this->drupalGet("/admin/config/workflow/workspace/{$bears->id()}/edit"); - $this->assertEquals(200, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(200); - $page = $session->getPage(); + $page = $this->getSession()->getPage(); $page->fillField('label', 'Bears again'); $page->fillField('id', 'bears'); $page->findButton(t('Save'))->click(); - $session->getPage()->hasContent('Bears again (bears)'); + $page->hasContent('Bears again (bears)'); // Now login as a different user and ensure they don't have edit access, // and vice versa. $admin = $this->drupalCreateUser(array_merge($permissions, ['edit any workspace'])); $this->drupalLogin($admin); - $session = $this->getSession(); - $this->createWorkspaceThroughUi('Packers', 'packers'); - $packers = Workspace::load('packers'); $this->drupalGet("/admin/config/workflow/workspace/{$packers->id()}/edit"); - $this->assertEquals(200, $session->getStatusCode()); + + $this->assertSession()->statusCodeEquals(200); $this->drupalGet("/admin/config/workflow/workspace/{$bears->id()}/edit"); - $this->assertEquals(200, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(200); } } diff --git a/core/modules/workspace/tests/src/Functional/WorkspaceSwitcherTest.php b/core/modules/workspace/tests/src/Functional/WorkspaceSwitcherTest.php index 8b640b4..78c6f8d 100644 --- a/core/modules/workspace/tests/src/Functional/WorkspaceSwitcherTest.php +++ b/core/modules/workspace/tests/src/Functional/WorkspaceSwitcherTest.php @@ -41,12 +41,11 @@ public function testSwitchingWorkspaces() { $this->drupalGet('/admin/config/workflow/workspace/' . $gravity->id() . '/activate'); - $session = $this->getSession(); - $this->assertEquals(200, $session->getStatusCode()); - $page = $session->getPage(); + $this->assertSession()->statusCodeEquals(200); + $page = $this->getSession()->getPage(); $page->findButton(t('Confirm'))->click(); - $session->getPage()->findLink($gravity->label()); + $page->findLink($gravity->label()); } } diff --git a/core/modules/workspace/tests/src/Functional/WorkspaceTest.php b/core/modules/workspace/tests/src/Functional/WorkspaceTest.php index dcbda85..097886f 100644 --- a/core/modules/workspace/tests/src/Functional/WorkspaceTest.php +++ b/core/modules/workspace/tests/src/Functional/WorkspaceTest.php @@ -56,13 +56,13 @@ public function testSpecialCharacters() { // Test and invalid workspace name. $this->drupalGet('/admin/config/workflow/workspace/add'); - $session = $this->getSession(); - $this->assertEquals(200, $session->getStatusCode()); - $page = $session->getPage(); + $this->assertSession()->statusCodeEquals(200); + + $page = $this->getSession()->getPage(); $page->fillField('label', 'workspace2'); $page->fillField('id', 'A!"£%^&*{}#~@?'); $page->findButton(t('Save'))->click(); - $session->getPage()->hasContent("This value is not valid"); + $page->hasContent("This value is not valid"); } /** diff --git a/core/modules/workspace/tests/src/Functional/WorkspaceViewTest.php b/core/modules/workspace/tests/src/Functional/WorkspaceViewTest.php index 16d13ef..27ca922 100644 --- a/core/modules/workspace/tests/src/Functional/WorkspaceViewTest.php +++ b/core/modules/workspace/tests/src/Functional/WorkspaceViewTest.php @@ -33,7 +33,6 @@ public function testViewOwnWorkspace() { // Login as a limited-access user and create a workspace. $this->drupalLogin($editor1); - $this->createWorkspaceThroughUi('Bears', 'bears'); $bears = Workspace::load('bears'); @@ -42,8 +41,6 @@ public function testViewOwnWorkspace() { $editor2 = $this->drupalCreateUser($permissions); $this->drupalLogin($editor2); - $session = $this->getSession(); - $this->createWorkspaceThroughUi('Packers', 'packers'); $packers = Workspace::load('packers'); @@ -51,11 +48,11 @@ public function testViewOwnWorkspace() { // Load the activate form for the Bears workspace. It should fail because // the workspace belongs to someone else. $this->drupalGet("admin/config/workflow/workspace/{$bears->id()}/activate"); - $this->assertEquals(403, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(403); // But editor 2 should be able to activate the Packers workspace. $this->drupalGet("admin/config/workflow/workspace/{$packers->id()}/activate"); - $this->assertEquals(200, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(200); } /** @@ -83,8 +80,6 @@ public function testViewAnyWorkspace() { $editor2 = $this->drupalCreateUser($permissions); $this->drupalLogin($editor2); - $session = $this->getSession(); - $this->createWorkspaceThroughUi('Packers', 'packers'); $packers = Workspace::load('packers'); @@ -92,11 +87,12 @@ public function testViewAnyWorkspace() { // Load the activate form for the Bears workspace. This user should be // able to see both workspaces because of the "view any" permission. $this->drupalGet("admin/config/workflow/workspace/{$bears->id()}/activate"); - $this->assertEquals(200, $session->getStatusCode()); + + $this->assertSession()->statusCodeEquals(200); // But editor 2 should be able to activate the Packers workspace. $this->drupalGet("admin/config/workflow/workspace/{$packers->id()}/activate"); - $this->assertEquals(200, $session->getStatusCode()); + $this->assertSession()->statusCodeEquals(200); } } diff --git a/core/modules/workspace/workspace.module b/core/modules/workspace/workspace.module index 3b3118e..c673f0e 100644 --- a/core/modules/workspace/workspace.module +++ b/core/modules/workspace/workspace.module @@ -39,15 +39,10 @@ function workspace_entity_load(array &$entities, $entity_type_id) { $workspace_manager = \Drupal::service('workspace.manager'); $entity_type_manager = \Drupal::entityTypeManager(); - // Don't alter the loaded entities if the entity type can not belong to a - // workspace. - if (!$workspace_manager->entityTypeCanBelongToWorkspaces($entity_type_manager->getDefinition($entity_type_id))) { - return; - } - - // Don't alter the loaded entities if the active workspace is the default one. - $active_workspace = $workspace_manager->getActiveWorkspace(); - if ($active_workspace->isDefaultWorkspace()) { + // Only run if the entity type can belong to a workspace and we are in a + // non-default workspace. + if (!$workspace_manager->entityTypeCanBelongToWorkspaces($entity_type_manager->getDefinition($entity_type_id)) + || (($active_workspace = $workspace_manager->getActiveWorkspace()) && $active_workspace->isDefaultWorkspace())) { return; } diff --git a/core/modules/workspace/workspace.services.yml b/core/modules/workspace/workspace.services.yml index ed83dbe..f68e0eb 100644 --- a/core/modules/workspace/workspace.services.yml +++ b/core/modules/workspace/workspace.services.yml @@ -24,4 +24,4 @@ services: - { name: cache.context } logger.channel.workspace: parent: logger.channel_base - arguments: ['cron'] + arguments: ['workspace']