Fix publishing bugs and clean up stale filesystem references

ddon · claude · ddon · commit 987be5abbbe4 · 2026-03-07T01:59:22.000+02:00
Fix slug conflict clearing passing wrong slug to DB cleanup, fix ngettext
interpolation showing literal %{count} in migration modal, rename
find_dialect_for_base_in_files to find_dialect_for_base_in_languages,
update stale comments and user-facing strings referencing filesystem storage.

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 ## 1.7.63 - 2026-03-06
-- Remove filesystem storage mode from publishing module — `storage_mode()` now always returns `:db`, remove ~780 lines of dead FS code, migration gate, and "Database migration required" banner
+- Remove filesystem storage from publishing module — delete Storage, DualWrite, and all storage/* submodules (~7k lines removed)
+- Add LanguageHelpers and SlugHelpers as standalone modules, simplify to DB-only throughout
+- Fix slug conflict clearing bug: `clear_url_slugs_for_conflicts` passed wrong slug to DB cleanup
+- Fix ngettext interpolation in primary language migration modal (literal `%{count}` in UI)
+- Clean up stale filesystem references in comments, docs, and user-facing strings
 - Fix V77/V78 migration crashes when UUID columns are missing (tables created after V56 ran)
 - Simplify V77/V78 migrations — remove over-engineered column detection, rely on idempotent patterns
 - Fix email tracking bug: `handle_delivery_result` used `get_log!` (raises) in a nil-matching branch; add `get_log/1` non-bang wrapper and remove unused public functions
diff --git a/lib/mix/tasks/phoenix_kit.migrate_blogging_to_publishing.ex b/lib/mix/tasks/phoenix_kit.migrate_blogging_to_publishing.ex
@@ -1,15 +1,13 @@
 defmodule Mix.Tasks.PhoenixKit.MigrateBloggingToPublishing do
-  @shortdoc "Migrates blogging module to publishing (storage path and settings)"
+  @shortdoc "Migrates blogging module to publishing (settings)"
   @moduledoc """
   Migrates the PhoenixKit blogging module to the new publishing module.
 
   ## What this task does
 
-  1. Renames `priv/blogging` directory to `priv/publishing` (if it exists)
-  2. Migrates settings keys from legacy names to new names:
+  Migrates settings keys from legacy names to new names:
      - `blogging_enabled` → `publishing_enabled`
      - `blogging_blogs` → `publishing_groups`
-     - `blogging_file_cache_enabled` → `publishing_file_cache_enabled`
      - `blogging_memory_cache_enabled` → `publishing_memory_cache_enabled`
      - `blogging_render_cache_enabled` → `publishing_render_cache_enabled`
      - `blogging_render_cache_enabled_*` → `publishing_render_cache_enabled_*`
diff --git a/lib/modules/publishing/pubsub.ex b/lib/modules/publishing/pubsub.ex
@@ -372,7 +372,7 @@ defmodule PhoenixKit.Modules.Publishing.PubSub do
   end
 
   @doc """
-  Broadcasts that the cache state has changed (file regenerated, memory loaded, etc).
+  Broadcasts that the cache state has changed (cache regenerated, memory loaded, etc).
   """
   def broadcast_cache_changed(group_slug) do
     Manager.broadcast(cache_topic(group_slug), {:cache_changed, group_slug})
diff --git a/lib/modules/publishing/slug_helpers.ex b/lib/modules/publishing/slug_helpers.ex
@@ -95,7 +95,7 @@ defmodule PhoenixKit.Modules.Publishing.SlugHelpers do
     case ListingCache.read(group_slug) do
       {:ok, posts} ->
         conflicts = find_conflicting_url_slugs(posts, directory_slug)
-        clear_url_slugs_for_conflicts(group_slug, conflicts)
+        clear_url_slugs_for_conflicts(group_slug, directory_slug, conflicts)
         log_cleared_conflicts(conflicts, directory_slug)
         conflicts
 
@@ -212,10 +212,9 @@ defmodule PhoenixKit.Modules.Publishing.SlugHelpers do
     end)
   end
 
-  defp clear_url_slugs_for_conflicts(group_slug, conflicts) do
+  defp clear_url_slugs_for_conflicts(group_slug, directory_slug, conflicts) do
     Enum.each(conflicts, fn {post_slug, _language} ->
-      # Clear via DB
-      DBStorage.clear_url_slug_from_post(group_slug, post_slug, post_slug)
+      DBStorage.clear_url_slug_from_post(group_slug, post_slug, directory_slug)
     end)
   end
 
diff --git a/lib/modules/publishing/web/controller/language.ex b/lib/modules/publishing/web/controller/language.ex
@@ -150,31 +150,31 @@ defmodule PhoenixKit.Modules.Publishing.Web.Controller.Language do
   # ============================================================================
 
   @doc """
-  Resolves a language code to an actual file language.
+  Resolves a language code to an actual content language.
   Handles base codes by finding a matching dialect in available languages.
   """
   def resolve_language_for_post(language, available_languages) do
     cond do
-      # Direct match - language exactly matches an available file
+      # Direct match - language exactly matches an available language
       language in available_languages ->
         language
 
       # Base code - try to find a dialect that matches
       base_code?(language) ->
-        find_dialect_for_base_in_files(language, available_languages) ||
+        find_dialect_for_base_in_languages(language, available_languages) ||
           DialectMapper.base_to_dialect(language)
 
       # Full dialect code not found - try base code match as fallback
       true ->
         base = DialectMapper.extract_base(language)
-        find_dialect_for_base_in_files(base, available_languages) || language
+        find_dialect_for_base_in_languages(base, available_languages) || language
     end
   end
 
   @doc """
-  Find a dialect in available files that matches the given base code.
+  Find a dialect in available languages that matches the given base code.
   """
-  def find_dialect_for_base_in_files(base_code, available_languages) do
+  def find_dialect_for_base_in_languages(base_code, available_languages) do
     base_lower = String.downcase(base_code)
 
     Enum.find(available_languages, fn lang ->
diff --git a/lib/modules/publishing/web/controller/listing.ex b/lib/modules/publishing/web/controller/listing.ex
@@ -234,12 +234,12 @@ defmodule PhoenixKit.Modules.Publishing.Web.Controller.Listing do
 
       # Base code - find a dialect that matches
       Language.base_code?(language) ->
-        Language.find_dialect_for_base_in_files(language, available_languages)
+        Language.find_dialect_for_base_in_languages(language, available_languages)
 
       # Full dialect not found - try base code match
       true ->
         base = DialectMapper.extract_base(language)
-        Language.find_dialect_for_base_in_files(base, available_languages)
+        Language.find_dialect_for_base_in_languages(base, available_languages)
     end
   end
 
diff --git a/lib/modules/publishing/web/controller/translations.ex b/lib/modules/publishing/web/controller/translations.ex
@@ -75,12 +75,12 @@ defmodule PhoenixKit.Modules.Publishing.Web.Controller.Translations do
   end
 
   # Check if a specific enabled language has published content in the group
-  # ONLY checks for EXACT file matches - no base code fallback
-  # This ensures only languages with actual files show in the public switcher
+  # ONLY checks for EXACT matches - no base code fallback
+  # This ensures only languages with actual content show in the public switcher
   # Uses passed posts to avoid redundant list_posts calls
   defp has_published_content_for_language?(posts, language) do
     Enum.any?(posts, fn post ->
-      # Check if there's a published file for this EXACT language only
+      # Check if there's published content for this EXACT language only
       # Use preloaded language_statuses map
       language in (post.available_languages || []) and
         Map.get(post.language_statuses, language) == "published"
@@ -249,30 +249,30 @@ defmodule PhoenixKit.Modules.Publishing.Web.Controller.Translations do
   defp normalize_languages([], current_language), do: [current_language]
   defp normalize_languages(languages, _current_language) when is_list(languages), do: languages
 
-  # Strict check for public display - only shows files that are:
+  # Strict check for public display - only shows languages that are:
   # 1. Directly in the enabled languages list, OR
   # 2. Base code files where any dialect of that base is enabled
   # This prevents showing en-US, en-GB etc when only en-CA is enabled
   defp language_enabled_for_public?(language, enabled_languages) do
     cond do
-      # Direct match - file code exactly matches an enabled language
+      # Direct match - language code exactly matches an enabled language
       language in enabled_languages ->
         true
 
-      # Base code file (e.g., "en") - show if any dialect is enabled
+      # Base code (e.g., "en") - show if any dialect is enabled
       Language.base_code?(language) ->
         Enum.any?(enabled_languages, fn enabled_lang ->
           DialectMapper.extract_base(enabled_lang) == language
         end)
 
-      # Dialect file (e.g., "en-US") not directly enabled - DON'T show
+      # Dialect (e.g., "en-US") not directly enabled - DON'T show
       # This is the key difference from language_enabled?
       true ->
         false
     end
   end
 
-  # Checks if the exact language file exists and is published
+  # Checks if the exact language translation exists and is published
   # Uses preloaded language_statuses map to avoid redundant DB reads
   defp translation_published_exact?(_group_slug, post, language) do
     language in (post.available_languages || []) and
diff --git a/lib/modules/publishing/web/listing.html.heex b/lib/modules/publishing/web/listing.html.heex
@@ -507,7 +507,8 @@
             {ngettext(
               "This will update the primary language setting for %{count} post in this publishing group.",
               "This will update the primary language setting for %{count} posts in this publishing group.",
-              total_needing
+              total_needing,
+              count: total_needing
             )}
           </p>
 
@@ -556,7 +557,9 @@
             phx-disable-with={gettext("Updating...")}
           >
             <.icon name="hero-arrow-path" class="w-4 h-4" />
-            {ngettext("Update %{count} Post", "Update %{count} Posts", total_needing)}
+            {ngettext("Update %{count} Post", "Update %{count} Posts", total_needing,
+              count: total_needing
+            )}
           </button>
         </div>
       </div>
diff --git a/lib/modules/publishing/web/new.html.heex b/lib/modules/publishing/web/new.html.heex
@@ -53,7 +53,7 @@
               />
               <div class="space-y-1">
                 <p class="text-xs text-base-content/60">
-                  {gettext("Used for the URL and the directory where files are stored.")}
+                  {gettext("Used for the URL and as the unique identifier for this group.")}
                 </p>
                 <p class="text-xs font-medium text-base-content/70">
                   <span class="font-semibold">{gettext("Format")}:</span>
diff --git a/lib/modules/publishing/web/settings.ex b/lib/modules/publishing/web/settings.ex
@@ -76,7 +76,7 @@ defmodule PhoenixKit.Modules.Publishing.Web.Settings do
          )}
 
       {:error, :not_found} ->
-        # Group directory doesn't exist, just remove from config
+        # Group not found in DB, just remove from config
         case Publishing.remove_group(slug) do
           {:ok, _} ->
             # The `Publishing.remove_group` call handles the broadcast. This
diff --git a/lib/modules/publishing/web/settings.html.heex b/lib/modules/publishing/web/settings.html.heex
@@ -169,9 +169,7 @@
                       phx-value-slug={group["slug"]}
                       class="btn btn-sm btn-square btn-outline text-error hover:bg-error hover:text-error-content"
                       data-confirm={
-                        gettext(
-                          "Move this group to trash? The directory will be renamed with a timestamp."
-                        )
+                        gettext("Move this group to trash? It will be renamed with a timestamp.")
                       }
                     >
                       <.icon name="hero-trash" class="w-4 h-4" />
diff --git a/lib/modules/publishing/workers/migrate_primary_language_worker.ex b/lib/modules/publishing/workers/migrate_primary_language_worker.ex
@@ -117,9 +117,9 @@ defmodule PhoenixKit.Modules.Publishing.Workers.MigratePrimaryLanguageWorker do
   @impl Oban.Worker
   def timeout(_job), do: :timer.minutes(5)
 
-  # Get post directory path from cached post
+  # Get post identifier from cached post
   # For slug mode: returns the slug (e.g., "hello")
-  # For timestamp mode: returns the date/time path (e.g., "2025-12-31/03:42")
+  # For timestamp mode: returns the date/time identifier (e.g., "2025-12-31/03:42")
   defp get_post_slug(post) do
     case post[:mode] do
       :timestamp -> derive_timestamp_post_dir(post[:path])
diff --git a/lib/modules/publishing/workers/translate_post_worker.ex b/lib/modules/publishing/workers/translate_post_worker.ex
@@ -470,8 +470,8 @@ defmodule PhoenixKit.Modules.Publishing.Workers.TranslatePostWorker do
 
     Logger.info("[TranslatePostWorker] Saving translation for #{language}...")
 
-    # Check if translation file already exists for this exact language
-    # We need to check if the file exists directly because read_post has fallback behavior
+    # Check if translation already exists for this exact language
+    # We need to check directly because read_post has fallback behavior
     # that returns a different language if the requested one doesn't exist
     case check_translation_exists(group_slug, post_slug, language, version) do
       {:ok, existing_post} ->
@@ -503,8 +503,8 @@ defmodule PhoenixKit.Modules.Publishing.Workers.TranslatePostWorker do
     case Publishing.read_post(group_slug, post_slug, language, version) do
       {:ok, post} ->
         # Verify the returned post is actually for the requested language
-        # AND that it's not a "new translation" stub (is_new_translation means the file
-        # doesn't exist and read_post returned a fallback with empty content)
+        # AND that it's not a "new translation" stub (is_new_translation means no record
+        # exists and read_post returned a fallback with empty content)
         is_new_translation = Map.get(post, :is_new_translation, false)
 
         if post.language == language && !is_new_translation do
@@ -619,9 +619,7 @@ defmodule PhoenixKit.Modules.Publishing.Workers.TranslatePostWorker do
       version: version
     } = opts
 
-    Logger.info(
-      "[TranslatePostWorker] Translation file already exists for #{language}, updating..."
-    )
+    Logger.info("[TranslatePostWorker] Translation already exists for #{language}, updating...")
 
     case Publishing.read_post(group_slug, post_slug, language, version) do
       {:ok, existing_post} ->
