Use a decorator class instead of an helper method

openSUSE · May 18, 2023 · 9fcd487 · 9fcd487
1 parent 1f186d9
commit 9fcd487
Show file tree

Hide file tree

Showing 11 changed files with 42 additions and 29 deletions.
diff --git a/src/api/app/controllers/webui/request_controller.rb b/src/api/app/controllers/webui/request_controller.rb
@@ -1,5 +1,4 @@
 class Webui::RequestController < Webui::WebuiController
-  helper 'webui/request'
   helper 'webui/package'
 
   before_action :require_login,
@@ -14,6 +13,8 @@ class Webui::RequestController < Webui::WebuiController
                                         if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
   before_action :set_action_id, only: [: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: [:show, :build_results, :rpm_lint, :changes, :mentioned_issues],
+                                        if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
   before_action :set_active_action, only: [:inline_comment, :show, :build_results, :rpm_lint, :changes, :mentioned_issues],
                                     if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
   before_action :set_superseded_request, only: [:show, :request_action, :request_action_changes, :build_results, :rpm_lint, :changes, :mentioned_issues]
@@ -307,13 +308,13 @@ def rpm_lint
   end
 
   def changes
-    redirect_to request_show_path(params[:number], params[:request_action_id]) unless ApplicationController.helpers.request_action_tab_visibility(@action, 'changes')
+    redirect_to request_show_path(params[:number], params[:request_action_id]) unless @bs_request_action.visible_tab('changes')
 
     @active_tab = 'changes'
   end
 
   def mentioned_issues
-    redirect_to request_show_path(params[:number], params[:request_action_id]) unless ApplicationController.helpers.request_action_tab_visibility(@action, 'mentioned_issues')
+    redirect_to request_show_path(params[:number], params[:request_action_id]) unless @bs_request_action.visible_tab('mentioned_issues')
 
     @active_tab = 'mentioned_issues'
   end
@@ -489,6 +490,10 @@ def set_action_id
     @action_id = params[:request_action_id] || @supported_actions.first&.id || @actions.first.id
   end
 
+  def set_bs_request_action
+    @bs_request_action = BsRequestAction.find(@action_id)
+  end
+
   def set_active_action
     @active_action = @actions.find(@action_id)
   end

diff --git a/src/api/app/helpers/webui/request_helper.rb b/src/api/app/helpers/webui/request_helper.rb
@@ -199,18 +199,4 @@ def next_prev_path(**opts)
       request_show_path(parameters)
     end
   end
-
-  # Handle tabs visibility
-  def request_action_tab_visibility(action, tab_name)
-    case tab_name
-    when 'build_results', 'rpm_lint'
-      action[:sprj] || action[:spkg]
-    when 'changes'
-      (action[:type] == :delete && action[:spkg]) || action[:type].in?([:submit, :maintenance_incident, :maintenance_release])
-    when 'mentioned_issues'
-      action[:type].in?([:submit, :maintenance_incident, :maintenance_release])
-    else
-      true
-    end
-  end
 end
diff --git a/src/api/app/models/bs_request_action.rb b/src/api/app/models/bs_request_action.rb
@@ -866,6 +866,11 @@ def event_parameters
     params
   end
 
+  def visible_tab(tab_name)
+    action_tab_visibility = BsRequestActionTabVisibility.new(self)
+    action_tab_visibility.is_visible(tab_name)
+  end
+
   private
 
   def cache_diffs

diff --git a/src/api/app/models/bs_request_action_tab_visibility.rb b/src/api/app/models/bs_request_action_tab_visibility.rb
@@ -0,0 +1,20 @@
+class BsRequestActionTabVisibility < ApplicationComponent
+  def initialize(bs_request_action)
+    super
+    @action = bs_request_action
+  end
+
+  # Handle tabs visibility
+  def is_visible(tab_name)
+    case tab_name
+    when 'build_results', 'rpm_lint'
+      @action.source_project || @action.source_package
+    when 'changes'
+      (@action.type == 'delete' && @action.source_package) || @action.type.in?(['submit', 'maintenance_incident', 'maintenance_release'])
+    when 'mentioned_issues'
+      @action.type.in?(['submit', 'maintenance_incident', 'maintenance_release'])
+    else
+      true
+    end
+  end
+end
diff --git a/src/api/app/views/webui/request/_request_tabs.html.haml b/src/api/app/views/webui/request/_request_tabs.html.haml
@@ -3,19 +3,19 @@
     %li.nav-item.scrollable-tab-link
       = link_to('Conversation', request_show_path(bs_request.number, actions_count > 1 ? active_action : nil),
                 class: "nav-link text-nowrap #{active_tab == 'conversation' ? 'active' : ''}")
-    - if request_action_tab_visibility(action, 'build_results')
+    - if bs_request_action.visible_tab('build_results')
       %li.nav-item.scrollable-tab-link.active
         = link_to('Build Results', request_build_results_path(bs_request.number, actions_count > 1 ? active_action : nil),
                   class: "nav-link text-nowrap #{active_tab == 'build_results' ? 'active' : ''}")
-    - if request_action_tab_visibility(action, 'rpm_lint')
+    - if bs_request_action.visible_tab('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),
                   class: "nav-link text-nowrap #{active_tab == 'rpm_lint' ? 'active' : ''}")
-    - if request_action_tab_visibility(action, 'changes')
+    - if bs_request_action.visible_tab('changes')
       %li.nav-item.scrollable-tab-link
         = link_to('Changes', request_changes_path(bs_request.number, actions_count > 1 ? active_action : nil),
                   class: "nav-link text-nowrap #{active_tab == 'changes' ? 'active' : ''}")
-    - if request_action_tab_visibility(action, 'mentioned_issues')
+    - if bs_request_action.visible_tab('mentioned_issues')
       %li.nav-item.scrollable-tab-link
         = link_to(request_mentioned_issues_path(bs_request.number, actions_count > 1 ? active_action : nil),
                   class: "nav-link text-nowrap #{active_tab == 'mentioned_issues' ? 'active' : ''}") do

diff --git a/src/api/app/views/webui/request/beta_show.html.haml b/src/api/app/views/webui/request/beta_show.html.haml
@@ -12,7 +12,7 @@
                   prev_action: @prev_action, next_action: @next_action, supported_actions: @supported_actions,
                   diff_to_superseded_id: @diff_to_superseded_id, diff_limit: @diff_limit, page_name: '' }
     = render partial: 'request_tabs',
-        locals: { bs_request: @bs_request, action: @action, issues: @issues, active_action: @active_action,
+        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 }
     .container.p-4
       .row

diff --git a/src/api/app/views/webui/request/build_results.html.haml b/src/api/app/views/webui/request/build_results.html.haml
@@ -10,7 +10,7 @@
                   prev_action: @prev_action, next_action: @next_action, supported_actions: @supported_actions,
                   diff_to_superseded_id: @diff_to_superseded_id, diff_limit: @diff_limit, page_name: 'request_build_results' }
     = render partial: 'request_tabs',
-        locals: { bs_request: @bs_request, action: @action, issues: @issues, active_action: @active_action,
+        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 }
     .container.p-4
       - if @buildable

diff --git a/src/api/app/views/webui/request/changes.html.haml b/src/api/app/views/webui/request/changes.html.haml
@@ -10,7 +10,7 @@
                   prev_action: @prev_action, next_action: @next_action, supported_actions: @supported_actions,
                   diff_to_superseded_id: @diff_to_superseded_id, diff_limit: @diff_limit, page_name: 'request_changes' }
     = render partial: 'request_tabs',
-        locals: { bs_request: @bs_request, action: @action, issues: @issues, active_action: @active_action,
+        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 }
     .container.p-4
       .tab-content.sourcediff

diff --git a/src/api/app/views/webui/request/mentioned_issues.html.haml b/src/api/app/views/webui/request/mentioned_issues.html.haml
@@ -10,7 +10,7 @@
                   prev_action: @prev_action, next_action: @next_action, supported_actions: @supported_actions,
                   diff_to_superseded_id: @diff_to_superseded_id, diff_limit: @diff_limit, page_name: 'request_mentioned_issues' }
     = render partial: 'request_tabs',
-        locals: { bs_request: @bs_request, action: @action, issues: @issues, active_action: @active_action,
+        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 }
     .container.p-4
       - if @issues.empty?

diff --git a/src/api/app/views/webui/request/rpm_lint.html.haml b/src/api/app/views/webui/request/rpm_lint.html.haml
@@ -10,7 +10,7 @@
                   prev_action: @prev_action, next_action: @next_action, supported_actions: @supported_actions,
                   diff_to_superseded_id: @diff_to_superseded_id, diff_limit: @diff_limit, page_name: 'request_rpm_lint' }
     = render partial: 'request_tabs',
-        locals: { bs_request: @bs_request, action: @action, issues: @issues, active_action: @active_action,
+        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 }
     .container.p-4
       - if @action[:spkg]

diff --git a/src/api/spec/helpers/webui/request_helper_spec.rb b/src/api/spec/helpers/webui/request_helper_spec.rb
@@ -151,9 +151,6 @@
 
         it { is_expected.to match(expected_regex) }
       end
-
-      it { expect(request_action_tab_visibility(action, 'conversation')).to be(true) }
-      it { expect(request_action_tab_visibility(action, 'mentioned_issues')).to be(false) }
     end
 
     context 'when action is :add_role' do