From 59e824ef9e2eed9c0c38d35882349305548665e6 Mon Sep 17 00:00:00 2001
From: aignatov-bio <47317017+aignatov-bio@users.noreply.github.com>
Date: Tue, 21 May 2019 15:53:34 +0200
Subject: [PATCH] Fix nil handling in different places [SCI-2840] (#1777)

* Fix nil handling in different places
---
 app/assets/javascripts/my_modules.js          |  5 +-
 app/controllers/assets_controller.rb          |  9 +-
 app/controllers/my_module_tags_controller.rb  |  5 +
 app/controllers/step_comments_controller.rb   |  4 +-
 app/controllers/steps_controller.rb           |  2 +-
 .../users/invitations_controller.rb           | 97 ++++++++-----------
 6 files changed, 58 insertions(+), 64 deletions(-)

diff --git a/app/assets/javascripts/my_modules.js b/app/assets/javascripts/my_modules.js
index ae6197a15..de71ca6df 100644
--- a/app/assets/javascripts/my_modules.js
+++ b/app/assets/javascripts/my_modules.js
@@ -298,7 +298,10 @@ function initTagsSelector() {
     var selectElement = e.target;
     if (params.id > 0) {
       newTag = { my_module_tag: { tag_id: e.params.data.id } };
-      $.post($('#module-tags-selector')[0].dataset.updateModuleTagsUrl, newTag);
+      $.post($('#module-tags-selector')[0].dataset.updateModuleTagsUrl, newTag)
+        .fail(function() {
+          $('.module-tags .select2-selection__choice').last().remove();
+        });
     } else {
       newTag = {
         tag: {
diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb
index af9269ae8..2081d7963 100644
--- a/app/controllers/assets_controller.rb
+++ b/app/controllers/assets_controller.rb
@@ -309,12 +309,9 @@ class AssetsController < ApplicationController
   end
 
   def append_wd_params(url)
-    wd_params = ''
-    params.keys.select { |i| i[/^wd.*/] }.each do |wd|
-      next if wd == 'wdPreviousSession' || wd == 'wdPreviousCorrelation'
-      wd_params += "&#{wd}=#{params[wd]}"
-    end
-    url + wd_params
+    exclude_params = %i(wdPreviousSession wdPreviousCorrelation)
+    wd_params = params.select { |key, _value| key[/^wd.*/] && !(exclude_params.include? key) }.to_query
+    url + '&' + wd_params
   end
 
   def asset_params
diff --git a/app/controllers/my_module_tags_controller.rb b/app/controllers/my_module_tags_controller.rb
index 40b31c4ae..7dc692a82 100644
--- a/app/controllers/my_module_tags_controller.rb
+++ b/app/controllers/my_module_tags_controller.rb
@@ -55,11 +55,14 @@ class MyModuleTagsController < ApplicationController
   end
 
   def create
+    return render_403 unless mt_params[:tag_id]
+
     @mt = MyModuleTag.new(mt_params.merge(my_module: @my_module))
     @mt.created_by = current_user
     @mt.save
 
     my_module = @mt.my_module
+
     Activities::CreateActivityService
       .call(activity_type: :add_task_tag,
             owner: current_user,
@@ -83,6 +86,8 @@ class MyModuleTagsController < ApplicationController
   def destroy
     @mt = MyModuleTag.find_by_id(params[:id])
 
+    return render_404 unless @mt
+
     Activities::CreateActivityService
       .call(activity_type: :remove_task_tag,
             owner: current_user,
diff --git a/app/controllers/step_comments_controller.rb b/app/controllers/step_comments_controller.rb
index ee47a41c3..efdace612 100644
--- a/app/controllers/step_comments_controller.rb
+++ b/app/controllers/step_comments_controller.rb
@@ -131,9 +131,9 @@ class StepCommentsController < ApplicationController
     @last_comment_id = params[:from].to_i
     @per_page = Constants::COMMENTS_SEARCH_LIMIT
     @step = Step.find_by_id(params[:step_id])
-    @protocol = @step.protocol
+    @protocol = @step&.protocol
 
-    unless @step
+    unless @step && @protocol
       render_404
     end
   end
diff --git a/app/controllers/steps_controller.rb b/app/controllers/steps_controller.rb
index 6ab985958..05fe5ab08 100644
--- a/app/controllers/steps_controller.rb
+++ b/app/controllers/steps_controller.rb
@@ -514,7 +514,7 @@ class StepsController < ApplicationController
 
   def load_vars
     @step = Step.find_by_id(params[:id])
-    @protocol = @step.protocol
+    @protocol = @step&.protocol
     if params[:checklistitem_id]
       @chk_item = ChecklistItem.find_by_id(params[:checklistitem_id])
     end
diff --git a/app/controllers/users/invitations_controller.rb b/app/controllers/users/invitations_controller.rb
index 6944bb223..4d8f70205 100644
--- a/app/controllers/users/invitations_controller.rb
+++ b/app/controllers/users/invitations_controller.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Users
   class InvitationsController < Devise::InvitationsController
     include InputSanitizeHelper
@@ -13,6 +15,7 @@ module Users
 
     def update
       return super unless Rails.configuration.x.new_team_on_signup
+
       # Instantialize a new team with the provided name
       @team = Team.new
       @team.name = params[:team][:name]
@@ -59,58 +62,46 @@ module Users
       @invite_results = []
       @too_many_emails = false
 
-      cntr = 0
-      @emails.each do |email|
-        cntr += 1
-
-        if cntr > Constants::INVITE_USERS_LIMIT
+      @emails.each_with_index do |email, email_counter|
+        # email_counter starts with 0
+        if email_counter >= Constants::INVITE_USERS_LIMIT
           @too_many_emails = true
           break
         end
 
-        password = generate_user_password
-
-        # Check if user already exists
-        user = nil
-        user = User.find_by_email(email) if User.exists?(email: email)
-
         result = { email: email }
+        unless Constants::BASIC_EMAIL_REGEX.match?(email)
+          result[:status] = :user_invalid
+          @invite_results << result
+          next
+        end
+        # Check if user already exists
+        user = User.find_by_email(email)
 
-        if user.present?
+        if user
           result[:status] = :user_exists
           result[:user] = user
         else
-          # Validate the user data
-          error = !(Constants::BASIC_EMAIL_REGEX === email)
-          error = validate_user(email, email, password).count > 0 unless error
+          user = User.invite!(
+            full_name: email,
+            email: email,
+            initials: email.upcase[0..1],
+            skip_invitation: true
+          )
+          user.update(invited_by: @user)
 
-          if !error
-            user = User.invite!(
-              full_name: email,
-              email: email,
-              initials: email.upcase[0..1],
-              skip_invitation: true
-            )
-            user.update(invited_by: @user)
+          result[:status] = :user_created
+          result[:user] = user
 
-            result[:status] = :user_created
-            result[:user] = user
-
-            # Sending email invitation is done in background job to prevent
-            # issues with email delivery. Also invite method must be call
-            # with :skip_invitation attribute set to true - see above.
-            user.delay.deliver_invitation
-          else
-            # Return invalid status
-            result[:status] = :user_invalid
-          end
+          # Sending email invitation is done in background job to prevent
+          # issues with email delivery. Also invite method must be call
+          # with :skip_invitation attribute set to true - see above.
+          user.delay.deliver_invitation
         end
 
-        if @team.present? && result[:status] != :user_invalid
-          if UserTeam.exists?(user: user, team: @team)
-            user_team =
-              UserTeam.where(user: user, team: @team).first
-
+        if @team && user
+          user_team = UserTeam.find_by_user_id_and_team_id(user.id, @team.id)
+          if user_team
             result[:status] = :user_exists_and_in_team
           else
             # Also generate user team relation
@@ -127,7 +118,6 @@ module Users
               user_team.role_str,
               user_team.team
             )
-
             Activities::CreateActivityService
               .call(activity_type: :invite_user_to_team,
                     owner: current_user,
@@ -139,13 +129,13 @@ module Users
                       role: user_team.role_str
                     })
 
-            if result[:status] == :user_exists && !user.confirmed?
-              result[:status] = :user_exists_unconfirmed_invited_to_team
-            elsif result[:status] == :user_exists
-              result[:status] = :user_exists_invited_to_team
-            else
-              result[:status] = :user_created_invited_to_team
-            end
+            result[:status] = if result[:status] == :user_exists && !user.confirmed?
+                                :user_exists_unconfirmed_invited_to_team
+                              elsif result[:status] == :user_exists
+                                :user_exists_invited_to_team
+                              else
+                                :user_created_invited_to_team
+                              end
           end
 
           result[:user_team] = user_team
@@ -196,9 +186,7 @@ module Users
         message: sanitize_input(message)
       )
 
-      if target_user.assignments_notification
-        UserNotification.create(notification: notification, user: target_user)
-      end
+      UserNotification.create(notification: notification, user: target_user) if target_user.assignments_notification
     end
 
     def check_captcha_for_invite
@@ -212,13 +200,14 @@ module Users
 
     def check_invite_users_permission
       @user = current_user
-      @emails = params[:emails].map(&:downcase)
+      @emails = params[:emails]&.map(&:downcase)
       @team = Team.find_by_id(params['teamId'])
       @role = params['role']
 
-      render_403 if @emails && @emails.empty?
-      render_403 if @team && !can_manage_team_users?(@team)
-      render_403 if @role && !UserTeam.roles.keys.include?(@role)
+      return render_403 unless @emails && @team && @role
+      return render_403 if @emails.empty?
+      return render_403 unless can_manage_team_users?(@team)
+      return render_403 unless UserTeam.roles.key?(@role)
     end
   end
 end