Add abort action to JobItemForm

Button has been added to JobItemForm that allows direct abortion of single Job item.

1. +++ b/src/Form/JobForm.php
   @@ -466,13 +466,13 @@ class JobForm extends TmgmtFormBase {
          $actions['abort_job'] = array(
   -        '#type' => 'submit',
   ...
   +         '#type' => 'submit',
   

   You wrongly changed the indentation here. Please revert.

2. +++ b/src/Form/JobItemForm.php
   @@ -32,6 +32,7 @@ class JobItemForm extends TmgmtFormBase {
   +  ¶
   

   Also your patch contains 7 tabs. See Drupal coding standards and fix.

3. +++ b/src/Form/JobItemForm.php
   @@ -163,8 +164,20 @@ class JobItemForm extends TmgmtFormBase {
   +    if($item->isActive() || $item->isNeedsReview()){
   +    $actions['abort_job'] = array(
   ...
   +      '#access' => $job->isAbortable(),
   

   Wrong indentation and spaces.. Also the button IMHO should be added unconditionally, the key should not be 'abort_job', and #access should check isAbortable() on $item with the conditions above.

   The JobItemListBuilder::getDefaultOperations() should also use the new isAbortable().

And finally a test should click this button.

Here are code changes based on review.

Good work!, but some small things should be fixed first.

+++ b/src/Entity/JobItem.php
@@ -571,6 +571,19 @@ class JobItem extends ContentEntityBase implements JobItemInterface {
+  public function  isAbortable() {

1. One extra space that should not be there, between function and isAbortable()

+++ b/src/Form/JobItemForm.php
@@ -163,8 +163,16 @@ class JobItemForm extends TmgmtFormBase {
+      '#access' => $item->isAbortable(),
+      '#weight' => 10,
+    );  ¶

2. I will vote for '#weight' => 15

+++ b/src/Tests/TMGMTUiTest.php
@@ -354,6 +354,7 @@ class TMGMTUiTest extends EntityTestBase {
+    $this->clickLink(t('Abort Item'));

3. This can work if first you get to the page, I think it should be 'admin/tmgmt/items/1' and post the form with the 'Abort Item' button.

+++ b/src/Entity/JobItem.php
@@ -571,6 +571,19 @@ class JobItem extends ContentEntityBase implements JobItemInterface {
+   * ¶

+++ b/src/Form/JobItemForm.php
@@ -32,7 +32,7 @@ class JobItemForm extends TmgmtFormBase {
+  ¶

@@ -163,8 +163,16 @@ class JobItemForm extends TmgmtFormBase {
+    ¶
...
+    );  ¶

@@ -196,6 +204,7 @@ class JobItemForm extends TmgmtFormBase {
+    ¶

@@ -206,6 +215,7 @@ class JobItemForm extends TmgmtFormBase {
+    ¶

@@ -260,7 +270,7 @@ class JobItemForm extends TmgmtFormBase {
+      else {  ¶

4. And finally some small white spaces that should not be there :)

Refactored code, bug fixing.

+++ b/src/Entity/JobItem.php
@@ -571,6 +571,19 @@ class JobItem extends ContentEntityBase implements JobItemInterface {
   /**
+   * ¶
+   * @return boolean

+++ b/src/Form/JobItemForm.php
@@ -32,7 +32,7 @@ class JobItemForm extends TmgmtFormBase {
   protected $entity;
-
+  ¶

@@ -160,11 +160,19 @@ class JobItemForm extends TmgmtFormBase {
     return $form;
   }
-
+  ¶
...
     $item = $this->entity;
-
+    ¶

+++ b/src/Tests/TMGMTUiTest.php
@@ -467,7 +468,7 @@ class TMGMTUiTest extends EntityTestBase {
     $this->assertText('test_source:test_suggestion_suggestion:7');
 
   }
-
+  ¶

1. We still have some white spaces.

+++ b/src/Form/JobItemForm.php
@@ -160,11 +160,19 @@ class JobItemForm extends TmgmtFormBase {
+      '#value' => t('Abort item'),

2. Please use 'Abort Item'

+++ b/src/Tests/TMGMTUiTest.php
@@ -354,6 +354,7 @@ class TMGMTUiTest extends EntityTestBase {
+    $this->drupalPostForm('admin/tmgmt/items/1', NULL, t('Abort Item'));

3. This looks fine but instead of hardcoding the id, should concatenate the id of $item4. Also please use a empty array instead of NULL. And finally after that line add a new line below, post again with a NULL path, empty array and 'Confirm' button. :)

+++ b/src/Form/JobItemForm.php
@@ -160,11 +160,19 @@ class JobItemForm extends TmgmtFormBase {
+      '#weight' => 15,

And also it seems that this weight is still too 'light', let's use 20 to improve the UI. :)

I changed test for button and remove white space.

I forgot patch.

Looks good. :)

Screenshot of JobItemForm.

Yeah that looks pretty good. But not yet RTBC. :-)

1. +++ b/src/Entity/JobItem.php
   @@ -571,6 +571,19 @@ class JobItem extends ContentEntityBase implements JobItemInterface {
   +    if ($this->isActive() || $this->isNeedsReview()) {
   +      return $this->isActive();
   

   I guess you want to return TRUE.

2. +++ b/src/Form/JobItemForm.php
   @@ -165,6 +165,14 @@ class JobItemForm extends TmgmtFormBase {
   +      '#value' => t('Abort Item'),
   
   +++ b/src/Tests/TMGMTUiTest.php
   @@ -354,6 +354,8 @@ class TMGMTUiTest extends EntityTestBase {
   +    $this->drupalPostForm('admin/tmgmt/items/' . $item4->id(), array(), t('Abort Item'));
   

   I would prefer "Abort".

Discussed with @miro_dietiker, it is fine if we use Abort Item. But yes, it should return TRUE if the item is active or if it needs review.

I changed text on button, and change return true instead of isActive(). First I wrote isActive(), because isActive() return 1(true);

I like it now :)

Created follow-up about merging abort button with save
#2705201: Make abort (item) a link instead of a button

Committed, as John proposed, with "Abort Item" variant.