diff --git a/apps/analytics/services.py b/apps/analytics/services.py index 4562c66b..59e5c67f 100644 --- a/apps/analytics/services.py +++ b/apps/analytics/services.py @@ -263,27 +263,40 @@ def account_analytics_bundle(account: SocialAccount, days: int) -> dict[str, Any ) ) by_metric: dict[str, dict[dt_date, float]] = defaultdict(dict) + captured_by_metric: dict[str, Any] = {} max_captured: Any = None metrics_with_account_data: set[str] = set() for r in rows: by_metric[r.metric_key][r.date] = r.value metrics_with_account_data.add(r.metric_key) + if r.metric_key not in captured_by_metric or r.captured_at > captured_by_metric[r.metric_key]: + captured_by_metric[r.metric_key] = r.captured_at if max_captured is None or r.captured_at > max_captured: max_captured = r.captured_at - # Hybrid fallback: for content-attribution metrics without account-level - # rows in the window, derive the daily series by summing per-post deltas - # so platforms without ``get_account_metrics`` (YouTube, TikTok, etc.) - # still get populated hero cards and charts. Roll the per-post - # ``captured_at`` into ``max_captured`` so freshness consumers don't - # report "no data" while the response is in fact populated. + # Hybrid fallback: for content-attribution metrics, derive a daily series + # by summing per-post deltas so platforms without ``get_account_metrics`` + # (YouTube, TikTok, etc.) still get populated hero cards and charts. + # + # Also prefer the per-post series when it is fresher than account-level + # rows. Facebook account insights are synced at most daily, while per-post + # metrics can refresh hourly; without this freshness check the main graph + # can lag behind the post drawer/table even though newer post snapshots are + # already stored. for m in platform_metrics: - if m in metrics_with_account_data or not _supports_post_fallback(m): + if not _supports_post_fallback(m): continue daily, fallback_captured = _post_summed_series_for_metric(account, m, start, end) - by_metric[m].update(daily) - if fallback_captured is not None and (max_captured is None or fallback_captured > max_captured): - max_captured = fallback_captured + if not daily: + continue + account_captured = captured_by_metric.get(m) + should_use_fallback = m not in metrics_with_account_data or ( + fallback_captured is not None and account_captured is not None and fallback_captured > account_captured + ) + if should_use_fallback: + by_metric[m].update(daily) + if fallback_captured is not None and (max_captured is None or fallback_captured > max_captured): + max_captured = fallback_captured series_map = { m: [by_metric[m].get(start + timedelta(days=i), 0.0) for i in range(2 * days)] for m in platform_metrics diff --git a/apps/analytics/tasks.py b/apps/analytics/tasks.py index 4ae75c43..81f0c846 100644 --- a/apps/analytics/tasks.py +++ b/apps/analytics/tasks.py @@ -380,6 +380,12 @@ def _write_post_snapshot( "youtube": frozenset({"watch_time", "avg_view_pct", "shares"}), } +_FOLLOWER_TOTAL_REFRESH_PLATFORMS: frozenset[str] = frozenset({"facebook", "instagram", "instagram_login"}) + + +def _needs_empty_follower_count_refresh(account) -> bool: + return account.follower_count <= 0 and account.platform in _FOLLOWER_TOTAL_REFRESH_PLATFORMS + def _sync_account_metrics(account, on_date: dt_date) -> None: """Fetch account-level metrics for ``on_date`` and any recent missing days. @@ -415,7 +421,9 @@ def _sync_account_metrics(account, on_date: dt_date) -> None: current_followers = None for offset in range(recent_days): target = on_date - timedelta(days=offset) - if AccountInsightsSnapshot.objects.filter(social_account=account, date=target).exists(): + has_rows_for_day = AccountInsightsSnapshot.objects.filter(social_account=account, date=target).exists() + needs_current_follower_refresh = target == on_date and _needs_empty_follower_count_refresh(account) + if has_rows_for_day and not needs_current_follower_refresh: continue start = datetime.combine(target, time.min, tzinfo=tz) end = datetime.combine(target, time.max, tzinfo=tz) @@ -687,9 +695,9 @@ def sync_all_account_analytics() -> None: # backfill (see backfill_account_analytics), so the cron resumes on its # own. The per-post Data-API loop below still runs (it uses the # publish/read scopes the account already has). - if ( - not account.analytics_needs_reconnect - and not AccountInsightsSnapshot.objects.filter(social_account=account, date=today).exists() + has_today_rows = AccountInsightsSnapshot.objects.filter(social_account=account, date=today).exists() + if not account.analytics_needs_reconnect and ( + not has_today_rows or _needs_empty_follower_count_refresh(account) ): _sync_account_metrics(account, today) diff --git a/apps/analytics/tests/test_services.py b/apps/analytics/tests/test_services.py new file mode 100644 index 00000000..6a2d6161 --- /dev/null +++ b/apps/analytics/tests/test_services.py @@ -0,0 +1,101 @@ +"""Tests for analytics read-side services.""" + +from datetime import timedelta + +import pytest +from django.utils import timezone + +from apps.social_accounts.models import SocialAccount + + +@pytest.fixture +def workspace(db, organization): + from apps.workspaces.models import Workspace + + return Workspace.objects.create(name="Analytics Services WS", organization=organization) + + +@pytest.fixture +def facebook_account(workspace): + return SocialAccount.objects.create( + workspace=workspace, + platform="facebook", + account_platform_id="page-1", + account_name="Facebook Page", + oauth_access_token="token", + connection_status=SocialAccount.ConnectionStatus.CONNECTED, + ) + + +def _published_platform_post(account): + from apps.composer.models import PlatformPost, Post + + post = Post.objects.create(workspace=account.workspace, caption="hello") + return PlatformPost.objects.create( + post=post, + social_account=account, + status=PlatformPost.Status.PUBLISHED, + published_at=timezone.now(), + platform_post_id="post-1", + ) + + +@pytest.mark.django_db +def test_account_bundle_prefers_fresher_post_fallback_for_content_metrics(facebook_account): + """Per-post Facebook analytics can refresh hourly while account snapshots are + daily. The main graph should use the fresher post-derived value instead of + leaving the overall insight chip/card stuck on the stale account row. + """ + from apps.analytics.models import AccountInsightsSnapshot, PostInsightsSnapshot + from apps.analytics.services import account_analytics_bundle + + today = timezone.now().date() + old_capture = timezone.now() - timedelta(hours=2) + new_capture = timezone.now() + platform_post = _published_platform_post(facebook_account) + + account_row = AccountInsightsSnapshot.objects.create( + social_account=facebook_account, + metric_key="views", + date=today, + value=10, + ) + AccountInsightsSnapshot.objects.filter(id=account_row.id).update(captured_at=old_capture) + + post_row = PostInsightsSnapshot.objects.create( + platform_post=platform_post, + metric_key="views", + date=today, + value=42, + ) + PostInsightsSnapshot.objects.filter(id=post_row.id).update(captured_at=new_capture) + + series = account_analytics_bundle(facebook_account, 7)["series_map"]["views"] + + assert series[-1] == 42 + + +@pytest.mark.django_db +def test_account_bundle_keeps_account_reach_instead_of_summing_post_reach(facebook_account): + from apps.analytics.models import AccountInsightsSnapshot, PostInsightsSnapshot + from apps.analytics.services import account_analytics_bundle + + today = timezone.now().date() + platform_post = _published_platform_post(facebook_account) + + AccountInsightsSnapshot.objects.create( + social_account=facebook_account, + metric_key="reach", + date=today, + value=10, + ) + PostInsightsSnapshot.objects.create( + platform_post=platform_post, + metric_key="reach", + date=today, + value=42, + ) + + series = account_analytics_bundle(facebook_account, 7)["series_map"]["reach"] + + assert series[-1] == 10 diff --git a/apps/analytics/tests/test_tasks.py b/apps/analytics/tests/test_tasks.py index 6f33201e..2a0d6627 100644 --- a/apps/analytics/tests/test_tasks.py +++ b/apps/analytics/tests/test_tasks.py @@ -157,3 +157,44 @@ def test_sync_account_metrics_recovers_followers_from_later_offset(workspace): assert not AccountInsightsSnapshot.objects.filter( social_account=account, date__lt=today, metric_key="followers" ).exists() + + +@pytest.mark.django_db +def test_sync_account_metrics_refreshes_empty_follower_count_when_today_rows_exist(workspace): + """Existing daily account snapshots must not strand the header follower total + at 0. This commonly affects Facebook accounts connected before + ``followers_count`` was persisted during page selection. + """ + from datetime import date + from unittest.mock import MagicMock, patch + + from apps.analytics.models import AccountInsightsSnapshot + from apps.analytics.tasks import _sync_account_metrics + from providers.types import AccountMetrics + + account = SocialAccount.objects.create( + workspace=workspace, + platform="facebook", + account_platform_id="page-1", + account_name="FB One", + follower_count=0, + oauth_access_token="token", + oauth_refresh_token="refresh", + connection_status=SocialAccount.ConnectionStatus.CONNECTED, + ) + today = date(2026, 6, 24) + AccountInsightsSnapshot.objects.create( + social_account=account, + date=today, + metric_key="views", + value=10, + ) + fake_provider = MagicMock() + fake_provider.account_metrics_supports_date_range = True + fake_provider.get_account_metrics.return_value = AccountMetrics(followers=1234, followers_gained=2) + + with patch("apps.analytics.tasks._resolve_provider", return_value=fake_provider): + _sync_account_metrics(account, today) + + account.refresh_from_db() + assert account.follower_count == 1234 diff --git a/apps/composer/migrations/0018_normalize_facebook_platform_post_ids.py b/apps/composer/migrations/0018_normalize_facebook_platform_post_ids.py new file mode 100644 index 00000000..f9a6900f --- /dev/null +++ b/apps/composer/migrations/0018_normalize_facebook_platform_post_ids.py @@ -0,0 +1,29 @@ +from django.db import migrations + + +def normalize_facebook_platform_post_ids(apps, schema_editor): + PlatformPost = apps.get_model("composer", "PlatformPost") + + queryset = PlatformPost.objects.filter( + social_account__platform="facebook", + platform_post_id__contains="_", + ).exclude(platform_post_id="") + + for platform_post in queryset.iterator(): + platform_post.platform_post_id = str(platform_post.platform_post_id).rsplit("_", 1)[1] + platform_post.save(update_fields=["platform_post_id"]) + + +def noop_reverse(apps, schema_editor): + pass + + +class Migration(migrations.Migration): + + dependencies = [ + ("composer", "0017_post_proposed_publish_at"), + ] + + operations = [ + migrations.RunPython(normalize_facebook_platform_post_ids, noop_reverse), + ] diff --git a/apps/publisher/engine.py b/apps/publisher/engine.py index 0d9fa0c9..9389e3ae 100644 --- a/apps/publisher/engine.py +++ b/apps/publisher/engine.py @@ -82,6 +82,8 @@ def _resolve_publish_credentials(account): "Bluesky PDS URL failed SSRF check for account %s", account.id, ) + elif platform == "facebook": + credentials["page_id"] = account.account_platform_id elif platform == "instagram": credentials["ig_user_id"] = account.account_platform_id @@ -220,6 +222,12 @@ def _publish_platform_post(self, platform_post): if result["success"]: platform_post.platform_post_id = result.get("platform_post_id", "") + response_extra = result.get("response") + if isinstance(response_extra, dict) and response_extra: + platform_post.platform_extra = { + **(platform_post.platform_extra or {}), + **response_extra, + } platform_post.status = PlatformPost.Status.PUBLISHED platform_post.published_at = timezone.now() platform_post.save() diff --git a/apps/publisher/tests.py b/apps/publisher/tests.py index 265a0bcb..5eb7a818 100644 --- a/apps/publisher/tests.py +++ b/apps/publisher/tests.py @@ -197,6 +197,17 @@ def test_does_not_inject_author_for_other_platforms(self, _mock_creds, mock_get_ class ResolvePublishCredentialsTest(SimpleTestCase): + @patch("apps.publisher.engine.resolve_platform_credentials", return_value={"client_id": "id"}) + def test_facebook_credentials_include_selected_page_id(self, _mock_resolve): + account = MagicMock() + account.platform = "facebook" + account.account_platform_id = "page-1" + account.workspace.organization_id = "org-1" + + credentials = _resolve_publish_credentials(account) + + self.assertEqual(credentials["page_id"], "page-1") + @patch("apps.publisher.engine.resolve_platform_credentials", return_value={"client_id": "id"}) def test_instagram_credentials_include_selected_ig_user_id(self, _mock_resolve): account = MagicMock() @@ -340,6 +351,30 @@ def test_publish_success_removes_queue_entry(self): # The QueueEntry is gone (slot freed), but the PlatformPost remains. self.assertFalse(QueueEntry.objects.filter(id=self.entry.id).exists()) + def test_publish_success_stores_response_extra_on_platform_extra(self): + from apps.composer.models import PlatformPost + + self.pp.platform_extra = {"post_type": "text"} + self.pp.save(update_fields=["platform_extra"]) + + engine = PublishEngine() + success = { + "success": True, + "platform_post_id": "post-1", + "status_code": 200, + "response": {"id": "page-1_post-1", "tracking": {"source": "graph"}}, + } + with patch.object(PublishEngine, "_dispatch_to_provider", return_value=success): + engine._publish_platform_post(self.pp) + + self.pp.refresh_from_db() + self.assertEqual(self.pp.status, PlatformPost.Status.PUBLISHED) + self.assertEqual(self.pp.platform_post_id, "post-1") + self.assertEqual( + self.pp.platform_extra, + {"post_type": "text", "id": "page-1_post-1", "tracking": {"source": "graph"}}, + ) + def test_publish_success_survives_queue_cleanup_failure(self): from apps.composer.models import PlatformPost diff --git a/providers/facebook.py b/providers/facebook.py index b443b2c7..37065262 100644 --- a/providers/facebook.py +++ b/providers/facebook.py @@ -294,8 +294,9 @@ def _publish_text_or_link(self, access_token: str, page_id: str, content: Publis json=payload, ) data = resp.json() + post_id = self._stored_post_id(data["id"]) return PublishResult( - platform_post_id=data["id"], + platform_post_id=post_id, url=f"https://www.facebook.com/{data['id']}", extra=data, ) @@ -314,8 +315,13 @@ def _publish_photo(self, access_token: str, page_id: str, content: PublishConten json=payload, ) data = resp.json() - post_id = data.get("post_id", data["id"]) - return PublishResult(platform_post_id=post_id, url=f"https://www.facebook.com/{post_id}", extra=data) + graph_post_id = data.get("post_id", data["id"]) + post_id = self._stored_post_id(graph_post_id) + return PublishResult( + platform_post_id=post_id, + url=f"https://www.facebook.com/{graph_post_id}", + extra=data, + ) def _publish_multi_photo(self, access_token: str, page_id: str, content: PublishContent) -> PublishResult: urls = content.media_urls @@ -363,8 +369,8 @@ def _publish_multi_photo(self, access_token: str, page_id: str, content: Publish access_token=access_token, json=payload, ).json() - post_id = data.get("id") - if not post_id: + graph_post_id = data.get("id") + if not graph_post_id: raise PublishError( "Failed to publish Facebook multi-photo post", platform=self.platform_name, @@ -378,8 +384,8 @@ def _publish_multi_photo(self, access_token: str, page_id: str, content: Publish raise return PublishResult( - platform_post_id=post_id, - url=f"https://www.facebook.com/{post_id}", + platform_post_id=self._stored_post_id(graph_post_id), + url=f"https://www.facebook.com/{graph_post_id}", extra={**data, "photo_ids": photo_ids}, ) @@ -433,8 +439,9 @@ def _publish_video(self, access_token: str, page_id: str, content: PublishConten # body making .json() raise). Catch broadly; fall back to video_id. logger.debug("Facebook video %s post_id unavailable: %s", video_id, exc) - post_id = video_fields.get("post_id") or video_id - url = video_fields.get("permalink_url") or f"https://www.facebook.com/{post_id}" + graph_post_id = video_fields.get("post_id") or video_id + post_id = self._stored_post_id(graph_post_id) + url = video_fields.get("permalink_url") or f"https://www.facebook.com/{graph_post_id}" return PublishResult( platform_post_id=post_id, url=url, @@ -446,6 +453,7 @@ def _publish_video(self, access_token: str, page_id: str, content: PublishConten # ------------------------------------------------------------------ def publish_comment(self, access_token: str, post_id: str, text: str) -> CommentResult: + post_id = self._page_scoped_post_id(post_id) resp = self._request( "POST", f"{BASE_URL}/{post_id}/comments", @@ -554,6 +562,23 @@ def _get_post_fields(self, access_token: str, post_id: str) -> dict: break return {} + @staticmethod + def _stored_post_id(graph_post_id: str) -> str: + """Store only Facebook's post object id from PAGEID_POSTID values.""" + post_id = str(graph_post_id or "") + if "_" in post_id: + return post_id.rsplit("_", 1)[1] + return post_id + + def _page_scoped_post_id(self, post_id: str) -> str: + post_id = str(post_id or "") + if "_" in post_id: + return post_id + page_id = self.credentials.get("page_id") + if page_id and post_id: + return f"{page_id}_{post_id}" + return post_id + @staticmethod def _summary_total(data: dict, key: str) -> int: value = data.get(key, {}) diff --git a/providers/meta_insights.py b/providers/meta_insights.py index 5a086eed..286dcabb 100644 --- a/providers/meta_insights.py +++ b/providers/meta_insights.py @@ -25,14 +25,22 @@ def parse_insights_response(data: dict[str, Any]) -> dict[str, Any]: values: dict[str, Any] = {} + periods: dict[str, str] = {} for entry in data.get("data", []): name = entry.get("name", "") if not name: continue + period = entry.get("period", "") + if periods.get(name) == "lifetime" and period != "lifetime": + continue if "total_value" in entry: - values[name] = entry.get("total_value", {}).get("value", 0) + value = entry.get("total_value", {}).get("value", 0) + else: + value = entry.get("values", [{}])[0].get("value", 0) + if name not in values or period == "lifetime": + values[name] = value + periods[name] = period continue - values[name] = entry.get("values", [{}])[0].get("value", 0) return values diff --git a/tests/providers/test_facebook.py b/tests/providers/test_facebook.py index 862844b1..ec794cd9 100644 --- a/tests/providers/test_facebook.py +++ b/tests/providers/test_facebook.py @@ -38,7 +38,7 @@ def test_publish_multi_photo_post_stages_photos_then_publishes_feed_post(): ), ) - assert result.platform_post_id == "page-1_post-1" + assert result.platform_post_id == "post-1" assert result.url == "https://www.facebook.com/page-1_post-1" assert result.extra["photo_ids"] == ["photo-1", "photo-2"] provider._request.assert_has_calls( @@ -124,7 +124,7 @@ def test_publish_single_photo_uses_photos_edge_without_staging(): ), ) - assert result.platform_post_id == "page-1_post-1" + assert result.platform_post_id == "post-1" assert result.url == "https://www.facebook.com/page-1_post-1" provider._request.assert_called_once_with( "POST", @@ -318,8 +318,8 @@ def test_get_post_metrics_uses_v25_media_view_metrics_and_object_counts(): metrics = provider.get_post_metrics("page-token", "page-1_post-1") - assert metrics.video_views == 54 assert metrics.reach == 42 + assert metrics.video_views == 54 assert metrics.clicks == 4 assert metrics.likes == 0 assert metrics.comments == 5 @@ -420,8 +420,8 @@ def test_get_post_metrics_resolves_photo_id_to_feed_post_for_comments_and_shares metrics = provider.get_post_metrics("page-token", "photo-1") - assert metrics.video_views == 500 assert metrics.reach == 300 + assert metrics.video_views == 500 assert metrics.clicks == 20 assert metrics.comments == 4 assert metrics.shares == 2 @@ -462,8 +462,8 @@ def test_get_post_metrics_tries_page_scoped_feed_id_for_numeric_object_id(): metrics = provider.get_post_metrics("page-token", "1668168861075953") - assert metrics.video_views == 90 assert metrics.reach == 70 + assert metrics.video_views == 90 assert metrics.comments == 6 assert metrics.shares == 8 assert metrics.extra["insight_post_id"] == "page-1_1668168861075953" @@ -501,8 +501,8 @@ def test_get_post_metrics_tries_next_candidate_when_feed_id_has_no_insights_edge metrics = provider.get_post_metrics("page-token", "1668168861075953") - assert metrics.video_views == 12 assert metrics.reach == 10 + assert metrics.video_views == 12 assert metrics.comments == 2 assert metrics.extra["insight_post_id"] == "1668168861075953" assert metrics.extra["attempted_insight_post_ids"] == ["page-1_1668168861075953", "1668168861075953"] @@ -539,6 +539,21 @@ def test_get_post_metrics_reports_batched_insights_failure_for_each_metric(): assert "post_total_media_view_unique" in metrics.extra["insight_errors"] +def test_publish_comment_reconstructs_page_scoped_facebook_post_id(): + provider = FacebookProvider({"client_id": "id", "client_secret": "secret", "page_id": "page-1"}) + provider._request = MagicMock(return_value=_resp({"id": "comment-1"})) + + result = provider.publish_comment("page-token", "post-1", "Nice") + + assert result.platform_comment_id == "comment-1" + provider._request.assert_called_once_with( + "POST", + "https://graph.facebook.com/v25.0/page-1_post-1/comments", + access_token="page-token", + json={"message": "Nice"}, + ) + + def test_get_account_metrics_uses_v25_page_media_view_metrics_and_followers_count(): provider = FacebookProvider({"client_id": "id", "client_secret": "secret", "page_id": "page-1"}) provider._request = MagicMock( @@ -683,7 +698,7 @@ def test_publish_video_resolves_feed_post_id_for_analytics(): ), ) - assert result.platform_post_id == "page-1_post-1" + assert result.platform_post_id == "post-1" assert result.url == "https://www.facebook.com/page-1/videos/video-1/" assert result.extra["video_id"] == "video-1" provider._request.assert_has_calls( diff --git a/tests/providers/test_meta_insights.py b/tests/providers/test_meta_insights.py index 12e55348..79a2ac75 100644 --- a/tests/providers/test_meta_insights.py +++ b/tests/providers/test_meta_insights.py @@ -3,7 +3,7 @@ import pytest from providers.exceptions import APIError -from providers.meta_insights import fetch_insights_safe +from providers.meta_insights import fetch_insights_safe, parse_insights_response def _resp(data): @@ -60,3 +60,35 @@ def test_fetch_insights_safe_reraises_permission_errors(): ) assert excinfo.value is permission_error + + +def test_parse_insights_response_prefers_lifetime_when_metric_name_repeats(): + values = parse_insights_response( + { + "data": [ + { + "name": "post_media_view", + "period": "lifetime", + "values": [{"value": 32741}], + }, + { + "name": "post_total_media_view_unique", + "period": "lifetime", + "values": [{"value": 21105}], + }, + { + "name": "post_total_media_view_unique", + "period": "day", + "values": [ + {"value": 0, "end_time": "2026-06-21T07:00:00+0000"}, + {"value": 0, "end_time": "2026-06-22T07:00:00+0000"}, + ], + }, + ] + } + ) + + assert values == { + "post_media_view": 32741, + "post_total_media_view_unique": 21105, + }