scinote-eln/scinote-web
1
1
Fork 0
mirror of https://github.com/scinote-eln/scinote-web.git synced 2024-12-26 09:42:46 +08:00
Code Issues Projects Releases Packages Wiki Activity

Refactor inventory sharing logic [SCI-3803]

Browse source
This commit is contained in:
Oleksii Kriuchykhin 2019-08-29 17:17:38 +02:00
parent d209b3abcd
commit 267c53c2d2
15 changed files with 118 additions and 205 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

56
app/controllers/team_repositories_controller.rb
View file

@ -2,23 +2,7 @@
class TeamRepositoriesController < ApplicationController
before_action :load_vars
before_action :check_sharing_permissions, only: %i(create destroy)
# POST :team_id/repositories/:repository_id/team_repositories
def create
team_repository = TeamRepository.new(repository: @repository,
team_id: create_params[:target_team_id],
permission_level: create_params[:permission_level])
if team_repository.save
log_activity(:share_inventory, team_repository)
render json: { team_repository: team_repository }, status: :ok
else
render json: { team_repository: { message: 'not saved!', errors: team_repository.errors } },
status: :unprocessable_entity
end
end
before_action :check_sharing_permissions
# DELETE :team_id/repositories/:repository_id/team_repositories/:id
def destroy
@ -29,21 +13,23 @@ class TeamRepositoriesController < ApplicationController
team_repository.destroy
render json: {}, status: :no_content
else
render json: { message: 'Can\'t find sharing relation for destroy' }, status: :unprocessable_entity
render json: { message: I18n.t('repositories.multiple_share_service.nothing_to_delete') },
status: :unprocessable_entity
end
end
# POST :team_id/repositories/:repository_id/multiple_update
def multiple_update
service_call = Repositories::MultipleShareUpdateService.call(repository_id: @repository.id,
user_id: current_user.id,
team_id: current_team.id,
# POST :team_id/repositories/:repository_id/update
def update
service_call = Repositories::MultipleShareUpdateService.call(repository: @repository,
user: current_user,
team: current_team,
team_ids_for_share: teams_to_share,
team_ids_for_unshare: teams_to_unshare,
team_ids_for_update: teams_to_update,
**share_all_params)
if service_call.succeed?
render json: { warnings: service_call.warnings.join(', '), status: @repository.i_shared?(current_team) }, status: :ok
render json: { warnings: service_call.warnings.join(', '), status: @repository.i_shared?(current_team) },
status: :ok
else
render json: { errors: service_call.errors.map { |_, v| v }.join(', ') }, status: :unprocessable_entity
end
@ -52,7 +38,7 @@ class TeamRepositoriesController < ApplicationController
private
def load_vars
@repository = Repository.find_by_id(params[:repository_id])
@repository = current_team.repositories.find_by_id(params[:repository_id])
render_404 unless @repository
end
@ -65,7 +51,7 @@ class TeamRepositoriesController < ApplicationController
params.permit(:team_id, :id)
end
def multiple_update_params
def update_params
params.permit(:permission_changes, share_team_ids: [], write_permissions: [])
end
@ -75,29 +61,29 @@ class TeamRepositoriesController < ApplicationController
def teams_to_share
existing_shares = @repository.teams_shared_with.pluck(:id)
teams_to_share = multiple_update_params[:share_team_ids]&.map(&:to_i).to_a - existing_shares
wp = multiple_update_params[:write_permissions]&.map(&:to_i)
teams_to_share = update_params[:share_team_ids]&.map(&:to_i).to_a - existing_shares
wp = update_params[:write_permissions]&.map(&:to_i)
teams_to_share.map { |e| { id: e, permission_level: wp&.include?(e) ? 'write' : 'read' } }
teams_to_share.map { |e| { id: e, permission_level: wp&.include?(e) ? 'shared_write' : 'shared_read' } }
end
def teams_to_unshare
existing_shares = @repository.teams_shared_with.pluck(:id)
existing_shares - multiple_update_params[:share_team_ids]&.map(&:to_i).to_a
existing_shares - update_params[:share_team_ids]&.map(&:to_i).to_a
end
def teams_to_update
teams_to_update = JSON.parse(multiple_update_params[:permission_changes]).keys.map(&:to_i).to_a &
multiple_update_params[:share_team_ids]&.map(&:to_i).to_a
wp = multiple_update_params[:write_permissions]&.map(&:to_i)
teams_to_update = JSON.parse(update_params[:permission_changes]).keys.map(&:to_i).to_a &
update_params[:share_team_ids]&.map(&:to_i).to_a
wp = update_params[:write_permissions]&.map(&:to_i)
teams_to_update.map { |e| { id: e, permission_level: wp&.include?(e) ? 'write' : 'read' } }
teams_to_update.map { |e| { id: e, permission_level: wp&.include?(e) ? 'shared_write' : 'shared_read' } }
end
def share_all_params
{
shared_with_all: params[:select_all_teams].present?,
shared_permissions_level: params[:select_all_write_permission].present? ? 'write' : 'read'
shared_permissions_level: params[:select_all_write_permission].present? ? 'shared_write' : 'shared_read'
}
end

17
app/models/repository.rb
View file

@ -34,7 +34,12 @@ class Repository < ApplicationRecord
left_outer_joins(:team_repositories)
.where('repositories.team_id IN (?) '\
'OR team_repositories.team_id IN (?) '\
'OR repositories.shared = true', teams, teams)
'OR repositories.permission_level = ? '\
'OR repositories.permission_level = ? ',
teams,
teams,
Extends::SHARED_INVENTORIES_PERMISSION_LEVELS[:shared_read],
Extends::SHARED_INVENTORIES_PERMISSION_LEVELS[:shared_write])
.distinct
}
@ -87,25 +92,25 @@ class Repository < ApplicationRecord
end
def shared_with_anybody?
(shared? || team_repositories.any?)
(!not_shared? || team_repositories.any?)
end
def shared_with?(team)
return false if self.team == team
shared? || private_shared_with?(team)
!not_shared? || private_shared_with?(team)
end
def shared_with_write?(team)
return false if self.team == team
shared? && write? || private_shared_with_write?(team)
shared_write? || private_shared_with_write?(team)
end
def shared_with_read?(team)
return false if self.team == team
shared? && read? || team_repositories.where(team: team, permission_level: :read).any?
shared_read? || team_repositories.where(team: team, permission_level: :shared_read).any?
end
def private_shared_with?(team)
@ -113,7 +118,7 @@ class Repository < ApplicationRecord
end
def private_shared_with_write?(team)
team_repositories.where(team: team, permission_level: :write).any?
team_repositories.where(team: team, permission_level: :shared_write).any?
end
def self.viewable_by_user(_user, teams)

8
app/models/team_repository.rb
View file

@ -8,5 +8,11 @@ class TeamRepository < ApplicationRecord
validates :permission_level, presence: true
validates :repository, uniqueness: { scope: :team_id }
validates :team_id, inclusion: { in: proc { |object| Team.pluck(:id) - [object.repository&.team&.id] } }
validate :team_cannot_be_the_same
private
def team_cannot_be_the_same
errors.add(:team_id, :same_team) if repository&.team_id == team_id
end
end

46
app/services/repositories/multiple_share_update_service.rb
View file

@ -7,17 +7,17 @@ module Repositories
attr_reader :repository, :user, :warnings, :errors
def initialize(repository_id:,
user_id:,
team_id:,
def initialize(repository:,
user:,
team:,
team_ids_for_share: [],
team_ids_for_unshare: [],
team_ids_for_update: [],
shared_with_all: nil,
shared_permissions_level: nil)
@repository = Repository.find_by_id repository_id
@user = User.find_by_id user_id
@team = Team.find_by_id team_id
@repository = repository
@user = user
@team = team
@team_ids_for_share = team_ids_for_share
@team_ids_for_unshare = team_ids_for_unshare
@team_ids_for_update = team_ids_for_update
@ -31,12 +31,10 @@ module Repositories
return self unless valid?
if !@shared_with_all.nil? && !@shared_permission_level.nil?
@repository.shared = @shared_with_all
@repository.permission_level = @shared_permission_level
old_permission_level = @repository.permission_level
@repository.permission_level = @shared_with_all ? @shared_permission_level : :not_shared
if @repository.changed?
change_type = @repository.changes.key?('shared') ? 'share' : 'update_permission_level'
log_activity_share_all(change_type, @repository) if @repository.save
log_activity_share_all(@repository.permission_level, old_permission_level, @repository) if @repository.save
end
end
@ -93,18 +91,11 @@ module Repositories
'user': @user,
'team': @team }
.map do |key, value|
"Can't find #{key.capitalize}" if value.nil?
I18n.t('repositories.multiple_share_service.invalid_arguments', key: key.capitalize) if value.nil?
end.compact
return false
end
if can_share_repository?(@user, @repository)
true
else
@errors[:user_without_permissions] =
['You are not allowed to share this repository']
false
end
true
end
def log_activity(type_of, team_repository)
@ -119,11 +110,16 @@ module Repositories
Extends::SHARED_INVENTORIES_PL_MAPPINGS[team_repository.permission_level.to_sym] })
end
def log_activity_share_all(change_type, repository)
type = if change_type == 'share'
@repository.shared ? :share_inventory_with_all : :unshare_inventory_with_all
else
:update_share_with_all_permission_level
def log_activity_share_all(permission_level, old_permission_level, repository)
type = case permission_level.to_sym
when :shared_read, :shared_write
if old_permission_level.to_sym == :not_shared
:share_inventory_with_all
else
:update_share_with_all_permission_level
end
when :not_shared
:unshare_inventory_with_all
end
Activities::CreateActivityService

10
app/views/repositories/_share_repository_modal.html.erb
View file

@ -1,6 +1,6 @@
<div class="modal share-repo-modal" tabindex="-1" role="dialog" >
<%= form_for :shares, url: multiple_update_team_repository_team_repositories_path(current_team, @repository), remote: :true,
data: {success_message: t("repositories.index.modal_share.success_message", inventory_name: @repository.name)} do |f| %>
<%= form_for :shares, url: team_repository_team_repositories_path(current_team, @repository), remote: :true,
data: { success_message: t("repositories.index.modal_share.success_message", inventory_name: @repository.name) } do |f| %>
<div class="modal-dialog" role="document">
<div class="modal-content">
<div class="modal-header">
@ -16,13 +16,13 @@
</div>
<div class="data-list">
<div class="all-teams">
<span class="team-selector" title="<%= t("repositories.index.modal_share.all_teams_tooltip") %>" >
<%= check_box_tag 'select_all_teams', 0, @repository.shared, {class: "simple-checkbox"} %>
<span class="team-selector" title="<%= t("repositories.index.modal_share.all_teams_tooltip") %>">
<%= check_box_tag 'select_all_teams', 0, @repository.shared_read? || @repository.shared_write?, { class: 'simple-checkbox' } %>
<span class="checkbox-label"></span>
<%= t("repositories.index.modal_share.all_teams") %>
</span>
<span class="permission-selector">
<%= check_box_tag 'select_all_write_permission', 0, @repository.write?, {class: 'hidden trigger-checkbox'}%>
<%= check_box_tag 'select_all_write_permission', 0, @repository.shared_write?, { class: 'hidden trigger-checkbox' }%>
<span class="checkbox-label"></span>
</span>
</div>

9
config/initializers/extends.rb
View file

@ -243,12 +243,13 @@ class Extends
}
SHARED_INVENTORIES_PERMISSION_LEVELS = {
read: 0,
write: 1
not_shared: 0,
shared_read: 1,
shared_write: 2
}.freeze
SHARED_INVENTORIES_PL_MAPPINGS = {
read: 'view-only',
write: 'edit'
shared_read: 'view-only',
shared_write: 'edit'
}.freeze
end

8
config/locales/en.yml
View file

@ -77,6 +77,10 @@ en:
not_unique: "State already exists for this user and parent object"
state:
wrong_state: "Wrong parameters"
team_repository:
attributes:
team_id:
same_team: "Inventory can't be shared to the same team as it belongs to"
helpers:
label:
@ -1106,7 +1110,9 @@ en:
unable_to_share: "Unable to share %{repository} inventory with %{team} team."
unable_to_unshare: "Unable to unshare %{repository} inventory with %{team} team."
unable_to_update: "Unable to update sharing %{repository} inventory with %{team} team."
not_allowed: "You are not allowed to share this repository!"
invalid_arguments: "Can't find %{key}"
nothing_to_delete: "Can't find sharing relation for destroy"
libraries:
manange_modal_column:
colum_type: "Column type"

4
config/routes.rb
View file

@ -179,9 +179,9 @@ Rails.application.routes.draw do
defaults: { format: 'json' }
get :share_modal
resources :team_repositories, only: %i(create destroy) do
resources :team_repositories, only: %i(destroy) do
collection do
post 'multiple_update'
post 'update'
end
end
end

1
db/migrate/20190812065432_add_share_flag_to_repository.rb
View file

@ -2,7 +2,6 @@
class AddShareFlagToRepository < ActiveRecord::Migration[5.2]
def change
add_column :repositories, :shared, :boolean, null: false, default: false
add_column :repositories, :permission_level, :integer, null: false, default: 0
end
end

1
db/migrate/20190812072649_add_indexes_to_sharing_columns.rb
View file

@ -3,7 +3,6 @@
class AddIndexesToSharingColumns < ActiveRecord::Migration[5.2]
def change
add_index :team_repositories, :permission_level
add_index :repositories, :shared
add_index :repositories, :permission_level
change_column_default :team_repositories, :permission_level, from: nil, to: 0
end

2
db/schema.rb
View file

@ -416,11 +416,9 @@ ActiveRecord::Schema.define(version: 2019_08_12_072649) do
t.datetime "created_at"
t.datetime "updated_at"
t.datetime "discarded_at"
t.boolean "shared", default: false, null: false
t.integer "permission_level", default: 0, null: false
t.index ["discarded_at"], name: "index_repositories_on_discarded_at"
t.index ["permission_level"], name: "index_repositories_on_permission_level"
t.index ["shared"], name: "index_repositories_on_shared"
t.index ["team_id"], name: "index_repositories_on_team_id"
end

76
spec/controllers/team_repositories_controller_spec.rb
View file

@ -11,68 +11,6 @@ describe TeamRepositoriesController, type: :controller do
let(:repository) { create :repository, team: team }
let(:target_team) { create :team }
describe 'POST create' do
context 'when resource can be saved' do
let(:action) do
post :create,
params: { team_id: team.id,
repository_id: repository.id,
target_team_id: target_team.id,
permission_level: 'read' },
format: :json
end
it 'renders 200' do
action
expect(response).to have_http_status(200)
end
it 'calls create activity for deleting inventory' do
expect(Activities::CreateActivityService)
.to(receive(:call)
.with(hash_including(activity_type: :share_inventory)))
action
end
it 'adds activity in DB' do
expect { action }.to(change { Activity.count })
end
end
context 'when resource cannot be saved' do
it 'renders 304' do
post :create,
params: { team_id: team.id, repository_id: repository.id, target_team_id: -1 },
format: :json
expect(response).to have_http_status(422)
end
end
context 'when user do not have permissions' do
let(:new_repository) { create :repository }
it 'renders 403' do
post :create,
params: { team_id: team.id, repository_id: new_repository.id },
format: :json
expect(response).to have_http_status(403)
end
end
context 'when repository is not found' do
it 'renders 404' do
post :create,
params: { team_id: team.id, repository_id: -1, target_team_id: 'id' },
format: :json
expect(response).to have_http_status(404)
end
end
end
describe 'DELETE destroy' do
let(:second_team) { create :team }
let!(:second_user_team) { create :user_team, user: user, team: second_team }
@ -110,18 +48,18 @@ describe TeamRepositoriesController, type: :controller do
end
end
context 'when user do not have permissions' do
context 'when user do not have access to inventory' do
let(:new_repository) { create :repository }
it 'renders 403' do
it 'renders 404' do
delete :destroy, params: { repository_id: new_repository.id, team_id: team.id, id: team_repository.id }
expect(response).to have_http_status(403)
expect(response).to have_http_status(404)
end
end
end
describe 'POST multiple_update' do
describe 'POST update' do
context 'when have valid params' do
before do
service = double('success_service')
@ -142,7 +80,7 @@ describe TeamRepositoriesController, type: :controller do
end
it 'renders status ok' do
post :multiple_update, params: params
post :update, params: params
expect(response).to have_http_status(200)
end
@ -160,7 +98,7 @@ describe TeamRepositoriesController, type: :controller do
let(:new_repository) { create :repository }
let(:params) do
{
repository_id: new_repository.id,
repository_id: repository.id,
team_id: team.id,
permission_changes: '{"5":true}',
share_team_ids: %w(1 2 3),
@ -169,7 +107,7 @@ describe TeamRepositoriesController, type: :controller do
end
it 'renders status 422' do
post :multiple_update, params: params
post :update, params: params
expect(response).to have_http_status(422)
end

6
spec/factories/repositories.rb
View file

@ -6,12 +6,10 @@ FactoryBot.define do
created_by { create :user }
team
trait :write_shared do
shared { true }
permission_level { :write }
permission_level { :shared_write }
end
trait :read_shared do
shared { true }
permission_level { :read }
permission_level { :shared_read }
end
end
end

4
spec/factories/team_repositories.rb
View file

@ -5,10 +5,10 @@ FactoryBot.define do
team
repository
trait :read do
permission_level { :read }
permission_level { :shared_read }
end
trait :write do
permission_level { :write }
permission_level { :shared_write }
end
end
end

75
spec/services/repositories/multiple_share_update_service_spec.rb
View file

@ -10,33 +10,12 @@ describe Repositories::MultipleShareUpdateService do
let(:team3) { create :team }
let(:repository) { create :repository, team: team }
context 'when user do not have permissions' do
it 'returns error about permissions' do
new_repo = create :repository
service_call = Repositories::MultipleShareUpdateService.call(repository_id: new_repo.id,
user_id: user.id,
team_id: team.id)
expect(service_call.errors).to have_key(:user_without_permissions)
end
end
context 'when repository not found' do
it 'returns error about repository' do
service_call = Repositories::MultipleShareUpdateService.call(repository_id: -1,
user_id: user.id,
team_id: team.id)
expect(service_call.errors).to have_key(:invalid_arguments)
end
end
context 'when share' do
let(:service_call) do
Repositories::MultipleShareUpdateService.call(repository_id: repository.id,
user_id: user.id,
team_id: team.id,
team_ids_for_share: [{ id: team2.id, permission_level: 'read' }])
Repositories::MultipleShareUpdateService.call(repository: repository,
user: user,
team: team,
team_ids_for_share: [{ id: team2.id, permission_level: :shared_read }])
end
it 'adds TeamRepository record' do
@ -49,10 +28,10 @@ describe Repositories::MultipleShareUpdateService do
context 'when cannot generate share' do
let(:service_call) do
Repositories::MultipleShareUpdateService.call(repository_id: repository.id,
user_id: user.id,
team_id: team.id,
team_ids_for_share: [{ id: -1, permission_level: 'read' }])
Repositories::MultipleShareUpdateService.call(repository: repository,
user: user,
team: team,
team_ids_for_share: [{ id: -1, permission_level: :shared_read }])
end
it 'returns error' do
@ -63,9 +42,9 @@ describe Repositories::MultipleShareUpdateService do
context 'when unshare' do
let(:service_call) do
Repositories::MultipleShareUpdateService.call(repository_id: repository.id,
user_id: user.id,
team_id: team.id,
Repositories::MultipleShareUpdateService.call(repository: repository,
user: user,
team: team,
team_ids_for_unshare: [team2.id])
end
@ -83,9 +62,9 @@ describe Repositories::MultipleShareUpdateService do
context 'when cannot delete share' do
let(:service_call) do
Repositories::MultipleShareUpdateService.call(repository_id: repository.id,
user_id: user.id,
team_id: team.id,
Repositories::MultipleShareUpdateService.call(repository: repository,
user: user,
team: team,
team_ids_for_unshare: [-1])
end
@ -97,10 +76,12 @@ describe Repositories::MultipleShareUpdateService do
context 'when updates share permissions' do
let(:service_call) do
Repositories::MultipleShareUpdateService.call(repository_id: repository.id,
user_id: user.id,
team_id: team.id,
team_ids_for_update: [{ id: team2.id, permission_level: 'read' }])
Repositories::MultipleShareUpdateService.call(
repository: repository,
user: user,
team: team,
team_ids_for_update: [{ id: team2.id, permission_level: :shared_read }]
)
end
it 'updates permission for share record' do
@ -117,10 +98,10 @@ describe Repositories::MultipleShareUpdateService do
context 'when cannot update share' do
let(:service_call) do
Repositories::MultipleShareUpdateService.call(repository_id: repository.id,
user_id: user.id,
team_id: team.id,
team_ids_for_update: [{ id: -1, permission_level: 'read' }])
Repositories::MultipleShareUpdateService.call(repository: repository,
user: user,
team: team,
team_ids_for_update: [{ id: -1, permission_level: :shared_read }])
end
it 'returns error' do
@ -131,11 +112,11 @@ describe Repositories::MultipleShareUpdateService do
context 'when share_with_all' do
let(:service_call) do
Repositories::MultipleShareUpdateService.call(repository_id: repository.id,
user_id: user.id,
team_id: team.id,
Repositories::MultipleShareUpdateService.call(repository: repository,
user: user,
team: team,
shared_with_all: true,
shared_permissions_level: :write)
shared_permissions_level: :shared_write)
end
it 'updates permission for share record' do