chore: reworked some transaction handling code

CODESMELL.md

@@ -51,7 +51,7 @@ _None._ All modules previously on the queue have been split; refresh the queue i
 
 ## 3. Side Effects in Transactions
 
-**Status:** ✅ done in `BDS.Media` (2026-04-30). Open elsewhere — no audit yet for `BDS.Posts`, `BDS.Publishing`, `BDS.Generation`.
+**Status:** ✅ done in `BDS.Media` (2026-04-30). Started elsewhere: `BDS.Templates.update_template/2` now keeps only DB writes inside its transaction and runs template-file rewrites, published-post rewrites, and tags JSON flushes after commit. Open elsewhere — no audit yet for `BDS.Posts`, `BDS.Publishing`, `BDS.Generation`.
 
 **Plan:** spot-check every `Repo.transaction/1` outside `BDS.Media`. Rule: only DB writes inside; filesystem and `Search.sync_*` after commit.
 

lib/bds/templates.ex

@@ -111,26 +111,38 @@ defmodule BDS.Templates do
           |> Map.put(:updated_at, now)
           |> Map.put(:status, next_status)
 
+        transaction_result =
           Repo.transaction(fn ->
             updated_template =
               template
               |> Template.changeset(updates)
               |> Repo.update!()
 
+            affected_posts =
               if slug_changed? do
                 cascade_template_slug_change(template, updated_template, now)
+              else
+                []
               end
 
-          if template.file_path not in [nil, ""] and next_file_path != template.file_path do
+            {updated_template, affected_posts}
-            rewrite_template_file(template, updated_template)
-          end
-
-          updated_template
           end)
-        |> case do
+
-          {:ok, updated_template} -> {:ok, updated_template}
+        case transaction_result do
+          {:ok, {updated_template, affected_posts}} ->
+            case sync_template_update_side_effects(
+                   template,
+                   updated_template,
+                   affected_posts,
+                   slug_changed?
+                 ) do
+              :ok -> {:ok, updated_template}
               {:error, reason} -> {:error, reason}
             end
+
+          {:error, reason} ->
+            {:error, reason}
+        end
     end
   end
 
@@ -215,7 +227,13 @@ defmodule BDS.Templates do
       %Template{file_path: file_path, status: status} = template
       when file_path not in [nil, ""] and status == :published ->
         full_path = full_file_path(template.project_id, template.file_path)
-        :ok = Persistence.atomic_write(full_path, serialize_template_file(template, published_template_body(template)))
+
+        :ok =
+          Persistence.atomic_write(
+            full_path,
+            serialize_template_file(template, published_template_body(template))
+          )
+
         {:ok, template}
 
       %Template{} ->
@@ -368,23 +386,43 @@ defmodule BDS.Templates do
     )
     |> Repo.update_all(set: [post_template_slug: updated_template.slug, updated_at: updated_at])
 
+    affected_posts
+  end
+
+  defp sync_template_update_side_effects(
+         original_template,
+         updated_template,
+         affected_posts,
+         slug_changed?
+       ) do
     Enum.each(affected_posts, fn post ->
       Posts.rewrite_published_post(post.id)
     end)
 
+    if slug_changed? do
       Tags.sync_tags_json(original_template.project_id)
     end
 
+    if original_template.file_path not in [nil, ""] and
+         updated_template.file_path != original_template.file_path do
+      rewrite_template_file(original_template, updated_template)
+    else
+      :ok
+    end
+  end
+
   defp rewrite_template_file(original_template, updated_template) do
     body = published_template_body(original_template)
     new_full_path = full_file_path(updated_template.project_id, updated_template.file_path)
-    :ok = Persistence.atomic_write(new_full_path, serialize_template_file(updated_template, body))
 
-    if original_template.file_path != updated_template.file_path do
+    result =
-      _ = delete_file_if_present(original_template.project_id, original_template.file_path)
+      Persistence.atomic_write(new_full_path, serialize_template_file(updated_template, body))
+
+    if result == :ok and original_template.file_path != updated_template.file_path do
+      delete_file_if_present(original_template.project_id, original_template.file_path)
+    else
+      result
     end
-
-    :ok
   end
 
   defp published_template_body(%Template{content: content}) when is_binary(content), do: content
@@ -444,7 +482,10 @@ defmodule BDS.Templates do
             template.file_path != "" and
             not is_nil(template.file_path)
     )
-    |> Enum.reject(&(MapSet.member?(tracked_paths, &1.file_path) or File.exists?(full_file_path(project_id, &1.file_path))))
+    |> Enum.reject(
+      &(MapSet.member?(tracked_paths, &1.file_path) or
+          File.exists?(full_file_path(project_id, &1.file_path)))
+    )
     |> Enum.each(fn template ->
       clear_template_references(template)
       Repo.delete!(template)

test/bds/templates_test.exs

@@ -218,6 +218,52 @@ defmodule BDS.TemplatesTest do
              Jason.decode!(File.read!(tags_path))
   end
 
+  test "update_template keeps committed database changes when renaming the published file fails",
+       %{project: project, temp_dir: temp_dir} do
+    assert {:ok, template} =
+             BDS.Templates.create_template(%{
+               project_id: project.id,
+               title: "Article View",
+               kind: :post,
+               content: "<article>{{ content }}</article>"
+             })
+
+    assert {:ok, published} = BDS.Templates.publish_template(template.id)
+
+    assert {:ok, post} =
+             BDS.Posts.create_post(%{
+               project_id: project.id,
+               title: "Uses Template",
+               content: "Body",
+               template_slug: published.slug
+             })
+
+    assert {:ok, published_post} = BDS.Posts.publish_post(post.id)
+
+    assert {:ok, _tag} =
+             BDS.Tags.create_tag(%{
+               project_id: project.id,
+               name: "Feature",
+               post_template_slug: published.slug
+             })
+
+    blocked_path = Path.join([temp_dir, "templates", "feature-view.liquid.tmp"])
+    File.mkdir_p!(blocked_path)
+
+    assert {:error, _reason} =
+             BDS.Templates.update_template(published.id, %{slug: "feature-view"})
+
+    reloaded_template = Repo.get!(BDS.Templates.Template, published.id)
+    assert reloaded_template.slug == "feature-view"
+    assert reloaded_template.file_path == "templates/feature-view.liquid"
+
+    reloaded_post = Repo.get!(Post, published_post.id)
+    assert reloaded_post.template_slug == "feature-view"
+
+    reloaded_tag = Repo.get_by!(Tag, project_id: project.id, name: "Feature")
+    assert reloaded_tag.post_template_slug == "feature-view"
+  end
+
   test "rebuild_templates_from_files recreates published templates from disk", %{
     project: project,
     temp_dir: temp_dir
@@ -264,7 +310,8 @@ defmodule BDS.TemplatesTest do
     assert template.updated_at == 202
   end
 
-  test "rebuild_templates_from_files removes stale published default templates when no local template files exist", %{
+  test "rebuild_templates_from_files removes stale published default templates when no local template files exist",
+       %{
          project: project
        } do
     now = BDS.Persistence.now_ms()