Consider disabling discounts by default

Hello rszrama,

I totally agree with you, working on my last client project made me realize that we need to expose extra options to prevent direct publishing of a discount.

From my perspective, adding submit buttons such as "Save and publish" and "Save unpublished" seams to be a good start. If we are moving to that direction, we also want to git rid of "enable/disable" dropdown list because the action buttons will take care of that. Moreover, it would be nice to display the current discount status somewhere in the discount form (i mean, the discount edition form).

I'm assigning this issue to myself, will propose a patch soon.

Hello guys,

Please find attached a first attempt to tackle this issue. I've updated both css and sass files (I'm not sure the sass files are still maintained).

The testing has failed, i'm updating the tests (the submit button id/name has changed).

Let me work on that.

@maintainers: In the meantime, may you test the new submit buttons and tell me what you think ?

Hello ryan,

I've updated my commit according to latest UI changes (the vertical tabs). The discount status is now clearly shown at page top and so i've removed the discount status options from the vertical tabs. The tests have normally been fixed, i'm encountering some troubles to run discount unit tests on my local machine.

New patch that aims to fix up unit tests.

Pulling strings, don't mind me...

1. +++ b/includes/commerce_discount.admin.inc
   @@ -288,10 +281,19 @@ function commerce_discount_form($form, &$form_state, CommerceDiscount $commerce_
   +    '#value' => t('Save unpublished'),
   

   This sounds a bit like it will leave the original discount live and save a draft, no? Maybe 'Save and unpublish' to be like the other one?

2. +++ b/sass/commerce_discount.scss
   @@ -7,6 +7,12 @@
   +    width: 180px;
   

   Try to avoid setting fixed widths in the CSS. It will reek a bit of havoc with other admin themes.

+++ b/commerce_discount.test
@@ -227,11 +227,10 @@ class CommerceDiscountTest extends CommerceDiscountTestBase {
+    $this->assertFieldById('edit-submit-publish', t('Save and publish'), 'Save discount button is present');
...
+    $this->drupalPost(NULL, array(), t('Save and publish'));

@@ -246,7 +245,7 @@ class CommerceDiscountTest extends CommerceDiscountTestBase {
+    $this->drupalPost(NULL, $values, t('Save and publish'));

@@ -295,8 +294,7 @@ class CommerceDiscountTest extends CommerceDiscountTestBase {
+    $this->assertFieldById('edit-submit-publish', t('Save and publish'), 'Save discount button is present');

@@ -308,7 +306,7 @@ class CommerceDiscountTest extends CommerceDiscountTestBase {
+    $this->drupalPost(NULL, $values, t('Save and publish'));

@@ -321,7 +319,7 @@ class CommerceDiscountTest extends CommerceDiscountTestBase {
+    $this->drupalPost(NULL, $values, t('Save and publish'));

+1 for changing to "Save and activate"

Thank you joelpittet, rszrama and mglaman,

I'll refactor the patch according to your observations.

Hey guys,

Here is an another attempt to solve this issue. I updated the action buttons' labels as following :

• Save and publish => Save and activate
• Save unpublished => Save and disable

The discount status is displayed on top of the discount page hence the operator/administrator can clearly see discount current status.

This patch will be impacted by #2600044: Merge Discount Date and Usage into Commerce Discount.

This no longer applies because of #2600044: Merge Discount Date and Usage into Commerce Discount

Re-roll

+++ b/sass/commerce_discount.scss
@@ -7,6 +7,16 @@
+  #edit-status {
+    background-color: #eee;
+    padding: 10px;
+    display: inline-block;
+
+    label {
+      display: inline;
+    }
+  }
+

Shouldn't this be removed, as it's now part of vertical tabs?

Instead of disabling as default, which creates a crazy workflow, how about a configuration option for Discounts in general that says, "only allow access to this discount via a coupon or Giftcard." Something like that?

Just a reroll.