E2256: Refactor late_policies_controller.rb

app/controllers/assignments_controller.rb

@@ -48,9 +48,10 @@ def create
         assignment_form_params[:assignment_questionnaire] = ques_array
         assignment_form_params[:due_date] = due_array
         @assignment_form.update(assignment_form_params, current_user)
-        aid = Assignment.find(@assignment_form.assignment.id).id
+        assignment_id = Assignment.find(@assignment_form.assignment.id).id
         ExpertizaLogger.info "Assignment created: #{@assignment_form.as_json}"
-        redirect_to edit_assignment_path aid
+        session[:assignment_id] = assignment_id
+        redirect_to edit_assignment_path assignment_id
         undo_link("Assignment \"#{@assignment_form.assignment.name}\" has been created successfully. ")
         return
       else
@@ -71,6 +72,7 @@ def create
 
   # edits an assignment's deadlines and assigned rubrics
   def edit
+    session[:assignment_id] = nil
     user_timezone_specified
     edit_params_setting
     assignment_staggered_deadline?
@@ -85,6 +87,8 @@ def edit
     @badges = Badge.all
     @use_bookmark = @assignment.use_bookmark
     @duties = Duty.where(assignment_id: @assignment_form.assignment.id)
+    @assignment = Assignment.find(params[:id])
+    session[:assignment_id] = @assignment.id
   end
 
   # updates an assignment via an assignment form
@@ -109,7 +113,9 @@ def update
 
   # displays an assignment via ID
   def show
+    session[:assignment_id] = nil
     @assignment = Assignment.find(params[:id])
+    session[:assignment_id] = @assignment.id
   end
 
   # gets an assignment's path/url
@@ -297,13 +303,13 @@ def update_nil_dd_description_url(due_date_all)
 
   # handle assignment form saved condition
   def assignment_form_save_handler
-    exist_assignment = Assignment.find_by(name: @assignment_form.assignment.name)
+    exist_assignment = Assignment.find(@assignment_form.assignment.id)
     assignment_form_params[:assignment][:id] = exist_assignment.id.to_s
     fix_assignment_missing_path
     update_assignment_form(exist_assignment)
-    aid = Assignment.find_by(name: @assignment_form.assignment.name).id
+    assignment_id = Assignment.find(@assignment_form.assignment.id).id
     ExpertizaLogger.info "Assignment created: #{@assignment_form.as_json}"
-    redirect_to edit_assignment_path aid
+    redirect_to edit_assignment_path assignment_id
     undo_link("Assignment \"#{@assignment_form.assignment.name}\" has been created successfully. ")
   end
 

app/controllers/late_policies_controller.rb

@@ -19,41 +19,47 @@ def action_allowed?
 
   # This method lists all the late policies records from late_policies table in database.
   def index
-    @penalty_policies = LatePolicy.where(['instructor_id = ? OR private = 0', instructor_id])
+    @late_policies = LatePolicy.where(['instructor_id = ? OR private = 0', instructor_id])
     respond_to do |format|
       format.html # index.html.erb
-      format.xml  { render xml: @penalty_policies }
+      format.xml  { render xml: @late_policies }
     end
   end
 
   # This method displays a certain record in late_policies table in the database.
   def show
-    @penalty_policy = LatePolicy.find(params[:id])
+    @late_policy = LatePolicy.find(params[:id])
     respond_to do |format|
       format.html # show.html.erb
-      format.xml  { render xml: @penalty_policy }
+      format.xml  { render xml: @late_policy }
     end
   end
 
   # New method creates instance of a late policy in the late_policies's table but does not saves in the database.
   def new
-    @penalty_policy = LatePolicy.new
+    @late_policy = LatePolicy.new
     respond_to do |format|
       format.html # new.html.erb
-      format.xml  { render xml: @penalty_policy }
+      format.xml  { render xml: @late_policy }
     end
   end
 
   # This method just fetch a particular record in LatePolicy table.
   def edit
-    @penalty_policy = LatePolicy.find(params[:id])
+    @late_policy = LatePolicy.find(params[:id])
   end
 
   # Create method can create a new late policy.
   # There are few check points before creating a late policy which are written in the if/else statements.
   def create
+    # this function validates the policy name
+    valid_name_penalty, error_message = check_if_policy_name_exists('')
+
     # First this function validates the input then save if the input is valid.
-    valid_penalty, error_message = validate_input
+    valid_attr_penalty, error_message = validate_input('', error_message)
+
+    valid_penalty = valid_name_penalty && valid_attr_penalty
+
     if error_message
       flash[:error] = error_message
     end
@@ -79,19 +85,27 @@ def create
 
   # Update method can update late policy. There are few check points before updating a late policy which are written in the if/else statements.
   def update
-    penalty_policy = LatePolicy.find(params[:id])
+    late_policy = LatePolicy.find(params[:id])
+
+    # this function checks if the policy name was update and then validates the policy name
+    prefix = 'Cannot edit the policy. '
+    if late_policy.policy_name != late_policy_params[:policy_name]
+      valid_name_penalty, error_message = check_if_policy_name_exists(prefix)
+    end
 
     # First this function validates the input then save if the input is valid.
-    _valid_penalty, error_message = validate_input(true)
+    valid_attr_penalty, error_message = validate_input(prefix, error_message)
+    _valid_penalty = valid_name_penalty && valid_attr_penalty
+
     if error_message
       flash[:error] = error_message
       redirect_to action: 'edit', id: params[:id]
     # If there are no errors, then save the record.
     else
       begin
-        penalty_policy.update_attributes(late_policy_params)
-        penalty_policy.save!
-        LatePolicy.update_calculated_penalty_objects(penalty_policy)
+        late_policy.update_attributes(late_policy_params)
+        late_policy.save!
+        LatePolicy.update_calculated_penalty_objects(late_policy)
         flash[:notice] = 'The late policy was successfully updated.'
         redirect_to action: 'index'
       # If something unexpected happens while updating, then redirect to the edit page of that policy again.
@@ -104,9 +118,9 @@ def update
 
   # This method fetches a particular record in the late_policy table and try to destroy's it.
   def destroy
-    @penalty_policy = LatePolicy.find(params[:id])
+    @late_policy = LatePolicy.find(params[:id])
     begin
-      @penalty_policy.destroy
+      @late_policy.destroy
     rescue StandardError
       flash[:error] = 'This policy is in use and hence cannot be deleted.'
     end
@@ -128,56 +142,44 @@ def instructor_id
 
   def late_policy
     # This function checks if the id exists in parameters and assigns it to the instance variable of penalty policy.
-    @penalty_policy ||= @late_policy || LatePolicy.find(params[:id]) if params[:id]
+    @late_policy ||= LatePolicy.find(params[:id]) if params[:id]
   end
 
-  # This function checks if the policy name already exists or not and returns boolean value for penalty and the error message.
-  def duplicate_name_check(is_update = false)
-    should_check = true
-    prefix = is_update ? "Cannot edit the policy. " : ""
-    valid_penalty, error_message = true, nil
-
-    if is_update
-      existing_late_policy = LatePolicy.find(params[:id])
-      if existing_late_policy.policy_name == params[:late_policy][:policy_name]
-        should_check = false
-      end
+  # This function checks if the policy name already exists or not and returns the error message.
+  def check_if_policy_name_exists(prefix)
+    error_message = nil
+    if LatePolicy.check_policy_with_same_name(late_policy_params[:policy_name], instructor_id)
+      error_message = prefix + 'A policy with the same name ' + late_policy_params[:policy_name] + ' already exists.'
+      return false, error_message
     end
-
-    if should_check
-      if LatePolicy.check_policy_with_same_name(params[:late_policy][:policy_name], instructor_id)
-        error_message = prefix + 'A policy with the same name ' + params[:late_policy][:policy_name] + ' already exists.'
-        valid_penalty = false
-      end
-    end
-    return valid_penalty, error_message
+    return true, error_message
   end
 
-  # This function validates the input.
-  def validate_input(is_update = false)
+  # This function validates the input and returns boolean for valid penalty abd error meassage.
+  def validate_input(prefix, error_message)
     # Validates input for create and update forms
-    max_penalty = params[:late_policy][:max_penalty].to_i
-    penalty_per_unit = params[:late_policy][:penalty_per_unit].to_i
-
-    valid_penalty, error_message = duplicate_name_check(is_update)
-    prefix = is_update ? "Cannot edit the policy. " : ""
+    max_penalty = late_policy_params[:max_penalty].to_i
+    penalty_per_unit = late_policy_params[:penalty_per_unit].to_i
+    valid_penalty = false
 
     # This check validates the maximum penalty.
     if max_penalty < penalty_per_unit
-      error_message = prefix + 'The maximum penalty cannot be less than penalty per unit.'
-      valid_penalty = false
+      error_message = "#{error_message.nil? ? "" : error_message + " "}" + prefix + 'The maximum penalty cannot be less than penalty per unit.'
     end
 
     # This check validates the penalty per unit for a late policy.
     if penalty_per_unit < 0
-      error_message = 'Penalty per unit cannot be negative.'
-      valid_penalty = false
+      error_message = "#{error_message.nil? ? "" : error_message + " "}" + "Penalty per unit cannot be negative."
     end
 
     # This checks maximum penalty does not exceed 100.
     if max_penalty >= 100
-      error_message = prefix + 'Maximum penalty cannot be greater than or equal to 100'
-      valid_penalty = false
+      error_message = "#{error_message.nil? ? "" : error_message + " "}" + prefix + 'Maximum penalty cannot be greater than or equal to 100.'
+    end
+
+    # This checks if any error message was generated.
+    if error_message.nil?
+      valid_penalty = true
     end
 
     return valid_penalty, error_message

app/views/assignments/edit.html.erb

@@ -122,10 +122,18 @@
           $('#ReviewStrategy').click();
         }
       });
+
 
       $('#go_to_tabs2').click(function(){
         $('#Rubrics').click();
       });
+      // Redirect to Due Date Tab  on Back link from late policy
+      jQuery(document).ready(function() {
+        var referrer =  document.referrer;
+        if(referrer.includes("late_policies")){                     
+          $('#DueDates').click();
+        }  
+}); 
     </script>
   <% end %>
 

app/views/late_policies/_form.html.erb

@@ -3,30 +3,30 @@
 <!--[form:late_policies]-->
 <p>
 <label for="late_policy_name">Late policy Name:</label><label style="color: #ff0000;">*</label><br/>
-  <%= f.text_field 'policy_name', :value => @penalty_policy.policy_name %>
+  <%= f.text_field 'policy_name', :value => @late_policy.policy_name %>
 </p>
 
 <p>
 <label for="late_policy_penalty_points_per_unit">Penalty Points Per Unit:</label><label style="color: #ff0000;">*</label><br/>
-  <%= f.text_field 'penalty_per_unit', :value => @penalty_policy.penalty_per_unit %>
+  <%= f.text_field 'penalty_per_unit', :value => @late_policy.penalty_per_unit %>
 </p>
 
 
 <p>
 <label for="private">Visibility:</label><label style="color: #ff0000;">*</label><br/>
 <% @units= [ ["Private", true], ["Public", false]]%>
-<%= f.select("private",@units,:selected=>@penalty_policy[:private]) %>
+<%= f.select("private",@units,:selected=>@late_policy[:private]) %>
 </p>
 
 <p>
 <label for="penalty_unit">Penalty Unit:</label><label style="color: #ff0000;">*</label><br/>
 <% @units= %w[Minute Hour Day]%>
-<%= f.select("penalty_unit",@units,:selected=>@penalty_policy[:penalty_unit]) %>
+<%= f.select("penalty_unit",@units,:selected=>@late_policy[:penalty_unit]) %>
 </p>
 
 <p>
   <label for="late_policy_penalty_maximum">Maximum Penalty:</label><label style="color: #ff0000;">*</label><br/>
-  <%= f.text_field 'max_penalty', :value =>  @penalty_policy.max_penalty %>
+  <%= f.text_field 'max_penalty', :value =>  @late_policy.max_penalty %>
 </p>
 <!--[eoform:late_policies]-->
 

app/views/late_policies/edit.html.erb

@@ -1,6 +1,6 @@
 <h1>Editing late policy</h1>
 
-<%= form_for @penalty_policy do |f| %>
+<%= form_for @late_policy do |f| %>
   <%= render partial: 'form', locals: { f: f } %>
   <%= f.submit 'Save' %>
 <% end %>

app/views/late_policies/index.html.erb

@@ -19,7 +19,7 @@ end
     <% end %>
   </tr>
 
-  <% @penalty_policies.each do |penalty_policy| %>
+  <% @late_policies.each do |penalty_policy| %>
     <% if instructor_id == penalty_policy.instructor_id || is_admin %>
       <tr>
         <td><%= penalty_policy.policy_name %></td>
@@ -39,4 +39,4 @@ end
 <br />
 
 <%= link_to 'New late policy', :action => 'new'%><br />
-<%= link_to 'Back', :controller=> 'assignments', :action => 'edit', :id=>session[:assignment]%>
+<%= link_to 'Back', :controller=> 'assignments', :action => 'edit', :id=>session[:assignment_id]%>

app/views/late_policies/new.html.erb

@@ -1,7 +1,7 @@
 <h1>New late policy</h1>
 
-<%= form_for @penalty_policy, :url => { :action => "create" } do |f| %>
-  <%= error_messages_for @penalty_policy %>
+<%= form_for @late_policy, :url => { :action => "create" } do |f| %>
+  <%= error_messages_for @late_policy %>
 
   <%= render partial: 'form', locals: { f: f } %>
 
@@ -10,4 +10,4 @@
 
 <br>
 <br>
-<%= link_to 'Back', :controller=> 'assignments', :action => 'edit', :id=>session[:assignment]%>
+<%= link_to 'Back', :controller=> 'assignments', :action => 'edit', :id=>session[:assignment_id]%>

app/views/late_policies/show.html.erb

@@ -1,3 +1,3 @@
 
-<%= link_to 'Edit', :action => 'edit', :id => @penalty_policy %> |
+<%= link_to 'Edit', :action => 'edit', :id => @late_policy %> |
 <%= link_to 'Back', :action => 'index' %>

spec/controllers/late_policies_controller_spec.rb

@@ -53,7 +53,7 @@
           id: 1
         }
         get :edit, params: request_params
-        expect(assigns(:penalty_policy).policy_name).to eq('Policy2')
+        expect(assigns(:late_policy).policy_name).to eq('Policy2')
       end
     end
   end
@@ -115,7 +115,7 @@
           }
         }
         post :create, params: request_params
-        expect(flash[:error]).to eq('Maximum penalty cannot be greater than or equal to 100')
+        expect(flash[:error]).to eq('Maximum penalty cannot be greater than or equal to 100.')
         expect(response).to redirect_to('/late_policies/new')
       end
     end