Convert ScanPostsJob tests to use SpecUtil.enqueued_job_args and add rescan tests

- Convert existing job mocking to use SpecUtil.enqueued_job_args helper - Remove allow(Domain::StaticFileJob).to receive(:perform_later) mocking - Add comprehensive test context for rescanning users with pending files - Create domain_post_file_bluesky_post_file factory for test objects - Add tests verifying enqueue_pending_files_job behavior during rescans - Ensure only pending files get jobs enqueued, not already processed files - Use force_scan: true to bypass scan frequency limits in tests
2025-08-10 20:49:26 +00:00
parent ded26741a8
commit 40c6d44100
13 changed files with 312 additions and 110 deletions
--- a/app/helpers/domain/posts_helper.rb
+++ b/app/helpers/domain/posts_helper.rb
@@ -211,50 +211,40 @@ module Domain::PostsHelper
          log_entry = file.log_entry

          # Generate thumbnail path
-          begin
-            if log_entry && (response_sha256 = log_entry.response_sha256)
-              thumbnail_path =
-                blob_path(
-                  HexUtil.bin2hex(response_sha256),
-                  format: "jpg",
-                  thumb: "small",
-                )
-            end
-          rescue StandardError
-            # thumbnail_path remains nil
+
+          if log_entry && (response_sha256 = log_entry.response_sha256)
+            thumbnail_path =
+              blob_path(
+                HexUtil.bin2hex(response_sha256),
+                format: "jpg",
+                thumb: "small",
+              )
          end

          # Generate content HTML
-          begin
-            content_html =
-              ApplicationController.renderer.render(
-                partial: "log_entries/content_container",
-                locals: {
-                  log_entry: log_entry,
-                },
-                assigns: {
-                  current_user: nil,
-                },
-              )
-          rescue StandardError
-            # content_html remains nil
-          end
+          content_html =
+            ApplicationController.renderer.render(
+              partial: "log_entries/content_container",
+              locals: {
+                log_entry: log_entry,
+              },
+              assigns: {
+                current_user: nil,
+              },
+            )

          # Generate file details HTML
-          begin
-            file_details_html =
-              ApplicationController.renderer.render(
-                partial: "log_entries/file_details_sky_section",
-                locals: {
-                  log_entry: log_entry,
-                },
-                assigns: {
-                  current_user: nil,
-                },
-              )
-          rescue StandardError
-            # file_details_html remains nil
-          end
+
+          file_details_html =
+            ApplicationController.renderer.render(
+              partial: "log_entries/file_details_sky_section",
+              locals: {
+                post_file: file,
+              },
+              assigns: {
+                current_user: nil,
+              },
+            )
        end

        {
--- a/app/jobs/domain/bluesky/job/base.rb
+++ b/app/jobs/domain/bluesky/job/base.rb
@@ -1,9 +1,11 @@
 # typed: strict
 class Domain::Bluesky::Job::Base < Scraper::JobBase
  abstract!
-  discard_on ActiveJob::DeserializationError
  include HasBulkEnqueueJobs

+  queue_as :bluesky
+  discard_on ActiveJob::DeserializationError
+
  sig { override.returns(Symbol) }
  def self.http_factory_method
    :get_generic_http_client
@@ -11,6 +13,46 @@ class Domain::Bluesky::Job::Base < Scraper::JobBase

  protected

+  sig { params(user: Domain::User::BlueskyUser).void }
+  def enqueue_scan_posts_job_if_due(user)
+    if user.posts_scan.due? || force_scan?
+      logger.info(
+        format_tags(
+          "enqueue posts scan",
+          make_tags(posts_scan: user.posts_scan.ago_in_words),
+        ),
+      )
+      defer_job(Domain::Bluesky::Job::ScanPostsJob, { user: })
+    else
+      logger.info(
+        format_tags(
+          "skipping enqueue of posts scan",
+          make_tags(scanned_at: user.posts_scan.ago_in_words),
+        ),
+      )
+    end
+  end
+
+  sig { params(user: Domain::User::BlueskyUser).void }
+  def enqueue_scan_user_job_if_due(user)
+    if user.profile_scan.due? || force_scan?
+      logger.info(
+        format_tags(
+          "enqueue user scan",
+          make_tags(profile_scan: user.profile_scan.ago_in_words),
+        ),
+      )
+      defer_job(Domain::Bluesky::Job::ScanUserJob, { user: })
+    else
+      logger.info(
+        format_tags(
+          "skipping enqueue of user scan",
+          make_tags(scanned_at: user.profile_scan.ago_in_words),
+        ),
+      )
+    end
+  end
+
  sig { returns(T.nilable(Domain::User::BlueskyUser)) }
  def user_from_args
    if (user = arguments[0][:user]).is_a?(Domain::User::BlueskyUser)
--- a/app/jobs/domain/bluesky/job/scan_posts_job.rb
+++ b/app/jobs/domain/bluesky/job/scan_posts_job.rb
@@ -6,10 +6,16 @@ class Domain::Bluesky::Job::ScanPostsJob < Domain::Bluesky::Job::Base
  def perform(args)
    user = user_from_args!
    logger.push_tags(make_arg_tag(user))
-    logger.info("starting posts scan")
+    logger.info(format_tags("starting posts scan"))

    return if buggy_user?(user)
-    return unless user.state_ok?
+    unless user.state_ok?
+      logger.error(
+        format_tags("skipping posts scan", make_tags(state: user.state)),
+      )
+      return
+    end
+
    if !user.posts_scan.due? && !force_scan?
      logger.info(
        format_tags(
@@ -19,6 +25,7 @@ class Domain::Bluesky::Job::ScanPostsJob < Domain::Bluesky::Job::Base
      )
      return
    end
+
    scan_user_posts(user)
    logger.info(format_tags("completed posts scan"))
  ensure
@@ -73,9 +80,7 @@ class Domain::Bluesky::Job::ScanPostsJob < Domain::Bluesky::Job::Base

          # Only process posts with media
          num_posts_with_media += 1
-          user_did = user.did
-          next unless user_did
-          if process_historical_post(user, record_data, record, user_did)
+          if process_historical_post(user, record_data, record)
            num_created_posts += 1
          end
        end
@@ -112,16 +117,18 @@ class Domain::Bluesky::Job::ScanPostsJob < Domain::Bluesky::Job::Base
      user: Domain::User::BlueskyUser,
      record_data: T::Hash[String, T.untyped],
      record: T::Hash[String, T.untyped],
-      user_did: String,
    ).returns(T::Boolean)
  end
-  def process_historical_post(user, record_data, record, user_did)
+  def process_historical_post(user, record_data, record)
    uri = record_data["uri"]
    rkey = record_data["uri"].split("/").last

    # Check if we already have this post
-    existing_post = Domain::Post::BlueskyPost.find_by(at_uri: uri)
-    return false if existing_post
+    existing_post = user.posts.find_by(bluesky_rkey: rkey)
+    if existing_post
+      enqueue_pending_files_job(existing_post)
+      return false
+    end

    begin
      post =
@@ -137,7 +144,7 @@ class Domain::Bluesky::Job::ScanPostsJob < Domain::Bluesky::Job::Base
      post.save!

      # Process media if present
-      process_post_media(post, record["embed"], user_did) if record["embed"]
+      process_post_media(post, record["embed"], user.did!) if record["embed"]

      logger.debug(
        format_tags(
@@ -179,6 +186,19 @@ class Domain::Bluesky::Job::ScanPostsJob < Domain::Bluesky::Job::Base
    end
  end

+  sig { params(post: Domain::Post::BlueskyPost).void }
+  def enqueue_pending_files_job(post)
+    post.files.each do |file|
+      if file.state_pending?
+        defer_job(
+          Domain::StaticFileJob,
+          { post_file: file },
+          { queue: "bluesky" },
+        )
+      end
+    end
+  end
+
  sig do
    params(
      post: Domain::Post::BlueskyPost,
@@ -194,10 +214,8 @@ class Domain::Bluesky::Job::ScanPostsJob < Domain::Bluesky::Job::Base

      post_file =
        post.files.build(
-          type: "Domain::PostFile::BlueskyPostFile",
          file_order: index,
          url_str: construct_blob_url(did, blob_data["ref"]["$link"]),
-          state: "pending",
          alt_text: image_data["alt"],
          blob_ref: blob_data["ref"]["$link"],
        )
@@ -209,7 +227,7 @@ class Domain::Bluesky::Job::ScanPostsJob < Domain::Bluesky::Job::Base
      end

      post_file.save!
-      Domain::StaticFileJob.perform_later({ post_file: })
+      defer_job(Domain::StaticFileJob, { post_file: }, { queue: "bluesky" })
      files << post_file
    end

@@ -236,12 +254,11 @@ class Domain::Bluesky::Job::ScanPostsJob < Domain::Bluesky::Job::Base
      post.files.build(
        file_order: 0,
        url_str: construct_blob_url(did, thumb_data["ref"]["$link"]),
-        state: "pending",
        blob_ref: thumb_data["ref"]["$link"],
      )

    post_file.save!
-    Domain::StaticFileJob.perform_later({ post_file: })
+    defer_job(Domain::StaticFileJob, { post_file: }, { queue: "bluesky" })

    logger.debug(
      format_tags(
--- a/app/jobs/domain/bluesky/job/scan_user_job.rb
+++ b/app/jobs/domain/bluesky/job/scan_user_job.rb
@@ -6,7 +6,7 @@ class Domain::Bluesky::Job::ScanUserJob < Domain::Bluesky::Job::Base
  def perform(args)
    user = user_from_args!
    logger.push_tags(make_arg_tag(user))
-    logger.info("starting profile scan")
+    logger.info(format_tags("starting profile scan"))

    return if buggy_user?(user)
    if !user.profile_scan.due? && !force_scan?
@@ -16,22 +16,13 @@ class Domain::Bluesky::Job::ScanUserJob < Domain::Bluesky::Job::Base
          make_tags(scanned_at: user.profile_scan.ago_in_words),
        ),
      )
+      enqueue_scan_posts_job_if_due(user)
      return
    end

-    # Scan user profile/bio
    scan_user_profile(user)
+    enqueue_scan_posts_job_if_due(user)
    logger.info(format_tags("completed profile scan"))
-
-    if user.posts_scan.due? || force_scan?
-      logger.info(
-        format_tags(
-          "enqueue posts scan",
-          make_tags(posts_scan: user.posts_scan.ago_in_words),
-        ),
-      )
-      defer_job(Domain::Bluesky::Job::ScanPostsJob, { user: })
-    end
  ensure
    user.save! if user
  end
--- a/app/models/domain/user/bluesky_user.rb
+++ b/app/models/domain/user/bluesky_user.rb
@@ -44,6 +44,11 @@ class Domain::User::BlueskyUser < Domain::User
    "Bluesky"
  end

+  sig { returns(String) }
+  def did!
+    T.must(did)
+  end
+
  sig { override.returns(T.nilable(String)) }
  def description_html_for_view
    description
--- a/app/views/domain/posts/_title_stat.html.erb
+++ b/app/views/domain/posts/_title_stat.html.erb
@@ -1,8 +0,0 @@
-<span>
-  <i class="fa-solid <%= icon_class %> mr-1"></i>
-  <%= label %>: <% if value.present? %>
-    <%= value %>
-  <% else %>
-    <span class="text-slate-400"> - </span>
-  <% end %>
-</span>
--- a/app/views/domain/posts/e621/_section_title_details.html.erb
+++ b/app/views/domain/posts/e621/_section_title_details.html.erb
@@ -1 +1 @@
-<%= render partial: "domain/posts/title_stat", locals: { label: "Rating", value: post.rating_for_view, icon_class: "fa-tag" } %>
+<%= render partial: "shared/title_stat", locals: { label: "Rating", value: post.rating_for_view, icon_class: "fa-tag" } %>
--- a/app/views/domain/posts/fa/_section_title_details.html.erb
+++ b/app/views/domain/posts/fa/_section_title_details.html.erb
@@ -1,6 +1,6 @@
-<%= render partial: "domain/posts/title_stat", locals: { label: "Views", value: post.num_views, icon_class: "fa-eye" } %>
-<%= render partial: "domain/posts/title_stat", locals: { label: "Comments", value: post.num_comments, icon_class: "fa-comment" } %>
-<%= render partial: "domain/posts/title_stat", locals: { label: "Status", value: post.status_for_view, icon_class: "fa-calendar-days" } %>
+<%= render partial: "shared/title_stat", locals: { label: "Views", value: post.num_views, icon_class: "fa-eye" } %>
+<%= render partial: "shared/title_stat", locals: { label: "Comments", value: post.num_comments, icon_class: "fa-comment" } %>
+<%= render partial: "shared/title_stat", locals: { label: "Status", value: post.status_for_view, icon_class: "fa-calendar-days" } %>
 <% if policy(post).view_tried_from_fur_archiver? %>
  <% if post.fuzzysearch_checked_at? %>
    <% hle = post.fuzzysearch_entry %>
--- a/app/views/domain/posts/ib/_section_title_details.html.erb
+++ b/app/views/domain/posts/ib/_section_title_details.html.erb
@@ -1,3 +1,3 @@
-<%= render partial: "domain/posts/title_stat", locals: { label: "Views", value: post.num_views, icon_class: "fa-eye" } %>
-<%= render partial: "domain/posts/title_stat", locals: { label: "Files", value: post.num_files, icon_class: "fa-file-image" } %>
-<%= render partial: "domain/posts/title_stat", locals: { label: "Comments", value: post.num_comments, icon_class: "fa-comment" } %>
+<%= render partial: "shared/title_stat", locals: { label: "Views", value: post.num_views, icon_class: "fa-eye" } %>
+<%= render partial: "shared/title_stat", locals: { label: "Files", value: post.num_files, icon_class: "fa-file-image" } %>
+<%= render partial: "shared/title_stat", locals: { label: "Comments", value: post.num_comments, icon_class: "fa-comment" } %>
--- a/app/views/log_entries/_file_details_sky_section.html.erb
+++ b/app/views/log_entries/_file_details_sky_section.html.erb
@@ -1,22 +1,38 @@
 <%= sky_section_tag("File Details") do %>
-  <div class="flex flex-wrap gap-x-4 text-sm text-slate-600">
-    <span>
-      <i class="fa-regular fa-file mr-1"></i>
-      <% ct = log_entry.content_type %>
-      <% ct = ct.split(";").first if ct %>
-      Type: <%= ct %>
-    </span>
-    <span>
-      <i class="fa-solid fa-weight-hanging mr-1"></i>
-      Size: <%= number_to_human_size(log_entry.response_size) %>
-    </span>
-    <span>
-      <i class="fa-solid fa-clock mr-1"></i>
-      Response Time: <%= log_entry.response_time_ms == -1 ? "(unknown)" : "#{log_entry.response_time_ms}ms" %>
-    </span>
-    <span>
-      <i class="fa-solid fa-signal mr-1"></i>
-      Status: <span class="<%= log_entry.status_code == 200 ? 'text-green-600' : 'text-red-600' %>"><%= log_entry.status_code %></span>
-    </span>
+  <% log_entry = post_file.log_entry %>
+  <div class="flex flex-wrap gap-x-4 text-sm text-slate-600 justify-between">
+    <% ct = log_entry.content_type %>
+    <% ct = ct.split(";").first if ct %>
+    <%= render partial: "shared/title_stat", locals: {
+      label: "Type",
+      value: ct,
+      icon_class: "fa-solid fa-file",
+    } %>
+    <%= render partial: "shared/title_stat", locals: {
+      label: "Size",
+      value: number_to_human_size(log_entry.response_size),
+      icon_class: "fa-solid fa-weight-hanging",
+    } %>
+    <%= render partial: "shared/title_stat", locals: {
+      label: "Time",
+      value: log_entry.response_time_ms == -1 ? nil : "#{log_entry.response_time_ms}ms",
+      icon_class: "fa-solid fa-clock",
+    } %>
+    <%= render partial: "shared/title_stat", locals: {
+      label: "Status",
+      value: log_entry.status_code.to_s,
+      value_class: log_entry.status_code == 200 ? "text-green-600" : "text-red-600",
+      icon_class: "fa-solid fa-signal",
+    } %>
+    <%= render partial: "shared/title_stat", locals: {
+      label: "State",
+      value: post_file.state,
+      icon_class: "fa-solid fa-circle-check",
+    } %>
+    <%= render partial: "shared/title_stat", locals: {
+      label: "Log Entry",
+      value: link_to("##{log_entry.id}", log_entry_path(log_entry), class: "text-blue-600"),
+      icon_class: "fa-solid fa-link",
+    } %>
  </div>
-<% end if log_entry %>
+<% end if post_file&.log_entry %>
--- a/app/views/shared/_title_stat.html.erb
+++ b/app/views/shared/_title_stat.html.erb
@@ -0,0 +1,9 @@
+<% value_class = local_assigns[:value_class] || 'text-slate-500' %>
+<span>
+  <i class="fa-regular <%= icon_class %> mr-1"></i>
+  <%= label %>: <% if value.present? %>
+    <span class="<%= value_class %>"><%= value %></span>
+  <% else %>
+    <span class="text-slate-400"> - </span>
+  <% end %>
+</span>
--- a/spec/factories/domain/post_file.rb
+++ b/spec/factories/domain/post_file.rb
@@ -84,4 +84,16 @@ FactoryBot.define do
    md5_initial { "d41d8cd98f00b204e9800998ecf8427e" }
    md5_full { "d41d8cd98f00b204e9800998ecf8427e" }
  end
+
+  factory :domain_post_file_bluesky_post_file,
+          class: "Domain::PostFile::BlueskyPostFile",
+          parent: :domain_post_file do
+    association :post, factory: :domain_post_bluesky_post
+    sequence(:blob_ref) { |n| "bafkreiimage#{n}" }
+    sequence(:url_str) do |n|
+      "https://bsky.social/xrpc/com.atproto.sync.getBlob?did=test&cid=bafkreiimage#{n}"
+    end
+    sequence(:file_order)
+    alt_text { "Test image" }
+  end
 end
--- a/spec/jobs/domain/bluesky/job/scan_posts_job_spec.rb
+++ b/spec/jobs/domain/bluesky/job/scan_posts_job_spec.rb
@@ -100,9 +100,6 @@ RSpec.describe Domain::Bluesky::Job::ScanPostsJob do

      before do
        @log_entries = HttpClientMockHelpers.init_with(client_mock_config)
-
-        # Mock static file job enqueueing
-        allow(Domain::StaticFileJob).to receive(:perform_later)
      end

      it "scans user posts and updates scanned_posts_at" do
@@ -159,9 +156,8 @@ RSpec.describe Domain::Bluesky::Job::ScanPostsJob do
        perform_now({ user: user })

        # Should enqueue 2 StaticFileJobs (one for image, one for external thumbnail)
-        expect(Domain::StaticFileJob).to have_received(:perform_later).exactly(
-          2,
-        ).times
+        enqueued_jobs = SpecUtil.enqueued_job_args(Domain::StaticFileJob)
+        expect(enqueued_jobs.length).to eq(2)
      end

      it "does not create duplicate posts" do
@@ -313,8 +309,6 @@ RSpec.describe Domain::Bluesky::Job::ScanPostsJob do

      before do
        @log_entries = HttpClientMockHelpers.init_with(client_mock_config)
-
-        allow(Domain::StaticFileJob).to receive(:perform_later)
      end

      it "handles pagination correctly" do
@@ -328,5 +322,139 @@ RSpec.describe Domain::Bluesky::Job::ScanPostsJob do
        expect(posts.second.text).to eq("Second post with image")
      end
    end
+
+    context "when rescanning user with existing posts but pending files" do
+      let(:existing_post) do
+        create(
+          :domain_post_bluesky_post,
+          at_uri: "at://#{user.did}/app.bsky.feed.post/post1",
+          bluesky_rkey: "post1",
+          creator: user,
+          text: "Hello world with image!",
+        )
+      end
+
+      let(:pending_file) do
+        create(
+          :domain_post_file_bluesky_post_file,
+          post: existing_post,
+          blob_ref: "bafkreiimage123",
+          state: "pending",
+          alt_text: "Test image",
+          aspect_ratio_width: 1920,
+          aspect_ratio_height: 1080,
+          file_order: 0,
+        )
+      end
+
+      let(:ok_file) do
+        create(
+          :domain_post_file_bluesky_post_file,
+          post: existing_post,
+          blob_ref: "bafkreiimage456",
+          state: "ok",
+          alt_text: "Already downloaded",
+          file_order: 1,
+        )
+      end
+
+      let(:posts_response_body) do
+        {
+          "records" => [
+            {
+              "uri" => "at://#{user.did}/app.bsky.feed.post/post1",
+              "cid" => "bafyreiapost123",
+              "value" => {
+                "text" => "Hello world with image!",
+                "createdAt" => "2025-01-08T12:00:00.000Z",
+                "embed" => {
+                  "$type" => "app.bsky.embed.images",
+                  "images" => [
+                    {
+                      "alt" => "Test image",
+                      "aspectRatio" => {
+                        "width" => 1920,
+                        "height" => 1080,
+                      },
+                      "image" => {
+                        "$type" => "blob",
+                        "ref" => {
+                          "$link" => "bafkreiimage123",
+                        },
+                        "mimeType" => "image/jpeg",
+                        "size" => 256_000,
+                      },
+                    },
+                  ],
+                },
+              },
+            },
+          ],
+          "cursor" => nil,
+        }.to_json
+      end
+
+      let(:client_mock_config) do
+        [
+          {
+            uri:
+              "https://bsky.social/xrpc/com.atproto.repo.listRecords?repo=#{user.did}&collection=app.bsky.feed.post&limit=100",
+            status_code: 200,
+            content_type: "application/json",
+            contents: posts_response_body,
+          },
+        ]
+      end
+
+      before do
+        @log_entries = HttpClientMockHelpers.init_with(client_mock_config)
+        # Set up existing post with files in different states
+        pending_file
+        ok_file
+        user.update!(scanned_posts_at: 1.hour.ago)
+      end
+
+      it "enqueues jobs only for pending files during rescan" do
+        perform_now({ user: user, force_scan: true })
+
+        enqueued_jobs = SpecUtil.enqueued_job_args(Domain::StaticFileJob)
+        expect(enqueued_jobs.length).to eq(1)
+
+        # Verify the enqueued job is for the pending file
+        enqueued_job = enqueued_jobs.first
+        expect(enqueued_job[:post_file]).to eq(pending_file)
+      end
+
+      it "does not enqueue jobs for files already in ok state" do
+        perform_now({ user: user, force_scan: true })
+
+        enqueued_jobs = SpecUtil.enqueued_job_args(Domain::StaticFileJob)
+        enqueued_file_ids = enqueued_jobs.map { |job| job[:post_file].id }
+
+        expect(enqueued_file_ids).to include(pending_file.id)
+        expect(enqueued_file_ids).not_to include(ok_file.id)
+      end
+
+      it "does not create duplicate posts during rescan" do
+        expect { perform_now({ user: user, force_scan: true }) }.not_to change(
+          Domain::Post::BlueskyPost,
+          :count,
+        )
+
+        # Verify the existing post wasn't duplicated
+        posts = Domain::Post::BlueskyPost.where(bluesky_rkey: "post1")
+        expect(posts.count).to eq(1)
+        expect(posts.first).to eq(existing_post)
+      end
+
+      it "updates scanned_posts_at timestamp during rescan" do
+        old_timestamp = user.scanned_posts_at
+
+        perform_now({ user: user, force_scan: true })
+
+        user.reload
+        expect(user.scanned_posts_at).to be > old_timestamp
+      end
+    end
  end
 end