fix: address notification handling review feedback

1. Fix misleading comment in async create_member path
2. Use skip_lock?: true in test case for create_member
3. Fix generate_cycles_for_all_members/1

lib/membership/member.ex

@@ -114,11 +114,12 @@ defmodule Mv.Membership.Member do
                if member.membership_fee_type_id && member.join_date do
                  if Application.get_env(:mv, :sql_sandbox, false) do
                    # Run synchronously in test environment for DB sandbox compatibility
+                   # Use skip_lock?: true to avoid nested transactions (after_action runs within action transaction)
                    # Return notifications to Ash so they are sent after commit
                    case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(
                           member.id,
                           today: Date.utc_today(),
-                          skip_lock?: false
+                          skip_lock?: true
                         ) do
                      {:ok, _cycles, notifications} ->
                        {:ok, member, notifications}
@@ -134,8 +135,7 @@ defmodule Mv.Membership.Member do
                    end
                  else
                    # Run asynchronously in other environments
-                   # Notifications cannot be returned in async case, so they will be lost
+                   # Send notifications explicitly since they cannot be returned via after_action
-                   # This is acceptable as cycle generation is not critical for member creation
                    Task.start(fn ->
                      case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id) do
                        {:ok, _cycles, notifications} ->

lib/mv/membership_fees/cycle_generator.ex

@@ -179,7 +179,19 @@ defmodule Mv.MembershipFees.CycleGenerator do
   defp process_batch(batch, today) do
     batch
     |> Task.async_stream(fn member ->
-      {member.id, generate_cycles_for_member(member, today: today)}
+      case generate_cycles_for_member(member, today: today) do
+        {:ok, _cycles, notifications} = ok ->
+          # Send notifications for batch job
+          # This is a top-level job, so we need to send notifications explicitly
+          if Enum.any?(notifications) do
+            Ash.Notifier.notify(notifications)
+          end
+
+          {member.id, ok}
+
+        {:error, _reason} = err ->
+          {member.id, err}
+      end
     end)
     |> Enum.map(fn
       {:ok, result} ->
@@ -193,7 +205,7 @@ defmodule Mv.MembershipFees.CycleGenerator do
   end
 
   defp build_results_summary(results) do
-    success_count = Enum.count(results, fn {_id, result} -> match?({:ok, _}, result) end)
+    success_count = Enum.count(results, fn {_id, result} -> match?({:ok, _, _}, result) end)
     failed_count = Enum.count(results, fn {_id, result} -> match?({:error, _}, result) end)
 
     %{success: success_count, failed: failed_count, total: length(results)}