-
Notifications
You must be signed in to change notification settings - Fork 132
160 remove complex widget #570
Changes from 206 commits
a658b0e
bce0c5e
bad2c09
c75b654
d49ac2b
1d061ee
d8591fb
97fb566
77eb562
9bb346e
ce13e41
d317614
0e10e5f
793715e
3d4013f
9c4858b
5346d09
1551922
3f69920
ca3bc5d
3e3c90d
78a427e
11f4bd2
8c94013
e98354c
1508540
afd3f71
586e677
c5fdf87
328c913
68a9abd
a117c7b
0940d99
521b508
ef4af0d
d5d34d6
7334e9c
9ac5241
71e903a
00525d9
a60a300
cb75f16
f7b0d29
43f8acd
cf30f3a
94db0c1
a2bdbab
e0c2655
ba47934
4d65046
2104177
4f47cc0
a988745
470e6ef
5c939ec
0fcf95e
81f1b28
f33eb3d
7762e4f
3a1929b
8f241b2
1b18142
1d5b92d
ef4d2db
8dc9aa1
d817fc4
d64af00
e2e82b9
ee625d3
2fdf2bd
a21eb83
85849ae
a1cc01e
7a18d5c
780b54a
352551d
7fafaef
093324c
8aa95f5
24a1221
6bdaa20
b174e23
4bb4819
0741340
ae94377
12838fb
8ab5433
55b3fcf
a2271be
7744bc3
9f96bc4
7ecdedd
86dee21
f20e279
3134064
8abc283
8c7cadb
564682f
a2d92a2
74e677d
2c5af11
df17662
779efee
1450634
961eb7d
0e030db
15debda
f43add7
2257ece
35b019f
12cc927
365da2f
f3894cd
16ff320
b7e2d5e
b7ac85b
3493f91
3439183
fd83a5d
c304aea
0625119
5fa4b7a
352ba22
57dfc44
fab2fe6
3d3365a
2faaad5
214f5cd
9c94943
84a85fc
c194f0e
e5b3b60
fde09bc
86301b0
5622d85
85d87ae
e9e2152
991ea33
d6ceee6
7e58f33
ffd5afe
412f880
aa82d18
34ffca4
bdc92db
417d548
0f87dbe
c980ace
38a25a7
d0b227f
6f6a733
fd95fdb
0439c0e
eff1923
40f677f
0e7bec1
037c38f
7bda98a
cdde6cd
bd428c6
5db820b
97623d4
c6559a7
6630f00
7b8245a
0076fba
8a35d7a
1ba32f8
68cd03e
fa93ef0
58caa78
29cc980
492825a
9884218
6409300
72fb63d
f8cbf12
e3fd886
7c591a4
79519fc
6236767
e1ce4b0
61967f9
f270eb2
b4be10f
9cc2202
1ea6272
e90010b
1e8d004
17f9b08
f87e4e5
9b2aaae
79931c4
e4f9334
a0c1e6d
3926543
146c778
748e78d
0bd6cc1
e80f686
5a5995a
d32da0b
7c3ce1b
16aad07
fc73c8e
75e3def
5d95ec6
528025d
2b43dec
6951d09
753e719
f1adcc7
1bb4998
437bb31
6494ed3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -158,6 +158,20 @@ function og_entity_access(EntityInterface $entity, $operation, AccountInterface | |
/** @var \Drupal\Core\Access\AccessResult $access */ | ||
$access = \Drupal::service('og.access')->userAccessEntityOperation($operation, $entity, $account); | ||
|
||
$audience_fields = \Drupal::service('og.group_audience_helper')->getAllGroupAudienceFields($entity_type_id, $bundle_id); | ||
if (count($audience_fields) === 1) { | ||
$field_definitions = \Drupal::service('entity_field.manager')->getFieldDefinitions($entity_type_id, $bundle_id); | ||
foreach ($field_definitions as $field_definition) { | ||
/** @var \Drupal\Core\Field\FieldDefinitionInterface $field_definition */ | ||
if (!in_array($field_definition->getName(), array_keys($audience_fields))) { | ||
continue; | ||
} | ||
|
||
// Check if the field needs to be required. | ||
$field_definition->setRequired($access); | ||
} | ||
} | ||
|
||
if ($access->isAllowed()) { | ||
return $access; | ||
} | ||
|
@@ -178,6 +192,11 @@ function og_entity_access(EntityInterface $entity, $operation, AccountInterface | |
function og_entity_create_access(AccountInterface $account, array $context, $bundle) { | ||
$entity_type_id = $context['entity_type_id']; | ||
|
||
if (!empty($context['skip_og_permission'])) { | ||
// We've already been here or we want to skip the access check. | ||
return; | ||
} | ||
Comment on lines
+195
to
+198
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary? This seems brittle to me and a potential security problem. We should not allow to skip access checks unless there is a very good reason for it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure about the correctness of the code here - but I remember the reason though. When you create a node, it still doesn't belong to a group. It might belong, but if for example the audience field is optional, than that node could be a "global" one (i.e. does not belong to a group). So we need to check if that node could be created outside of OG context. If it can, then we don't have the final say. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this is intended to avoid an endless loop, because this is calling itself again later on. This approach feels very strange to me. The code below calls the access check, passing exactly the same parameters, from inside the access check? What information could this possibly be retrieving that we don't already have at this point. |
||
|
||
if (!Og::isGroupContent($entity_type_id, $bundle)) { | ||
// Not a group content. | ||
return AccessResult::neutral(); | ||
|
@@ -203,23 +222,41 @@ function og_entity_create_access(AccountInterface $account, array $context, $bun | |
// @see \Drupal\og\Plugin\EntityReferenceSelection\OgSelection::buildEntityQuery() | ||
$required = FALSE; | ||
|
||
/** @var \Drupal\og\OgGroupAudienceHelperInterface $audience_helper */ | ||
$audience_helper = \Drupal::service('og.group_audience_helper'); | ||
$audience_fields = $audience_helper->getAllGroupAudienceFields($entity_type_id, $bundle); | ||
$field_definitions = \Drupal::service('entity_field.manager')->getFieldDefinitions($entity_type_id, $bundle); | ||
foreach ($field_definitions as $field_definition) { | ||
/** @var \Drupal\Core\Field\FieldDefinitionInterface $field_definition */ | ||
if (!\Drupal::service('og.group_audience_helper')->isGroupAudienceField($field_definition)) { | ||
if (!in_array($field_definition->getName(), array_keys($audience_fields))) { | ||
continue; | ||
} | ||
|
||
if (count($audience_fields) === 1) { | ||
// Set the field to required only if we have a single audience field. | ||
// If we have multiple fields then the require logic will perform at the | ||
// entity constraint level. | ||
$field_definition->setRequired(!\Drupal::entityTypeManager() | ||
->getAccessControlHandler($entity_type_id) | ||
->createAccess($bundle, $account, ['skip_og_permission' => TRUE])); | ||
} | ||
Comment on lines
+235
to
+242
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we again setting fields to be required inside an access hook? This doesn't belong here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it's related to https://github.com/Gizra/og/pull/570/files#r377001979 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should investigate a different solution, this is a hack. We should not alter any field definitions in It looks like this field definition is used as temporary storage for what could be a simple |
||
|
||
$options = [ | ||
'target_type' => $field_definition->getFieldStorageDefinition()->getSetting('target_type'), | ||
'handler' => $field_definition->getSetting('handler'), | ||
'field_mode' => 'admin', | ||
]; | ||
/** @var \Drupal\Core\Entity\EntityReferenceSelection\SelectionPluginManager $handler */ | ||
$handler = \Drupal::service('plugin.manager.entity_reference_selection'); | ||
$handler = $handler->getInstance($options); | ||
|
||
// @todo Remove deprecated call to Og::getSelectionHandler. | ||
$handler = Og::getSelectionHandler($field_definition); | ||
MPParsley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if ($handler->getInstance($options)) { | ||
return AccessResult::neutral(); | ||
if ($handler && $handler->countReferenceableEntities()) { | ||
// At least one of the fields has referenceable groups. That mean that | ||
// the user can create group content when referencing the group content in | ||
// at least one of the fields. | ||
return AccessResult::allowed(); | ||
} | ||
// Allow users to create content outside of groups, if none of the | ||
// audience fields is required. | ||
|
@@ -289,7 +326,16 @@ function og_field_formatter_info_alter(array &$info) { | |
* Implements hook_field_widget_info_alter(). | ||
*/ | ||
function og_field_widget_info_alter(array &$info) { | ||
$info['options_buttons']['field_types'][] = OgGroupAudienceHelperInterface::GROUP_REFERENCE; | ||
$field_types = [ | ||
'entity_reference_autocomplete', | ||
'entity_reference_autocomplete_tags', | ||
'options_buttons', | ||
'options_select', | ||
]; | ||
|
||
foreach ($field_types as $field_type) { | ||
$info[$field_type]['field_types'][] = OgGroupAudienceHelperInterface::GROUP_REFERENCE; | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -304,7 +350,21 @@ function og_field_widget_info_alter(array &$info) { | |
function og_entity_type_alter(array &$entity_types) { | ||
/** @var \Drupal\Core\Entity\EntityTypeInterface $entity_type */ | ||
foreach ($entity_types as $entity_type_id => $entity_type) { | ||
|
||
// Add link template to groups. We add it to all the entity types, and later | ||
// on return the correct access, depending if the bundle is indeed a group | ||
// and accessible. We do not filter here the entity type by groups, so | ||
// whenever GroupTypeManager::addGroup is called, it's enough to mark route | ||
// to be rebuilt via RouteBuilder::setRebuildNeeded. | ||
$entity_type->setLinkTemplate('og-admin-routes', "/group/$entity_type_id/{{$entity_type_id}}/admin"); | ||
|
||
// We won't check if an entity might be group content because we need to | ||
// check if that entity has any bundle that could be a group entity and this | ||
// would cause a recursion. The constraint will check there if the entity is | ||
// group content or not. | ||
if ($entity_type->entityClassImplements(ContentEntityInterface::class)) { | ||
$entity_type->addConstraint('ValidOgMembershipMultipleReference'); | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,8 +4,18 @@ | |||||||||||
|
||||||||||||
namespace Drupal\og\Plugin\EntityReferenceSelection; | ||||||||||||
|
||||||||||||
use Drupal\Core\Entity\EntityFieldManagerInterface; | ||||||||||||
use Drupal\Core\Entity\EntityTypeBundleInfoInterface; | ||||||||||||
use Drupal\Core\Entity\EntityTypeManagerInterface; | ||||||||||||
use Drupal\Core\Entity\EntityRepositoryInterface; | ||||||||||||
use Drupal\Core\Entity\Plugin\EntityReferenceSelection\DefaultSelection; | ||||||||||||
use Drupal\Core\Extension\ModuleHandlerInterface; | ||||||||||||
use Drupal\Core\Form\FormStateInterface; | ||||||||||||
use Drupal\Core\Session\AccountInterface; | ||||||||||||
use Drupal\og\MembershipManagerInterface; | ||||||||||||
use Drupal\og\OgAccessInterface; | ||||||||||||
use Drupal\og\Og; | ||||||||||||
use Symfony\Component\DependencyInjection\ContainerInterface; | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Provide default OG selection handler. | ||||||||||||
|
@@ -26,6 +36,71 @@ | |||||||||||
*/ | ||||||||||||
class OgSelection extends DefaultSelection { | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* The OG access service. | ||||||||||||
* | ||||||||||||
* @var \Drupal\og\OgAccessInterface | ||||||||||||
*/ | ||||||||||||
protected $ogAccess; | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* The OG membership manager. | ||||||||||||
* | ||||||||||||
* @var \Drupal\og\MembershipManagerInterface | ||||||||||||
*/ | ||||||||||||
protected $ogMembershipManager; | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Constructs a new SelectionBase object. | ||||||||||||
* | ||||||||||||
* @param array $configuration | ||||||||||||
* A configuration array containing information about the plugin instance. | ||||||||||||
* @param string $plugin_id | ||||||||||||
* The plugin_id for the plugin instance. | ||||||||||||
* @param mixed $plugin_definition | ||||||||||||
* The plugin implementation definition. | ||||||||||||
* @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager | ||||||||||||
* The entity manager service. | ||||||||||||
* @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler | ||||||||||||
* The module handler service. | ||||||||||||
* @param \Drupal\Core\Session\AccountInterface $current_user | ||||||||||||
* The current user. | ||||||||||||
* @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager | ||||||||||||
* The entity field manager. | ||||||||||||
* @param \Drupal\Core\Entity\EntityTypeBundleInfoInterface $entity_type_bundle_info | ||||||||||||
* The entity type bundle info service. | ||||||||||||
* @param \Drupal\Core\Entity\EntityRepositoryInterface $entity_repository | ||||||||||||
* The entity repository. | ||||||||||||
* @param \Drupal\og\OgAccessInterface $og_access | ||||||||||||
* The OG access service. | ||||||||||||
* @param \Drupal\og\MembershipManagerInterface $og_membership_manager | ||||||||||||
* The OG membership service. | ||||||||||||
*/ | ||||||||||||
public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, ModuleHandlerInterface $module_handler, AccountInterface $current_user, EntityFieldManagerInterface $entity_field_manager, EntityTypeBundleInfoInterface $entity_type_bundle_info, EntityRepositoryInterface $entity_repository, OgAccessInterface $og_access, MembershipManagerInterface $og_membership_manager) { | ||||||||||||
parent::__construct($configuration, $plugin_id, $plugin_definition, $entity_type_manager, $module_handler, $current_user, $entity_field_manager, $entity_type_bundle_info, $entity_repository); | ||||||||||||
$this->ogAccess = $og_access; | ||||||||||||
$this->ogMembershipManager = $og_membership_manager; | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* {@inheritdoc} | ||||||||||||
*/ | ||||||||||||
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { | ||||||||||||
return new static( | ||||||||||||
$configuration, | ||||||||||||
$plugin_id, | ||||||||||||
$plugin_definition, | ||||||||||||
$container->get('entity_type.manager'), | ||||||||||||
$container->get('module_handler'), | ||||||||||||
$container->get('current_user'), | ||||||||||||
$container->get('entity_field.manager'), | ||||||||||||
$container->get('entity_type.bundle.info'), | ||||||||||||
$container->get('entity.repository'), | ||||||||||||
$container->get('og.access'), | ||||||||||||
$container->get('og.membership_manager') | ||||||||||||
); | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Get the selection handler of the field. | ||||||||||||
* | ||||||||||||
|
@@ -62,7 +137,6 @@ public function getSelectionHandler() { | |||||||||||
* it. | ||||||||||||
*/ | ||||||||||||
protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS') { | ||||||||||||
|
||||||||||||
// Getting the original entity selection handler. OG selection handler using | ||||||||||||
// the default selection handler of the entity, which the field reference | ||||||||||||
// to, and add another logic to the query object i.e. check if the entities | ||||||||||||
|
@@ -81,37 +155,45 @@ protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS') | |||||||||||
$query->condition($bundle_key, $bundles, 'IN'); | ||||||||||||
} | ||||||||||||
|
||||||||||||
$user_groups = $this->getUserGroups(); | ||||||||||||
if (!$user_groups) { | ||||||||||||
// Get the identifier key of the entity. | ||||||||||||
$identifier_key = $definition->getKey('id'); | ||||||||||||
|
||||||||||||
if ($this->currentUser->isAnonymous()) { | ||||||||||||
// @todo: Check if anonymous users should have access to any referenced | ||||||||||||
MPParsley marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
// groups? What about groups that allow anonymous posts? | ||||||||||||
return $query->condition($identifier_key, -1); | ||||||||||||
} | ||||||||||||
|
||||||||||||
if ($this->currentUser->hasPermission('administer organic groups')) { | ||||||||||||
// User can see all the groups. | ||||||||||||
return $query; | ||||||||||||
} | ||||||||||||
|
||||||||||||
$identifier_key = $definition->getKey('id'); | ||||||||||||
if (empty($this->configuration['entity'])) { | ||||||||||||
// @todo Find out why we have this scenario. | ||||||||||||
return $query; | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** @var \Drupal\Core\Entity\EntityInterface $entity */ | ||||||||||||
$entity = $this->configuration['entity']; | ||||||||||||
$bundle = $entity->bundle(); | ||||||||||||
|
||||||||||||
$ids = []; | ||||||||||||
if (!empty($this->getConfiguration()['field_mode']) && $this->getConfiguration()['field_mode'] === 'admin') { | ||||||||||||
// Don't include the groups, the user doesn't have create permission. | ||||||||||||
foreach ($user_groups as $group) { | ||||||||||||
foreach ($this->getUserGroups() as $group) { | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||
// Check user has "create" permission on this entity. | ||||||||||||
$permission = "create $bundle content"; | ||||||||||||
if ($this->ogAccess->userAccess($group, $permission, $this->currentUser)->isAllowed()) { | ||||||||||||
Comment on lines
+183
to
+185
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
$ids[] = $group->id(); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
if ($ids) { | ||||||||||||
$query->condition($identifier_key, $ids, 'NOT IN'); | ||||||||||||
} | ||||||||||||
if ($ids) { | ||||||||||||
$query->condition($identifier_key, $ids, 'IN'); | ||||||||||||
} | ||||||||||||
else { | ||||||||||||
// Determine which groups should be selectable. | ||||||||||||
foreach ($user_groups as $group) { | ||||||||||||
$ids[] = $group->id(); | ||||||||||||
} | ||||||||||||
if ($ids) { | ||||||||||||
$query->condition($identifier_key, $ids, 'IN'); | ||||||||||||
} | ||||||||||||
else { | ||||||||||||
// User doesn't have permission to select any group so falsify this | ||||||||||||
// query. | ||||||||||||
$query->condition($identifier_key, -1, '='); | ||||||||||||
} | ||||||||||||
// User doesn't have permission to select any group so falsify this | ||||||||||||
// query. | ||||||||||||
$query->condition($identifier_key, -1); | ||||||||||||
} | ||||||||||||
|
||||||||||||
return $query; | ||||||||||||
|
@@ -130,4 +212,28 @@ protected function getUserGroups() { | |||||||||||
return isset($other_groups[$this->configuration['target_type']]) ? $other_groups[$this->configuration['target_type']] : []; | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* {@inheritdoc} | ||||||||||||
*/ | ||||||||||||
public function buildConfigurationForm(array $form, FormStateInterface $form_state) { | ||||||||||||
$form = parent::buildConfigurationForm($form, $form_state); | ||||||||||||
|
||||||||||||
// Filter out the bundles that are not groups. | ||||||||||||
$entity_type_id = $this->configuration['target_type']; | ||||||||||||
$entity_type = $this->entityTypeManager->getDefinition($entity_type_id); | ||||||||||||
$bundles_info = $this->entityTypeBundleInfo->getBundleInfo($entity_type_id); | ||||||||||||
|
||||||||||||
if ($entity_type->hasKey('bundle')) { | ||||||||||||
$bundles = Og::groupTypeManager()->getGroupBundleIdsByEntityType($entity_type_id); | ||||||||||||
foreach ($bundles as $bundle) { | ||||||||||||
$bundle_options[$bundle] = $bundles_info[$bundle]['label']; | ||||||||||||
} | ||||||||||||
|
||||||||||||
natsort($bundle_options); | ||||||||||||
$form['target_bundles']['#options'] = $bundle_options; | ||||||||||||
} | ||||||||||||
|
||||||||||||
return $form; | ||||||||||||
} | ||||||||||||
|
||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MPParsley this one is related to https://github.com/Gizra/og/pull/570/files#r377001979
We try to make sure that if a node must be created inside an OG context, it cannot be created without choosing a group. Again not sure about the correctness of code - but rather on the reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this code is not working. This is calling
FieldConfigInterface::setRequired()
but never callingFieldConfigInterface::save()
after making the change so this code is basically doing nothing.We should not alter field definitions in
hook_entity_access()
in any case.