fix some issues with pod-checking

add tooltips in the frontend
fix a JS problem with empty hostname
use `find_in_batches` correctly
add a migration to clean up the pods table + unique index on hostname

app/assets/javascripts/app/collections/pods.js

@@ -3,7 +3,8 @@ app.collections.Pods = Backbone.Collection.extend({
   model: app.models.Pod,
 
   comparator: function(model) {
-    return model.get("host").toLowerCase();
+    var host = model.get("host") || "";
+    return host.toLowerCase();
   }
 });
 // @license-end

app/assets/javascripts/app/pages/admin_pods.js

@@ -3,6 +3,8 @@
 app.pages.AdminPods = app.views.Base.extend({
   templateName: "pod_table",
 
+  tooltipSelector: "th i",
+
   initialize: function() {
     this.pods = new app.collections.Pods(app.parsePreload("pods"));
     this.rows = []; // contains the table row views

app/assets/javascripts/app/views/pod_entry_view.js

@@ -10,6 +10,8 @@ app.views.PodEntry = app.views.Base.extend({
     "click .recheck": "recheckPod"
   },
 
+  tooltipSelector: ".ssl-status i, .actions i",
+
   className: function() {
     if( this.model.get("offline") ) { return "bg-danger"; }
     if( this.model.get("status")==="version_failed" ) { return "bg-warning"; }

app/assets/templates/pod_table_entry_tpl.jst.hbs

@@ -1,5 +1,5 @@
 
-<td>
+<td class="ssl-status"">
   {{#if ssl}}
     <i title="{{t 'admin.pods.ssl_enabled'}}" class="entypo-check">
   {{else}}

app/models/pod.rb

@@ -44,7 +44,7 @@ class Pod < ActiveRecord::Base
     end
 
     def check_all!
-      Pod.find_in_batches(batch_size: 20).each(&:test_connection!)
+      Pod.find_in_batches(batch_size: 20) {|batch| batch.each(&:test_connection!) }
     end
   end
 

db/migrate/20150828132451_remove_duplicate_and_empty_pods.rb

@@ -0,0 +1,24 @@
+class RemoveDuplicateAndEmptyPods < ActiveRecord::Migration
+  def up
+    remove_dupes
+    remove_empty_or_nil
+
+    add_index :pods, :host, unique: true, length: 190 # =190*4 for utf8mb4
+  end
+
+  def down
+    remove_index :pods, :host
+  end
+
+  private
+
+  def remove_dupes
+    duplicates = Pod.group(:host).count.select {|_, v| v > 1 }.keys
+    ids = duplicates.flat_map {|pod| Pod.where(host: pod).order(created_at: :asc).pluck(:id).tap(&:shift) }
+    Pod.where(id: ids).destroy_all
+  end
+
+  def remove_empty_or_nil
+    Pod.where(host: [nil, ""]).destroy_all
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150731123114) do
+ActiveRecord::Schema.define(version: 20150828132451) do
 
   create_table "account_deletions", force: :cascade do |t|
     t.string   "diaspora_handle", limit: 255
@@ -319,6 +319,7 @@ ActiveRecord::Schema.define(version: 20150731123114) do
   end
 
   add_index "pods", ["checked_at"], name: "index_pods_on_checked_at", using: :btree
+  add_index "pods", ["host"], name: "index_pods_on_host", unique: true, length: {"host"=>190}, using: :btree
   add_index "pods", ["offline_since"], name: "index_pods_on_offline_since", using: :btree
   add_index "pods", ["status"], name: "index_pods_on_status", using: :btree
 

spec/factories.rb

@@ -211,7 +211,7 @@ FactoryGirl.define do
   end
 
   factory :pod do
-    host "pod.example.com"
+    sequence(:host) {|n| "pod#{n}.example#{r_str}.com" }
     ssl true
   end
 

spec/javascripts/app/collections/pods_spec.js

@@ -0,0 +1,16 @@
+
+describe("app.collections.Pods", function() {
+  describe("#comparator", function() {
+    it("should handle empty hostnames", function() {
+      var collection = new app.collections.Pods([
+        {id: 1},
+        {id: 2, host: "zzz.zz"},
+        {id: 3, host: "aaa.aa"},
+        {id: 4, host: ""}
+      ]);
+      expect(collection.length).toBe(4);
+      expect(collection.first().get("host")).toBeFalsy(); // also empty string
+      expect(collection.last().get("host")).toBe("zzz.zz");
+    });
+  });
+});

spec/migrations/remove_duplicate_and_empty_pods_spec.rb

@@ -0,0 +1,64 @@
+
+require "spec_helper"
+require Rails.root.join("db/migrate/20150828132451_remove_duplicate_and_empty_pods.rb")
+
+describe RemoveDuplicateAndEmptyPods do
+  self.use_transactional_fixtures = false
+
+  before do
+    @previous_migration = 20_150_731_123_114
+    @my_migration = 20_150_828_132_451
+    Pod.delete_all
+  end
+
+  after :all do
+    migrate_to(nil) # up
+    Pod.delete_all # cleanup manually, transactions are disabled
+  end
+
+  def migrate_to(version)
+    ActiveRecord::Migrator.migrate(Rails.root.join("db", "migrate"), version)
+  end
+
+  describe "#up" do
+    before do
+      migrate_to(@previous_migration)
+
+      FactoryGirl.create(:pod, host: nil)
+      FactoryGirl.create(:pod, host: "")
+      4.times { FactoryGirl.create(:pod, host: "aaa.aa") }
+    end
+
+    it "removes duplicates" do
+      expect(Pod.where(host: "aaa.aa").count).to eql(4)
+      migrate_to(@my_migration)
+      expect(Pod.where(host: "aaa.aa").count).to eql(1)
+    end
+
+    it "removes empty hostnames" do
+      expect(Pod.where(host: [nil, ""]).count).to eql(2)
+      migrate_to(@my_migration)
+      expect(Pod.where(host: [nil, ""]).count).to eql(0)
+    end
+
+    it "adds an index on the host column" do
+      expect {
+        migrate_to(@my_migration)
+        Pod.reset_column_information
+      }.to change { Pod.connection.indexes(Pod.table_name).count }.by(1)
+    end
+  end
+
+  describe "#down" do
+    before do
+      migrate_to(@my_migration)
+    end
+
+    it "removes the index on the host column" do
+      expect {
+        migrate_to(@previous_migration)
+        Pod.reset_column_information
+      }.to change { Pod.connection.indexes(Pod.table_name).count }.by(-1)
+    end
+  end
+end

spec/models/pod_spec.rb

@@ -20,7 +20,7 @@ describe Pod, type: :model do
           expect(pod).to receive(:test_connection!)
         end
       end
-      allow(Pod).to receive(:find_in_batches) { @pods }
+      allow(Pod).to receive(:find_in_batches).and_yield(@pods)
     end
 
     it "calls #test_connection! on every pod" do

spec/workers/recurring_pod_check_spec.rb

@@ -4,11 +4,11 @@ require "spec_helper"
 describe Workers::RecurringPodCheck do
   before do
     @pods = (0..4).map do
-      FactoryGirl.create(:pod).tap { |pod|
+      FactoryGirl.build(:pod).tap { |pod|
         expect(pod).to receive(:test_connection!)
       }
     end
-    allow(Pod).to receive(:find_in_batches) { @pods }
+    allow(Pod).to receive(:find_in_batches).and_yield(@pods)
   end
 
   it "performs a connection test on all existing pods" do