Merge pull request #3963 from bgeuken/refactor_request_controller

Refactor request controller

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

@@ -5,7 +5,7 @@ class Webui::RequestController < Webui::WebuiController
   # requests do not really add much value for our page rank :)
   before_action :lockout_spiders
 
-  before_action :require_request, only: [:changerequest]
+  before_action :require_request, only: [:changerequest, :show]
 
   before_action :set_project, only: [:change_devel_request_dialog]
 
@@ -79,13 +79,7 @@ def modify_review
   end
 
   def show
-    @bsreq = BsRequest.find_by_number(params[:number])
-    unless @bsreq
-      flash[:error] = "Can't find request #{params[:number]}"
-      redirect_back(fallback_location: user_show_path(User.current)) && return
-    end
-
-    @req = @bsreq.webui_infos
+    @req = @bs_request.webui_infos
     @id = @req['id']
     @number = @req['number']
     @state = @req['state'].to_s
@@ -100,7 +94,7 @@ def show
     @can_add_reviews = @state.in?(["new", "review"]) && (@is_author || @is_target_maintainer || @my_open_reviews.present?) && !User.current.is_nobody?
     @can_handle_request = @state.in?(["new", "review", "declined"]) && (@is_target_maintainer || @is_author) && !User.current.is_nobody?
 
-    @history = @bsreq.history_elements
+    @history = @bs_request.history_elements
     @actions = @req['actions']
 
     # retrieve a list of all package maintainers that are assigned to at least one target package
@@ -117,14 +111,14 @@ def show
     @request_before = nil
     @request_after = nil
 
-    index = session[:request_numbers].try(:index, @bsreq.number)
+    index = session[:request_numbers].try(:index, @bs_request.number)
     if index
       @request_before = session[:request_numbers][index - 1] if index > 0
       # will be nil for after end
       @request_after = session[:request_numbers][index + 1]
     end
 
-    @comments = @bsreq.comments
+    @comments = @bs_request.comments
     @comment = Comment.new
   end
 
@@ -136,21 +130,7 @@ def sourcediff
                                                     no_border: true, uid: params[:uid]}
   end
 
-  def require_request
-    required_parameters :number
-    @req = BsRequest.find_by_number params[:number]
-    return if @req
-    flash[:error] = "Can't find request #{params[:number]}"
-    redirect_back(fallback_location: user_show_path(User.current)) && return
-  end
-
   def changerequest
-    @req = BsRequest.find_by_number(params[:number])
-    unless @req
-      flash[:error] = "Can't find request #{params[:number]}"
-      redirect_back(fallback_location: user_show_path(User.current)) && return
-    end
-
     changestate = nil
     %w(accepted declined revoked new).each do |s|
       if params.has_key? s
@@ -174,7 +154,7 @@ def changerequest
           if target.can_be_modified_by?(User.current)
             # the request action type might be permitted in future, but that doesn't mean we
             # are allowed to modify the object
-            target.add_user(@req.creator, 'maintainer')
+            target.add_user(@bs_request.creator, 'maintainer')
             target.save
             target.store if target.kind_of? Project
           end
@@ -377,6 +357,14 @@ def main_object
 
   private
 
+  def require_request
+    required_parameters :number
+    @bs_request = BsRequest.find_by_number params[:number]
+    return if @bs_request
+    flash[:error] = "Can't find request #{params[:number]}"
+    redirect_back(fallback_location: user_show_path(User.current)) && return
+  end
+
   def get_target_package_maintainers(actions)
     actions = actions.uniq{ |action| action[:tpkg] }
     actions.flat_map { |action| Package.find_by_project_and_name(action[:tprj], action[:tpkg]).try(:maintainers) }.compact.uniq
@@ -424,7 +412,7 @@ def forward_request_to(fwd)
       BsRequest.transaction do
         req = BsRequest.new( state: "new")
         req.description = params[:description]
-        @req.bs_request_actions.each do |action|
+        @bs_request.bs_request_actions.each do |action|
           rev = Directory.hashed(project: action.target_project, package: action.target_package)['rev']
 
           opts = { source_project: action.target_project,
@@ -443,7 +431,7 @@ def forward_request_to(fwd)
         end
       end
     rescue APIException, ActiveRecord::RecordInvalid => e
-      Airbrake.notify("Failed to forward BsRequest: #{@req.number}: #{req} #{e}")
+      Airbrake.notify("Failed to forward BsRequest: #{@bs_request.number}: #{req} #{e}")
       flash[:error] = "Unable to forward submit request: #{e.message}"
       return
     end

src/api/app/views/webui/request/_recent_events_table.html.erb

@@ -3,18 +3,18 @@
     <!-- add a history item based on the request creation information -->
     <tr>
       <td class="nowrap" style="width: 1%;">
-        <%= user_with_realname_and_icon @bsreq.creator, short: true %>
+        <%= user_with_realname_and_icon @bs_request.creator, short: true %>
       </td>
       <td>
         <span style="color: black;">created request</span>
       </td>
       <td class="nowrap" style="width: 1%;">
-        <span class="hidden"><%= @bsreq.created_at.to_i %></span>
-        <%= fuzzy_time(@bsreq.created_at) %>
+        <span class="hidden"><%= @bs_request.created_at.to_i %></span>
+        <%= fuzzy_time(@bs_request.created_at) %>
       </td>
     </tr>
     <tr class="odd">
-      <td colspan="3" class="expandable_event_comment"><pre class="plain"><%= toggle_sliced_text(@bsreq.description, 48) %></pre></td>
+      <td colspan="3" class="expandable_event_comment"><pre class="plain"><%= toggle_sliced_text(@bs_request.description, 48) %></pre></td>
     </tr>
     <!-- and now everything from the history log -->
     <% @history.each do |element| %>

src/api/app/views/webui/request/show.html.erb

@@ -222,7 +222,7 @@
 
     <div class="grid_10 box box-shadow alpha omega">
       <h2 class="box-header">Comments for request <%= @number %> (<%= @comments.length %>)</h2>
-      <%= render :partial => 'webui/comment/show', locals: { commentable: @bsreq } %>
+      <%= render :partial => 'webui/comment/show', locals: { commentable: @bs_request } %>
     </div>
 
     <div class="grid_10 alpha">

src/api/spec/controllers/webui/request_controller_spec.rb

@@ -41,9 +41,9 @@
       expect(response).to have_http_status(:success)
     end
 
-    it 'assigns @bsreq' do
+    it 'assigns @bs_request' do
       get :show, params: { number: bs_request.number }
-      expect(assigns(:bsreq)).to eq(bs_request)
+      expect(assigns(:bs_request)).to eq(bs_request)
     end
 
     it 'redirects to root_path if request does not exist' do