diff --git a/lib/revstack/tracking/service.ex b/lib/revstack/tracking/service.ex index 29db683..1ec9216 100644 --- a/lib/revstack/tracking/service.ex +++ b/lib/revstack/tracking/service.ex @@ -26,7 +26,8 @@ defmodule Revstack.Tracking.Service do method = Map.get(attrs, :method) with {:ok, visitor} <- find_or_create_visitor(ip, user_agent, referrer), - {:ok, _page_visit} <- create_page_visit(visitor, path, full_url, query_string, method) do + {:ok, _page_visit} <- + create_page_visit(visitor, path, full_url, query_string, method, referrer) do maybe_enrich_location(visitor) {:ok, visitor} end @@ -39,7 +40,9 @@ defmodule Revstack.Tracking.Service do def find_or_create_visitor(ip_address, user_agent \\ nil, referrer \\ nil) do case Visitor.by_ip(ip_address, authorize?: false) do {:ok, visitor} -> - Visitor.record_visit(visitor, authorize?: false) + with {:ok, visitor} <- maybe_backfill_referrer(visitor, referrer) do + Visitor.record_visit(visitor, authorize?: false) + end {:error, _} -> Visitor.create( @@ -56,19 +59,42 @@ defmodule Revstack.Tracking.Service do @doc """ Creates a page visit record for a visitor. """ - def create_page_visit(visitor, path, full_url \\ nil, query_string \\ nil, method \\ nil) do + def create_page_visit( + visitor, + path, + full_url \\ nil, + query_string \\ nil, + method \\ nil, + referrer \\ nil + ) do VisitorPageVisit.create( %{ visitor_id: visitor.id, path: path, full_url: full_url, query_string: query_string, - method: method + method: method, + referrer: referrer }, authorize?: false ) end + defp maybe_backfill_referrer(visitor, referrer) do + cond do + present_referrer?(visitor.referrer) -> + {:ok, visitor} + + present_referrer?(referrer) -> + Visitor.backfill_referrer(visitor, %{referrer: referrer}, authorize?: false) + + true -> + {:ok, visitor} + end + end + + defp present_referrer?(value), do: is_binary(value) and String.trim(value) != "" + @doc """ Triggers location enrichment for a visitor if location data is missing. Runs asynchronously to avoid blocking page rendering. diff --git a/lib/revstack/tracking/visitor.ex b/lib/revstack/tracking/visitor.ex index c9736e6..c6a752f 100644 --- a/lib/revstack/tracking/visitor.ex +++ b/lib/revstack/tracking/visitor.ex @@ -15,6 +15,7 @@ defmodule Revstack.Tracking.Visitor do define :read, action: :read define :by_ip, action: :by_ip, args: [:ip_address] define :record_visit, action: :record_visit + define :backfill_referrer, action: :backfill_referrer define :enrich_location, action: :enrich_location define :destroy, action: :destroy end @@ -53,6 +54,12 @@ defmodule Revstack.Tracking.Visitor do change increment(:visit_count) end + update :backfill_referrer do + require_atomic? false + + accept [:referrer] + end + update :enrich_location do require_atomic? false diff --git a/lib/revstack/tracking/visitor_page_visit.ex b/lib/revstack/tracking/visitor_page_visit.ex index b8440a4..d6d86c0 100644 --- a/lib/revstack/tracking/visitor_page_visit.ex +++ b/lib/revstack/tracking/visitor_page_visit.ex @@ -26,6 +26,7 @@ defmodule Revstack.Tracking.VisitorPageVisit do :full_url, :query_string, :method, + :referrer, :visitor_id ] @@ -80,6 +81,11 @@ defmodule Revstack.Tracking.VisitorPageVisit do public? true end + attribute :referrer, :string do + allow_nil? true + public? true + end + attribute :visited_at, :utc_datetime do allow_nil? false public? true diff --git a/lib/revstack_web/plugs/visitor_tracking.ex b/lib/revstack_web/plugs/visitor_tracking.ex index 7872641..f999541 100644 --- a/lib/revstack_web/plugs/visitor_tracking.ex +++ b/lib/revstack_web/plugs/visitor_tracking.ex @@ -17,7 +17,7 @@ defmodule RevstackWeb.Plugs.VisitorTracking do def call(conn, _opts) do ip = Service.extract_ip(conn) user_agent = get_user_agent(conn) - referrer = get_referrer(conn) + referrer = get_referrer(conn) || Plug.Conn.get_session(conn, :visitor_referrer) conn |> Plug.Conn.assign(:visitor_ip, ip) @@ -54,5 +54,15 @@ defmodule RevstackWeb.Plugs.VisitorTracking do conn |> Plug.Conn.get_req_header("referer") |> List.first() + |> normalize_blank() end + + defp normalize_blank(value) when is_binary(value) do + case String.trim(value) do + "" -> nil + trimmed -> trimmed + end + end + + defp normalize_blank(_value), do: nil end diff --git a/priv/repo/migrations/20260413202952_add_referrer_to_visitor_page_visits.exs b/priv/repo/migrations/20260413202952_add_referrer_to_visitor_page_visits.exs new file mode 100644 index 0000000..7b674f4 --- /dev/null +++ b/priv/repo/migrations/20260413202952_add_referrer_to_visitor_page_visits.exs @@ -0,0 +1,21 @@ +defmodule Revstack.Repo.Migrations.AddReferrerToVisitorPageVisits do + @moduledoc """ + Updates resources based on their most recent snapshots. + + This file was autogenerated with `mix ash_postgres.generate_migrations` + """ + + use Ecto.Migration + + def up do + alter table(:visitor_page_visits) do + add(:referrer, :text) + end + end + + def down do + alter table(:visitor_page_visits) do + remove(:referrer) + end + end +end diff --git a/priv/resource_snapshots/repo/visitor_page_visits/20260413202952.json b/priv/resource_snapshots/repo/visitor_page_visits/20260413202952.json new file mode 100644 index 0000000..aeedddd --- /dev/null +++ b/priv/resource_snapshots/repo/visitor_page_visits/20260413202952.json @@ -0,0 +1,159 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"gen_random_uuid()\")", + "generated?": false, + "precision": null, + "primary_key?": true, + "references": null, + "scale": null, + "size": null, + "source": "id", + "type": "uuid" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "path", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "full_url", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "query_string", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "method", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "referrer", + "type": "text" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "visited_at", + "type": "utc_datetime" + }, + { + "allow_nil?": false, + "default": "fragment(\"(now() AT TIME ZONE 'utc')\")", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "inserted_at", + "type": "utc_datetime_usec" + }, + { + "allow_nil?": false, + "default": "fragment(\"(now() AT TIME ZONE 'utc')\")", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "updated_at", + "type": "utc_datetime_usec" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": { + "deferrable": false, + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "index?": false, + "match_type": null, + "match_with": null, + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "name": "visitor_page_visits_visitor_id_fkey", + "on_delete": null, + "on_update": null, + "primary_key?": true, + "schema": null, + "table": "visitors" + }, + "scale": null, + "size": null, + "source": "visitor_id", + "type": "uuid" + } + ], + "base_filter": null, + "check_constraints": [], + "create_table_options": null, + "custom_indexes": [], + "custom_statements": [], + "has_create_action": true, + "hash": "CA26469D09F600007777880AC5713745DD76FBCAF8FE41A0B74034325D189045", + "identities": [], + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "repo": "Elixir.Revstack.Repo", + "schema": null, + "table": "visitor_page_visits" +} \ No newline at end of file diff --git a/test/revstack/tracking/service_test.exs b/test/revstack/tracking/service_test.exs index 71776ae..eb82e8d 100644 --- a/test/revstack/tracking/service_test.exs +++ b/test/revstack/tracking/service_test.exs @@ -49,6 +49,28 @@ defmodule Revstack.Tracking.ServiceTest do assert visitor.referrer == "https://google.com" end + + test "backfills referrer for existing visitor when missing" do + {:ok, visitor1} = Service.find_or_create_visitor("203.0.113.6", "TestAgent", nil) + assert is_nil(visitor1.referrer) + + {:ok, visitor2} = + Service.find_or_create_visitor("203.0.113.6", "TestAgent", "https://google.com") + + assert visitor2.id == visitor1.id + assert visitor2.referrer == "https://google.com" + end + + test "does not overwrite existing referrer for existing visitor" do + {:ok, visitor1} = + Service.find_or_create_visitor("203.0.113.7", "TestAgent", "https://google.com") + + {:ok, visitor2} = + Service.find_or_create_visitor("203.0.113.7", "TestAgent", "https://bing.com") + + assert visitor2.id == visitor1.id + assert visitor2.referrer == "https://google.com" + end end describe "create_page_visit/5" do @@ -78,6 +100,22 @@ defmodule Revstack.Tracking.ServiceTest do assert visit.method == "GET" end + test "stores referrer on page visit" do + {:ok, visitor} = Service.find_or_create_visitor("203.0.113.13") + + assert {:ok, visit} = + Service.create_page_visit( + visitor, + "/services", + "http://localhost/services", + nil, + "GET", + "https://example.com/source" + ) + + assert visit.referrer == "https://example.com/source" + end + test "creates multiple page visits for same visitor" do {:ok, visitor} = Service.find_or_create_visitor("203.0.113.12") @@ -96,7 +134,7 @@ defmodule Revstack.Tracking.ServiceTest do Service.track_page_visit(%{ ip_address: "203.0.113.20", user_agent: "TestUA", - referrer: nil, + referrer: "https://example.com/source", path: "/", full_url: "http://localhost/", query_string: nil, @@ -109,6 +147,7 @@ defmodule Revstack.Tracking.ServiceTest do visits = VisitorPageVisit.for_visitor!(visitor.id, authorize?: false) assert length(visits) == 1 assert hd(visits).path == "/" + assert hd(visits).referrer == "https://example.com/source" end test "tracks multiple pages for the same visitor" do diff --git a/test/revstack/tracking/visitor_page_visit_resource_test.exs b/test/revstack/tracking/visitor_page_visit_resource_test.exs index cf46e89..048b473 100644 --- a/test/revstack/tracking/visitor_page_visit_resource_test.exs +++ b/test/revstack/tracking/visitor_page_visit_resource_test.exs @@ -41,6 +41,20 @@ defmodule Revstack.Tracking.VisitorPageVisitResourceTest do assert visit.method == "GET" end + test "stores optional referrer", %{visitor: visitor} do + {:ok, visit} = + VisitorPageVisit.create( + %{ + visitor_id: visitor.id, + path: "/services", + referrer: "https://example.com/source" + }, + authorize?: false + ) + + assert visit.referrer == "https://example.com/source" + end + test "requires visitor_id", %{} do assert {:error, _} = VisitorPageVisit.create(%{path: "/test"}, authorize?: false) diff --git a/test/revstack_web/plugs/visitor_tracking_test.exs b/test/revstack_web/plugs/visitor_tracking_test.exs index ab296fe..d036e5c 100644 --- a/test/revstack_web/plugs/visitor_tracking_test.exs +++ b/test/revstack_web/plugs/visitor_tracking_test.exs @@ -24,6 +24,18 @@ defmodule RevstackWeb.Plugs.VisitorTrackingTest do assert get_session(conn, :visitor_referrer) == "https://example.com/source" end + test "preserves existing session referrer when request has no referer", %{conn: conn} do + conn = + conn + |> init_test_session(%{visitor_referrer: "https://example.com/original"}) + |> put_req_header("user-agent", "TrackingTest/1.0") + |> get("/about") + + conn = fetch_session(conn) + + assert get_session(conn, :visitor_referrer) == "https://example.com/original" + end + test "stores visitor IP in session for LiveView access", %{conn: conn} do conn = conn