fix(reactivation): address remaining CodeRabbit findings from PR #751 review

- renew(): capture $previous_status before set_status() so the
  date_cancellation guard compares against the pre-update status.
  Previously the check was always false because get_status() returned
  the new 'active' value after set_status() ran.

- reactivate(): remove ': bool' return type (renew() can return
  WP_Error); update PHPDoc to 'bool|\WP_Error'; change success check
  from 'if ($result)' to 'if (true === $result)' so WP_Error objects
  (truthy) don't trigger the post-reactivation hooks.

- build_from_membership() reactivation path: set duration/duration_unit
  from the plan BEFORE calling add_product() so line items are created
  with the correct billing period. Replace get_addon_ids() (discards
  quantities) with get_addon_products() (product_id => quantity map) so
  addon quantities are preserved when rebuilding the cart.

Closes #752

Author: superdav42

inc/checkout/class-cart.php

@@ -882,37 +882,46 @@ protected function build_from_membership($membership_id): bool {
 			 */
 			$plan_id = $membership->get_plan_id();
 
-			if ($plan_id) {
-				$this->attributes->products = [$plan_id];
-
-				$addon_ids = $membership->get_addon_ids();
-
-				if (! empty($addon_ids)) {
-					$this->attributes->products = array_merge($this->attributes->products, $addon_ids);
-				}
-			}
-
-			// Set up country and currency, then add products and return
+			// Set up country and currency before building products
 			$this->country = $this->country ?: $this->customer->get_country();
 
 			$this->set_currency($membership->get_currency());
 
-			if (empty($this->attributes->products)) {
+			/*
+			 * Set duration from the plan BEFORE calling add_product() so that
+			 * line items are created with the correct billing period. Previously
+			 * this was set after add_product(), meaning products were added with
+			 * the default/empty duration and the correct values were never used.
+			 *
+			 * @since 2.5.0
+			 */
+			$plan_product = $membership->get_plan();
+
+			if ($plan_product && ! $membership->is_free()) {
+				$this->duration      = $plan_product->get_duration();
+				$this->duration_unit = $plan_product->get_duration_unit();
+			}
+
+			if (! $plan_id) {
 				$this->errors->add('no_plan', __('This membership has no plan to reactivate.', 'ultimate-multisite'));
 
 				return true;
 			}
 
-			foreach ($this->attributes->products as $product_id) {
-				$this->add_product($product_id);
-			}
+			/*
+			 * Rebuild the product list from the membership, preserving addon
+			 * quantities. get_addon_ids() discards quantities; get_addon_products()
+			 * returns the full product_id => quantity map so each addon is added
+			 * with the correct quantity.
+			 *
+			 * @since 2.5.0
+			 */
+			$this->add_product($plan_id);
 
-			// Set duration from the plan
-			$plan_product = $membership->get_plan();
+			$addon_products = $membership->get_addon_products();
 
-			if ($plan_product && ! $membership->is_free()) {
-				$this->duration      = $plan_product->get_duration();
-				$this->duration_unit = $plan_product->get_duration_unit();
+			foreach ($addon_products as $addon_id => $quantity) {
+				$this->add_product((int) $addon_id, (int) $quantity);
 			}
 
 			return true;

inc/models/class-membership.php

@@ -2304,6 +2304,16 @@ public function renew($auto_renew = false, $status = 'active', $expiration = '')
 		 */
 		do_action('wu_membership_pre_renew', $expiration, $this->get_id(), $this);
 
+		/*
+		 * Capture the current status BEFORE calling set_status() so the
+		 * cancellation-clear guard below can compare against the previous
+		 * state. After set_status() runs, get_status() returns the new value
+		 * and the CANCELLED check would never be true.
+		 *
+		 * @since 2.5.0
+		 */
+		$previous_status = $this->get_status();
+
 		$this->set_date_expiration($expiration);
 
 		if ( ! empty($status)) {
@@ -2312,13 +2322,14 @@ public function renew($auto_renew = false, $status = 'active', $expiration = '')
 
 		/*
 		 * Clear the cancellation date only when reactivating a previously
-		 * cancelled membership. Checks the current status (before the update)
-		 * to avoid clearing historical cancellation data on regular recurring
-		 * renewals where the membership was never in a cancelled state.
+		 * cancelled membership. Uses $previous_status (captured before the
+		 * set_status() call above) to avoid clearing historical cancellation
+		 * data on regular recurring renewals where the membership was never
+		 * in a cancelled state.
 		 *
 		 * @since 2.5.0
 		 */
-		if ('active' === $status && Membership_Status::CANCELLED === $this->get_status() && ! empty($this->get_date_cancellation())) {
+		if (Membership_Status::ACTIVE === $status && Membership_Status::CANCELLED === $previous_status && ! empty($this->get_date_cancellation())) {
 			$this->set_date_cancellation(null);
 		}
 
@@ -2363,9 +2374,9 @@ public function renew($auto_renew = false, $status = 'active', $expiration = '')
 	 *
 	 * @param bool   $auto_renew Whether to auto-renew.
 	 * @param string $expiration Optional expiration date in MySQL format.
-	 * @return bool Whether the reactivation was successful.
+	 * @return bool|\WP_Error True on success, WP_Error if renew()/save() fails.
 	 */
-	public function reactivate($auto_renew = false, $expiration = ''): bool {
+	public function reactivate($auto_renew = false, $expiration = '') {
 
 		$id = $this->get_id();
 
@@ -2394,7 +2405,7 @@ public function reactivate($auto_renew = false, $expiration = ''): bool {
 
 		$result = $this->renew($auto_renew, 'active', $expiration);
 
-		if ($result) {
+		if (true === $result) {
 			wu_log_add("membership-{$id}", sprintf('Membership #%d reactivated successfully. Old cancel date: %s', $id, $old_cancel_date ?: 'none'));
 
 			/**