fix(self-scheduling): stop reporting a committed booking as a failure

CHANGELOG.md

@@ -17,6 +17,9 @@ The format follows [Keep a Changelog] (https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- **Um agendamento era criado mas reportado como falha** (#1058): `$appointment['id']` chega do `$wpdb` como string e `AppointmentReceiptHandler::get_receipt_url()` declara `int` sob `strict_types` — o `TypeError` fatalava no e-mail de confirmação, *depois* do commit da reserva, e o visitante recebia HTTP 500 para um agendamento que existia. O `catch` do handler passou a `\Throwable` e o bloco pós-commit foi isolado por etapa: uma reserva commitada nunca mais é reportada como falha.
+- **Toda linha de activity log de agendamento criado era perdida** (#1058): `on_appointment_created()` passava o id do agendamento no 4º argumento de `ActivityLog::log()`, que é `$user_id` — com `fk_ffc_activity_log_user` ativa o MySQL rejeitava o insert.
+
 - **O captcha se atrapalhava com dois formulários na mesma página** (#1056): o refresh do agendamento reescrevia a pergunta em *todos* os formulários mas, casando por id, trocava o token só do primeiro — o segundo passava a exibir uma pergunta que seu token não respondia. Agora é escopado ao formulário e casa por `name`. Os ids do captcha passam a ser únicos por render, corrigindo também a associação `<label for>` para leitores de tela.
 
 ## [6.22.0] (2026-09-04) — `aa5ba5b`

includes/core/class-ffc-activity-log-subscriber.php

@@ -153,7 +153,12 @@ public function on_appointment_created( int $appointment_id, array $data, array
 				'user_id'        => $data['user_id'] ?? null,
 				'ip'             => $data['user_ip'] ?? '',
 			),
-			$appointment_id
+			// 4th argument is $user_id, not the appointment. Passing the
+			// appointment id here attributed every booking to whichever user
+			// happened to hold that id — and once MigrationForeignKeys added
+			// `fk_ffc_activity_log_user`, the insert was rejected outright, so
+			// the row was silently lost instead of merely wrong.
+			(int) ( $data['user_id'] ?? 0 )
 		);
 	}
 

includes/self-scheduling/class-ffc-self-scheduling-appointment-ajax-handler.php

@@ -58,6 +58,9 @@ public function __construct( AppointmentHandler $handler ) {
 	 * AJAX: Book appointment
 	 */
 	public function ajax_book_appointment(): void {
+		// Set once the appointment row is committed; read by the catch below.
+		$booked = null;
+
 		try {
 			check_ajax_referer( 'ffc_self_scheduling_nonce', 'nonce' );
 
@@ -113,6 +116,12 @@ public function ajax_book_appointment(): void {
 
 			$result = $this->handler->process_appointment( $appointment_data );
 
+			// From here the booking is committed. The catch at the bottom reads
+			// this to know it must not report a failure for it.
+			if ( ! is_wp_error( $result ) ) {
+				$booked = $result;
+			}
+
 			if ( is_wp_error( $result ) ) {
 				wp_send_json_error(
 					\FreeFormCertificate\Core\SecurityService::with_fresh_challenge(
@@ -190,17 +199,44 @@ public function ajax_book_appointment(): void {
 			}
 
 			wp_send_json_success( $response );
-		} catch ( \Exception $e ) {
-			if ( class_exists( '\FreeFormCertificate\Core\Utils' ) ) {
-				\FreeFormCertificate\Core\Debug::log_self_scheduling(
-					'Appointment AJAX error',
+		} catch ( \Throwable $e ) {
+			// `\Throwable`, not `\Exception`: a TypeError or a call on null is an
+			// `\Error`, which the narrower catch let escape as an uncaught fatal —
+			// a 500 with no JSON body, which the client can only render as its
+			// generic "an error occurred" while the booking sits in the database.
+			\FreeFormCertificate\Core\Debug::log_self_scheduling(
+				'Appointment AJAX error',
+				array(
+					'booked'  => null !== $booked ? ( $booked['appointment_id'] ?? 'yes' ) : 'no',
+					'message' => $e->getMessage(),
+					'file'    => $e->getFile(),
+					'line'    => $e->getLine(),
+				)
+			);
+
+			// The booking committed before this threw. Reporting failure would
+			// send the visitor to book again — duplicating the appointment or
+			// colliding with the duplicate guard — so answer with what is true:
+			// it worked, and the parts that did not are flagged rather than
+			// silently claimed.
+			if ( null !== $booked ) {
+				wp_send_json_success(
 					array(
-						'message' => $e->getMessage(),
-						'file'    => $e->getFile(),
-						'line'    => $e->getLine(),
+						'message'            => __( 'Appointment booked successfully!', 'ffcertificate' ),
+						'appointment_id'     => $booked['appointment_id'],
+						'confirmation_token' => $booked['confirmation_token'] ?? null,
+						'validation_code'    => null,
+						'receipt_url'        => $booked['receipt_url'] ?? '',
+						'requires_approval'  => $booked['requires_approval'] ?? false,
+						'waitlisted'         => ! empty( $booked['waitlisted'] ),
+						'email_sent'         => false,
+						'degraded'           => true,
 					)
 				);
 			}
+
+			// Nothing was committed: a real failure, and retrying is the right
+			// advice — so this is the one path that still hands back a challenge.
 			wp_send_json_error(
 				\FreeFormCertificate\Core\SecurityService::with_fresh_challenge(
 					array(

includes/self-scheduling/class-ffc-self-scheduling-appointment-email-handler.php

@@ -93,7 +93,7 @@ public function send_booking_confirmation( array $appointment, array $calendar )
 		$receipt_url = '';
 		if ( class_exists( '\FreeFormCertificate\SelfScheduling\AppointmentReceiptHandler' ) ) {
 			$receipt_url = \FreeFormCertificate\SelfScheduling\AppointmentReceiptHandler::get_receipt_url(
-				$appointment['id'],
+				(int) ( $appointment['id'] ?? 0 ),
 				$appointment['confirmation_token'] ?? ''
 			);
 		}
@@ -371,7 +371,7 @@ public function send_approval_notification( array $appointment, array $calendar
 		$receipt_url = '';
 		if ( class_exists( '\FreeFormCertificate\SelfScheduling\AppointmentReceiptHandler' ) ) {
 			$receipt_url = AppointmentReceiptHandler::get_receipt_url(
-				$appointment['id'],
+				(int) ( $appointment['id'] ?? 0 ),
 				$appointment['confirmation_token'] ?? ''
 			);
 		}
@@ -469,7 +469,7 @@ public function send_promotion_notification( array $appointment, array $calendar
 		$receipt_url = '';
 		if ( class_exists( '\FreeFormCertificate\SelfScheduling\AppointmentReceiptHandler' ) ) {
 			$receipt_url = AppointmentReceiptHandler::get_receipt_url(
-				$appointment['id'],
+				(int) ( $appointment['id'] ?? 0 ),
 				$appointment['confirmation_token'] ?? ''
 			);
 		}

includes/self-scheduling/class-ffc-self-scheduling-appointment-handler.php

@@ -198,44 +198,91 @@ public function process_appointment( array $data ) {
 		}
 		// === END TRANSACTION ===.
 
-		/**
-		 * Fires after an appointment is created.
-		 *
-		 * @since 4.6.4
-		 * @param int   $appointment_id New appointment ID.
-		 * @param array $data           Appointment data.
-		 * @param array<string, mixed> $calendar       Calendar configuration.
-		 */
-		do_action( 'ffcertificate_after_appointment_create', $appointment_id, $data, $calendar );
+		// Everything from here on is *after* the commit: the appointment row
+		// exists and the booking is real. A failure in the notification hook,
+		// the confirmation email or the receipt link is a degraded booking, not
+		// a failed one — so it must never propagate out of this method as an
+		// error. Before this guard a throw here surfaced to the visitor as
+		// "an error occurred, please try again" for a booking that was already
+		// in the database, and the natural response — booking again — either
+		// duplicated it or hit the duplicate guard.
+		$appointment = null;
+		$receipt_url = '';
+		$emails_ok   = true;
 
-		// Get appointment for email (outside transaction — read-only).
-		$appointment = $this->appointment_repository->findById( $appointment_id );
+		try {
+			/**
+			 * Fires after an appointment is created.
+			 *
+			 * @since 4.6.4
+			 * @param int   $appointment_id New appointment ID.
+			 * @param array $data           Appointment data.
+			 * @param array<string, mixed> $calendar       Calendar configuration.
+			 */
+			do_action( 'ffcertificate_after_appointment_create', $appointment_id, $data, $calendar );
+
+			// Get appointment for email (outside transaction — read-only).
+			$appointment = $this->appointment_repository->findById( $appointment_id );
+		} catch ( \Throwable $e ) {
+			$this->log_post_commit_failure( 'after_create_hook', $appointment_id, $e );
+		}
 
 		// Schedule email notifications. A waitlisted booking gets the "you're on
 		// the waitlist" email instead of the booking confirmation (#941 phase 2).
 		if ( is_array( $appointment ) ) {
-			$this->schedule_email_notifications( $appointment, $calendar, $is_waitlist ? 'waitlisted' : 'created' );
+			try {
+				$this->schedule_email_notifications( $appointment, $calendar, $is_waitlist ? 'waitlisted' : 'created' );
+			} catch ( \Throwable $e ) {
+				$emails_ok = false;
+				$this->log_post_commit_failure( 'email_notifications', $appointment_id, $e );
+			}
 		}
 
 		// Generate receipt URL (magic link to /valid/ page).
-		$receipt_url        = '';
-		$confirmation_token = $appointment['confirmation_token'] ?? '';
-		if ( ! empty( $confirmation_token ) && class_exists( '\\FreeFormCertificate\\Generators\\MagicLinkHelper' ) ) {
-			$receipt_url = \FreeFormCertificate\Generators\MagicLinkHelper::generate_magic_link( $confirmation_token );
-		} elseif ( class_exists( '\FreeFormCertificate\SelfScheduling\AppointmentReceiptHandler' ) ) {
-			$receipt_url = \FreeFormCertificate\SelfScheduling\AppointmentReceiptHandler::get_receipt_url(
-				$appointment_id,
-				$confirmation_token
-			);
+		$confirmation_token = is_array( $appointment ) ? ( $appointment['confirmation_token'] ?? '' ) : '';
+
+		try {
+			if ( ! empty( $confirmation_token ) && class_exists( '\\FreeFormCertificate\\Generators\\MagicLinkHelper' ) ) {
+				$receipt_url = \FreeFormCertificate\Generators\MagicLinkHelper::generate_magic_link( $confirmation_token );
+			} elseif ( class_exists( '\FreeFormCertificate\SelfScheduling\AppointmentReceiptHandler' ) ) {
+				$receipt_url = \FreeFormCertificate\SelfScheduling\AppointmentReceiptHandler::get_receipt_url(
+					$appointment_id,
+					$confirmation_token
+				);
+			}
+		} catch ( \Throwable $e ) {
+			$this->log_post_commit_failure( 'receipt_url', $appointment_id, $e );
 		}
 
 		return array(
 			'success'            => true,
 			'appointment_id'     => $appointment_id,
-			'confirmation_token' => $appointment['confirmation_token'] ?? null,
+			'confirmation_token' => '' !== $confirmation_token ? $confirmation_token : null,
 			'requires_approval'  => 1 === $calendar['requires_approval'],
 			'waitlisted'         => $is_waitlist,
 			'receipt_url'        => $receipt_url,
+			'notifications_ok'   => $emails_ok,
+		);
+	}
+
+	/**
+	 * Record a post-commit failure without failing the booking.
+	 *
+	 * @param string     $stage          Which post-commit step threw.
+	 * @param int        $appointment_id Committed appointment id.
+	 * @param \Throwable $e              The failure.
+	 * @return void
+	 */
+	private function log_post_commit_failure( string $stage, int $appointment_id, \Throwable $e ): void {
+		\FreeFormCertificate\Core\Debug::log_self_scheduling(
+			'Post-commit failure — the appointment is booked, this step is not',
+			array(
+				'stage'          => $stage,
+				'appointment_id' => $appointment_id,
+				'message'        => $e->getMessage(),
+				'file'           => $e->getFile(),
+				'line'           => $e->getLine(),
+			)
 		);
 	}
 

tests/Unit/ActivityLogAppointmentAttributionTest.php

@@ -0,0 +1,123 @@
+<?php
+declare(strict_types=1);
+
+namespace FreeFormCertificate\Tests\Unit;
+
+use Brain\Monkey;
+use Brain\Monkey\Functions;
+use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
+use PHPUnit\Framework\TestCase;
+use FreeFormCertificate\Core\ActivityLogSubscriber;
+
+/**
+ * Regression guard for the user attributed to an appointment activity-log row.
+ *
+ * `ActivityLog::log()` takes `$user_id` as its fourth argument. The appointment
+ * subscriber passed `$appointment_id` there, so every booking was logged
+ * against whichever user happened to hold that id — and once
+ * `MigrationForeignKeys` added `fk_ffc_activity_log_user`, MySQL rejected the
+ * insert outright and the row was lost rather than merely wrong. Observed in
+ * production as `Cannot add or update a child row` on every booking.
+ *
+ * Alias-mocking `ActivityLog` is process-global, so this lives in its own class
+ * rather than inside ActivityLogSubscriberTest, whose other cases exercise the
+ * real buffering.
+ *
+ * @covers \FreeFormCertificate\Core\ActivityLogSubscriber
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ */
+class ActivityLogAppointmentAttributionTest extends TestCase {
+
+	use MockeryPHPUnitIntegration;
+
+	protected function setUp(): void {
+		parent::setUp();
+		Monkey\setUp();
+
+		class_exists( '\FreeFormCertificate\Core\ActivityLogSubscriber' );
+
+		// An alias mock stands in for a class that must not be loaded, so it
+		// carries no class constants — the subscriber reads LEVEL_INFO off it.
+		\Mockery::getConfiguration()->setConstantsMap(
+			array(
+				'FreeFormCertificate\Core\ActivityLog' => array(
+					'LEVEL_INFO'    => 'info',
+					'LEVEL_WARNING' => 'warning',
+					'LEVEL_ERROR'   => 'error',
+				),
+			)
+		);
+
+		Functions\when( 'add_action' )->justReturn( true );
+	}
+
+	protected function tearDown(): void {
+		\Mockery::getConfiguration()->setConstantsMap( array() );
+		Monkey\tearDown();
+		parent::tearDown();
+	}
+
+	/**
+	 * Appointment data as the handler passes it.
+	 *
+	 * @param array<string, mixed> $overrides Fields to override.
+	 * @return array<string, mixed>
+	 */
+	private function bookingData( array $overrides = array() ): array {
+		return array_merge(
+			array(
+				'calendar_id'      => 1,
+				'appointment_date' => '2030-01-15',
+				'start_time'       => '10:00',
+				'status'           => 'confirmed',
+				'user_id'          => 42,
+				'user_ip'          => '127.0.0.1',
+			),
+			$overrides
+		);
+	}
+
+	public function test_logs_the_booking_user_not_the_appointment_id(): void {
+		$captured = null;
+
+		\Mockery::mock( 'alias:FreeFormCertificate\Core\ActivityLog' )
+			->shouldReceive( 'log' )
+			->once()
+			->andReturnUsing(
+				function ( $action, $level = '', $context = array(), $user_id = 0, $submission_id = 0 ) use ( &$captured ) {
+					$captured = $user_id;
+					return true;
+				}
+			);
+
+		// 777 is the appointment id; 42 is the visitor who booked.
+		( new ActivityLogSubscriber() )->on_appointment_created( 777, $this->bookingData(), array() );
+
+		$this->assertSame( 42, $captured, 'the fourth argument is $user_id, not the appointment' );
+	}
+
+	public function test_falls_back_to_anonymous_when_the_booking_has_no_user(): void {
+		$captured = null;
+
+		\Mockery::mock( 'alias:FreeFormCertificate\Core\ActivityLog' )
+			->shouldReceive( 'log' )
+			->once()
+			->andReturnUsing(
+				function ( $action, $level = '', $context = array(), $user_id = 0, $submission_id = 0 ) use ( &$captured ) {
+					$captured = $user_id;
+					return true;
+				}
+			);
+
+		// A guest booking carries no user_id. 0 is the anonymous sentinel the
+		// column allows; the appointment id would be a foreign-key violation.
+		( new ActivityLogSubscriber() )->on_appointment_created(
+			777,
+			$this->bookingData( array( 'user_id' => null ) ),
+			array()
+		);
+
+		$this->assertSame( 0, $captured );
+	}
+}