Problem/Motivation

The delete operation for job items does not work. When you try to delete an item (/admin/tmgmt/items/7/delete, the delete operation for job items on /admin/tmgmt/jobs/7). The error is:

Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException: The "tmgmt_job_item" entity type did not specify a "delete" form class. in Drupal\Core\Entity\EntityManager->getFormObject() (line 309 of core/lib/Drupal/Core/Entity/EntityManager.php).

And when you take a look at the JobItem: "delete" = "Drupal\tmgmt\Form\JobItemDeleteForm"

That class does not exist though...

Proposed resolution

Add JobItemDeleteForm which extends from EntityDeleteForm and redirects back to the job to which the item used to belong.

Remaining tasks

Review patch, commit.

User interface changes

None.

API changes

None.

Data model changes

None.

Comments

joshi.rohit100’s picture

Status: Active » Needs review
StatusFileSize
new624 bytes
berdir’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

we can't just use that. the default delete form assumes the existence of a collection route/link template and we don't have that and can't add it.

So we still need a custom class that extends from that and overrides the cancel/redirect Url methods to go back to the job this item belongs to.

Sounds like we need test coverage here.

joshi.rohit100’s picture

Assigned: Unassigned » joshi.rohit100

working !

joshi.rohit100’s picture

Status: Needs work » Needs review
StatusFileSize
new973 bytes

Without tests

joshi.rohit100’s picture

StatusFileSize
new2.24 KB

With Test case.

Status: Needs review » Needs work

The last submitted patch, 5: 2545356-5-with-test-case.patch, failed testing.

LKS90’s picture

Issue summary: View changes
joshi.rohit100’s picture

Status: Needs work » Needs review
StatusFileSize
new2.17 KB

Just changes in the test case (in job creation).

LKS90’s picture

  1. +++ b/src/Tests/TMGMTUiTest.php
    @@ -813,4 +813,34 @@ class TMGMTUiTest extends TMGMTTestBase {
    +    $this->assertLink('Delete');
    +
    +    $this->clickLink('Delete');
    

    Wouldn't just clicking the link be enough? (delete the two lines at the top, leave only the clickLink)

  2. +++ b/src/Tests/TMGMTUiTest.php
    @@ -813,4 +813,34 @@ class TMGMTUiTest extends TMGMTTestBase {
    +    // Delete the share message.
    

    The what? I guess copy paste? :D

joshi.rohit100’s picture

StatusFileSize
new2.17 KB
new471 bytes

Sorry my bad ):

The last submitted patch, 8: 2545356-8.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 10: 2545356-10.patch, failed testing.

joshi.rohit100’s picture

Don't know why delete link is not there ):

LKS90’s picture

The delete link only appears after submitting the job to the translator. Insert a drupalPostForm('admin/tmgmt/jobs/' . $job->id(), [], t('Submit to translator')) before you do the drupalGet to the job and it should work.

Edit: You are also missing the permission to delete stuff (I think, there is a 403 in the test when you try to delete the job [you want to delete job items by the way]).
Edit-2: Yep, $this->loginAsAdmin(); in the beginning of the method and it passes :).

berdir’s picture

There's a special delete permission, you might be missing that with your test user.

joshi.rohit100’s picture

Status: Needs work » Needs review
StatusFileSize
new2.29 KB

@berdir : I couldn't find the delete permission so I just commanded the admin user to do that.

@LKS90 : thanks for the help.

joshi.rohit100’s picture

Finally green sigh!

berdir’s picture

Status: Needs review » Needs work
Issue tags: -Needs tests
+++ b/src/Form/JobItemDeleteForm.php
@@ -0,0 +1,37 @@
+    $job_id = $this->getEntity()->getJobId();
+    $job_entity = \Drupal::entityManager()
+       ->getStorage('tmgmt_job')
+       ->load($job_id);
+    return $job_entity->urlInfo('canonical');

You can write this in a single line: return $this->getEntity()->getJob()->urlInfo();

joshi.rohit100’s picture

I will do that :)

joshi.rohit100’s picture

Status: Needs work » Needs review
StatusFileSize
new2.13 KB
new663 bytes
joshi.rohit100’s picture

StatusFileSize
new2.11 KB
new353 bytes

forgot minor change.

Diff with #20.

berdir’s picture

Status: Needs review » Fixed

Thanks, committed.

Status: Fixed » Closed (fixed)

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