Skip to content

Extract Newsletters module to phoenix_kit_newsletters package - #413

Merged
ddon merged 14 commits into
BeamLabEU:devfrom
timujinne:dev
Mar 15, 2026
Merged

Extract Newsletters module to phoenix_kit_newsletters package#413
ddon merged 14 commits into
BeamLabEU:devfrom
timujinne:dev

Conversation

@timujinne

Copy link
Copy Markdown
Contributor

Summary

Extracts PhoenixKit.Modules.Newsletters from phoenix_kit core into a standalone package (phoenix_kit_newsletters), and adds a cross-module dependency system to PhoenixKit.ModuleRegistry.

Changes

New: required_modules/0 callback system

  • PhoenixKit.Module — new optional callback required_modules/0 :: [String.t()] (default: [])
  • PhoenixKit.ModuleRegistry — new dependency_warnings/0 function that detects enabled modules with disabled dependencies
  • Admin Modules page — displays dependency warnings in UI when required modules are not enabled

New: Auto-discovery for external module public routes

  • integration.ex — new compile_module_public_routes/1 that auto-discovers public (non-admin) routes from external PhoenixKit modules via route_module/0
  • Replaces the previous hardcoded safe_route_call(NewslettersRoutes, ...) pattern
  • Module public routes placed before publishing/localized catch-all routes to prevent /:language/:group from intercepting paths like /newsletters/unsubscribe

Removed: Newsletters module from core

  • Deleted lib/modules/newsletters/ (15 files)
  • Deleted lib/phoenix_kit_web/routes/newsletters.ex
  • Removed PhoenixKit.Modules.Newsletters from internal_modules/0 in ModuleRegistry
  • sqs_processor.ex and process_scheduled_jobs_worker.ex — replaced direct Newsletters.* calls with ModuleRegistry.get_by_key("newsletters") + Code.ensure_loaded? guards

V79 migration stays in core

Tables (phoenix_kit_newsletters_*) continue to be created by phoenix_kit's versioned migration system. The new package contains only Elixir code.

Related

New package: BeamLabEU/phoenix_kit_newsletters#1

Test plan

  • mix compile --warnings-as-errors passes
  • mix credo --strict — no issues
  • mix dialyzer — passes
  • Verified in Decor 3D Print parent app with {:phoenix_kit_newsletters, path: "..."} added:
    • ModuleRegistry.all_modules() includes PhoenixKit.Modules.Newsletters
    • dependency_warnings/0 returns correct warnings when Emails disabled
    • Auto-discovered unsubscribe routes load correctly

timujinne and others added 14 commits March 14, 2026 22:14
Module was renamed from PhoenixKit.Modules.Mailing to
PhoenixKit.Modules.Newsletters but the smoke test list
was not updated, causing setup_all to fail on Code.ensure_loaded!/1.
Fix module_test: replace Mailing with Newsletters
…ix task

SeedTemplates.run() calls System.halt(1) on failure, which kills the
migration process. Use Templates.seed_system_templates() directly
(same approach as V31) so validation failures are logged gracefully
without aborting the migration.
System.halt(1) kills the entire VM when called from migrations (V15
calls SeedTemplates.run() and has a try/rescue wrapper, but System.halt
bypasses all rescue handlers). Raising an exception instead allows
migration rescue blocks to handle the failure gracefully.
Fix SeedTemplates: raise exception instead of System.halt on failure
…ion settings

Co-authored-by: alexdont <alexdont@users.noreply.github.com>
- Add TableRowMenu Phoenix component (table_row_menu.ex) with link,
  button, and divider sub-components supporting color variants
- Add RowMenu JS hook using position:fixed to escape overflow-clip
  table containers; handles viewport edge detection, keyboard nav,
  and outside-click dismissal
- Replace inline action buttons in Users, Roles, Sessions,
  Live Sessions, and Emails tables with dropdown menus
- Add toggleable card/table view to Roles, Sessions, and Live Sessions
  tables (Users and Emails already had it); replaces manual
  md:hidden/md:block split in Sessions with unified table_default
- Register TableRowMenu import in phoenix_kit_web.ex
@ddon
ddon merged commit 02a6929 into BeamLabEU:dev Mar 15, 2026
6 checks passed
ddon added a commit that referenced this pull request Mar 15, 2026
…le SQS broadcast counter

- Extract duplicated permission check in roles template into @can_manage_permissions assign
- Replace Module.concat("Broadcast") with newsletters_mod.increment_broadcast_counter/2 call
- Remove unused Ecto.Query import from SQS processor
- Add PR review files for #413 and #414

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ddon added a commit that referenced this pull request Mar 15, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@timujinne

Copy link
Copy Markdown
Contributor Author

Code review

Found 2 issues:

  1. Test suite broken: PhoenixKit.Modules.Newsletters added to @all_internal_modules but the module was extracted and deleted in this same PR. setup_all calls Code.ensure_loaded!/1 on every entry in this list, which will raise ArgumentError: could not load module PhoenixKit.Modules.Newsletters due to reason :nofile, preventing all tests in the file from running. The entry should be removed (or replaced with the correct internal module name if MailingNewsletters was a rename).

PhoenixKit.Modules.Sync,
PhoenixKit.Modules.CustomerService,
PhoenixKit.Modules.Newsletters,
PhoenixKit.Jobs
]
# Ensure all modules are loaded so function_exported? works even when
# running individual tests (where the "loadable" test may be excluded).
setup_all do
Enum.each(@all_internal_modules, &Code.ensure_loaded!/1)
:ok
end

  1. Asymmetric function_exported? guard in process_scheduled_jobs_worker.ex: The code guards newsletters_mod.enabled?() with function_exported?(newsletters_mod, :enabled?, 0) but calls newsletters_mod.process_scheduled_broadcasts() on the next line without a corresponding guard. If an external Newsletters package implements enabled?/0 but not process_scheduled_broadcasts/0 (e.g., a version mismatch), the Oban worker will crash with an UndefinedFunctionError. Compare with sqs_processor.ex which consistently applies function_exported? before every dynamic call.

newsletters_mod = PhoenixKit.ModuleRegistry.get_by_key("newsletters")
if newsletters_mod && Code.ensure_loaded?(newsletters_mod) &&
function_exported?(newsletters_mod, :enabled?, 0) &&
newsletters_mod.enabled?() do
{:ok, newsletters_count} = newsletters_mod.process_scheduled_broadcasts()
if newsletters_count > 0 do

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants