Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir created an issue. See original summary.

DiDebru’s picture

Subscribe.

DiDebru’s picture

I tried to put an operation link to the view but the Url::routeMatch(); give me

Symfony\Component\Routing\Exception\MissingMandatoryParametersException: Some mandatory parameters are missing ("node") to generate a URL for route "entity.node.replicate". in Drupal\Core\Routing\UrlGenerator->doGenerate() (line 180 of core/lib/Drupal/Core/Routing/UrlGenerator.php).
if ($entity instanceof NodeInterface) {
    $operations['replicate'] = [
      'title' => 'Replicate',
      'weight' => 45,
      'url' => $entity->toUrl('replicate'),
      'query' => [
        'destination' => '/admin/content',
      ]
    ];
  }

worked :)

casey’s picture

Status: Active » Needs review
FileSize
1.07 KB
casey’s picture

FileSize
1.14 KB
slydevil’s picture

Status: Needs review » Needs work

This patch no longer applies cleanly...need to re-roll.

slydevil’s picture

slydevil’s picture

Status: Needs work » Needs review

This patch requires the patch from #2904662 first.

slydevil’s picture

Related issues: +#2904662: missing hook_help
slydevil’s picture

Status: Needs review » Reviewed & tested by the community

All good!

Berdir’s picture

+++ b/replicate_ui.module
@@ -45,3 +46,31 @@ function replicate_ui_entity_type_alter(array $entity_types) {
+        'query' => \Drupal::service('redirect.destination')->getAsArray(),

does this really make sense? I don't want to be redirected back to the current page, I most likely want to edit the entity that I just duplicated like with the tab?

Should also have tests now that we have branch test enabled. And I'm not quite sure yet if this really makes sense to have by default everywhere.

Berdir’s picture

Status: Reviewed & tested by the community » Needs work
mbovan’s picture

I addressed #11 and wrote a test.

bucefal91’s picture

Hello guys!

I am just willing to make a small adjustment in the latest patch related to UX. I think Berdir meant a bit different thing in his #12 comment (at least I understood it differently): We are not willing set up any destination at all so the replicate form in its submission handler naturally would redirect us to the just created (replicated) entity. If we place the destination and point it to the source of replication (like patch #13 does), it produces poor UX -- you see a message of successful replication but the entity you see on your screen is the old (without this " (copy)" prefix) and it creates a feeling of something going wrong.

So I am unsetting the 'destination' from the "replicate" operation. But there's one extra problem. If you just unset it and then use the "Operations" dropdown button in any view. Many admin views enable the "Include destination" checkbox so user is taken back to the view. I do not think this is the case for the "replicate" operation where the admin user is likely to need to further edit the replicated entity. So just to make sure the EntityOperations views field does not inject its undesired (in this specific case of "replicate" operation) destination, I am setting it to NULL.

In this adjusted patch I am attaching I do confirm replicating from an operation dropdown in a view still takes you to the replicated entity.

Oh, and there was a double new line in the patch #13, which I took care of too.

Berdir’s picture

Status: Needs review » Needs work

Yes, that makes more sense, it shouldn't redirect to the entity that is being cloned.

We should also improve the test a bit and not just assert a 200 response but also that we're on the correct page and possibly also the confirmation message.

mbovan’s picture

Status: Needs work » Needs review
FileSize
3.55 KB
2.42 KB

Improved tests and addressed #15.

Status: Needs review » Needs work

The last submitted patch, 16: 2900862-replicate-operation-16.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mbovan’s picture

Status: Needs work » Needs review
FileSize
3.54 KB
803 bytes

Fixed tests.

  • Berdir committed 77fd155 on 8.x-1.x authored by mbovan
    Issue #2900862 by mbovan, casey, bucefal91: Add an entity operation to...
Berdir’s picture

Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.