Can you please explain why is this a VBO issue?
Yes, probably most of the frontend can be done later if there's nothing critical that'd block UI from functioning. We can remove that from the alpha plan.
Thanks!
Ok, added to the issue description.
Thank you for sharing your findings, hopefully it’ll help someone.
Basically the core bulk form needs that extra configuration while the Views Bulk Operations field doesn’t, it discovers all available actions.
On case of the patch referenced in #21 📌 Provide a way to disable "select all on all pages" RTBC tests fail. The MR on the other hand looks good byt the select description needs updating so users know what's the "Default behavior". Also, in the MR there's a schema update which means we'll need an update hook for any existing config.
Still needs work in any case.
Thank you, our composer.json will look much better now 🙂
Why do you want to use addWhere conditionally and not in all cases?
@LOBsTerr a bit later means? ;)
Thank you!
That basically ;)
We need to get back to the primary solution unfortunately - the "great improvement" will make VBO useless on block displays, those dedicated routes were needed.
@Kristiaan, general thoughts: context - based access is something that should be avoided as it creates ways to bypass restrictions - an individual either has access or doesn't and not maybe has and maybe not, so site builders can accidentally leave some doors open and create security issues. Also, it seems bad that group prevents access to routes without group context as usually in cases outside of some module scope neutral would be returned. Can we just return TRUE if no context is available?
Fine with me, I guess we can add some hook or dispatch an event when filter is replaced allowing other modules to replace their filters as well. We'll also need to document that in module.api.php. Further thoughts?
This is big unfortunately and will need a (sub) major release. No BC issues expected though.
Moving this to VBO, needs quite some refactoring but It'll be a big step forward for VBO so definitely worth an effort.
Thank you, will release this along with sortable actions and all that made its way to dev in a week if there'll be no further issues.
Please check with that last commit.
Thank you for finding this. I think we should just add the missing weights before sorting in a foreach with $i++
Ok, it's a core issue actually, very edgy though. Updating.
I feel this is a deeper hole after all.
Reroll + copy-pasta fix plus some phpcs stuff.
array_key_exists is strict, we know the argument must be an array, ?? works with everything which may lead to unexpected results. If array_key_exists wasn't used, we wouldn't have this issue and we wouldn't know there are changes in core that could silently break our functionalities and we'd have issues that would be much harder to discover and investigate.
Strict approach leaves no place for such issues, array must be an array, int must be int etc.
This needs more work with new Drupal\Component\Plugin\Definition\PluginDefinition.. we need to account for that as well probably as it can now be returned by getPluginDefinition().
Please try:
// Set the API version.
$action_definition = $this->actionManager->getDefinition($this->action->getPluginId());
$output['api_version'] = \array_key_exists('api_version', $action_definition) ? $action_definition['api_version'] : '1';
Instead.
I don't remember where did I commit that but the README is as in the MR. Just committing a small fix with a credit for all.
Thanks!
Now why didn't I commit this? Something must've distracted me.
Can we check if the view has been built (if (!$this->view->built))?
BTW.. I cannot reproduce this locally now, created 100 articles using devel_generate and saved them all on a standard content view with VBO field in place of the core bulk operations field, batching off, no error.
Thanks all!
Fixed according to my last comment. No big deal and this'll not hang here forever ;) Thanks all!
Please see #9 ✨ Allow sorting of actions Needs work before marking this as RTBC again.
We don't need to store weight here at all, it's enough to reorder actions in view config.
Otherwise this is great work and it's very close, hopefully someone will fix the config issue sometime soon.
This issue is closed, please create a follow-up.
OFC you can open a generic issue to correct return types across the entire module as we found 2 already.
The thing is that every entity loading method can return NULL. See Drupal\Core\Entity\EntityStorageInterface::load() that is at the very bottom of all this and has a correct return type specified.
I think for the sake of correct docs we should change the return type to ?EntityInterface as proposed by Alec but the real issue here is that Drupal\group\Plugin\Group\RelationHandlerDefault::getRelationshipLabel() doesn't account for the situation when NULL is actually returned and that can always be a case. Can we change it to something like:
/**
* {@inheritdoc}
*/
public function getRelationshipLabel(GroupRelationshipInterface $group_relationship) {
$entity = $group_relationship->getEntity();
if ($entity === NULL) {
$label = $this->t('Deleted entity: @plugin (@id)', [
'@plugin' => $group_relationship->getPluginId(),
'@id' => $group_relationship->getEntityId(),
];
}
else {
$label = $group_relationship->getEntity()->label();
}
return $label;
}
By the way, Drupal\group\Plugin\Group\RelationHandler\UiTextProviderInterface::getRelationshipLabel() return type is also incorrect as it can return a string and not only \Drupal\Core\StringTranslation\TranslatableMarkup. I'd probably start using type hints instead of those misleading comments across the entire project.
I'd say let's solve the issue with getRelationshipLabel here and leave type hints to the maintainer as it's a much bigger general work and out of scope here.
As I stated many times already - we cannot add code for every module that is not compatible with VBO as we'd end with 100 of if ($moduleHandler->moduleExists('module_X')) {. What we can do is dispatch an event or invoke a hook for the incompatible module to use and work together.
Thank you for the work done here!
Please check
https://www.drupal.org/project/group_clone/releases/3.0.x-dev →
that also fixes the deep cloning setting as previously referenced entities were not cloned if set. If you'd like to add some improvements there, please feel free to create additional issues.
I didn't change things like the cloner plugin name as that'd break backwards compatibility and every custom plugin would have to be renamed as well..
We'll have to close those V3 issues sometime soon.
Not sure if I included all..
Maybe you can consider using https://www.drupal.org/project/vbo_export → on the view instead?
alexpott → credited Graber → .
Haven't checked actions_permissions since quite some time, it has no test coverage and it's not used in any of my current projects so it may be something there got broken after some recent VBO update.
Contributions welcome, I can review when there's a MR ready.
$language->id() -> $language->getId()
See ViewsBulkOperationsFormTrait::calculateEntityBulkFormKey(), where base field value is clearly on position 0 and not 3. It's not always equal to entity ID.
Thanks!
Here's a patch for 1.0.x including a language negotiator plugin. We can include that in 3.0.x as well.
Also wording is worse than in the original. Although +1 for including the field name.
Thanks, can't believe I missed this. The solution is to add a use statement.
I think that's fixed in https://www.drupal.org/project/group_clone/issues/3417819 📌 Group entity reference field values always deep-cloned Fixed .