The last submitted patch, 1: drupal_2201137_1.patch, failed testing.
- ๐ฌ๐งUnited Kingdom Xano Southampton
I changed the order of the checks when building entity operation links for two reasons:
- Checking whether the required link path must always be done, but is faster than checking for entity access. Therefore if an entity does not support duplication, there is less of a performance hit in this order.
BlockAccessController
does not support checking access for this operation and this order prevents it from being called unnecessarily, which prevents the test failures.
- ๐ฎ๐นItaly mondrake ๐ฎ๐น
This is very nice and works well :)
For testing, I have tried to implement this in image module, to duplicate Image Styles. See do not test patch attached (which also works well :)) I suppose that will be a followup anyway.
- ๐ฌ๐งUnited Kingdom Xano Southampton
I see your route uses a permission, rather than entity access, which defeats the purpose of this issue :) My patch does not only add the operation link, but also adds access control for the
duplicate
operation. - ๐ฎ๐นItaly mondrake ๐ฎ๐น
Aha. I was just copying from the other form routes :) which are checking permission. I suppose we may even change those too as the 'admin_permission' key in the ImageStyle annotation would take over the fact that no custom access controller is there?
Anyway we are probably going OT here, created #2204087: Introduce a "duplicate" image style operation โ as a follow up, see you there :)
- ๐ฌ๐งUnited Kingdom damiankloip
+++ b/core/lib/Drupal/Core/Entity/EntityListController.php @@ -106,6 +106,12 @@ public function getOperations(EntityInterface $entity) { + 'title' => t('Duplicate'),
Some sort of $this->t?
- ๐ฌ๐งUnited Kingdom damiankloip
+++ b/core/lib/Drupal/Core/Entity/EntityAccessController.php @@ -129,6 +129,13 @@ protected function processAccessHookResults(array $access) { + // Duplication is the act of creating a new entity based on the information + // that an existing entity contains. A user therefore needs to 'view' the + // existing entity in order to create a new one.
This just doesn't see right. If you are duplicating an entity. You need create access atleast?
So maybe this could be simplified more, and create == duplicate or something? Rather than having to have a cumbersome condition in the EntityAccessController?
- ๐ฌ๐งUnited Kingdom Xano Southampton
Re-roll.
Some sort of $this->t?
Done.
- ๐ฌ๐งUnited Kingdom Xano Southampton
I just talked to @dawehner and he was okay with converting Views' clone operation to duplicate in the UI, so that's what this patch adds.
- ๐ฌ๐งUnited Kingdom Xano Southampton
10: drupal_2201137_10.patch queued for re-testing.
The last submitted patch, 10: drupal_2201137_10.patch, failed testing.
- ๐ฌ๐งUnited Kingdom Xano Southampton
Re-roll.
Classifying this as a task, as it simply consolidates the different duplicate/clone operations in core.
The last submitted patch, 13: drupal_2201137_13.patch, failed testing.
- ๐ฌ๐งUnited Kingdom Xano Southampton
15: drupal_2201137_15.patch queued for re-testing.
The last submitted patch, 15: drupal_2201137_15.patch, failed testing.
- ๐ฉ๐ชGermany tstoeckler Essen, Germany
+++ b/core/modules/views_ui/views_ui.module @@ -54,7 +54,7 @@ function views_ui_entity_type_build(array &$entity_types) { + ->setLinkTemplate('duplicate-form', 'views_ui.clone')
Hmm... should we also rename the route to "views_ui.duplicate"?
Other than that, looks RTBC to me.
- ๐ฌ๐งUnited Kingdom Xano Southampton
I decided not to change any of Views' internals where I didn't have to, as not to make this patch too complex. I'd rather do all that in a follow-up.
- ๐ฌ๐งUnited Kingdom damiankloip
You might as well just change the route name too? As you seem to have changed basically every other instance of views UI using 'clone' to 'Duplicate' anyway. In fact that is 90% of the patch? :)
- ๐ฌ๐งUnited Kingdom Xano Southampton
I only changed UI texts, with the exception of the actual operation so it inherits from the default list builder and access controller (which is what the patch is about). There is a lot more that will need to be changed, among which are route and form names.
- ๐ฌ๐งUnited Kingdom damiankloip
Sorry, but some UI text was changed but others not :)
We have a 'Duplicate' value for the submit label, but still 'Clone' for the form title, name default value, and placeholder text. AS well are a path still using 'clone' - That seems confusing for people, even if it is only in the interim.
Plus the changes are pretty minimal IMO.
The last submitted patch, 25: 2201137-25.patch, failed testing.
- ๐ฉ๐ชGermany tstoeckler Essen, Germany
OK, awesome. This time I actually applied the patch and clicked around and verified that all instances of 'Clone' are now 'Duplicate'....
...except for the "Clone attachment" thing, which is still there (see screenshot). I don't know if that has to done now or can be done in a follow-up, though. If the latter, this is RTBC.
- ๐ฌ๐งUnited Kingdom damiankloip
Oh, that part is totally different :) That is implemented internally in the views UI.
- ๐ฌ๐งUnited Kingdom damiankloip
Created an issue for that, #2252743: Rename 'clone' to 'duplicate' for displays in Views UI โ .
- ๐ฉ๐ชGermany tstoeckler Essen, Germany
Awesome, thanks. Since there's agreement on not tackling that here, marking RTBC.
- ๐ฌ๐งUnited Kingdom damiankloip
Thanks Xano, tstoeckler. Sorry for gatecrashing.
- ๐ฉ๐ชGermany tstoeckler Essen, Germany
Not at all! If I would have applied and tried out the patch the first time around (which I should have done regardless!) I would have complained about the same thing :-)
- ๐ฌ๐งUnited Kingdom Xano Southampton
Cool. Thanks for the work, guys!
The last submitted patch, 27: 2201137-27.patch, failed testing.
- ๐ฉ๐ชGermany sun Karlsruhe
+++ b/core/lib/Drupal/Core/Entity/EntityAccessController.php @@ -132,6 +132,14 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A + // Duplication is the act of creating a new entity based on the information + // that an existing entity contains. A user therefore needs to 'view' the + // existing entity in order to create a new one. + if ($operation == 'duplicate') { + return $this->access($entity, 'view', $langcode, $account) && $this->createAccess($entity->bundle(), $account); + }
Hm. This doesn't look correct to me. The base permission should be 'edit', not 'view'.
If you only have view access, then you do not know what values the entity actually contains. In order to see the (raw) values and duplicate them, you need 'edit' access.
Otherwise, you'd be able to duplicate an entity that contains a private value of another user. Previously you may only had 'view' access, because it wasn't your entity โ just duplicate it, and the duplicate is yours, tadaa, Happy Easter Egg! :)
- ๐ฌ๐งUnited Kingdom Xano Southampton
If you only have view access, then you do not know what values the entity actually contains.
Not really. You're allowed to view an entity, but whether you get to see everything that's contained within it, is up to the site builder.
It should definitely not be edit. What if I don't want anyone to mess with my entities, but I'm fine with them seeing the contents?
I do understand your concern. Maybe we must leave this open to the individual entity access controllers?
- ๐ฆ๐บAustralia realityloop
To me View and Create makes sense.
A user with view rights could theoretically create a duplicate manually, so why not let them do it programatically.
The last submitted patch, 36: drupal_2201137_36.patch, failed testing.
- ๐ฉ๐ชGermany tstoeckler Essen, Germany
Otherwise, you'd be able to duplicate an entity that contains a private value of another user. Previously you may only had 'view' access, because it wasn't your entity โ just duplicate it, and the duplicate is yours, tadaa, Happy Easter Egg! :)
This still needs to be resolved.
A user with view rights could theoretically create a duplicate manually, so why not let them do it programatically.
I don't understand this argument. Programmatically, you can absolutely do anything. You can drop the entire database if you feel like it. And you can load/edit/update/delete entities like crazy, no matter what. There is no notion of 'current_user' in code, that is bound to a request.
- ๐ฌ๐งUnited Kingdom Xano Southampton
So the conflict here is tha we have view access/permissions for entities, but they don't define what exactly people can see. That all depends on whatever display modes the site builder configured, but theoretically the permissions and access check let you see everything that's in an entity.
- ๐จ๐ฆCanada mgifford Ottawa, Ontario
This no longer applies. Not sure what happened to the EntityAccessController.php
- ๐จ๐ฆCanada mgifford Ottawa, Ontario
There has been no new work on this issue in quite some time. So I'm assuming the person assigned is no longer being actively pursuing it. Sincere apologies if this is wrong.
Drupal 8.1.0-beta1 โ was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule โ and the Allowed changes during the Drupal 8 release cycle โ .
- ๐ง๐ชBelgium swentel
Sounds like a cool feature for 8.1 to me.
I've looked at the permission that node_clone uses and it checks for view and create, which sounds sane to me. The module goes even further and defines additional separate permissions so that you can even kill the ability of duplicating even if the user has view and create access, which in a way make much sense too, but that would mean we'd have to auto generate those based on any entity type in your installation which could make the permission screen way bigger. Granted, it would be extremely powerful then, but maybe not worth it.
// $access also takes a separate permission 'clone node' & 'clone own node' $access = $access && node_access('view', $node) && node_access('create', $node->type);
The last submitted patch, 49: drupal_2201137_49.patch, failed testing.
The last submitted patch, 51: drupal_2201137_51.patch, failed testing.
The last submitted patch, 51: drupal_2201137_51.patch, failed testing.
Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule โ and the Allowed changes during the Drupal 8 release cycle โ .
Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule โ and the Allowed changes during the Drupal 8 release cycle โ .
Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule โ and the Allowed changes during the Drupal 8 release cycle โ .
Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule โ and the Allowed changes during the Drupal 8 release cycle โ .
Drupal 8.2.0-beta1 โ was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule โ and the Allowed changes during the Drupal 8 release cycle โ .
Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule โ and the Allowed changes during the Drupal 8 release cycle โ .
- ๐ฌ๐งUnited Kingdom AaronMcHale Edinburgh, Scotland
Not sure if this is something that people still want as there hasn't been recent activity, but I'm of the opinion that it would be better to define a new "clone"/"duplicate" permission.
I'm sure there would be many advantages to this but off the top of my head, it gives the administrator more control over where and who can have access to this (or they can simply use permissions to disable this altogether if they don't want it), and it means we don't need to try and guess what the optimal combination of permissions are (maybe some sites only want those with edit access to duplicate, maybe some are fine with those who have view access being able to duplicate).
I think for us to do anything other than provide a new permission risks us making too many assumptions about Drupal sites, and so I don't think us making those assumptions is in line with the Drupal ethos.
Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9โs release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule โ and the Allowed changes during the Drupal 8 and 9 release cycles โ .
Drupal 8.9.0-beta1 โ was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule โ and the Allowed changes during the Drupal 8 and 9 release cycles โ .
- ๐ฉ๐ชGermany geek-merlin Freiburg, Germany
This still makes sense to me.
- ๐ฉ๐ชGermany geek-merlin Freiburg, Germany
Still makes sense, but needs reroll.
The last submitted patch, 68: 2201137_68.patch, failed testing. View results โ
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.Drupal 9.1.0-alpha1 โ will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule โ and the Allowed changes during the Drupal 9 release cycle โ .
Drupal 9.2.0-alpha1 โ will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule โ and the Allowed changes during the Drupal core release cycle โ .
- ๐ฉ๐ชGermany geek-merlin Freiburg, Germany
@sanjayk Great work! I can only provide a pointer here:
We are in a KernelTest which means we can only rely on parts of Drupal that we explicitly set up.
So \Drupal::getContainer coughs on any ::service() call.
The stack trace looks like this line is to blame:
assert(\Drupal::service('cache_contexts_manager')->assertValidTokens($cache_contexts));
Background on this (but no practical experience on my side):
- http://drupalcampohio.org/sites/default/files/slides/dco2015-mocking-dru...
- https://drupal.stackexchange.com/questions/226340/drupalcontainer-is-not... - ๐ญ๐บHungary pasqualle ๐ญ๐บ Budapest
Core definitely need to provide some guidance here..
https://www.drupal.org/project/entity_clone โ
https://www.drupal.org/project/replicate โ
https://www.drupal.org/project/cloner โ Drupal core is moving towards using a โmainโ branch. As an interim step, a new 11.x branch has been opened โ , as Drupal.org infrastructure cannot currently fully support a branch named
main
. New developments and disruptive changes should now be targeted for the11.x
branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule โ and the Allowed changes during the Drupal core release cycle โ .Drupal 9.5.0-beta2 โ and Drupal 10.0.0-beta2 โ were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule โ and the Allowed changes during the Drupal core release cycle โ .
Drupal 9.4.0-alpha1 โ was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule โ and the Allowed changes during the Drupal core release cycle โ .
Drupal 9.3.0-rc1 โ was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule โ and the Allowed changes during the Drupal core release cycle โ .
- ๐ซ๐ฎFinland jhuhta
I'm not sure if this issue is relevant anymore, or could the related issue ๐ It is not possible to react to an entity being duplicated Needs work provide some of what's needed here. Closing as duplicate, but feel free to reopen if appropriate.
- Status changed to Active
about 1 year ago 12:22pm 10 November 2023 - ๐ฉ๐ชGermany Anybody Porta Westfalica
@jhuhta reading the issue summary I don't think this is a duplicate. This is for example about the controller / ui for duplication in core.
- Status changed to Needs work
11 months ago 10:57pm 8 January 2024 - ๐ซ๐ทFrance andypost
There's dependent issue to duplicate image styles!
so here patches needs to be converted to merge request, change covered with test and change recordIt's a part of global #2095603: [meta] Complete Entity Field API โ