Compute actions count instead of using a var of the value

src/api/app/controllers/webui/request_controller.rb

@@ -8,19 +8,14 @@
   before_action :require_request,
                 only: %i[changerequest show request_action request_action_changes inline_comment build_results rpm_lint
                          changes mentioned_issues chart_build_results complete_build_results]
-  before_action :set_actions, only: %i[inline_comment show build_results rpm_lint changes mentioned_issues chart_build_results complete_build_results],
+  before_action :set_actions, only: %i[inline_comment show build_results rpm_lint changes mentioned_issues chart_build_results complete_build_results request_action_changes],
                               if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
+  before_action :set_actions, only: [:show]
   before_action :build_results_data, only: [:show], if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
-  before_action :set_supported_actions, only: %i[inline_comment show build_results rpm_lint changes mentioned_issues],
-                                        if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
-  before_action :set_action_id, only: %i[inline_comment show build_results rpm_lint changes mentioned_issues],
-                                if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
-  before_action :set_bs_request_action, only: %i[show build_results rpm_lint changes mentioned_issues],
-                                        if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
+  before_action :set_action, only: %i[inline_comment show build_results rpm_lint changes mentioned_issues],
+                             if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
   before_action :set_influxdb_data_request_actions, only: %i[show build_results rpm_lint changes mentioned_issues],
                                                     if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
-  before_action :set_active_action, only: %i[inline_comment show build_results rpm_lint changes mentioned_issues],
-                                    if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
   before_action :set_superseded_request, only: %i[show request_action request_action_changes build_results rpm_lint changes mentioned_issues]
   before_action :check_ajax, only: :sourcediff
   before_action :prepare_request_data, only: %i[show build_results rpm_lint changes mentioned_issues],
@@ -178,7 +173,7 @@
   end
 
   def request_action_changes
-    @action = @bs_request.bs_request_actions.where(id: params['id'].to_i).first
+    @action = @actions.where(id: params['id'].to_i).first
 
     cache_diff_data
 
@@ -281,15 +276,15 @@
   end
 
   def build_results
-    redirect_to request_show_path(params[:number], params[:request_action_id]) unless @bs_request_action.tab_visibility.build
+    redirect_to request_show_path(params[:number], params[:request_action_id]) unless @action.tab_visibility.build
 
     @active_tab = 'build_results'
     @project = @staging_project || @action.source_project
     @buildable = @action.source_package || @project
   end
 
   def rpm_lint
-    redirect_to request_show_path(params[:number], params[:request_action_id]) unless @bs_request_action.tab_visibility.rpm_lint
+    redirect_to request_show_path(params[:number], params[:request_action_id]) unless @action.tab_visibility.rpm_lint
 
     @active_tab = 'rpm_lint'
     @ajax_data = {}
@@ -299,13 +294,13 @@
   end
 
   def changes
-    redirect_to request_show_path(params[:number], params[:request_action_id]) unless @bs_request_action.tab_visibility.changes
+    redirect_to request_show_path(params[:number], params[:request_action_id]) unless @action.tab_visibility.changes
 
     @active_tab = 'changes'
   end
 
   def mentioned_issues
-    redirect_to request_show_path(params[:number], params[:request_action_id]) unless @bs_request_action.tab_visibility.issues
+    redirect_to request_show_path(params[:number], params[:request_action_id]) unless @action.tab_visibility.issues
 
     @active_tab = 'mentioned_issues'
   end
@@ -344,7 +339,7 @@
   end
 
   def any_project_maintained_by_current_user?
-    projects = @bs_request.bs_request_actions.select(:target_project).distinct.pluck(:target_project)
+    projects = @actions.select(:target_project).distinct.pluck(:target_project)
     maintainer_role = Role.find_by_title('maintainer')
     projects.any? { |project| Project.find_by_name(project).user_has_role?(User.possibly_nobody, maintainer_role) }
   end
@@ -373,7 +368,7 @@
   end
 
   def target_package_maintainers
-    distinct_bs_request_actions = @bs_request.bs_request_actions.select(:target_project, :target_package).distinct
+    distinct_bs_request_actions = @actions.select(:target_project, :target_package).distinct
     distinct_bs_request_actions.flat_map do |action|
       Package.find_by_project_and_name(action.target_project, action.target_package).try(:maintainers)
     end.compact.uniq
@@ -475,22 +470,10 @@
     ActionBuildResultsService::ChartDataExtractor.new(actions: @actions).call
   end
 
-  def set_supported_actions
-    # Change supported_actions below into actions here when all actions are supported
-    @supported_actions = @actions.where(type: %i[add_role change_devel delete submit maintenance_incident maintenance_release set_bugowner])
-  end
-
-  def set_action_id
-    # In case the request doesn't have supported actions, we display the first unsupported action.
-    @action_id = params[:request_action_id] || @supported_actions.first&.id || @actions.first.id
-  end
-
-  def set_bs_request_action
-    @bs_request_action = @bs_request.bs_request_actions.find(@action_id)
-  end
-
-  def set_active_action
-    @active_action = @actions.find(@action_id)
+  def set_action
+    # If no specific request_action_id, take the first
+    action_id = params[:request_action_id] || @actions.first.id
+    @action = @actions.find(action_id)
   end
 
   def staging_status(request, target_project)
@@ -530,11 +513,11 @@
     @diff_to_superseded_id = params[:diff_to_superseded]
 
     # Handling request actions
-    @action = @active_action || @bs_request.bs_request_actions.first
-    active_action_index = @supported_actions.index(@action)
-    if active_action_index
-      @prev_action = @supported_actions[active_action_index - 1] unless active_action_index.zero?
-      @next_action = @supported_actions[active_action_index + 1] if active_action_index + 1 < @supported_actions.length
+    @action ||= @actions.first
+    action_index = @actions.index(@action)
+    if action_index
+      @prev_action = @actions[action_index - 1] unless action_index.zero?
+      @next_action = @actions[action_index + 1] if action_index + 1 < @actions.length
     end
 
     target_project = Project.find_by_name(@bs_request.target_project_name)
@@ -562,7 +545,7 @@
 
   def set_influxdb_data_request_actions
     InfluxDB::Rails.current.tags = {
-      bs_request_action_type: @bs_request_action.class.name
+      bs_request_action_type: @action.class.name
     }
   end
 end

src/api/app/views/webui/request/_actions_details.html.haml

@@ -2,7 +2,7 @@
 :ruby
   page_name ||= ''
 
-- if actions_count > 1
+- if actions.count > 1
   .d-flex.flex-wrap.align-items-center
     .input-group.w-auto
       = link_to('Previous', next_prev_path(number: bs_request.number, request_action_id: prev_action&.id,
@@ -12,7 +12,7 @@
       .dropdown#request-actions
         %button.btn.btn-outline-primary.btn-sm.dropdown-toggle.rounded-0{ 'aria-expanded' => 'false', 'data-bs-toggle' => 'dropdown',
                                                                           :type => 'button', 'data-boundary' => 'viewport' }
-          = active_action.short_name
+          = action.short_name
         %ul.dropdown-menu.dropdown-menu-start.scrollable-dropdown.pt-0
           %li.card-header.px-1.sticky-top.bg-body-tertiary
             %h6.dropdown-header
@@ -26,31 +26,31 @@
                                                         next_prev_path(number: bs_request.number,
                                                                        request_action_id: action_item.id,
                                                                        diff_to_superseded: diff_to_superseded_id, page_name: page_name),
-                                                        class: "dropdown-item #{action_item.id == active_action.id ? 'active' : ''}",
+                                                        class: "dropdown-item #{action_item.id == action.id ? 'active' : ''}",
                                                         'aria-current': 'true')
       = link_to('Next', next_prev_path(number: bs_request.number, request_action_id: next_action&.id,
                                        diff_to_superseded: diff_to_superseded_id, page_name: page_name),
                 class: "btn btn-primary btn-sm #{next_action ? '' : 'disabled'}",
                 id: 'next-action-button')
 
-    - active_action_index = actions.find_index(active_action) + 1
-    %span.ms-2 Showing ##{active_action_index} (of #{actions_count})
+    - action_index = actions.find_index(action) + 1
+    %span.ms-2 Showing ##{action_index} (of #{actions.count})
 %h5.mt-4
   Action details
-  - if actions_count > 1 && User.session
+  - if actions.count > 1 && User.session
     %span#action-seen-by-user-wrapper
-      = render ActionSeenByUserComponent.new(action: active_action, user: User.session!)
+      = render ActionSeenByUserComponent.new(action: action, user: User.session!)
 
 %p.fst-italic
-  = render BsRequestActionDescriptionComponent.new(action: active_action)
-- if active_action.target_releaseproject
-  %p.fst-italic Release in #{project_or_package_link(project: active_action.target_releaseproject)}
+  = render BsRequestActionDescriptionComponent.new(action: action)
+- if action.target_releaseproject
+  %p.fst-italic Release in #{project_or_package_link(project: action.target_releaseproject)}
 
-- if %w[add_role set_bugowner].include?(active_action.type)
+- if %w[add_role set_bugowner].include?(action.type)
   :ruby
-    parameters = { project: active_action.target_project, request_action_id: active_action.id, notification_id: @current_notification }
-    roles_link = if active_action.target_package
-                   package_users_path(parameters.merge!(package: active_action.target_package))
+    parameters = { project: action.target_project, request_action_id: action.id, notification_id: @current_notification }
+    roles_link = if action.target_package
+                   package_users_path(parameters.merge!(package: action.target_package))
                  else
                    project_users_path(parameters)
                  end
@@ -60,10 +60,10 @@
       check the current roles
     of
     %span<
-      = project_or_package_text(active_action.target_project, active_action.target_package)
+      = project_or_package_text(action.target_project, action.target_package)
       \.
 
-- if active_action.type == 'change_devel'
-  - current_devel_package = Package.find_by_project_and_name(active_action.target_project, active_action.target_package).develpackage
+- if action.type == 'change_devel'
+  - current_devel_package = Package.find_by_project_and_name(action.target_project, action.target_package).develpackage
   -  if current_devel_package
     Development is currently happening on #{project_or_package_link(project: current_devel_package.project.name, package: current_devel_package.name)}

src/api/app/views/webui/request/_request_header.html.haml

@@ -73,9 +73,7 @@
         request
   .mt-4
     -# action description, prev + dropdown select + next
-        TODO: once we support all types of actions, we have to send @actions instead of @supported_actions
-    = render partial: 'actions_details', locals: { bs_request: bs_request, action: action, active_action: active_action,
+    = render partial: 'actions_details', locals: { bs_request: bs_request, action: action,
                                                     prev_action: prev_action, next_action: next_action,
-                                                    actions: supported_actions, actions_count: supported_actions.count,
-                                                    diff_to_superseded_id: diff_to_superseded_id,
+                                                    actions: actions, diff_to_superseded_id: diff_to_superseded_id,
                                                     page_name: page_name }

src/api/app/views/webui/request/_request_tabs.html.haml

@@ -1,23 +1,23 @@
 .border-bottom
   %ul.nav.scrollable-tabs.border-0#request-tabs
     %li.nav-item.scrollable-tab-link
-      = link_to('Conversation', request_show_path(bs_request.number, actions_count > 1 ? active_action : nil),
+      = link_to('Conversation', request_show_path(bs_request.number, actions_count > 1 ? action : nil),
                 class: "nav-link text-nowrap #{active_tab == 'conversation' ? 'active' : ''}")
-    - if bs_request_action.tab_visibility.build
+    - if action.tab_visibility.build
       %li.nav-item.scrollable-tab-link.active
-        = link_to('Build Results', request_build_results_path(bs_request.number, actions_count > 1 ? active_action : nil),
+        = link_to('Build Results', request_build_results_path(bs_request.number, actions_count > 1 ? action : nil),
                   class: "nav-link text-nowrap #{active_tab == 'build_results' ? 'active' : ''}")
-    - if bs_request_action.tab_visibility.rpm_lint
+    - if action.tab_visibility.rpm_lint
       %li.nav-item.scrollable-tab-link
-        = link_to('RPM Lint', request_rpm_lint_path(bs_request.number, actions_count > 1 ? active_action : nil),
+        = link_to('RPM Lint', request_rpm_lint_path(bs_request.number, actions_count > 1 ? action : nil),
                   class: "nav-link text-nowrap #{active_tab == 'rpm_lint' ? 'active' : ''}")
-    - if bs_request_action.tab_visibility.changes
+    - if action.tab_visibility.changes
       %li.nav-item.scrollable-tab-link
-        = link_to('Changes', request_changes_path(bs_request.number, actions_count > 1 ? active_action : nil),
+        = link_to('Changes', request_changes_path(bs_request.number, actions_count > 1 ? action : nil),
                   class: "nav-link text-nowrap #{active_tab == 'changes' ? 'active' : ''}")
-    - if bs_request_action.tab_visibility.issues
+    - if action.tab_visibility.issues
       %li.nav-item.scrollable-tab-link
-        = link_to(request_mentioned_issues_path(bs_request.number, actions_count > 1 ? active_action : nil),
+        = link_to(request_mentioned_issues_path(bs_request.number, actions_count > 1 ? action : nil),
                   class: "nav-link text-nowrap #{active_tab == 'mentioned_issues' ? 'active' : ''}") do
           Mentioned Issues
           %span.badge.text-bg-primary.align-text-top= issues.size

src/api/app/views/webui/request/beta_show.html.haml

@@ -9,13 +9,13 @@
 .card
   .card-body.p-0
     = render partial: 'request_header',
-        locals: { bs_request: @bs_request, staging_status: @staging_status, action: @action, active_action: @active_action,
-                  prev_action: @prev_action, next_action: @next_action, supported_actions: @supported_actions,
+        locals: { bs_request: @bs_request, staging_status: @staging_status, action: @action,
+                  prev_action: @prev_action, next_action: @next_action, actions: @actions,
                   diff_to_superseded_id: @diff_to_superseded_id, page_name: '',
                   bs_requests: @watched_requests, packages: @watched_packages, projects: @watched_projects }
     = render partial: 'request_tabs',
-        locals: { bs_request: @bs_request, bs_request_action: @bs_request_action, issues: @issues, active_action: @active_action,
-                  actions_count: @supported_actions.count, active_tab: @active_tab }
+        locals: { bs_request: @bs_request, action: @action, issues: @issues,
+                  actions_count: @actions.count, active_tab: @active_tab }
     .container.p-4
       .row
         .col-md-4.order-md-2.order-sm-1.mb-4

src/api/app/views/webui/request/build_results.html.haml

@@ -6,13 +6,13 @@
 .card
   .card-body.p-0
     = render partial: 'request_header',
-        locals: { bs_request: @bs_request, staging_status: @staging_status, action: @action, active_action: @active_action,
-                  prev_action: @prev_action, next_action: @next_action, supported_actions: @supported_actions,
+        locals: { bs_request: @bs_request, staging_status: @staging_status, action: @action,
+                  prev_action: @prev_action, next_action: @next_action, actions: @actions,
                   diff_to_superseded_id: @diff_to_superseded_id, page_name: 'request_build_results',
                   bs_requests: @watched_requests, packages: @watched_packages, projects: @watched_projects }
     = render partial: 'request_tabs',
-        locals: { bs_request: @bs_request, bs_request_action: @bs_request_action, issues: @issues, active_action: @active_action,
-                  actions_count: @supported_actions.count, active_tab: @active_tab }
+        locals: { bs_request: @bs_request, action: @action, issues: @issues,
+                  actions_count: @actions.count, active_tab: @active_tab }
     .container.p-4
       - if @buildable
         .build-results-content

src/api/app/views/webui/request/changes.html.haml

@@ -6,13 +6,13 @@
 .card
   .card-body.p-0
     = render partial: 'request_header',
-        locals: { bs_request: @bs_request, staging_status: @staging_status, action: @action, active_action: @active_action,
-                  prev_action: @prev_action, next_action: @next_action, supported_actions: @supported_actions,
+        locals: { bs_request: @bs_request, staging_status: @staging_status, action: @action,
+                  prev_action: @prev_action, next_action: @next_action, actions: @actions,
                   diff_to_superseded_id: @diff_to_superseded_id, page_name: 'request_changes',
                   bs_requests: @watched_requests, packages: @watched_packages, projects: @watched_projects }
     = render partial: 'request_tabs',
-        locals: { bs_request: @bs_request, bs_request_action: @bs_request_action, issues: @issues, active_action: @active_action,
-                  actions_count: @supported_actions.count, active_tab: @active_tab }
+        locals: { bs_request: @bs_request, action: @action, issues: @issues,
+                  actions_count: @actions.count, active_tab: @active_tab }
     .container.p-4
       .tab-content.sourcediff
         - if @diff_to_superseded

src/api/app/views/webui/request/inline_comment.js.erb

@@ -1,6 +1,6 @@
 $('#flash').empty();
-$('#comment<%= @line %> .diff-comments').html("<%= escape_javascript(render(partial: 'webui/request/inline_comment', locals: { comment: @comment, commentable: @active_action, diff_ref: @line })) %>");
+$('#comment<%= @line %> .diff-comments').html("<%= escape_javascript(render(partial: 'webui/request/inline_comment', locals: { comment: @comment, commentable: @action, diff_ref: @line })) %>");
 $("#comment<%= @line %> .diff-comments textarea").focus();
 $("#comment<%= @line %>").on('click', '.cancel-comment', function (e) {
-  $('#comment<%= @line %> .diff-comments').html("<%= escape_javascript(render(partial: 'webui/request/add_inline_comment', locals: { commentable: @active_action, diff_ref: @line })) %>");
+  $('#comment<%= @line %> .diff-comments').html("<%= escape_javascript(render(partial: 'webui/request/add_inline_comment', locals: { commentable: @action, diff_ref: @line })) %>");
 });