Merge pull request 'Let a studio choose its default payment method, and disconnect Stripe again' (#174) from feature/173-default-payment-method into main

Reviewed-on: #174

CHANGELOG.md

@@ -13,6 +13,10 @@ each change under the current top section as you work.
 
 ## [1.5.2]
 
+### Added
+- **You can now choose which payment method the studio bills by, instead of it following your Stripe keys.** Saving Stripe keys used to move every student onto credit-card billing the moment they were entered — there was no way to have Stripe live and still bill by e-transfer while you satisfied yourself that card payments worked. **Studio Settings → Billing → Default payment method** now makes that an explicit choice between **Credit card** and **E-transfer**. Leaving it on E-transfer with Stripe configured lets you switch one student at a time to Credit card on their student detail page and watch their bookings charge for real; when you are satisfied, changing this one setting moves everyone over. Credit card remains the default, so a studio that adds keys and changes nothing else behaves exactly as before, and it still falls back to e-transfer until keys are saved — a card cannot be charged without them.
+- **Stripe can now be disconnected from Studio Settings.** Keys could be replaced but never removed, so a studio that set Stripe up to try it had no way back to e-transfer short of editing the database. **Clear Stripe configuration**, at the foot of the settings page whenever any Stripe value is stored, forgets the publishable key, the secret key and the webhook signing secret, and returns the mode to Test — billing falls back to e-transfer until keys are entered again. Payments already recorded are untouched, as are your currency, HST, e-transfer and registration settings. If you are disconnecting for good, delete the webhook endpoint in the Stripe Dashboard too, or it will keep sending events this site can no longer verify.
+
 ## [1.5.1]
 
 ### Fixed

docs/features/payments.md

@@ -34,6 +34,15 @@ page (`manage_billing`, studio admin only):
 | `us_currency`                | Default ISO 4217 currency, e.g. `CAD`  |
 | `us_etransfer_email`         | Studio-default e-transfer destination  |
 | `us_hst_rate`                | Default HST/tax percentage, e.g. `13`  |
+| `us_default_payment_method`  | Studio-default billing method (`card` \| `etransfer`) |
+
+Secrets are write-only in the form: a stored secret is never echoed back, and a
+blank field keeps it. To disconnect Stripe entirely, **Clear Stripe
+configuration** (shown once any Stripe value is stored) deletes the publishable
+key, secret key and webhook secret and drops the mode back to `test`
+(`StudioSettings::clearStripeConfig()`). Currency, HST, e-transfer and
+registration settings are untouched, as are payments already recorded; billing
+falls back to e-transfer until keys are entered again.
 
 ## HST / Tax
 
@@ -52,8 +61,9 @@ total when tax applies.
 ## Per-Student Billing Method
 Each student's billing method is stored in user meta `us_payment_method`, set by the
 studio admin (`Students → student detail → Billing method`). When unset, the studio
-default applies — `card` if Stripe is configured, otherwise `etransfer`
-(`BillingMethodResolver`):
+default applies (`BillingMethodResolver::defaultMethod()`): the
+`us_default_payment_method` option, degraded to `etransfer` whenever Stripe is not
+configured, since a card cannot be charged without keys.
 
 | Method     | Behaviour                                                              |
 |------------|-----------------------------------------------------------------------|
@@ -61,6 +71,22 @@ default applies — `card` if Stripe is configured, otherwise `etransfer`
 | `etransfer`| Payment row created `pending`; admin marks it `paid` when funds arrive |
 | `comp`     | No charge; registration is confirmed immediately, no payment row required |
 
+## Studio Default Billing Method
+**Studio Settings → Billing → Default payment method** (`manage_billing`) chooses
+between `card` and `etransfer` for every student without an override. Card is the
+default, so a studio that adds Stripe keys and changes nothing else behaves as it
+always has.
+
+Setting it to `etransfer` is the **staged rollout** path: Stripe stays live, but
+the studio keeps billing by e-transfer while individual students are switched to
+`card` on their student detail page. Their bookings exercise real Stripe charges
+end to end; once that is proven, flipping the studio default to `card` moves
+everyone at once and the per-student overrides can be cleared.
+
+`comp` is deliberately not offered as a studio default — it is a per-student
+decision, and a studio-wide `comp` would silently stop billing everybody. A stored
+value that is neither `card` nor `etransfer` reads back as `card`.
+
 ## E-transfer Destination Email
 Where students send e-transfers is resolved and **frozen onto the payment** at
 booking time (`us_payments.etransfer_email`), so each record keeps the destination

src/Payment/BillingMethodResolver.php

@@ -7,8 +7,8 @@ use Unsupervised\Schedular\Val;
 
 /**
  * Resolves the billing method for a student: a per-student override if set,
- * otherwise the studio default — card when Stripe is configured, e-transfer when
- * it is not.
+ * otherwise the studio default chosen on Studio Settings — which itself falls
+ * back to e-transfer whenever Stripe is not configured.
  */
 class BillingMethodResolver {
 
@@ -27,10 +27,17 @@ class BillingMethodResolver {
 
 	/**
 	 * The studio default when a student has no explicit override.
+	 *
+	 * Card is only ever the default when the studio asked for it *and* Stripe is
+	 * configured; without keys there is nothing to charge a card with. A studio
+	 * that sets the default to e-transfer keeps every student on e-transfer even
+	 * with Stripe live, so card billing can be proven on a few students — each
+	 * given a per-student override — before the whole studio moves over.
 	 */
 	public function defaultMethod(): string {
-		return $this->settings->isStripeConfigured()
-			? Payment::METHOD_CARD
-			: Payment::METHOD_ETRANSFER;
+		return Payment::METHOD_CARD === $this->settings->defaultPaymentMethod()
+			&& $this->settings->isStripeConfigured()
+				? Payment::METHOD_CARD
+				: Payment::METHOD_ETRANSFER;
 	}
 }

src/Payment/StudioSettings.php

@@ -16,6 +16,14 @@ class StudioSettings {
 	public const OPT_ETRANSFER_EMAIL = 'us_etransfer_email';
 	public const OPT_HST_RATE        = 'us_hst_rate';
 
+	/**
+	 * The studio-wide default billing method for students with no per-student
+	 * override. Card is the default; setting it to e-transfer holds every student
+	 * on e-transfer even once Stripe is live, so card billing can be trialled on a
+	 * few students before the whole studio moves over.
+	 */
+	public const OPT_DEFAULT_PAYMENT_METHOD = 'us_default_payment_method';
+
 	/**
 	 * Studio-default cancellation cutoff, stored in hours. A student may not
 	 * cancel a lesson once it starts within this many hours. Displayed to the
@@ -56,6 +64,17 @@ class StudioSettings {
 		return 'live' === get_option( self::OPT_MODE, 'test' ) ? 'live' : 'test';
 	}
 
+	/**
+	 * The studio-default billing method: `card` or `etransfer`. A card default
+	 * still degrades to e-transfer while Stripe is unconfigured — see
+	 * BillingMethodResolver, which owns that fallback.
+	 */
+	public function defaultPaymentMethod(): string {
+		return Payment::METHOD_ETRANSFER === get_option( self::OPT_DEFAULT_PAYMENT_METHOD, Payment::METHOD_CARD )
+			? Payment::METHOD_ETRANSFER
+			: Payment::METHOD_CARD;
+	}
+
 	public function currency(): string {
 		$currency = Val::string( get_option( self::OPT_CURRENCY, 'CAD' ) );
 
@@ -112,13 +131,32 @@ class StudioSettings {
 		return self::MODE_SELF_APPROVAL === $this->registrationMode();
 	}
 
+	/**
+	 * Forget every Stripe credential, returning the studio to e-transfer billing.
+	 * The mode drops back to `test` so a later re-configuration cannot go live by
+	 * inheriting the old setting.
+	 */
+	public function clearStripeConfig(): void {
+		delete_option( self::OPT_PUBLISHABLE );
+		delete_option( self::OPT_SECRET );
+		delete_option( self::OPT_WEBHOOK_SECRET );
+		delete_option( self::OPT_MODE );
+	}
+
 	public function renderPage(): void {
 		if ( ! current_user_can( RoleManager::CAP_MANAGE_BILLING ) ) {
 			wp_die( esc_html__( 'You do not have permission to manage billing settings.', 'unsupervised-schedular' ) );
 		}
 
+		$notice = '';
 		if ( isset( $_POST['usc_action'] ) && check_admin_referer( 'usc_settings_action' ) ) {
-			$this->save();
+			// phpcs:ignore WordPress.Security.NonceVerification.Missing -- verified immediately above.
+			if ( 'clear_stripe' === sanitize_key( Val::string( wp_unslash( $_POST['usc_action'] ) ) ) ) {
+				$this->clearStripeConfig();
+				$notice = __( 'Stripe configuration cleared. New registrations default to e-transfer until Stripe is set up again.', 'unsupervised-schedular' );
+			} else {
+				$this->save();
+			}
 		}
 
 		$publishableKey = $this->publishableKey();
@@ -133,6 +171,10 @@ class StudioSettings {
 		$etransferEmail   = $this->etransferEmail();
 		$hstRate          = $this->hstRate();
 		$stripeConfigured = $this->isStripeConfigured();
+		$defaultMethod    = $this->defaultPaymentMethod();
+		// Offer the clear button whenever any Stripe value lingers, not only when
+		// the pair of keys makes Stripe fully usable.
+		$stripeAnySet     = '' !== $publishableKey || $secretKeySet || $webhookSecretSet;
 		$openRegistration = $this->openRegistrationEnabled();
 		// Stored in hours, surfaced to the admin in whole days.
 		$cancellationCutoffDays = (int) round( $this->cancellationCutoffHours() / 24 );
@@ -158,6 +200,13 @@ class StudioSettings {
 		update_option( self::OPT_MODE, 'live' === $mode ? 'live' : 'test' );
 		update_option( self::OPT_CURRENCY, strtoupper( sanitize_text_field( Val::string( wp_unslash( $_POST['currency'] ?? 'CAD' ) ) ) ) );
 		update_option( self::OPT_ETRANSFER_EMAIL, sanitize_email( Val::string( wp_unslash( $_POST['etransfer_email'] ?? '' ) ) ) );
+		// Anything but an explicit e-transfer choice means card, so a mangled or
+		// missing field can never silently disable card billing studio-wide.
+		$defaultMethod = sanitize_key( Val::string( wp_unslash( $_POST['default_payment_method'] ?? Payment::METHOD_CARD ) ) );
+		update_option(
+			self::OPT_DEFAULT_PAYMENT_METHOD,
+			Payment::METHOD_ETRANSFER === $defaultMethod ? Payment::METHOD_ETRANSFER : Payment::METHOD_CARD
+		);
 		// phpcs:ignore WordPress.Security.ValidatedSanitizedInput.MissingUnslash, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Val::float() coerces to float; slashes cannot survive numeric coercion.
 		$hstRate = isset( $_POST['hst_rate'] ) ? Val::float( $_POST['hst_rate'] ) : 0.0;
 		update_option( self::OPT_HST_RATE, max( 0.0, $hstRate ) );

templates/admin/settings.php

@@ -15,6 +15,9 @@ if (! defined('ABSPATH')) {
  * @var string $etransferEmail
  * @var float  $hstRate
  * @var bool   $stripeConfigured
+ * @var string $defaultMethod
+ * @var bool   $stripeAnySet
+ * @var string $notice
  * @var bool   $openRegistration
  * @var int    $cancellationCutoffDays
  */
@@ -22,10 +25,18 @@ if (! defined('ABSPATH')) {
 <div class="wrap">
     <h1><?php esc_html_e('Studio Settings', 'unsupervised-schedular'); ?></h1>
 
+    <?php if ('' !== $notice) : ?>
+        <div class="notice notice-success inline">
+            <p><?php echo esc_html($notice); ?></p>
+        </div>
+    <?php endif; ?>
+
     <div class="notice notice-info inline">
         <p>
-            <?php if ($stripeConfigured) : ?>
+            <?php if ($stripeConfigured && 'card' === $defaultMethod) : ?>
                 <?php esc_html_e('Stripe is configured — new registrations default to credit-card billing.', 'unsupervised-schedular'); ?>
+            <?php elseif ($stripeConfigured) : ?>
+                <?php esc_html_e('Stripe is configured, but the studio default is e-transfer — only students you switch to credit card individually are billed by card.', 'unsupervised-schedular'); ?>
             <?php else : ?>
                 <?php esc_html_e('Stripe is not configured — new registrations default to e-transfer, which a studio admin marks paid on receipt. Add your Stripe keys below to enable card billing.', 'unsupervised-schedular'); ?>
             <?php endif; ?>
@@ -80,6 +91,31 @@ if (! defined('ABSPATH')) {
             </tr>
         </table>
 
+        <h2><?php esc_html_e('Billing', 'unsupervised-schedular'); ?></h2>
+        <table class="form-table">
+            <tr>
+                <th scope="row"><?php esc_html_e('Default payment method', 'unsupervised-schedular'); ?></th>
+                <td>
+                    <fieldset>
+                        <label>
+                            <input type="radio" name="default_payment_method" value="card" <?php checked($defaultMethod, 'card'); ?>>
+                            <?php esc_html_e('Credit card (requires Stripe)', 'unsupervised-schedular'); ?>
+                        </label><br>
+                        <label>
+                            <input type="radio" name="default_payment_method" value="etransfer" <?php checked($defaultMethod, 'etransfer'); ?>>
+                            <?php esc_html_e('E-transfer', 'unsupervised-schedular'); ?>
+                        </label>
+                        <p class="description">
+                            <?php esc_html_e('Applies to every student without their own billing method. Keep this on e-transfer while you trial card payments: switch individual students to Credit card under Students → student detail → Billing method, confirm their bookings charge correctly, then move the whole studio over by changing this setting.', 'unsupervised-schedular'); ?>
+                        </p>
+                        <?php if (! $stripeConfigured) : ?>
+                            <p class="description"><?php esc_html_e('Credit card has no effect until Stripe keys are saved above — until then every student is billed by e-transfer.', 'unsupervised-schedular'); ?></p>
+                        <?php endif; ?>
+                    </fieldset>
+                </td>
+            </tr>
+        </table>
+
         <h2><?php esc_html_e('E-transfer', 'unsupervised-schedular'); ?></h2>
         <table class="form-table">
             <tr>
@@ -124,4 +160,16 @@ if (! defined('ABSPATH')) {
         </table>
         <?php submit_button(esc_html__('Save Settings', 'unsupervised-schedular')); ?>
     </form>
+
+    <?php if ($stripeAnySet) : ?>
+        <h2><?php esc_html_e('Clear Stripe configuration', 'unsupervised-schedular'); ?></h2>
+        <p class="description">
+            <?php esc_html_e('Forgets the publishable key, secret key and webhook signing secret, and returns the mode to Test. Billing falls back to e-transfer until Stripe is set up again; payments already recorded are untouched.', 'unsupervised-schedular'); ?>
+        </p>
+        <form method="post" onsubmit="return confirm('<?php echo esc_js(esc_html__('Clear the stored Stripe keys? Card billing stops until you enter them again.', 'unsupervised-schedular')); ?>');">
+            <?php wp_nonce_field('usc_settings_action'); ?>
+            <input type="hidden" name="usc_action" value="clear_stripe">
+            <?php submit_button(esc_html__('Clear Stripe configuration', 'unsupervised-schedular'), 'delete', 'submit', true); ?>
+        </form>
+    <?php endif; ?>
 </div>

tests/Unit/Payment/BillingMethodResolverTest.php

@@ -22,13 +22,13 @@ class BillingMethodResolverTest extends TestCase
         self::assertSame(Payment::METHOD_COMP, $resolver->resolve(5));
     }
 
-    public function testDefaultsToCardWhenStripeConfigured(): void
+    public function testDefaultsToCardWhenStripeConfiguredAndCardChosen(): void
     {
         Functions\when('get_user_meta')->justReturn('');
 
-        $settings = Mockery::mock(StudioSettings::class);
-        $settings->shouldReceive('isStripeConfigured')->andReturn(true);
-        $resolver = new BillingMethodResolver($settings);
+        $resolver = new BillingMethodResolver(
+            $this->settings(Payment::METHOD_CARD, true)
+        );
 
         self::assertSame(Payment::METHOD_CARD, $resolver->resolve(5));
     }
@@ -37,22 +37,56 @@ class BillingMethodResolverTest extends TestCase
     {
         Functions\when('get_user_meta')->justReturn('');
 
-        $settings = Mockery::mock(StudioSettings::class);
-        $settings->shouldReceive('isStripeConfigured')->andReturn(false);
-        $resolver = new BillingMethodResolver($settings);
+        $resolver = new BillingMethodResolver(
+            $this->settings(Payment::METHOD_CARD, false)
+        );
 
         self::assertSame(Payment::METHOD_ETRANSFER, $resolver->resolve(5));
         self::assertSame(Payment::METHOD_ETRANSFER, $resolver->defaultMethod());
     }
 
+    public function testEtransferDefaultHoldsEvenWhenStripeIsLive(): void
+    {
+        Functions\when('get_user_meta')->justReturn('');
+
+        $resolver = new BillingMethodResolver(
+            $this->settings(Payment::METHOD_ETRANSFER, true)
+        );
+
+        self::assertSame(Payment::METHOD_ETRANSFER, $resolver->resolve(5));
+        self::assertSame(Payment::METHOD_ETRANSFER, $resolver->defaultMethod());
+    }
+
+    public function testPerStudentCardOverrideStillWinsUnderAnEtransferDefault(): void
+    {
+        // The trial path: the studio bills by e-transfer, one student is moved to
+        // card to prove Stripe end to end.
+        Functions\when('get_user_meta')->justReturn(Payment::METHOD_CARD);
+
+        $resolver = new BillingMethodResolver(
+            $this->settings(Payment::METHOD_ETRANSFER, true)
+        );
+
+        self::assertSame(Payment::METHOD_CARD, $resolver->resolve(5));
+    }
+
     public function testInvalidOverrideFallsBackToDefault(): void
     {
         Functions\when('get_user_meta')->justReturn('bogus');
 
-        $settings = Mockery::mock(StudioSettings::class);
-        $settings->shouldReceive('isStripeConfigured')->andReturn(true);
-        $resolver = new BillingMethodResolver($settings);
+        $resolver = new BillingMethodResolver(
+            $this->settings(Payment::METHOD_CARD, true)
+        );
 
         self::assertSame(Payment::METHOD_CARD, $resolver->resolve(5));
     }
+
+    private function settings(string $default, bool $stripeConfigured): StudioSettings
+    {
+        $settings = Mockery::mock(StudioSettings::class);
+        $settings->shouldReceive('defaultPaymentMethod')->andReturn($default);
+        $settings->shouldReceive('isStripeConfigured')->andReturn($stripeConfigured);
+
+        return $settings;
+    }
 }

tests/Unit/Payment/StudioSettingsTest.php

@@ -5,6 +5,7 @@ namespace Unsupervised\Schedular\Tests\Unit\Payment;
 
 use Brain\Monkey\Functions;
 use Unsupervised\Schedular\Auth\RoleManager;
+use Unsupervised\Schedular\Payment\Payment;
 use Unsupervised\Schedular\Payment\StudioSettings;
 use Unsupervised\Schedular\Tests\Unit\TestCase;
 
@@ -103,6 +104,60 @@ class StudioSettingsTest extends TestCase
         $this->applyMode(true);
     }
 
+    public function testDefaultPaymentMethodIsCardWhenUnset(): void
+    {
+        Functions\when('get_option')->alias(static fn (string $name, $default) => $default);
+
+        self::assertSame(Payment::METHOD_CARD, (new StudioSettings())->defaultPaymentMethod());
+    }
+
+    public function testDefaultPaymentMethodReadsStoredEtransfer(): void
+    {
+        Functions\when('get_option')->alias(static fn (string $name) =>
+            $name === StudioSettings::OPT_DEFAULT_PAYMENT_METHOD ? Payment::METHOD_ETRANSFER : '');
+
+        self::assertSame(Payment::METHOD_ETRANSFER, (new StudioSettings())->defaultPaymentMethod());
+    }
+
+    public function testUnrecognisedStoredDefaultPaymentMethodFallsBackToCard(): void
+    {
+        // `comp` is a per-student choice only: it must never become the studio
+        // default and quietly stop billing anyone.
+        Functions\when('get_option')->alias(static fn (string $name) =>
+            $name === StudioSettings::OPT_DEFAULT_PAYMENT_METHOD ? Payment::METHOD_COMP : '');
+
+        self::assertSame(Payment::METHOD_CARD, (new StudioSettings())->defaultPaymentMethod());
+    }
+
+    public function testClearStripeConfigDeletesEveryStripeOption(): void
+    {
+        Functions\expect('delete_option')->once()->with(StudioSettings::OPT_PUBLISHABLE);
+        Functions\expect('delete_option')->once()->with(StudioSettings::OPT_SECRET);
+        Functions\expect('delete_option')->once()->with(StudioSettings::OPT_WEBHOOK_SECRET);
+        Functions\expect('delete_option')->once()->with(StudioSettings::OPT_MODE);
+
+        (new StudioSettings())->clearStripeConfig();
+    }
+
+    public function testClearStripeConfigLeavesNonStripeSettingsAlone(): void
+    {
+        $deleted = [];
+        Functions\when('delete_option')->alias(static function (string $name) use (&$deleted): bool {
+            $deleted[] = $name;
+
+            return true;
+        });
+        Functions\expect('update_option')->never();
+
+        (new StudioSettings())->clearStripeConfig();
+
+        self::assertNotContains(StudioSettings::OPT_CURRENCY, $deleted);
+        self::assertNotContains(StudioSettings::OPT_ETRANSFER_EMAIL, $deleted);
+        self::assertNotContains(StudioSettings::OPT_HST_RATE, $deleted);
+        self::assertNotContains(StudioSettings::OPT_REGISTRATION_MODE, $deleted);
+        self::assertNotContains(StudioSettings::OPT_CANCELLATION_CUTOFF_HOURS, $deleted);
+    }
+
     private function applyMode(bool $enable): void
     {
         $method = new \ReflectionMethod(StudioSettings::class, 'applyRegistrationMode');