From 3e286119dbe979e7fc797a55528cbc6b7b9989ca Mon Sep 17 00:00:00 2001
From: Julian Flynn <julian.flynn@ezcater.com>
Date: Tue, 16 Jul 2024 22:19:32 -0400
Subject: [PATCH] feat: return job ID from client push

What did we change?

Have `SidekiqPublisher::Worker.client_push` return a job ID string, which
matches the sidekiq gem behavior. Currently `.client_push` returns the inserted
`SidekiqPublisher::Job` ActiveRecord instance.

Why are we doing this?

Having a consistent return value between the native sidekiq gem and the
sidekiq_publisher gem makes things easier to handle. Otherwise clients have to
juggle using `#job_id` method (for sidekiq_publisher gem) or the job ID string
(for sidekiq gem). For example:

```rb
job = ExampleJob.perform_async
logger.info msg: "some example", job_id: job.try(:job_id) || job
```
---
 lib/sidekiq_publisher/worker.rb       |  2 +-
 spec/sidekiq_publisher/worker_spec.rb | 30 +++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/lib/sidekiq_publisher/worker.rb b/lib/sidekiq_publisher/worker.rb
index 7accbb1..87fe79e 100644
--- a/lib/sidekiq_publisher/worker.rb
+++ b/lib/sidekiq_publisher/worker.rb
@@ -11,7 +11,7 @@ def self.included(base)
     module ClassMethods
       def client_push(item)
         if SidekiqPublisher::DatabaseConnection.transaction_open?
-          SidekiqPublisher::Job.create_job!(item)
+          SidekiqPublisher::Job.create_job!(item).job_id
         else
           super
         end
diff --git a/spec/sidekiq_publisher/worker_spec.rb b/spec/sidekiq_publisher/worker_spec.rb
index f6fc5d4..779befe 100644
--- a/spec/sidekiq_publisher/worker_spec.rb
+++ b/spec/sidekiq_publisher/worker_spec.rb
@@ -31,6 +31,18 @@
       expect(sidekiq_job.display_class).to eq("TestWorker")
       expect(sidekiq_job.display_args).to eq(args)
     end
+
+    it "returns job ID" do
+      fake_job_id = SecureRandom.hex(12)
+      allow(SidekiqPublisher::Job).to receive(:generate_sidekiq_jid).and_return(fake_job_id)
+
+      result = TestWorker.sidekiq_client_push(
+        "class" => TestWorker,
+        "args" => args
+      )
+
+      expect(result).to eq(fake_job_id)
+    end
   end
 
   describe ".perform_async" do
@@ -48,6 +60,15 @@
         queue = Sidekiq::Queue.new("default")
         expect(queue.size).to eq(0)
       end
+
+      it "returns job ID" do
+        fake_job_id = SecureRandom.hex(12)
+        allow(SidekiqPublisher::Job).to receive(:generate_sidekiq_jid).and_return(fake_job_id)
+
+        result = TestWorker.perform_async(*args)
+
+        expect(result).to eq(fake_job_id)
+      end
     end
 
     context "when not in a transaction", skip_db_clean: true do
@@ -67,6 +88,15 @@
         expect(sidekiq_job.display_class).to eq("TestWorker")
         expect(sidekiq_job.display_args).to eq(args)
       end
+
+      it "returns job ID" do
+        fake_job_id = SecureRandom.hex(12)
+        allow(SidekiqPublisher::Job).to receive(:generate_sidekiq_jid).and_return(fake_job_id)
+
+        result = TestWorker.perform_async(*args)
+
+        expect(result).to eq(fake_job_id)
+      end
     end
 
     context "when Sidekiq::Testing mode is inline" do