From e73996c7611064d072fdc89a49487fd5dc3aec76 Mon Sep 17 00:00:00 2001
From: Nitish Aggarwal <royalnitish21@gmail.com>
Date: Thu, 28 May 2020 10:30:41 +0530
Subject: [PATCH 01/10] feat: Add collaboration API endpoints

---
 .../api/v1/collaborations_controller.rb       | 77 +++++++++++++++++++
 .../api/v1/collaboration_serializer.rb        | 20 +++++
 config/routes.rb                              |  1 +
 3 files changed, 98 insertions(+)
 create mode 100644 app/controllers/api/v1/collaborations_controller.rb
 create mode 100644 app/serializers/api/v1/collaboration_serializer.rb

diff --git a/app/controllers/api/v1/collaborations_controller.rb b/app/controllers/api/v1/collaborations_controller.rb
new file mode 100644
index 0000000000..3fda5a38ec
--- /dev/null
+++ b/app/controllers/api/v1/collaborations_controller.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+class Api::V1::CollaborationsController < Api::V1::BaseController
+  before_action :authenticate_user!
+  before_action :set_project, only: %i[index create]
+  before_action :check_author_access, only: %i[create]
+  before_action :set_collaboration, only: %i[destroy]
+
+  # /api/v1/projects/:project_id/collaborations
+  def index
+    @collaborations = paginate(@project.collaborations)
+    @options = { links: link_attrs(@collaborations, api_v1_project_collaborations_url) }
+    render json: Api::V1::CollaborationSerializer.new(@collaborations, @options)
+  end
+
+  # POST /api/v1/projects/:project_id/collaborations
+  def create
+    parse_mails_except_current_user(params[:emails])
+    newly_added = @valid_mails - @existing_mails
+
+    @added_mails = []
+
+    newly_added.each do |email|
+      user = User.find_by(email: email)
+      if user.present?
+        @added_mails.push(email)
+        Collaboration.where(project_id: @project.id, user_id: user.id).first_or_create
+      end
+    end
+
+    render json: { added: @added_mails, invalid: @invalid_mails }
+  end
+
+  # DELETE /api/v1/collaborations/:id
+  def destroy
+    authorize @collaboration.project, :author_access?
+    @collaboration.destroy!
+    render json: {}, status: :no_content
+  end
+
+  private
+
+    def set_project
+      @project = Project.find(params[:project_id])
+    end
+
+    def check_author_access
+      authorize @project, :author_access?
+    end
+
+    def set_collaboration
+      @collaboration = Collaboration.find(params[:id])
+    end
+
+    def collaboration_params
+      params.require(:collaboration).permit(:user_id, :project_id, :emails)
+    end
+
+    def parse_mails_except_current_user(mails)
+      @valid_mails = []
+      @invalid_mails = []
+
+      mails.split(",").each do |email|
+        email = email.strip
+        if email.present? && email != @current_user.email \
+          && Devise.email_regexp.match?(email)
+          @valid_mails.push(email)
+        else
+          @invalid_mails.push(email)
+        end
+      end
+
+      @existing_mails = User.where(
+        id: @project.collaborations.pluck(:user_id)
+      ).pluck(:email)
+    end
+end
diff --git a/app/serializers/api/v1/collaboration_serializer.rb b/app/serializers/api/v1/collaboration_serializer.rb
new file mode 100644
index 0000000000..12e8a66986
--- /dev/null
+++ b/app/serializers/api/v1/collaboration_serializer.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class Api::V1::CollaborationSerializer
+  include FastJsonapi::ObjectSerializer
+
+  attribute :project do |object|
+    {
+      "id": object.project.id,
+      "name": object.project.name
+    }
+  end
+
+  attribute :user do |object|
+    {
+      "id": object.user.id,
+      "name": object.user.name,
+      "email": object.user.email
+    }
+  end
+end
diff --git a/config/routes.rb b/config/routes.rb
index 8688153078..2e2b448fc8 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -129,6 +129,7 @@
           get 'fork', to: 'projects#create_fork'
           get 'image_preview', to: 'projects#image_preview'
         end
+        resources :collaborations, shallow: true
       end
       resources :users do
         get 'projects', to: 'projects#user_projects', on: :member

From c0a8b4b03cbdbab6eec0bad1a41e74a87bf343a1 Mon Sep 17 00:00:00 2001
From: Nitish Aggarwal <royalnitish21@gmail.com>
Date: Sun, 31 May 2020 21:27:13 +0530
Subject: [PATCH 02/10] ptch: Update users_serializer selective serializing
 params naming

---
 .../api/v1/collaborations_controller.rb       | 77 -------------------
 app/controllers/api/v1/users_controller.rb    |  2 +-
 .../api/v1/collaboration_serializer.rb        | 20 -----
 app/serializers/api/v1/user_serializer.rb     |  4 +-
 4 files changed, 3 insertions(+), 100 deletions(-)
 delete mode 100644 app/controllers/api/v1/collaborations_controller.rb
 delete mode 100644 app/serializers/api/v1/collaboration_serializer.rb

diff --git a/app/controllers/api/v1/collaborations_controller.rb b/app/controllers/api/v1/collaborations_controller.rb
deleted file mode 100644
index 3fda5a38ec..0000000000
--- a/app/controllers/api/v1/collaborations_controller.rb
+++ /dev/null
@@ -1,77 +0,0 @@
-# frozen_string_literal: true
-
-class Api::V1::CollaborationsController < Api::V1::BaseController
-  before_action :authenticate_user!
-  before_action :set_project, only: %i[index create]
-  before_action :check_author_access, only: %i[create]
-  before_action :set_collaboration, only: %i[destroy]
-
-  # /api/v1/projects/:project_id/collaborations
-  def index
-    @collaborations = paginate(@project.collaborations)
-    @options = { links: link_attrs(@collaborations, api_v1_project_collaborations_url) }
-    render json: Api::V1::CollaborationSerializer.new(@collaborations, @options)
-  end
-
-  # POST /api/v1/projects/:project_id/collaborations
-  def create
-    parse_mails_except_current_user(params[:emails])
-    newly_added = @valid_mails - @existing_mails
-
-    @added_mails = []
-
-    newly_added.each do |email|
-      user = User.find_by(email: email)
-      if user.present?
-        @added_mails.push(email)
-        Collaboration.where(project_id: @project.id, user_id: user.id).first_or_create
-      end
-    end
-
-    render json: { added: @added_mails, invalid: @invalid_mails }
-  end
-
-  # DELETE /api/v1/collaborations/:id
-  def destroy
-    authorize @collaboration.project, :author_access?
-    @collaboration.destroy!
-    render json: {}, status: :no_content
-  end
-
-  private
-
-    def set_project
-      @project = Project.find(params[:project_id])
-    end
-
-    def check_author_access
-      authorize @project, :author_access?
-    end
-
-    def set_collaboration
-      @collaboration = Collaboration.find(params[:id])
-    end
-
-    def collaboration_params
-      params.require(:collaboration).permit(:user_id, :project_id, :emails)
-    end
-
-    def parse_mails_except_current_user(mails)
-      @valid_mails = []
-      @invalid_mails = []
-
-      mails.split(",").each do |email|
-        email = email.strip
-        if email.present? && email != @current_user.email \
-          && Devise.email_regexp.match?(email)
-          @valid_mails.push(email)
-        else
-          @invalid_mails.push(email)
-        end
-      end
-
-      @existing_mails = User.where(
-        id: @project.collaborations.pluck(:user_id)
-      ).pluck(:email)
-    end
-end
diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb
index 52f59aa243..25a598d150 100644
--- a/app/controllers/api/v1/users_controller.rb
+++ b/app/controllers/api/v1/users_controller.rb
@@ -9,7 +9,7 @@ class Api::V1::UsersController < Api::V1::BaseController
   # GET api/v1/users
   def index
     @users = paginate(User.all)
-    @options = { params: { are_all_users_fetched: true } }
+    @options = { params: { only_name: true } }
     @options[:links] = link_attrs(@users, api_v1_users_url)
     render json: Api::V1::UserSerializer.new(@users, @options)
   end
diff --git a/app/serializers/api/v1/collaboration_serializer.rb b/app/serializers/api/v1/collaboration_serializer.rb
deleted file mode 100644
index 12e8a66986..0000000000
--- a/app/serializers/api/v1/collaboration_serializer.rb
+++ /dev/null
@@ -1,20 +0,0 @@
-# frozen_string_literal: true
-
-class Api::V1::CollaborationSerializer
-  include FastJsonapi::ObjectSerializer
-
-  attribute :project do |object|
-    {
-      "id": object.project.id,
-      "name": object.project.name
-    }
-  end
-
-  attribute :user do |object|
-    {
-      "id": object.user.id,
-      "name": object.user.name,
-      "email": object.user.email
-    }
-  end
-end
diff --git a/app/serializers/api/v1/user_serializer.rb b/app/serializers/api/v1/user_serializer.rb
index b5a918b5c4..e0efb6105f 100644
--- a/app/serializers/api/v1/user_serializer.rb
+++ b/app/serializers/api/v1/user_serializer.rb
@@ -3,7 +3,7 @@
 class Api::V1::UserSerializer
   include FastJsonapi::ObjectSerializer
 
-  # only name is serialized if all users are fetched
+  # only name is serialized if all users/collaborators are fetched
   attribute :name
 
   # only serialized if user fetches own details
@@ -14,6 +14,6 @@ class Api::V1::UserSerializer
 
   attributes :admin, :country, :educational_institute,
              if: proc { |record, params|
-               params[:are_all_users_fetched] != true || record.admin
+               params[:only_name] != true || record.admin
              }
 end

From 440b08eb92d7fffeded1910e3278ffbac46cbbe8 Mon Sep 17 00:00:00 2001
From: Nitish Aggarwal <royalnitish21@gmail.com>
Date: Sun, 31 May 2020 21:27:48 +0530
Subject: [PATCH 03/10] feat: Add collaborators endpoints

---
 .../api/v1/collaborators_controller.rb        | 81 +++++++++++++++++++
 config/routes.rb                              |  2 +-
 2 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 app/controllers/api/v1/collaborators_controller.rb

diff --git a/app/controllers/api/v1/collaborators_controller.rb b/app/controllers/api/v1/collaborators_controller.rb
new file mode 100644
index 0000000000..dc36a1c24f
--- /dev/null
+++ b/app/controllers/api/v1/collaborators_controller.rb
@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+class Api::V1::CollaboratorsController < Api::V1::BaseController
+  before_action :authenticate_user!
+  before_action :set_project
+  before_action :check_author_access, only: %i[create destroy]
+  before_action :set_collaborator, only: %i[destroy]
+
+  # /api/v1/projects/:project_id/collaborators
+  def index
+    @collaborators = paginate(@project.collaborators)
+    # options for serializing collaborators
+    @options = {
+      params: { only_name: true },
+      links: link_attrs(@collaborators, api_v1_project_collaborators_url)
+    }
+    render json: Api::V1::UserSerializer.new(@collaborators, @options)
+  end
+
+  # POST /api/v1/projects/:project_id/collaborators
+  def create
+    parse_mails_except_current_user(params[:emails])
+    newly_added = @valid_mails - @existing_mails
+
+    @added_mails = []
+
+    newly_added.each do |email|
+      user = User.find_by(email: email)
+      if user.present?
+        @added_mails.push(email)
+        Collaboration.where(project_id: @project.id, user_id: user.id).first_or_create
+      end
+    end
+
+    render json: { added: @added_mails, existing: @existing_mails, invalid: @invalid_mails }
+  end
+
+  # DELETE /api/v1//projects/:project_id/collaborators/:id
+  # :id is essentially the user_id for the user to be removed from project
+  def destroy
+    @collaborator.destroy!
+    render json: {}, status: :no_content
+  end
+
+  private
+
+    def set_project
+      @project = Project.find(params[:project_id])
+    end
+
+    def check_author_access
+      authorize @project, :author_access?
+    end
+
+    def set_collaborator
+      @collaborator = @project.collaborators.find(params[:id])
+    end
+
+    def collaborator_params
+      params.require(:collaborator).permit(:project_id, :emails)
+    end
+
+    def parse_mails_except_current_user(mails)
+      @valid_mails = []
+      @invalid_mails = []
+
+      mails.split(",").each do |email|
+        email = email.strip
+        if email.present? && email != @current_user.email \
+          && Devise.email_regexp.match?(email)
+          @valid_mails.push(email)
+        else
+          @invalid_mails.push(email)
+        end
+      end
+
+      @existing_mails = User.where(
+        id: @project.collaborations.pluck(:user_id)
+      ).pluck(:email)
+    end
+end
diff --git a/config/routes.rb b/config/routes.rb
index 2e2b448fc8..529a6f71e4 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -129,7 +129,7 @@
           get 'fork', to: 'projects#create_fork'
           get 'image_preview', to: 'projects#image_preview'
         end
-        resources :collaborations, shallow: true
+        resources :collaborators, only: [:index, :create, :destroy]
       end
       resources :users do
         get 'projects', to: 'projects#user_projects', on: :member

From ed167f1a7273c54755f936c834696df774b3b077 Mon Sep 17 00:00:00 2001
From: Nitish Aggarwal <royalnitish21@gmail.com>
Date: Sun, 31 May 2020 22:37:29 +0530
Subject: [PATCH 04/10] test: Add collaborators specs

---
 .../collaborators_controller/create_spec.rb   | 78 +++++++++++++++++++
 .../collaborators_controller/destroy_spec.rb  | 64 +++++++++++++++
 .../v1/collaborators_controller/index_spec.rb | 52 +++++++++++++
 3 files changed, 194 insertions(+)
 create mode 100644 spec/requests/api/v1/collaborators_controller/create_spec.rb
 create mode 100644 spec/requests/api/v1/collaborators_controller/destroy_spec.rb
 create mode 100644 spec/requests/api/v1/collaborators_controller/index_spec.rb

diff --git a/spec/requests/api/v1/collaborators_controller/create_spec.rb b/spec/requests/api/v1/collaborators_controller/create_spec.rb
new file mode 100644
index 0000000000..5d3c890d01
--- /dev/null
+++ b/spec/requests/api/v1/collaborators_controller/create_spec.rb
@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe Api::V1::CollaboratorsController, "#create", type: :request do
+  describe "create/add collaborators" do
+    let!(:author) { FactoryBot.create(:user) }
+    let!(:project) { FactoryBot.create(:project, author: author) }
+    let!(:user) { FactoryBot.create(:user) }
+
+    context "when not authenticated" do
+      before do
+        post "/api/v1/projects/#{project.id}/collaborators/", as: :json
+      end
+
+      it "returns status unautheticated" do
+        expect(response).to have_http_status(401)
+        expect(response.parsed_body).to have_jsonapi_errors
+      end
+    end
+
+    context "when authenticated as random user and don't have author_access?" do
+      before do
+        token = get_auth_token(FactoryBot.create(:user))
+        post "/api/v1/projects/#{project.id}/collaborators/",
+             headers: { "Authorization": "Token #{token}" },
+             params: create_params, as: :json
+      end
+
+      it "returns status unauthorized" do
+        expect(response).to have_http_status(403)
+        expect(response.parsed_body).to have_jsonapi_errors
+      end
+    end
+
+    context "when authorized but tries to add collaborator to non existent project" do
+      before do
+        token = get_auth_token(author)
+        post "//api/v1/projects/0/collaborators/",
+             headers: { "Authorization": "Token #{token}" },
+             params: create_params, as: :json
+      end
+
+      it "returns status not_found" do
+        expect(response).to have_http_status(404)
+        expect(response.parsed_body).to have_jsonapi_errors
+      end
+    end
+
+    context "when authorized and has access to add collaborator" do
+      before do
+        # creates a collaboration
+        existing = FactoryBot.create(:user, email: "existing@test.com")
+        FactoryBot.create(:collaboration, user: existing, project: project)
+        token = get_auth_token(author)
+        post "//api/v1/projects/#{project.id}/collaborators/",
+             headers: { "Authorization": "Token #{token}" },
+             params: create_params, as: :json
+      end
+
+      it "returns status code 200" do
+        expect(response).to have_http_status(200)
+      end
+
+      it "returns the added, already_existing & invalid mails (author being invalid)" do
+        expect(response.parsed_body["added"]).to eq([user.email])
+        expect(response.parsed_body["existing"]).to eq(["existing@test.com"])
+        expect(response.parsed_body["invalid"]).to eq(["invalid", author.email])
+      end
+    end
+
+    def create_params
+      {
+        "emails": "#{user.email}, existing@test.com, invalid, #{author.email}"
+      }
+    end
+  end
+end
diff --git a/spec/requests/api/v1/collaborators_controller/destroy_spec.rb b/spec/requests/api/v1/collaborators_controller/destroy_spec.rb
new file mode 100644
index 0000000000..52bc104d32
--- /dev/null
+++ b/spec/requests/api/v1/collaborators_controller/destroy_spec.rb
@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe Api::V1::CollaboratorsController, "#destroy", type: :request do
+  describe "delete specific collaborator" do
+    let!(:author) { FactoryBot.create(:user) }
+    let!(:project) { FactoryBot.create(:project, author: author) }
+    let!(:collaborator) { FactoryBot.create(:user) }
+    let!(:collaboration) { FactoryBot.create(:collaboration, user: collaborator, project: project) }
+
+    context "when not authenticated" do
+      before do
+        delete "/api/v1/projects/#{project.id}/collaborators/#{collaboration.user.id}", as: :json
+      end
+
+      it "returns status unauthenticated" do
+        expect(response).to have_http_status(401)
+        expect(response.parsed_body).to have_jsonapi_errors
+      end
+    end
+
+    context "when authenticated as random user and don't have author_access?" do
+      before do
+        token = get_auth_token(FactoryBot.create(:user))
+        delete "/api/v1/projects/#{project.id}/collaborators/#{collaboration.user.id}",
+               headers: { "Authorization": "Token #{token}" }, as: :json
+      end
+
+      it "returns status unauthorized" do
+        expect(response).to have_http_status(403)
+        expect(response.parsed_body).to have_jsonapi_errors
+      end
+    end
+
+    context "when authenticated but tries to delete non existent collaborator" do
+      before do
+        token = get_auth_token(author)
+        delete "/api/v1/projects/#{project.id}/collaborators/0",
+               headers: { "Authorization": "Token #{token}" }, as: :json
+      end
+
+      it "returns status not_found" do
+        expect(response).to have_http_status(404)
+        expect(response.parsed_body).to have_jsonapi_errors
+      end
+    end
+
+    context "when authenticated and has access to delete collaborator" do
+      before do
+        token = get_auth_token(author)
+        delete "/api/v1/projects/#{project.id}/collaborators/#{collaboration.user.id}",
+               headers: { "Authorization": "Token #{token}" }, as: :json
+      end
+
+      it "deletes collaborator & return status no_content" do
+        expect { Collaboration.find_by!(user: collaborator, project: project) }.to raise_exception(
+          ActiveRecord::RecordNotFound
+        )
+        expect(response).to have_http_status(204)
+      end
+    end
+  end
+end
diff --git a/spec/requests/api/v1/collaborators_controller/index_spec.rb b/spec/requests/api/v1/collaborators_controller/index_spec.rb
new file mode 100644
index 0000000000..e6cb7fe69e
--- /dev/null
+++ b/spec/requests/api/v1/collaborators_controller/index_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe Api::V1::CollaboratorsController, "#index", type: :request do
+  describe "list all collaborators" do
+    let!(:author) { FactoryBot.create(:user) }
+    let!(:project) { FactoryBot.create(:project, author: author) }
+
+    context "when not authenticated" do
+      before do
+        get "/api/v1/projects/#{project.id}/collaborators/", as: :json
+      end
+
+      it "returns status unauthorized" do
+        expect(response).to have_http_status(401)
+        expect(response.parsed_body).to have_jsonapi_errors
+      end
+    end
+
+    context "when authorized but invalid/non-existent project" do
+      before do
+        token = get_auth_token(FactoryBot.create(:user))
+        get "/api/v1/projects/0/collaborators/",
+            headers: { "Authorization": "Token #{token}" }, as: :json
+      end
+
+      it "returns status not_found" do
+        expect(response).to have_http_status(404)
+        expect(response.parsed_body).to have_jsonapi_errors
+      end
+    end
+
+    context "when auhtorized to fetch project collaborators" do
+      before do
+        # create 3 collaborators for a project
+        FactoryBot.create_list(:user, 3).each do |u|
+          FactoryBot.create(:collaboration, user: u, project: project)
+        end
+        token = get_auth_token(FactoryBot.create(:user))
+        get "/api/v1/projects/#{project.id}/collaborators/",
+            headers: { "Authorization": "Token #{token}" }, as: :json
+      end
+
+      it "returns all the collaborators for the given project" do
+        expect(response).to have_http_status(200)
+        expect(response).to match_response_schema("users")
+        expect(response.parsed_body["data"].length).to eq(3)
+      end
+    end
+  end
+end

From cee899fd4692d08be583f6117c1b632ac441383b Mon Sep 17 00:00:00 2001
From: Nitish Aggarwal <royalnitish21@gmail.com>
Date: Sun, 31 May 2020 23:05:16 +0530
Subject: [PATCH 05/10] bfix: Fix user being deleted in #destroy

---
 app/controllers/api/v1/collaborators_controller.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/controllers/api/v1/collaborators_controller.rb b/app/controllers/api/v1/collaborators_controller.rb
index dc36a1c24f..5ebf7a73a3 100644
--- a/app/controllers/api/v1/collaborators_controller.rb
+++ b/app/controllers/api/v1/collaborators_controller.rb
@@ -38,7 +38,8 @@ def create
   # DELETE /api/v1//projects/:project_id/collaborators/:id
   # :id is essentially the user_id for the user to be removed from project
   def destroy
-    @collaborator.destroy!
+    @collaboration = Collaboration.find_by(user: @collaborator, project: @project)
+    @collaboration.destroy!
     render json: {}, status: :no_content
   end
 

From b9252544cd172a2967e3222154027503884da143 Mon Sep 17 00:00:00 2001
From: Nitish Aggarwal <royalnitish21@gmail.com>
Date: Tue, 2 Jun 2020 12:06:24 +0530
Subject: [PATCH 06/10] rfac: refactor mails logic to separate mails handler

---
 .../api/v1/collaborators_controller.rb        | 42 ++++------------
 .../collaborators_controller/mails_handler.rb | 48 +++++++++++++++++++
 .../collaborators_controller/create_spec.rb   |  5 +-
 3 files changed, 60 insertions(+), 35 deletions(-)
 create mode 100644 app/controllers/api/v1/collaborators_controller/mails_handler.rb

diff --git a/app/controllers/api/v1/collaborators_controller.rb b/app/controllers/api/v1/collaborators_controller.rb
index 5ebf7a73a3..c6ef045d07 100644
--- a/app/controllers/api/v1/collaborators_controller.rb
+++ b/app/controllers/api/v1/collaborators_controller.rb
@@ -19,20 +19,15 @@ def index
 
   # POST /api/v1/projects/:project_id/collaborators
   def create
-    parse_mails_except_current_user(params[:emails])
-    newly_added = @valid_mails - @existing_mails
-
-    @added_mails = []
-
-    newly_added.each do |email|
-      user = User.find_by(email: email)
-      if user.present?
-        @added_mails.push(email)
-        Collaboration.where(project_id: @project.id, user_id: user.id).first_or_create
-      end
-    end
-
-    render json: { added: @added_mails, existing: @existing_mails, invalid: @invalid_mails }
+    mails_handler = MailsHandler.new(params[:emails], @project, @current_user)
+    # parse mails as valid or invalid
+    mails_handler.parse
+
+    render json: {
+      added: mails_handler.added_mails,
+      existing: mails_handler.existing_mails,
+      invalid: mails_handler.invalid_mails
+    }
   end
 
   # DELETE /api/v1//projects/:project_id/collaborators/:id
@@ -60,23 +55,4 @@ def set_collaborator
     def collaborator_params
       params.require(:collaborator).permit(:project_id, :emails)
     end
-
-    def parse_mails_except_current_user(mails)
-      @valid_mails = []
-      @invalid_mails = []
-
-      mails.split(",").each do |email|
-        email = email.strip
-        if email.present? && email != @current_user.email \
-          && Devise.email_regexp.match?(email)
-          @valid_mails.push(email)
-        else
-          @invalid_mails.push(email)
-        end
-      end
-
-      @existing_mails = User.where(
-        id: @project.collaborations.pluck(:user_id)
-      ).pluck(:email)
-    end
 end
diff --git a/app/controllers/api/v1/collaborators_controller/mails_handler.rb b/app/controllers/api/v1/collaborators_controller/mails_handler.rb
new file mode 100644
index 0000000000..3aaf352e61
--- /dev/null
+++ b/app/controllers/api/v1/collaborators_controller/mails_handler.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+class Api::V1::CollaboratorsController
+  class MailsHandler
+    attr_reader :valid_mails, :invalid_mails, :existing_mails
+
+    # initialize the class with mails, project and current_user to be used in class
+    def initialize(mails, project, current_user)
+      @mails = mails
+      @project = project
+      @current_user = current_user
+      # initialize empty valid and invalid mails
+      @valid_mails = []
+      @invalid_mails = []
+    end
+
+    # parse emails as valid, invalid or exisitng mails
+    def parse
+      @mails.split(",").each do |email|
+        email = email.strip
+        if email.present? && email != @current_user.email && Devise.email_regexp.match?(email)
+          @valid_mails.push(email)
+        else
+          @invalid_mails.push(email)
+        end
+      end
+
+      @existing_mails = User.where(
+        id: @project.collaborations.pluck(:user_id)
+      ).pluck(:email)
+    end
+
+    def added_mails
+      newly_added = valid_mails - existing_mails
+      added_mails = []
+      # checks if user exists and adds to added_mails
+      newly_added.each do |email|
+        user = User.find_by(email: email)
+        if user.present?
+          added_mails.push(email)
+          Collaboration.where(project_id: @project.id, user_id: user.id).first_or_create
+        end
+      end
+      # returns added_mails
+      added_mails
+    end
+  end
+end
diff --git a/spec/requests/api/v1/collaborators_controller/create_spec.rb b/spec/requests/api/v1/collaborators_controller/create_spec.rb
index 5d3c890d01..a215fcadd7 100644
--- a/spec/requests/api/v1/collaborators_controller/create_spec.rb
+++ b/spec/requests/api/v1/collaborators_controller/create_spec.rb
@@ -36,7 +36,7 @@
     context "when authorized but tries to add collaborator to non existent project" do
       before do
         token = get_auth_token(author)
-        post "//api/v1/projects/0/collaborators/",
+        post "/api/v1/projects/0/collaborators/",
              headers: { "Authorization": "Token #{token}" },
              params: create_params, as: :json
       end
@@ -53,7 +53,7 @@
         existing = FactoryBot.create(:user, email: "existing@test.com")
         FactoryBot.create(:collaboration, user: existing, project: project)
         token = get_auth_token(author)
-        post "//api/v1/projects/#{project.id}/collaborators/",
+        post "/api/v1/projects/#{project.id}/collaborators/",
              headers: { "Authorization": "Token #{token}" },
              params: create_params, as: :json
       end
@@ -64,6 +64,7 @@
 
       it "returns the added, already_existing & invalid mails (author being invalid)" do
         expect(response.parsed_body["added"]).to eq([user.email])
+        puts user.email
         expect(response.parsed_body["existing"]).to eq(["existing@test.com"])
         expect(response.parsed_body["invalid"]).to eq(["invalid", author.email])
       end

From b0d5d9dd09cdc63cfb057d0b1a9f67aeabf7dadb Mon Sep 17 00:00:00 2001
From: Nitish Aggarwal <royalnitish21@gmail.com>
Date: Tue, 2 Jun 2020 13:12:30 +0530
Subject: [PATCH 07/10] ptch: Use except in before_action to prevent accidental
 exposure

---
 app/controllers/api/v1/collaborators_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/api/v1/collaborators_controller.rb b/app/controllers/api/v1/collaborators_controller.rb
index c6ef045d07..04e55c82a5 100644
--- a/app/controllers/api/v1/collaborators_controller.rb
+++ b/app/controllers/api/v1/collaborators_controller.rb
@@ -3,7 +3,7 @@
 class Api::V1::CollaboratorsController < Api::V1::BaseController
   before_action :authenticate_user!
   before_action :set_project
-  before_action :check_author_access, only: %i[create destroy]
+  before_action :check_author_access, except: %i[index]
   before_action :set_collaborator, only: %i[destroy]
 
   # /api/v1/projects/:project_id/collaborators

From 63ea89d2e18af04a9871f1cdce54eb9c78e8161d Mon Sep 17 00:00:00 2001
From: Nitish Aggarwal <royalnitish21@gmail.com>
Date: Wed, 10 Jun 2020 18:59:17 +0530
Subject: [PATCH 08/10] rfac: move mails handler to app/services

---
 .../api/v1/collaborators_controller/mails_handler.rb              | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename app/{controllers => services}/api/v1/collaborators_controller/mails_handler.rb (100%)

diff --git a/app/controllers/api/v1/collaborators_controller/mails_handler.rb b/app/services/api/v1/collaborators_controller/mails_handler.rb
similarity index 100%
rename from app/controllers/api/v1/collaborators_controller/mails_handler.rb
rename to app/services/api/v1/collaborators_controller/mails_handler.rb

From fc0796aadbdd26fe505109a0256da9383749a750 Mon Sep 17 00:00:00 2001
From: Nitish Aggarwal <royalnitish21@gmail.com>
Date: Tue, 23 Jun 2020 21:15:25 +0530
Subject: [PATCH 09/10] ptch: Fix Typos

---
 app/services/api/v1/collaborators_controller/mails_handler.rb | 2 +-
 spec/requests/api/v1/collaborators_controller/create_spec.rb  | 2 +-
 spec/requests/api/v1/collaborators_controller/index_spec.rb   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/services/api/v1/collaborators_controller/mails_handler.rb b/app/services/api/v1/collaborators_controller/mails_handler.rb
index 3aaf352e61..61d19a7af8 100644
--- a/app/services/api/v1/collaborators_controller/mails_handler.rb
+++ b/app/services/api/v1/collaborators_controller/mails_handler.rb
@@ -14,7 +14,7 @@ def initialize(mails, project, current_user)
       @invalid_mails = []
     end
 
-    # parse emails as valid, invalid or exisitng mails
+    # parse emails as valid, invalid or existing mails
     def parse
       @mails.split(",").each do |email|
         email = email.strip
diff --git a/spec/requests/api/v1/collaborators_controller/create_spec.rb b/spec/requests/api/v1/collaborators_controller/create_spec.rb
index a215fcadd7..c02858d57d 100644
--- a/spec/requests/api/v1/collaborators_controller/create_spec.rb
+++ b/spec/requests/api/v1/collaborators_controller/create_spec.rb
@@ -13,7 +13,7 @@
         post "/api/v1/projects/#{project.id}/collaborators/", as: :json
       end
 
-      it "returns status unautheticated" do
+      it "returns status unauthenticated" do
         expect(response).to have_http_status(401)
         expect(response.parsed_body).to have_jsonapi_errors
       end
diff --git a/spec/requests/api/v1/collaborators_controller/index_spec.rb b/spec/requests/api/v1/collaborators_controller/index_spec.rb
index e6cb7fe69e..af842a3fe2 100644
--- a/spec/requests/api/v1/collaborators_controller/index_spec.rb
+++ b/spec/requests/api/v1/collaborators_controller/index_spec.rb
@@ -31,7 +31,7 @@
       end
     end
 
-    context "when auhtorized to fetch project collaborators" do
+    context "when authorized to fetch project collaborators" do
       before do
         # create 3 collaborators for a project
         FactoryBot.create_list(:user, 3).each do |u|

From f48db35e7987fec713dc03c5335b8718d72432a9 Mon Sep 17 00:00:00 2001
From: Nitish Aggarwal <royalnitish21@gmail.com>
Date: Thu, 25 Jun 2020 09:31:01 +0530
Subject: [PATCH 10/10] ptch: Adds check for project's collaborators which user
 has no access to

---
 .../api/v1/collaborators_controller.rb        |  5 +++
 .../v1/collaborators_controller/index_spec.rb | 32 +++++++++++++++----
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/app/controllers/api/v1/collaborators_controller.rb b/app/controllers/api/v1/collaborators_controller.rb
index 04e55c82a5..6e325a3f8d 100644
--- a/app/controllers/api/v1/collaborators_controller.rb
+++ b/app/controllers/api/v1/collaborators_controller.rb
@@ -4,6 +4,7 @@ class Api::V1::CollaboratorsController < Api::V1::BaseController
   before_action :authenticate_user!
   before_action :set_project
   before_action :check_author_access, except: %i[index]
+  before_action :check_view_access, only: %i[index]
   before_action :set_collaborator, only: %i[destroy]
 
   # /api/v1/projects/:project_id/collaborators
@@ -48,6 +49,10 @@ def check_author_access
       authorize @project, :author_access?
     end
 
+    def check_view_access
+      authorize @project, :check_view_access?
+    end
+
     def set_collaborator
       @collaborator = @project.collaborators.find(params[:id])
     end
diff --git a/spec/requests/api/v1/collaborators_controller/index_spec.rb b/spec/requests/api/v1/collaborators_controller/index_spec.rb
index af842a3fe2..f9d3f04fe2 100644
--- a/spec/requests/api/v1/collaborators_controller/index_spec.rb
+++ b/spec/requests/api/v1/collaborators_controller/index_spec.rb
@@ -5,11 +5,14 @@
 RSpec.describe Api::V1::CollaboratorsController, "#index", type: :request do
   describe "list all collaborators" do
     let!(:author) { FactoryBot.create(:user) }
-    let!(:project) { FactoryBot.create(:project, author: author) }
+    let!(:public_project) do
+      FactoryBot.create(:project, author: author, project_access_type: "Public")
+    end
+    let!(:private_project) { FactoryBot.create(:project, author: author) }
 
     context "when not authenticated" do
       before do
-        get "/api/v1/projects/#{project.id}/collaborators/", as: :json
+        get "/api/v1/projects/#{public_project.id}/collaborators/", as: :json
       end
 
       it "returns status unauthorized" do
@@ -31,14 +34,14 @@
       end
     end
 
-    context "when authorized to fetch project collaborators" do
+    context "when authorized to fetch project's collaborators which user has view access to" do
       before do
-        # create 3 collaborators for a project
+        # create 3 collaborators for a public project
         FactoryBot.create_list(:user, 3).each do |u|
-          FactoryBot.create(:collaboration, user: u, project: project)
+          FactoryBot.create(:collaboration, user: u, project: public_project)
         end
         token = get_auth_token(FactoryBot.create(:user))
-        get "/api/v1/projects/#{project.id}/collaborators/",
+        get "/api/v1/projects/#{public_project.id}/collaborators/",
             headers: { "Authorization": "Token #{token}" }, as: :json
       end
 
@@ -48,5 +51,22 @@
         expect(response.parsed_body["data"].length).to eq(3)
       end
     end
+
+    context "when fetching project's collaborators which user doesn't have view access to" do
+      before do
+        # create 3 collaborators for a private project
+        FactoryBot.create_list(:user, 3).each do |u|
+          FactoryBot.create(:collaboration, user: u, project: private_project)
+        end
+        token = get_auth_token(FactoryBot.create(:user))
+        get "/api/v1/projects/#{private_project.id}/collaborators/",
+            headers: { "Authorization": "Token #{token}" }, as: :json
+      end
+
+      it "returns status unauthorized" do
+        expect(response).to have_http_status(403)
+        expect(response.parsed_body).to have_jsonapi_errors
+      end
+    end
   end
 end