Add tests for background jobs stuff

.app_version

@@ -1 +1 @@
-0.8.7
+0.9.0

CHANGELOG.md

@@ -5,6 +5,20 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [0.9.0] — 2024-07-12
+
+### Added
+
+- Background jobs page. You can find it in Settings -> Backgroun Jobs.
+- Queue clearing buttons. You can clear all jobs in the queue.
+- Reverse geocoding restart button. You can restart the reverse geocoding process for all of your points.
+- Reverse geocoding continue button. Click on this button will start reverse geocoding process only for points that were not processed yet.
+- A lot more data is now being saved in terms of reverse geocoding process. It will be used in the future to create more insights about your data.
+
+### Changed
+
+- Point reference to a user is no longer optional. It should not cause any problems, but if you see any issues, please let me know.
+
 ## [0.8.7] — 2024-07-09
 
 ### Changed

app/controllers/settings/background_jobs_controller.rb

@@ -8,13 +8,6 @@ class Settings::BackgroundJobsController < ApplicationController
     @queues = Sidekiq::Queue.all
   end
 
-  def show; end
-
-  def new
-  end
-
-  def edit; end
-
   def create
     EnqueueReverseGeocodingJob.perform_later(params[:job_name], current_user.id)
 
@@ -23,9 +16,6 @@ class Settings::BackgroundJobsController < ApplicationController
     redirect_to settings_background_jobs_path, notice: 'Job was successfully created.'
   end
 
-  def update
-  end
-
   def destroy
     # Clear all jobs in the queue, params[:id] contains queue name
     queue = Sidekiq::Queue.new(params[:id])

app/helpers/settings/background_jobs_helper.rb

@@ -1,2 +0,0 @@
-module Settings::BackgroundJobsHelper
-end

app/jobs/reverse_geocoding_job.rb

@@ -6,14 +6,6 @@ class ReverseGeocodingJob < ApplicationJob
   def perform(point_id)
     return unless REVERSE_GEOCODING_ENABLED
 
-    point = Point.find(point_id)
-    return if point.city.present? && point.country.present?
-
-    result = Geocoder.search([point.latitude, point.longitude])
-    return if result.blank?
-
-    point.update!(city: result.first.city, country: result.first.country)
-  rescue ActiveRecord::RecordNotFound => e
-    Rails.logger.error("Point with id #{point_id} not found: #{e.message}")
+    ReverseGeocoding::FetchData.new(point_id).call
   end
 end

app/models/point.rb

@@ -1,8 +1,10 @@
 # frozen_string_literal: true
 
 class Point < ApplicationRecord
+  reverse_geocoded_by :latitude, :longitude
+
   belongs_to :import, optional: true
-  belongs_to :user, optional: true
+  belongs_to :user
 
   validates :latitude, :longitude, :timestamp, presence: true
 

app/services/reverse_geocoding/fetch_data.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+class ReverseGeocoding::FetchData
+  attr_reader :point
+
+  def initialize(point_id)
+    @point = Point.find(point_id)
+  rescue ActiveRecord::RecordNotFound => e
+    Rails.logger.error("Point with id #{point_id} not found: #{e.message}")
+  end
+
+  def call
+    return if reverse_geocoded?
+
+    response = Geocoder.search([point.latitude, point.longitude]).first
+    return if response.blank? || response.data['error'].present?
+
+    point.update!(city: response.city, country: response.country, geodata: response.data)
+  end
+
+  private
+
+  def reverse_geocoded?
+    point.city.present? && point.country.present? || point.geodata.present?
+  end
+end

app/views/settings/background_jobs/_background_job.html.erb

@@ -1,2 +0,0 @@
-<div id="<%= dom_id background_job %>">
-</div>

app/views/settings/background_jobs/_form.html.erb

@@ -1,17 +0,0 @@
-<%= form_with(model: settings_background_job, class: "contents") do |form| %>
-  <% if settings_background_job.errors.any? %>
-    <div id="error_explanation" class="bg-red-50 text-red-500 px-3 py-2 font-medium rounded-lg mt-3">
-      <h2><%= pluralize(settings_background_job.errors.count, "error") %> prohibited this settings_background_job from being saved:</h2>
-
-      <ul>
-        <% settings_background_job.errors.each do |error| %>
-          <li><%= error.full_message %></li>
-        <% end %>
-      </ul>
-    </div>
-  <% end %>
-
-  <div class="inline">
-    <%= form.submit class: "rounded-lg py-3 px-5 bg-blue-600 text-white inline-block font-medium cursor-pointer" %>
-  </div>
-<% end %>

app/views/settings/background_jobs/edit.html.erb

@@ -1,8 +0,0 @@
-<div class="mx-auto md:w-2/3 w-full">
-  <h1 class="font-bold text-4xl">Editing background job</h1>
-
-  <%= render "form", settings_background_job: @settings_background_job %>
-
-  <%= link_to "Show this background job", @settings_background_job, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %>
-  <%= link_to "Back to background jobs", settings_background_jobs_path, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %>
-</div>

app/views/settings/background_jobs/index.html.erb

@@ -7,7 +7,7 @@
     <h1 class="font-bold text-4xl">Background jobs</h1>
   </div>
 
-  <div role="alert" class="alert my-5">
+  <div role="alert" class="alert m-5">
     <svg
       xmlns="http://www.w3.org/2000/svg"
       fill="none"
@@ -19,7 +19,7 @@
         stroke-width="2"
         d="M13 16h-1v-4h-1m1-4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z"></path>
     </svg>
-    <span>Spamming new many jobs at once is a bad idea. Let them work or clear the queue beforehand.</span>
+    <span>Spamming many new jobs at once is a bad idea. Let them work or clear the queue beforehand.</span>
   </div>
 
   <div class='flex'>
@@ -46,7 +46,7 @@
 
   <div id="settings_background_jobs" class="min-w-full">
     <% @queues.each do |queue| %>
-      <div class="card shadow-2xl bg-base-300 p-5 my-5">
+      <div class="card shadow-2xl bg-base-300 p-5 m-5">
         <h2 class="text-2xl font-bold"><%= queue.name %></h2>
         <div class="flex justify-between items-center">
           <p class="text-lg">Jobs in queue: <%= queue.size %></p>

app/views/settings/background_jobs/new.html.erb

@@ -1,7 +0,0 @@
-<div class="mx-auto md:w-2/3 w-full">
-  <h1 class="font-bold text-4xl">New background job</h1>
-
-  <%= render "form", settings_background_job: @settings_background_job %>
-
-  <%= link_to "Back to background jobs", settings_background_jobs_path, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %>
-</div>

app/views/settings/background_jobs/show.html.erb

@@ -1,15 +0,0 @@
-<div class="mx-auto md:w-2/3 w-full flex">
-  <div class="mx-auto">
-    <% if notice.present? %>
-      <p class="py-2 px-3 bg-green-50 mb-5 text-green-500 font-medium rounded-lg inline-block" id="notice"><%= notice %></p>
-    <% end %>
-
-    <%= render @settings_background_job %>
-
-    <%= link_to "Edit this background job", edit_settings_background_job_path(@settings_background_job), class: "mt-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %>
-    <%= link_to "Back to background jobs", settings_background_jobs_path, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %>
-    <div class="inline-block ml-2">
-      <%= button_to "Destroy this background job", @settings_background_job, method: :delete, class: "mt-2 rounded-lg py-3 px-5 bg-gray-100 font-medium" %>
-    </div>
-  </div>
-</div>

config/initializers/geocoder.rb

@@ -1,4 +1,5 @@
-# config/initializers/geocoder.rb
+# frozen_string_literal: true
+
 Geocoder.configure(
   # geocoding service request timeout, in seconds (default 3):
   # timeout: 5,
@@ -9,7 +10,7 @@ Geocoder.configure(
   # caching (see Caching section below for details):
   cache: Redis.new,
   cache_options: {
-    expiration: 1.day, # Defaults to `nil`
+    expiration: 1.day # Defaults to `nil`
     # prefix: "another_key:" # Defaults to `geocoder:`
   }
 )

config/routes.rb

@@ -9,7 +9,7 @@ Rails.application.routes.draw do
 
   resources :settings, only: :index
   namespace :settings do
-    resources :background_jobs
+    resources :background_jobs, only: %i[index create destroy]
     resources :users, only: :create
   end
 

db/migrate/20240712141303_add_geodata_to_points.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class AddGeodataToPoints < ActiveRecord::Migration[7.1]
+  def change
+    add_column :points, :geodata, :jsonb, null: false, default: {}
+    add_index :points, :geodata, using: :gin
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_07_03_105734) do
+ActiveRecord::Schema[7.1].define(version: 2024_07_12_141303) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -109,12 +109,14 @@ ActiveRecord::Schema[7.1].define(version: 2024_07_03_105734) do
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.bigint "user_id"
+    t.jsonb "geodata", default: {}, null: false
     t.index ["altitude"], name: "index_points_on_altitude"
     t.index ["battery"], name: "index_points_on_battery"
     t.index ["battery_status"], name: "index_points_on_battery_status"
     t.index ["city"], name: "index_points_on_city"
     t.index ["connection"], name: "index_points_on_connection"
     t.index ["country"], name: "index_points_on_country"
+    t.index ["geodata"], name: "index_points_on_geodata", using: :gin
     t.index ["import_id"], name: "index_points_on_import_id"
     t.index ["latitude", "longitude"], name: "index_points_on_latitude_and_longitude"
     t.index ["timestamp"], name: "index_points_on_timestamp"

spec/factories/points.rb

@@ -25,5 +25,6 @@ FactoryBot.define do
     import_id { '' }
     city { nil }
     country { nil }
+    user
   end
 end

spec/jobs/enqueue_reverse_geocoding_job_spec.rb

@@ -1,5 +1,14 @@
+# frozen_string_literal: true
+
 require 'rails_helper'
 
 RSpec.describe EnqueueReverseGeocodingJob, type: :job do
-  pending "add some examples to (or delete) #{__FILE__}"
+  let(:job_name) { 'start_reverse_geocoding' }
+  let(:user_id) { 1 }
+
+  it 'calls job creation service' do
+    expect(Jobs::Create).to receive(:new).with(job_name, user_id).and_return(double(call: nil))
+
+    EnqueueReverseGeocodingJob.perform_now(job_name, user_id)
+  end
 end

spec/jobs/reverse_geocoding_job_spec.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'rails_helper'
 
 RSpec.describe ReverseGeocodingJob, type: :job do
@@ -17,44 +19,27 @@ RSpec.describe ReverseGeocodingJob, type: :job do
         expect { perform }.not_to(change { point.reload.city })
       end
 
-      it 'does not call Geocoder' do
+      it 'does not call ReverseGeocoding::FetchData' do
+        allow(ReverseGeocoding::FetchData).to receive(:new).and_call_original
+
         perform
 
-        expect(Geocoder).not_to have_received(:search)
+        expect(ReverseGeocoding::FetchData).not_to have_received(:new)
       end
     end
 
     context 'when REVERSE_GEOCODING_ENABLED is true' do
       before { stub_const('REVERSE_GEOCODING_ENABLED', true) }
 
-      it 'updates point with city and country' do
-        expect { perform }.to change { point.reload.city }.from(nil)
-      end
+      let(:stubbed_geocoder) { OpenStruct.new(data: { city: 'City', country: 'Country' }) }
 
       it 'calls Geocoder' do
+        allow(Geocoder).to receive(:search).and_return([stubbed_geocoder])
+        allow(ReverseGeocoding::FetchData).to receive(:new).and_call_original
+
         perform
 
-        expect(Geocoder).to have_received(:search).with([point.latitude, point.longitude])
-      end
-
-      context 'when point has city and country' do
-        let(:point) { create(:point, city: 'City', country: 'Country') }
-
-        before do
-          allow(Geocoder).to receive(:search).and_return(
-            [double(city: 'Another city', country: 'Some country')]
-          )
-        end
-
-        it 'does not update point' do
-          expect { perform }.not_to change { point.reload.city }
-        end
-
-        it 'does not call Geocoder' do
-          perform
-
-          expect(Geocoder).not_to have_received(:search)
-        end
+        expect(ReverseGeocoding::FetchData).to have_received(:new).with(point.id)
       end
     end
   end

spec/models/point_spec.rb

@@ -5,7 +5,7 @@ require 'rails_helper'
 RSpec.describe Point, type: :model do
   describe 'associations' do
     it { is_expected.to belong_to(:import).optional }
-    it { is_expected.to belong_to(:user).optional }
+    it { is_expected.to belong_to(:user) }
   end
 
   describe 'validations' do
@@ -13,4 +13,24 @@ RSpec.describe Point, type: :model do
     it { is_expected.to validate_presence_of(:longitude) }
     it { is_expected.to validate_presence_of(:timestamp) }
   end
+
+  describe 'scopes' do
+    describe '.reverse_geocoded' do
+      let(:point) { create(:point, country: 'Country', city: 'City') }
+      let(:point_without_address) { create(:point, city: nil, country: nil) }
+
+      it 'returns points with reverse geocoded address' do
+        expect(described_class.reverse_geocoded).to eq([point])
+      end
+    end
+
+    describe '.not_reverse_geocoded' do
+      let(:point) { create(:point, country: 'Country', city: 'City') }
+      let(:point_without_address) { create(:point, city: nil, country: nil) }
+
+      it 'returns points without reverse geocoded address' do
+        expect(described_class.not_reverse_geocoded).to eq([point_without_address])
+      end
+    end
+  end
 end

spec/requests/settings/background_jobs_spec.rb

@@ -1,135 +1,69 @@
+# frozen_string_literal: true
+
 require 'rails_helper'
 
-# This spec was generated by rspec-rails when you ran the scaffold generator.
-# It demonstrates how one might use RSpec to test the controller code that
-# was generated by Rails when you ran the scaffold generator.
-#
-# It assumes that the implementation code is generated by the rails scaffold
-# generator. If you are using any extension libraries to generate different
-# controller code, this generated spec may or may not pass.
-#
-# It only uses APIs available in rails and/or rspec-rails. There are a number
-# of tools you can use to make these specs even more expressive, but we're
-# sticking to rails and rspec-rails APIs to keep things simple and stable.
+RSpec.describe '/settings/background_jobs', type: :request do
+  before do
+    stub_request(:any, 'https://api.github.com/repos/Freika/dawarich/tags')
+      .to_return(status: 200, body: '[{"name": "1.0.0"}]', headers: {})
+  end
 
-RSpec.describe "/settings/background_jobs", type: :request do
-  
-  # This should return the minimal set of attributes required to create a valid
-  # Settings::BackgroundJob. As you add validations to Settings::BackgroundJob, be sure to
-  # adjust the attributes here as well.
-  let(:valid_attributes) {
-    skip("Add a hash of attributes valid for your model")
-  }
-
-  let(:invalid_attributes) {
-    skip("Add a hash of attributes invalid for your model")
-  }
-
-  describe "GET /index" do
-    it "renders a successful response" do
-      Settings::BackgroundJob.create! valid_attributes
+  context 'when user is not authenticated' do
+    it 'redirects to sign in page' do
       get settings_background_jobs_url
-      expect(response).to be_successful
+
+      expect(response).to redirect_to(new_user_session_url)
     end
   end
 
-  describe "GET /show" do
-    it "renders a successful response" do
-      background_job = Settings::BackgroundJob.create! valid_attributes
-      get settings_background_job_url(background_job)
-      expect(response).to be_successful
+  context 'when user is authenticated' do
+    let(:user) { create(:user) }
+
+    before do
+      sign_in user
     end
-  end
 
-  describe "GET /new" do
-    it "renders a successful response" do
-      get new_settings_background_job_url
-      expect(response).to be_successful
-    end
-  end
+    describe 'GET /index' do
+      it 'renders a successful response' do
+        get settings_background_jobs_url
 
-  describe "GET /edit" do
-    it "renders a successful response" do
-      background_job = Settings::BackgroundJob.create! valid_attributes
-      get edit_settings_background_job_url(background_job)
-      expect(response).to be_successful
-    end
-  end
-
-  describe "POST /create" do
-    context "with valid parameters" do
-      it "creates a new Settings::BackgroundJob" do
-        expect {
-          post settings_background_jobs_url, params: { settings_background_job: valid_attributes }
-        }.to change(Settings::BackgroundJob, :count).by(1)
-      end
-
-      it "redirects to the created settings_background_job" do
-        post settings_background_jobs_url, params: { settings_background_job: valid_attributes }
-        expect(response).to redirect_to(settings_background_job_url(Settings::BackgroundJob.last))
+        expect(response).to be_successful
       end
     end
 
-    context "with invalid parameters" do
-      it "does not create a new Settings::BackgroundJob" do
-        expect {
-          post settings_background_jobs_url, params: { settings_background_job: invalid_attributes }
-        }.to change(Settings::BackgroundJob, :count).by(0)
-      end
+    describe 'POST /create' do
+      let(:params) { { job_name: 'start_reverse_geocoding' } }
 
-    
-      it "renders a response with 422 status (i.e. to display the 'new' template)" do
-        post settings_background_jobs_url, params: { settings_background_job: invalid_attributes }
-        expect(response).to have_http_status(:unprocessable_entity)
-      end
-    
-    end
-  end
+      context 'with valid parameters' do
+        it 'enqueues a new job' do
+          expect do
+            post settings_background_jobs_url, params:
+          end.to have_enqueued_job(EnqueueReverseGeocodingJob)
+        end
 
-  describe "PATCH /update" do
-    context "with valid parameters" do
-      let(:new_attributes) {
-        skip("Add a hash of attributes valid for your model")
-      }
+        it 'redirects to the created settings_background_job' do
+          post(settings_background_jobs_url, params:)
 
-      it "updates the requested settings_background_job" do
-        background_job = Settings::BackgroundJob.create! valid_attributes
-        patch settings_background_job_url(background_job), params: { settings_background_job: new_attributes }
-        background_job.reload
-        skip("Add assertions for updated state")
-      end
-
-      it "redirects to the settings_background_job" do
-        background_job = Settings::BackgroundJob.create! valid_attributes
-        patch settings_background_job_url(background_job), params: { settings_background_job: new_attributes }
-        background_job.reload
-        expect(response).to redirect_to(settings_background_job_url(background_job))
+          expect(response).to redirect_to(settings_background_jobs_url)
+        end
       end
     end
 
-    context "with invalid parameters" do
-    
-      it "renders a response with 422 status (i.e. to display the 'edit' template)" do
-        background_job = Settings::BackgroundJob.create! valid_attributes
-        patch settings_background_job_url(background_job), params: { settings_background_job: invalid_attributes }
-        expect(response).to have_http_status(:unprocessable_entity)
+    describe 'DELETE /destroy' do
+      it 'clears the Sidekiq queue' do
+        queue = instance_double(Sidekiq::Queue)
+        allow(Sidekiq::Queue).to receive(:new).and_return(queue)
+
+        expect(queue).to receive(:clear)
+
+        delete settings_background_job_url('queue_name')
       end
-    
-    end
-  end
 
-  describe "DELETE /destroy" do
-    it "destroys the requested settings_background_job" do
-      background_job = Settings::BackgroundJob.create! valid_attributes
-      expect {
-        delete settings_background_job_url(background_job)
-      }.to change(Settings::BackgroundJob, :count).by(-1)
-    end
+      it 'redirects to the settings_background_jobs list' do
+        delete settings_background_job_url('queue_name')
 
-    it "redirects to the settings_background_jobs list" do
-      background_job = Settings::BackgroundJob.create! valid_attributes
-      delete settings_background_job_url(background_job)
-      expect(response).to redirect_to(settings_background_jobs_url)
+        expect(response).to redirect_to(settings_background_jobs_url)
+      end
     end
   end
 end

spec/services/jobs/create_spec.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Jobs::Create do
+  describe '#call' do
+    context 'when job_name is start_reverse_geocoding' do
+      let(:user) { create(:user) }
+      let(:points) { create_list(:point, 4, user:) }
+      let(:job_name) { 'start_reverse_geocoding' }
+
+      it 'enqueues reverse geocoding for all user points' do
+        allow(ReverseGeocodingJob).to receive(:perform_later).and_return(nil)
+
+        described_class.new(job_name, user.id).call
+
+        points.each do |point|
+          expect(ReverseGeocodingJob).to have_received(:perform_later).with(point.id)
+        end
+      end
+    end
+
+    context 'when job_name is continue_reverse_geocoding' do
+      let(:user) { create(:user) }
+      let(:points_without_address) { create_list(:point, 4, user:, country: nil, city: nil) }
+      let(:points_with_address) { create_list(:point, 5, user:, country: 'Country', city: 'City') }
+
+      let(:job_name) { 'continue_reverse_geocoding' }
+
+      it 'enqueues reverse geocoding for all user points without address' do
+        allow(ReverseGeocodingJob).to receive(:perform_later).and_return(nil)
+
+        described_class.new(job_name, user.id).call
+
+        points_without_address.each do |point|
+          expect(ReverseGeocodingJob).to have_received(:perform_later).with(point.id)
+        end
+      end
+    end
+
+    context 'when job_name is invalid' do
+      let(:user) { create(:user) }
+      let(:job_name) { 'invalid_job_name' }
+
+      it 'raises an error' do
+        expect { described_class.new(job_name, user.id).call }.to raise_error(Jobs::Create::InvalidJobName)
+      end
+    end
+  end
+end

spec/services/reverse_geocoding/fetch_data_spec.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe ReverseGeocoding::FetchData do
+  subject(:fetch_data) { described_class.new(point.id).call }
+
+  let(:point) { create(:point) }
+
+  context 'when Geocoder returns city and country' do
+    before do
+      allow(Geocoder).to receive(:search).and_return([double(city: 'City', country: 'Country',
+                                                             data: { 'address' => 'Address' })])
+    end
+
+    context 'when point does not have city and country' do
+      it 'updates point with city and country' do
+        expect { fetch_data }.to change { point.reload.city }
+          .from(nil).to('City')
+          .and change { point.reload.country }.from(nil).to('Country')
+      end
+
+      it 'updates point with geodata' do
+        expect { fetch_data }.to change { point.reload.geodata }.from({}).to('address' => 'Address')
+      end
+
+      it 'calls Geocoder' do
+        fetch_data
+
+        expect(Geocoder).to have_received(:search).with([point.latitude, point.longitude])
+      end
+    end
+
+    context 'when point has city and country' do
+      let(:point) { create(:point, city: 'City', country: 'Country') }
+
+      before do
+        allow(Geocoder).to receive(:search).and_return(
+          [double(city: 'Another city', country: 'Some country')]
+        )
+      end
+
+      it 'does not update point' do
+        expect { fetch_data }.not_to(change { point.reload.city })
+      end
+
+      it 'does not call Geocoder' do
+        fetch_data
+
+        expect(Geocoder).not_to have_received(:search)
+      end
+    end
+  end
+
+  context 'when Geocoder returns an error' do
+    before do
+      allow(Geocoder).to receive(:search).and_return([double(data: { 'error' => 'Error' })])
+    end
+
+    it 'does not update point' do
+      expect { fetch_data }.not_to(change { point.reload.city })
+    end
+  end
+end