From 89cfc261a34a39bd9597b8404a855d41ca7335dd Mon Sep 17 00:00:00 2001 From: ObayM Date: Sun, 31 May 2026 23:46:47 +0300 Subject: [PATCH 1/4] feat(shipwrights): added an api to be integrated with #ask-the-shipwrights bot --- .../api/v1/certification/base_controller.rb | 7 ++ .../api/v1/certification/ships_controller.rb | 94 +++++++++++++++++++ config/routes.rb | 4 + 3 files changed, 105 insertions(+) create mode 100644 app/controllers/api/v1/certification/base_controller.rb create mode 100644 app/controllers/api/v1/certification/ships_controller.rb diff --git a/app/controllers/api/v1/certification/base_controller.rb b/app/controllers/api/v1/certification/base_controller.rb new file mode 100644 index 000000000..740c84840 --- /dev/null +++ b/app/controllers/api/v1/certification/base_controller.rb @@ -0,0 +1,7 @@ +class Api::V1::Certification::BaseController < Api::V1::BaseController + private + + def credential_api_keys + Array.wrap(Rails.application.credentials.dig(:certification_shipwrights, :api_keys)) + end +end diff --git a/app/controllers/api/v1/certification/ships_controller.rb b/app/controllers/api/v1/certification/ships_controller.rb new file mode 100644 index 000000000..0035fc990 --- /dev/null +++ b/app/controllers/api/v1/certification/ships_controller.rb @@ -0,0 +1,94 @@ +class Api::V1::Certification::ShipsController < Api::V1::Certification::BaseController + # GET /api/v1/certification/ships + # + # Returns ship certifications within a specific time frame! + # + # Query params: + # hours (integer, default 24) + # since (ISO 8601 datetime) + # until (ISO 8601 datetime) + # status (pending|approved|returned|all, default all) + def index + window_start, window_end = parse_time_window + status_filter = params[:status].presence_in(%w[pending approved returned]) || "all" + + scope = ::Certification::Ship + .includes(:reviewer, project: { memberships: :user }) + .where(created_at: window_start..window_end) + + scope = scope.where(status: status_filter) unless status_filter == "all" + + render json: { + window: { + from: window_start.iso8601, + to: window_end.iso8601 + }, + status_filter: status_filter, + count: scope.size, + ships: scope.order(created_at: :desc).map { |ship| serialize_ship(ship) } + } + end + + private + + def parse_time_window + now = Time.current + + window_end = parse_time_param(params[:until]) || now + + window_start = if params[:since].present? + parse_time_param(params[:since]) || (now - 24.hours) + else + hours = params[:hours].to_i + hours = 24 if hours <= 0 + window_end - hours.hours + end + + [ window_start, window_end ] + end + + def parse_time_param(value) + return nil if value.blank? + + Time.zone.parse(value.to_s) + rescue ArgumentError, TypeError + nil + end + + def serialize_ship(ship) + project = ship.project + owner_membership = project.memberships.find { |m| m.role == "owner" } + owner = owner_membership&.user + reviewer = ship.reviewer + + { + id: ship.id, + status: ship.status, + created_at: ship.created_at.iso8601, + updated_at: ship.updated_at.iso8601, + decided_at: ship.decided_at&.iso8601, + claimed_at: ship.claimed_at&.iso8601, + feedback: ship.feedback, + project: { + id: project.id, + title: project.title, + ship_status: project.ship_status, + demo_url: project.demo_url, + repo_url: project.repo_url, + description: project.description, + duration_seconds: project.duration_seconds, + shipped_at: project.shipped_at&.iso8601 + }, + owner: owner ? { + id: owner.id, + display_name: owner.display_name, + slack_id: owner.slack_id + } : nil, + reviewer: reviewer ? { + id: reviewer.id, + display_name: reviewer.display_name, + slack_id: reviewer.slack_id + } : nil + } + end +end diff --git a/config/routes.rb b/config/routes.rb index 687fb31f9..ec4cafe86 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -439,6 +439,10 @@ namespace :api, defaults: { format: :json } do namespace :v1 do resources :ambassador_referrals, only: [ :index, :show ] + + namespace :certification do + resources :ships, only: [ :index ] + end end end From 9fc06f4b15125f3f5a6f0c56e3771591c4661fc9 Mon Sep 17 00:00:00 2001 From: ObayM Date: Sun, 31 May 2026 23:55:45 +0300 Subject: [PATCH 2/4] Updated the API to allow fetching the full ship certs history if no params are specified --- .../api/v1/certification/ships_controller.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/app/controllers/api/v1/certification/ships_controller.rb b/app/controllers/api/v1/certification/ships_controller.rb index 0035fc990..a3eef5d0d 100644 --- a/app/controllers/api/v1/certification/ships_controller.rb +++ b/app/controllers/api/v1/certification/ships_controller.rb @@ -12,17 +12,13 @@ def index window_start, window_end = parse_time_window status_filter = params[:status].presence_in(%w[pending approved returned]) || "all" - scope = ::Certification::Ship - .includes(:reviewer, project: { memberships: :user }) - .where(created_at: window_start..window_end) + scope = ::Certification::Ship.includes(:reviewer, project: { memberships: :user }) + scope = scope.where(created_at: window_start..window_end) if window_start scope = scope.where(status: status_filter) unless status_filter == "all" render json: { - window: { - from: window_start.iso8601, - to: window_end.iso8601 - }, + window: window_start ? { from: window_start.iso8601, to: window_end.iso8601 } : nil, status_filter: status_filter, count: scope.size, ships: scope.order(created_at: :desc).map { |ship| serialize_ship(ship) } @@ -34,6 +30,9 @@ def index def parse_time_window now = Time.current + # No time params at all then return nil so caller skips the filter :) + return [ nil, nil ] if params[:hours].blank? && params[:since].blank? && params[:until].blank? + window_end = parse_time_param(params[:until]) || now window_start = if params[:since].present? From cef1e809091f9db64d909bb2a79e502fc7ca75de Mon Sep 17 00:00:00 2001 From: Dhamari Trice-Hanson <39872667+dhamariT@users.noreply.github.com> Date: Wed, 10 Jun 2026 16:46:55 -0400 Subject: [PATCH 3/4] Harden certification ships endpoint Exclude ships of soft-deleted projects, default to a 24h window instead of returning the whole table when no time params are given, return 400 on unparseable time params rather than silently substituting defaults, and drop the extra COUNT query. --- .../api/v1/certification/ships_controller.rb | 57 +++++++++++-------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/app/controllers/api/v1/certification/ships_controller.rb b/app/controllers/api/v1/certification/ships_controller.rb index a3eef5d0d..bc6d52b59 100644 --- a/app/controllers/api/v1/certification/ships_controller.rb +++ b/app/controllers/api/v1/certification/ships_controller.rb @@ -1,57 +1,66 @@ class Api::V1::Certification::ShipsController < Api::V1::Certification::BaseController + class InvalidParam < StandardError; end + + rescue_from InvalidParam do |error| + render json: { error: error.message }, status: :bad_request + end + # GET /api/v1/certification/ships # - # Returns ship certifications within a specific time frame! + # Returns ship certification reviews within a time window (default: the last 24 hours). # # Query params: - # hours (integer, default 24) + # hours (positive integer, default 24; ignored when since is given) # since (ISO 8601 datetime) - # until (ISO 8601 datetime) + # until (ISO 8601 datetime, default now) # status (pending|approved|returned|all, default all) def index window_start, window_end = parse_time_window status_filter = params[:status].presence_in(%w[pending approved returned]) || "all" - scope = ::Certification::Ship.includes(:reviewer, project: { memberships: :user }) - scope = scope.where(created_at: window_start..window_end) if window_start - + scope = ::Certification::Ship + .joins(:project) + .where(projects: { deleted_at: nil }) + .where(created_at: window_start..window_end) + .includes(:reviewer, project: { memberships: :user }) scope = scope.where(status: status_filter) unless status_filter == "all" + ships = scope.order(created_at: :desc).map { |ship| serialize_ship(ship) } + render json: { - window: window_start ? { from: window_start.iso8601, to: window_end.iso8601 } : nil, + window: { from: window_start.iso8601, to: window_end.iso8601 }, status_filter: status_filter, - count: scope.size, - ships: scope.order(created_at: :desc).map { |ship| serialize_ship(ship) } + count: ships.size, + ships: ships } end private def parse_time_window - now = Time.current + window_end = parse_time_param(:until) || Time.current + window_start = parse_time_param(:since) || window_end - window_hours.hours + raise InvalidParam, "since must be earlier than until" if window_start > window_end - # No time params at all then return nil so caller skips the filter :) - return [ nil, nil ] if params[:hours].blank? && params[:since].blank? && params[:until].blank? + [ window_start, window_end ] + end - window_end = parse_time_param(params[:until]) || now + def window_hours + return 24 if params[:hours].blank? - window_start = if params[:since].present? - parse_time_param(params[:since]) || (now - 24.hours) - else - hours = params[:hours].to_i - hours = 24 if hours <= 0 - window_end - hours.hours - end + hours = Integer(params[:hours], exception: false) + raise InvalidParam, "hours must be a positive integer" unless hours&.positive? - [ window_start, window_end ] + hours end - def parse_time_param(value) + def parse_time_param(key) + value = params[key] return nil if value.blank? - Time.zone.parse(value.to_s) + Time.zone.parse(value.to_s) || raise(InvalidParam, "#{key} is not a valid ISO 8601 datetime") rescue ArgumentError, TypeError - nil + raise InvalidParam, "#{key} is not a valid ISO 8601 datetime" end def serialize_ship(ship) From 2aaf7cfdbc144c5aa929fe6ce55c42f0d637e3c5 Mon Sep 17 00:00:00 2001 From: Dhamari Trice-Hanson <39872667+dhamariT@users.noreply.github.com> Date: Wed, 10 Jun 2026 19:26:15 -0400 Subject: [PATCH 4/4] Add request tests for certification ships endpoint Covers auth rejection, the default 24h window, explicit since/until windows, the status filter, the soft-deleted project exclusion, and 400s on invalid time params. --- .../v1/certification/ships_controller_test.rb | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 test/controllers/api/v1/certification/ships_controller_test.rb diff --git a/test/controllers/api/v1/certification/ships_controller_test.rb b/test/controllers/api/v1/certification/ships_controller_test.rb new file mode 100644 index 000000000..b08db54c7 --- /dev/null +++ b/test/controllers/api/v1/certification/ships_controller_test.rb @@ -0,0 +1,96 @@ +require "test_helper" + +class Api::V1::Certification::ShipsControllerTest < ActionDispatch::IntegrationTest + API_KEY = "test-shipwrights-key" + + setup do + credentials_options[:certification_shipwrights] = { api_keys: [ API_KEY ] } + + @owner = create_user(slack_id: "U0OWNER", display_name: "owner") + @reviewer = create_user(slack_id: "U0REVIEWER", display_name: "reviewer") + + @project = Project.create!(title: "certifiable") + @project.memberships.create!(user: @owner, role: :owner) + @ship = Certification::Ship.create!(project: @project) + end + + test "rejects requests without a valid api key" do + fetch_ships(key: nil) + assert_response :unauthorized + + fetch_ships(key: "wrong-key") + assert_response :unauthorized + end + + test "returns ships in the default 24 hour window with owner and reviewer" do + @ship.update_columns(reviewer_id: @reviewer.id) + + old_ship = Certification::Ship.create!(project: Project.create!(title: "old")) + old_ship.update_columns(created_at: 3.days.ago, updated_at: 3.days.ago) + + fetch_ships + assert_response :success + + body = response.parsed_body + assert_equal 1, body["count"] + ship = body["ships"].first + assert_equal @ship.id, ship["id"] + assert_equal "pending", ship["status"] + assert_equal @owner.slack_id, ship.dig("owner", "slack_id") + assert_equal @reviewer.slack_id, ship.dig("reviewer", "slack_id") + end + + test "filters by status" do + approved = Certification::Ship.create!(project: Project.create!(title: "done")) + approved.update_columns(status: 1) + + fetch_ships(params: { status: "approved" }) + assert_response :success + assert_equal [ approved.id ], response.parsed_body["ships"].map { |s| s["id"] } + end + + test "respects an explicit since/until window" do + @ship.update_columns(created_at: 10.days.ago, updated_at: 10.days.ago) + + fetch_ships(params: { since: 11.days.ago.iso8601, until: 9.days.ago.iso8601 }) + assert_response :success + assert_equal [ @ship.id ], response.parsed_body["ships"].map { |s| s["id"] } + end + + test "excludes ships of soft-deleted projects" do + @project.update_columns(deleted_at: Time.current) + + fetch_ships + assert_response :success + assert_equal 0, response.parsed_body["count"] + end + + test "rejects invalid time params" do + fetch_ships(params: { since: "banana" }) + assert_response :bad_request + + fetch_ships(params: { hours: "0" }) + assert_response :bad_request + + fetch_ships(params: { since: 1.hour.ago.iso8601, until: 2.hours.ago.iso8601 }) + assert_response :bad_request + end + + teardown do + credentials_options.delete(:certification_shipwrights) + end + + private + + # Credentials are read through a memoized options hash; mutating it is the + # only way to inject test keys, since EncryptedConfiguration's method_missing + # swallows minitest's Object#stub as a credential lookup. + def credentials_options + Rails.application.credentials.send(:options) + end + + def fetch_ships(key: API_KEY, params: {}) + headers = key ? { "Authorization" => "Bearer #{key}" } : {} + get api_v1_certification_ships_path, params: params, headers: headers + end +end