Refactored the logic

spec/requests/api/health/show_spec.cr

@@ -0,0 +1,13 @@
+require "../../../spec_helper"
+
+describe Api::Health::Show do
+  it "returns ok and version" do
+    response = ApiClient.exec(Api::Health::Show)
+
+    response.status_code.should eq(200)
+    body = JSON.parse(response.body).as_h
+
+    body["ok"].as_bool.should be_true
+    body["version"].as_s.should eq("0.1.0")
+  end
+end

spec/requests/api/services/create_spec.cr

@@ -0,0 +1,39 @@
+require "../../../spec_helper"
+
+describe Api::Services::Create do
+  it "creates a service from JSON" do
+    payload = {
+      "name"          => "auth-service",
+      "type"          => "Rails",
+      "url"           => "https://auth.example.com",
+      "checkInterval" => 60,
+      "timeout"       => 30,
+    }.to_json
+
+    response = ApiClient.new
+      .exec_raw(Api::Services::Create, payload)
+
+    response.status_code.should eq(201)
+
+    service = ServiceQuery.new.name("auth-service").first?
+    service.should_not be_nil
+    service.not_nil!.kind.should eq("Rails")
+  end
+
+  it "returns 422 on invalid data" do
+    # missing name
+    payload = {
+      "type"          => "Rails",
+      "checkInterval" => 60,
+      "timeout"       => 30,
+    }.to_json
+
+    response = ApiClient.new
+      .exec_raw(Api::Services::Create, payload)
+
+    response.status_code.should eq(422)
+    body = JSON.parse(response.body).as_h
+
+    body.has_key?("errors").should be_true
+  end
+end

spec/requests/api/services/delete_spec.cr

@@ -0,0 +1,29 @@
+require "../../../spec_helper"
+
+describe Api::Services::Delete do
+  it "deletes an existing service" do
+    op = Service::SaveOperation.new
+    op.name.value = "email-processor"
+    op.kind.value = "Node.js"
+    op.status.value = "ok"
+    op.avg_response_ms.value = 10
+    op.check_interval_seconds.value = 60
+    op.timeout_seconds.value = 30
+    op.save!
+
+    service = ServiceQuery.new.name("email-processor").first
+
+    response = ApiClient.exec(Api::Services::Delete.with(id: service.id))
+
+    response.status_code.should eq(204)
+    ServiceQuery.new.id(service.id).first?.should be_nil
+  end
+
+  it "returns 404 when trying to delete non-existing service" do
+    response = ApiClient.exec(Api::Services::Delete.with(id: 999_i64))
+
+    # if you switched to `ServiceQuery.find`, this would be 404 via error handler
+    # if you kept the if/else, it's also 404 explicitly
+    response.status_code.should eq(404)
+  end
+end

spec/requests/api/services/index_spec.cr

@@ -0,0 +1,36 @@
+require "../../../spec_helper"
+
+describe Api::Services::Index do
+  it "returns an empty list when there are no services" do
+    response = ApiClient.exec(Api::Services::Index)
+
+    response.status_code.should eq(200)
+    body = JSON.parse(response.body)
+
+    body.as_a.size.should eq(0)
+  end
+
+  it "lists existing services" do
+    # create a service using the operation
+    op = Service::SaveOperation.new
+    op.name.value = "rails-api"
+    op.kind.value = "Rails"
+    op.status.value = "ok"
+    op.avg_response_ms.value = 145
+    op.check_interval_seconds.value = 60
+    op.timeout_seconds.value = 30
+    op.save!
+
+    response = ApiClient.exec(Api::Services::Index)
+
+    response.status_code.should eq(200)
+    body = JSON.parse(response.body).as_a
+
+    body.size.should eq(1)
+    s = body.first.as_h
+    s["name"].as_s.should eq("rails-api")
+    s["status"].as_s.should eq("ok")
+    s["type"].as_s.should eq("Rails")
+    s["responseTime"].as_i.should eq(145)
+  end
+end

src/actions/api/health/show.cr

@@ -1,4 +1,6 @@
 class Api::Health::Show < ApiAction
+  include Api::Auth::SkipRequireAuthToken
+
   get "/api/health" do
     json({ok: true, version: "0.1.0"})
   end

src/actions/api/services/create.cr

@@ -1,21 +1,20 @@
 class Api::Services::Create < ApiAction
+  include Api::Auth::SkipRequireAuthToken
+
   post "/api/services" do
     data = params.from_json
 
-    op = Service::SaveOperation.new
-    op.name.value = data["name"].as_s
+    op = SaveService.new
+    op.name.value = data["name"]?.try(&.as_s)
     op.kind.value = data["type"]?.try(&.as_s) || "Generic"
     op.url.value = data["url"]?.try(&.as_s)
-    op.status.value = "ok"
-    op.avg_response_ms.value = 0
-    op.last_check.value = Time.utc
     op.check_interval_seconds.value = data["checkInterval"]?.try(&.as_i) || 60
     op.timeout_seconds.value = data["timeout"]?.try(&.as_i) || 30
 
     if op.save
       head 201
     else
-      json({errors: op.errors}, 422)
+      json({errors: op.full_messages}, 422)
     end
   end
 end

src/actions/api/services/delete.cr

@@ -1,11 +1,9 @@
 class Api::Services::Delete < ApiAction
+  include Api::Auth::SkipRequireAuthToken
+
   delete "/api/services/:id" do
-    id = params.get(:id)
-    if service = ServiceQuery.new.id(id).first?
-      Service::DeleteOperation.delete!(service)
-      head 204
-    else
-      head 404
-    end
+    service = ServiceQuery.find(params.get(:id))
+    DeleteService.delete!(service)
+    head 204
   end
 end

src/actions/api/services/index.cr

@@ -2,7 +2,7 @@ class Api::Services::Index < ApiAction
   include Api::Auth::SkipRequireAuthToken
 
   get "/api/services" do
-    services = ServiceQuery.new.id.asc_order
-    json services.map { |s| ServiceSerializer.new(s).to_json }
+    services = ServiceQuery.new.id.asc_order.results
+    json ServiceSerializer.for_collection(services)
   end
 end

src/operations/mixins/operation_error_helpers.cr

@@ -0,0 +1,11 @@
+module OperationErrorHelpers
+  # Works with Avram::Operation#errors (Hash(Symbol, Array(String)))
+  def full_messages : Array(String)
+    errors.flat_map do |attr, messages|
+      messages.map do |msg|
+        # Turn `:name` + "is required" into "name is required"
+        "#{attr} #{msg}"
+      end
+    end
+  end
+end

src/operations/save_service.cr

@@ -1,6 +1,13 @@
 class SaveService < Service::SaveOperation
-  # To save user provided params to the database, you must permit them
-  # https://luckyframework.org/guides/database/saving-records#perma-permitting-columns
-  #
-  # permit_columns name, kind, url, status, avg_response_ms, last_check, check_interval_seconds, timeout_seconds
+  include OperationErrorHelpers
+
+  permit_columns name, kind, url, check_interval_seconds, timeout_seconds
+
+  before_save do
+    status.value ||= "ok"
+    avg_response_ms.value ||= 0
+    last_check.value ||= Time.utc
+
+    validate_required name
+  end
 end