From 45047c923710804ebdb192a5a287895165416c9f Mon Sep 17 00:00:00 2001
From: Michael Grosser <michael@grosser.it>
Date: Sat, 25 May 2024 07:34:54 +0200
Subject: [PATCH] fixes

---
 lib/kennel/importer.rb         | 13 ++++++----
 lib/kennel/models/slo.rb       |  9 ++++---
 test/kennel/models/slo_test.rb | 44 ++++++++++++++++------------------
 3 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/lib/kennel/importer.rb b/lib/kennel/importer.rb
index 8cf55aa..7354546 100644
--- a/lib/kennel/importer.rb
+++ b/lib/kennel/importer.rb
@@ -2,8 +2,15 @@
 
 module Kennel
   class Importer
+    # title will have the lock symbol we need to remove when re-importing
     TITLES = [:name, :title].freeze
-    SORT_ORDER = [*TITLES, :id, :kennel_id, :type, :tags, :query, :sli_specification, *Models::Record.subclasses.map { |k| k::TRACKING_FIELDS }, :template_variables].freeze
+
+    # bring important fields to the top
+    SORT_ORDER = [
+      *TITLES, :id, :kennel_id, :type, :tags, :query, :sli_specification,
+      *Models::Record.subclasses.flat_map { |k| k::TRACKING_FIELDS },
+      :template_variables
+    ].freeze
 
     def initialize(api)
       @api = api
@@ -19,15 +26,11 @@ def import(resource, id)
         raise(ArgumentError, "#{resource} is not supported")
 
       data = @api.show(model.api_resource, id)
-      # get sli specification before normalization.
-      sli_specification = data[:sli_specification] || data.dig(:sli_specification, :time_slice)
 
       id = data.fetch(:id) # keep native value
       model.normalize({}, data) # removes id
       data[:id] = id
 
-      data[:sli_specification] = sli_specification if sli_specification
-
       title_field = TITLES.detect { |f| data[f] }
       title = data.fetch(title_field)
       title.tr!(Kennel::Models::Record::LOCK, "") # avoid double lock icon
diff --git a/lib/kennel/models/slo.rb b/lib/kennel/models/slo.rb
index cf27aee..ae0a279 100644
--- a/lib/kennel/models/slo.rb
+++ b/lib/kennel/models/slo.rb
@@ -4,7 +4,10 @@ module Models
     class Slo < Record
       include TagsValidation
 
-      READONLY_ATTRIBUTES = superclass::READONLY_ATTRIBUTES + [:type_id, :monitor_tags, :target_threshold, :timeframe, :warning_threshold]
+      READONLY_ATTRIBUTES = [
+        *superclass::READONLY_ATTRIBUTES,
+        :type_id, :monitor_tags, :target_threshold, :timeframe, :warning_threshold
+      ].freeze
       TRACKING_FIELD = :description
       DEFAULTS = {
         description: nil,
@@ -35,8 +38,8 @@ def build_json
           type: type
         )
 
-        if type == 'time_slice'
-          data[:sliSpecification] = :sli_specification
+        if type == "time_slice"
+          data[:sli_specification] = sli_specification
         elsif v = query
           data[:query] = v
         end
diff --git a/test/kennel/models/slo_test.rb b/test/kennel/models/slo_test.rb
index 1292da4..d490a3b 100644
--- a/test/kennel/models/slo_test.rb
+++ b/test/kennel/models/slo_test.rb
@@ -11,15 +11,12 @@ class TestSlo < Kennel::Models::Slo
 
   def slo(options = {})
     Kennel::Models::Slo.new(
-      type = options[:type] || "metric"
       options.delete(:project) || project,
-      slo_options ={
-        type: -> { type },
+      {
+        type: -> { "metric" },
         name: -> { "Foo" },
         kennel_id: -> { "m1" }
-      }
-      slo_options[:sliSpecification] = -> { options[:sliSpecification] } if type == "time_slice"
-      Kennel::Models::Slo.new(project, slo_options.merge(options))
+      }.merge(options)
     )
   end
 
@@ -62,6 +59,22 @@ def slo(options = {})
       )
     end
 
+    it "sets id when updating by id" do
+      expected_basic_json[:id] = 123
+      assert_json_equal(
+        slo(id: -> { 123 }).build_json,
+        expected_basic_json
+      )
+    end
+
+    it "sets groups when given" do
+      expected_basic_json[:groups] = ["foo"]
+      assert_json_equal(
+        slo(groups: -> { ["foo"] }).build_json,
+        expected_basic_json
+      )
+    end
+
     it "includes sliSpecification for time slices" do
       sli_spec = {
         timeSlice: {
@@ -81,28 +94,13 @@ def slo(options = {})
         }
       }
 
-      expected_basic_json[:sliSpecification] = sli_spec
+      expected_basic_json[:sli_specification] = sli_spec
+      expected_basic_json[:type] = "time_slice"
       assert_json_equal(
         slo(type: "time_slice", sli_specification: sli_spec).build_json,
         expected_basic_json
       )
     end
-
-    it "sets id when updating by id" do
-      expected_basic_json[:id] = 123
-      assert_json_equal(
-        slo(id: -> { 123 }).build_json,
-        expected_basic_json
-      )
-    end
-
-    it "sets groups when given" do
-      expected_basic_json[:groups] = ["foo"]
-      assert_json_equal(
-        slo(groups: -> { ["foo"] }).build_json,
-        expected_basic_json
-      )
-    end
   end
 
   describe "#resolve_linked_tracking_ids!" do