Implement status checks for requests

in order to do this it was also necessary to refactor the whole status checks module

* Dropped Status::RepositoryPublish model and introduced generic Status::Report model instead
* Dropped all routes except Checks#update and Reports#index as discussed with the stakeholders
  that these are the only requested routes for now. Checks#update will create or update the
  check. Report#index will list all checks of this report.
* Moved common functionality to a concern
* Changed routes like requested by stakeholders
* Made controllers polymorphic to support requests and repositories for now, more to probably come
* Updated controller specs

Co-authored-by: Björn Geuken <[email protected]>
Co-authored-by: Eduardo Navarro <[email protected]>

src/api/app/controllers/status/checks_controller.rb

@@ -1,71 +1,41 @@
 class Status::ChecksController < ApplicationController
-  before_action :require_project
-  before_action :require_repository
-  before_action :require_checkable, only: [:index, :show, :destroy, :update]
-  before_action :require_or_initialize_checkable, only: :create
-  before_action :require_check, only: [:show, :destroy, :update]
-  before_action :set_xml_check, only: [:create, :update]
-  skip_before_action :require_login, only: [:show, :index]
+  include Status::Concerns::SetCheckable
+  before_action :set_xml_check
+  before_action :set_status_report
+  before_action :set_check
   after_action :verify_authorized
 
-  # GET /projects/:project_name/repositories/:repository_name/repository_publishes/:repository_publish_build_id/checks
-  def index
-    @checks = @checkable.checks
-    @missing_checks = @checkable.missing_checks
-    authorize @checks
-  end
-
-  # GET /projects/:project_name/repositories/:repository_name/repository_publishes/:repository_publish_build_id/checks/:id
-  def show
-    authorize @check
-  end
+  # PUT /status_reports/published/:project_name/:repository_name/reports/:uuid
+  # PUT /status_reports/requests/:number
+  def update
+    authorize @status_report
 
-  # POST /projects/:project_name/repositories/:repository_name/repository_publishes/:repository_publish_build_id/checks
-  def create
-    @xml_check[:checkable] = @checkable
-    @check = Status::Check.new(@xml_check)
-    authorize @check
-    if @check.save
-      render :show
+    if @check
+      @check.assign_attributes(@xml_check)
     else
-      render_error(status: 422, errorcode: 'invalid_check', message: "Could not save check: #{@check.errors.full_messages.to_sentence}")
+      @xml_check[:status_report] = @status_report
+      @check = Status::Check.new(@xml_check)
     end
-  end
 
-  # PATCH /projects/:project_name/repositories/:repository_name/repository_publishes/:repository_publish_build_id/checks/:id
-  def update
-    authorize @check
-    if @check.update(@xml_check)
+    if @check.save
       render :show
     else
       render_error(status: 422, errorcode: 'invalid_check', message: "Could not save check: #{@check.errors.full_messages.to_sentence}")
     end
   end
 
-  # DELETE /projects/:project_name/repositories/:repository_name/repository_publishes/:repository_publish_build_id/checks/:id
-  def destroy
-    authorize @check
-    if @check.destroy
-      render_ok
-    else
-      render_error(status: 422, errorcode: 'invalid_check', message: "Could not delete check: #{@check.errors.full_messages.to_sentence}")
-    end
-  end
-
   private
 
-  def require_or_initialize_checkable
-    @checkable = @repository.status_publishes.find_or_initialize_by(build_id: params[:repository_publish_build_id])
-  end
-
-  def require_checkable
-    @checkable = Status::RepositoryPublish.find_by(build_id: params[:repository_publish_build_id]) if params[:repository_publish_build_id]
-    render_error(status: 404, errorcode: 'not_found', message: "Unable to find status_repository_publish with id '#{params[:repository_publish_build_id]}'") unless @checkable
+  def set_status_report
+    if params[:report_uuid]
+      @status_report = @checkable.status_reports.find_or_initialize_by(uuid: params[:report_uuid])
+    else
+      @status_report = @checkable.status_reports.first_or_initialize
+    end
   end
 
-  def require_check
-    @check = @checkable.checks.find_by(id: params[:id])
-    render_error(status: 404, errorcode: 'not_found', message: "Unable to find check with id '#{params[:id]}'") unless @check
+  def set_check
+    @check = @status_report.checks.find_by(name: @xml_check[:name])
   end
 
   def set_xml_check
@@ -74,17 +44,12 @@ def set_xml_check
     render_error status: 404, errorcode: 'empty_body', message: 'Request body is empty!'
   end
 
-  def require_project
-    @project = Project.get_by_name(params[:project_name])
-  end
-
-  def require_repository
-    @repository = @project.repositories.find_by(name: params[:repository_name])
-    raise UnknownRepository, "Repository does not exist #{params[:repository_name]}" unless @repository
-  end
-
   def xml_hash
-    result = (Xmlhash.parse(request.body.read) || {}).with_indifferent_access
-    result.slice(:url, :state, :short_description, :name)
+    result = HashWithIndifferentAccess.new
+    parsed_body = Xmlhash.parse(request.body.read)
+    return if parsed_body.blank?
+    %w[url state short_description name].each { |key| result[key] = parsed_body.value(key) if parsed_body.value(key) }
+
+    result
   end
 end

src/api/app/controllers/status/concerns/set_checkable.rb

@@ -0,0 +1,49 @@
+module Status
+  module Concerns
+    module SetCheckable
+      extend ActiveSupport::Concern
+
+      included do
+         before_action :set_checkable
+      end
+
+      private
+
+      def set_checkable
+        set_repository || set_bs_request
+        return if @checkable
+
+        @error_message = 'Provide at least project_name and repository_name or request number.' unless @error_message
+        render_error(
+          status: 404,
+          errorcode: 'not_found',
+          message: @error_message
+        )
+      end
+
+      def set_project
+        @project = Project.find_by(name: params[:project_name])
+
+        return @project if @project
+        @error_message = "Project '#{params[:project_name]}' not found."
+      end
+
+      def set_repository
+        set_project
+        return unless @project
+
+        @checkable = @project.repositories.find_by(name: params[:repository_name])
+        return @checkable if @checkable
+
+        @error_message = "Repository '#{params[:project_name]}/#{params[:repository_name]}' not found."
+      end
+
+      def set_bs_request
+        @checkable = BsRequest.with_submit_requests.find_by(number: params[:bs_request_number])
+        return @checkable if @checkable
+
+        @error_message = "Submit request with number '#{params[:bs_request_number]}' not found."
+      end
+    end
+  end
+end

src/api/app/controllers/status/reports_controller.rb

@@ -0,0 +1,30 @@
+class Status::ReportsController < ApplicationController
+  include Status::Concerns::SetCheckable
+  before_action :set_status_report
+  skip_before_action :require_login, only: [:show]
+
+  # GET /status_reports/published/:project_name/:repository_name/reports/:uuid
+  def show
+    @checks = @status_report.checks
+    @missing_checks = @status_report.missing_checks
+  end
+
+  private
+
+  def set_status_report
+    if params[:uuid]
+      @status_report = @checkable.status_reports.find_by(uuid: params[:uuid])
+      return if @status_report
+      @error_message = "Status report with uuid '#{params[:uuid]} not found.'"
+    else
+      @status_report = @checkable.status_reports.first
+      return if @status_report
+      @error_message = "Status report not found."
+    end
+    render_error(
+      status: 404,
+      errorcode: 'not_found',
+      message: @error_message
+    )
+  end
+end

src/api/app/controllers/status/required_checks_controller.rb

@@ -14,13 +14,15 @@ class Status::RequiredChecksController < ApplicationController
 
   #### CRUD actions
 
-  # GET /projects/:project_name/repositories/:repository_name/required_checks
+  # GET /status_reports/projects/:project_name/required_checks
+  # GET /status_reports/repositories/:project_name/:repository_name/required_checks
   def index
     authorize @checkable
     @required_checks = @checkable.required_checks
   end
 
-  # POST /projects/:project_name/repositories/:repository_name/required_checks
+  # POST /status_reports/projects/:project_name/required_checks/:name
+  # POST /status_reports/repositories/:project_name/:repository_name/required_checks/:name
   def create
     authorize @checkable
     @required_checks = @checkable.required_checks |= names
@@ -36,7 +38,8 @@ def create
     end
   end
 
-  # DELETE /projects/:project_name/repositories/:repository_name/required_checks/:name
+  # DELETE /status_reports/projects/:project_name/required_checks/:name
+  # DELETE /status_reports/repositories/:project_name/:repository_name/required_checks/:name
   def destroy
     authorize @checkable
     @checkable.required_checks -= [params[:name]]
@@ -63,20 +66,10 @@ def set_checkable
     if params[:repository_name]
       @project = Project.get_by_name(params[:project_name])
       @checkable = @project.repositories.find_by(name: params[:repository_name])
-      return if @checkable
-
-      render_error(
-        status: 404,
-        errorcode: 'not_found',
-        message: "Unable to find repository with name '#{params[:project_name]}/#{params[:repository_name]}'"
-      )
     else
-      render_error(
-        status: 404,
-        errorcode: 'not_found',
-        message: 'No repository name specified.'
-      )
+      @checkable = Project.get_by_name(params[:project_name])
     end
+    raise ActiveRecord::RecordNotFound unless @checkable
   end
 
   # Use callbacks to share common setup or constraints between actions.

src/api/app/controllers/webui/obs_factory/staging_projects_controller.rb

@@ -44,7 +44,7 @@ def show
           return if images_repository.blank?
 
           @build_id = images_repository.build_id
-          status = images_repository.status_publishes.find_by(build_id: @build_id)
+          status = images_repository.status_reports.find_by(build_id: @build_id)
           return if status.nil?
           @missing_checks = status.missing_checks
           @checks = status.checks

src/api/app/helpers/status/required_checks_helper.rb

@@ -5,7 +5,7 @@ def build_header(project, checkable)
     elsif checkable.is_a?(Package)
       { project: project.name, package: checkable.name }
     else
-      { project: project.name }
+      { project: checkable.name }
     end
   end
 end

src/api/app/helpers/webui/obs_factory/application_helper.rb

@@ -9,7 +9,9 @@ def distribution_tests_url(distribution, version = nil)
     path
   end
 
-  def icon_for_checks(checks)
+  def icon_for_checks(checks, missing_checks)
+    return 'eye' if missing_checks.present?
+    return 'accept' if checks.blank?
     return 'eye' if checks.any? { |check| check.state == 'pending' }
     return 'accept' if checks.all? { |check| check.state == 'success' }
     'error'

src/api/app/models/bs_request.rb

@@ -65,6 +65,7 @@ class SaveError < APIError
   }
 
   scope :with_actions_and_reviews, -> { joins(:bs_request_actions).left_outer_joins(:reviews).distinct.order(priority: :asc, id: :desc) }
+  scope :with_submit_requests, -> { joins(:bs_request_actions).where(bs_request_actions: { type: 'submit' }) }
 
   scope :by_user_reviews, ->(user_ids) { where(reviews: { user: user_ids }) }
   scope :by_project_reviews, ->(project_ids) { where(reviews: { project: project_ids }) }
@@ -84,6 +85,8 @@ class SaveError < APIError
   has_many :comments, as: :commentable, dependent: :delete_all
   has_many :request_history_elements, -> { order(:created_at) }, class_name: 'HistoryElement::Request', foreign_key: :op_object_id
   has_many :review_history_elements, through: :reviews, source: :history_elements
+  has_many :status_reports, as: :checkable, class_name: 'Status::Report', dependent: :destroy
+  has_many :target_project_objects, through: :bs_request_actions
 
   validates :state, inclusion: { in: VALID_REQUEST_STATES }
   validates :creator, presence: true
@@ -1228,6 +1231,10 @@ def forward_to(project:, package: nil, options: {})
     new_request
   end
 
+  def required_checks
+    target_project_objects.pluck(:required_checks).flatten.uniq
+  end
+
   private
 
   #

src/api/app/models/project.rb

@@ -21,6 +21,7 @@ class Project < ApplicationRecord
   after_rollback :discard_cache
   after_initialize :init
 
+  serialize :required_checks, Array
   attr_reader :commit_opts
   attr_writer :commit_opts
   after_initialize do

src/api/app/models/repository.rb

@@ -17,13 +17,13 @@ class Repository < ApplicationRecord
   has_many :repository_architectures, -> { order('position') }, dependent: :destroy, inverse_of: :repository
   has_many :architectures, -> { order('position') }, through: :repository_architectures
   has_many :status_publishes, class_name: 'Status::RepositoryPublish'
-  has_many :checks, through: :status_publishes, class_name: 'Status::Check' do
-    def for_build_id(build_id)
-      where(status_repository_publishes: { build_id: build_id })
+  has_many :status_reports, as: :checkable, class_name: 'Status::Report', dependent: :destroy do
+    def for_uuid(uuid)
+      where(status_reports: { uuid: uuid })
     end
 
     def latest
-      for_build_id(proxy_association.owner.build_id)
+      for_uuid(proxy_association.owner.build_id)
     end
   end
 

src/api/app/models/status/check.rb

@@ -7,13 +7,13 @@ class Status::Check < ApplicationRecord
   #### Self config
 
   #### Attributes
-  validates :state, :name, :checkable, presence: true
+  validates :state, :name, presence: true
   # TODO: This should be an ENUM
   validates :state, inclusion: { in: %w[pending error failure success] }
-  validates :name, uniqueness: { scope: :checkable }
+  validates :name, uniqueness: { scope: :status_report }
 
   #### Associations macros (Belongs to, Has one, Has many)
-  belongs_to :checkable, polymorphic: true
+  belongs_to :status_report, class_name: 'Status::Report', foreign_key: 'status_reports_id'
 
   #### Callbacks macros: before_save, after_save, etc.
 
@@ -28,10 +28,7 @@ class Status::Check < ApplicationRecord
 
   #### Instance methods (public and then protected/private)
   def required?
-    # TODO: we will remove the checkable polymorphic Association
-    # in a follow up refactoring and this will be then
-    # report.checkable.required_checks.include?(name)
-    checkable.repository.required_checks.include?(name)
+    status_report.checkable.required_checks.include?(name)
   end
 
   #### Alias of methods