Skip to content
GitLab
Menu
Projects
Groups
Snippets
Help
Help
Support
Community forum
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
Menu
Open sidebar
heptapod
heptapod
Commits
1971e54b3bda
Commit
6eb6229c
authored
Jul 27, 2018
by
Jarka Kadlecová
Browse files
Remove todos when project feature visibility changes
parent
95e14eb76d18
Changes
10
Hide whitespace changes
Inline
Side-by-side
app/services/projects/update_service.rb
View file @
1971e54b
...
...
@@ -42,9 +42,18 @@ def run_auto_devops_pipeline?
private
def
after_update
todos_features_changes
=
%w(
issues_access_level
merge_requests_access_level
repository_access_level
)
project_changed_feature_keys
=
project
.
project_feature
.
previous_changes
.
keys
if
project
.
previous_changes
.
include?
(
:visibility_level
)
&&
project
.
private?
# don't enqueue immediately to prevent todos removal in case of a mistake
TodosDestroyer
::
ProjectPrivateWorker
.
perform_in
(
1
.
hour
,
project
.
id
)
elsif
(
project_changed_feature_keys
&
todos_features_changes
).
present?
TodosDestroyer
::
PrivateFeaturesWorker
.
perform_in
(
1
.
hour
,
project
.
id
)
end
if
project
.
previous_changes
.
include?
(
'path'
)
...
...
app/services/todos/destroy/base_service.rb
View file @
1971e54b
...
...
@@ -18,15 +18,15 @@ def authorized_users
end
def
todos
# overridden in subclasses
raise
NotImplementedError
end
def
project_ids
# overridden in subclasses
raise
NotImplementedError
end
def
todos_to_remove?
# overridden in subclasses
raise
NotImplementedError
end
end
end
...
...
app/services/todos/destroy/entity_leave_service.rb
View file @
1971e54b
...
...
@@ -21,24 +21,30 @@ def todos
if
entity
.
private?
Todo
.
where
(
project_id:
project_ids
,
user_id:
user_id
)
else
Todo
.
where
(
target_id:
confidential_issues
.
select
(
:id
),
target_type:
Issue
)
project_ids
.
each
do
|
project_id
|
TodosDestroyer
::
PrivateFeaturesWorker
.
perform_async
(
project_id
,
user_id
)
end
Todo
.
where
(
target_id:
confidential_issues
.
select
(
:id
),
target_type:
Issue
,
user_id:
user_id
)
end
end
override
:project_ids
def
project_ids
if
entity
.
is_a?
(
Project
)
entity
.
id
else
case
entity
when
Project
[
entity
.
id
]
when
Namespace
Project
.
select
(
:id
).
where
(
namespace_id:
entity
.
self_and_descendants
.
select
(
:id
))
end
end
override
:todos_to_remove?
def
todos_to_remove?
return
unless
entity
entity
.
private?
||
confidential_issues
.
count
>
0
# if an entity is provided we want to check always at least private features
!!
entity
end
def
confidential_issues
...
...
app/services/todos/destroy/private_features_service.rb
0 → 100644
View file @
1971e54b
module
Todos
module
Destroy
class
PrivateFeaturesService
<
::
Todos
::
Destroy
::
BaseService
attr_reader
:project_ids
,
:user_id
def
initialize
(
project_ids
,
user_id
=
nil
)
@project_ids
=
project_ids
@user_id
=
user_id
end
def
execute
ProjectFeature
.
where
(
project_id:
project_ids
).
each
do
|
project_features
|
target_types
=
[]
target_types
<<
Issue
if
private
?(
project_features
.
issues_access_level
)
target_types
<<
MergeRequest
if
private
?(
project_features
.
merge_requests_access_level
)
target_types
<<
Commit
if
private
?(
project_features
.
repository_access_level
)
next
if
target_types
.
empty?
remove_todos
(
project_features
.
project_id
,
target_types
)
end
end
private
def
private?
(
feature_level
)
feature_level
==
ProjectFeature
::
PRIVATE
end
def
remove_todos
(
project_id
,
target_types
)
items
=
Todo
.
where
(
project_id:
project_id
)
items
=
items
.
where
(
user_id:
user_id
)
if
user_id
items
.
where
(
'user_id NOT IN (?)'
,
authorized_users
)
.
where
(
target_type:
target_types
)
.
delete_all
end
end
end
end
app/workers/all_queues.yml
View file @
1971e54b
...
...
@@ -76,6 +76,7 @@
-
todos_destroyer:todos_destroyer_confidential_issue
-
todos_destroyer:todos_destroyer_entity_leave
-
todos_destroyer:todos_destroyer_project_private
-
todos_destroyer:todos_destroyer_private_features
-
default
-
mailers
# ActionMailer::DeliveryJob.queue_name
...
...
app/workers/todos_destroyer/private_features_worker.rb
0 → 100644
View file @
1971e54b
module
TodosDestroyer
class
PrivateFeaturesWorker
include
ApplicationWorker
include
TodosDestroyerQueue
def
perform
(
project_id
,
user_id
=
nil
)
::
Todos
::
Destroy
::
PrivateFeaturesService
.
new
(
project_id
,
user_id
).
execute
end
end
end
spec/services/projects/update_service_spec.rb
View file @
1971e54b
...
...
@@ -188,6 +188,20 @@
end
end
context
'when changing feature visibility to private'
do
it
'updates the visibility correctly'
do
expect
(
TodosDestroyer
::
PrivateFeaturesWorker
)
.
to
receive
(
:perform_in
).
with
(
1
.
hour
,
project
.
id
)
result
=
update_project
(
project
,
user
,
project_feature_attributes:
{
issues_access_level:
ProjectFeature
::
PRIVATE
}
)
expect
(
result
).
to
eq
({
status: :success
})
expect
(
project
.
project_feature
.
issues_access_level
).
to
be
(
ProjectFeature
::
PRIVATE
)
end
end
context
'when updating a project that contains container images'
do
before
do
stub_container_registry_config
(
enabled:
true
)
...
...
spec/services/todos/destroy/entity_leave_service_spec.rb
View file @
1971e54b
require
'spec_helper'
describe
Todos
::
Destroy
::
EntityLeaveService
do
let
(
:group
)
{
create
(
:group
,
:private
)
}
let
(
:project
)
{
create
(
:project
,
group:
group
)
}
let
(
:user
)
{
create
(
:user
)
}
let
(
:project_member
)
{
create
(
:user
)
}
let
(
:issue
)
{
create
(
:issue
,
:confidential
,
project:
project
)
}
let
(
:group
)
{
create
(
:group
,
:private
)
}
let
(
:project
)
{
create
(
:project
,
group:
group
)
}
let
(
:user
)
{
create
(
:user
)
}
let
(
:user2
)
{
create
(
:user
)
}
let
(
:issue
)
{
create
(
:issue
,
project:
project
)
}
let
(
:mr
)
{
create
(
:merge_request
,
source_project:
project
)
}
let!
(
:todo_
non_member
)
{
create
(
:todo
,
user:
user
,
project:
project
)
}
let!
(
:todo_
conf_
issue_
non_memb
er
)
{
create
(
:todo
,
user:
user
,
target:
issue
,
project:
project
)
}
let!
(
:todo_
conf_
issue_
member
)
{
create
(
:todo
,
user:
project_memb
er
,
target:
issue
,
project:
project
)
}
let!
(
:todo_
mr_user
)
{
create
(
:todo
,
user:
user
,
target:
mr
,
project:
project
)
}
let!
(
:todo_issue_
us
er
)
{
create
(
:todo
,
user:
user
,
target:
issue
,
project:
project
)
}
let!
(
:todo_issue_
user2
)
{
create
(
:todo
,
user:
us
er
2
,
target:
issue
,
project:
project
)
}
describe
'#execute'
do
before
do
project
.
add_developer
(
project_member
)
end
context
'when a user leaves a project'
do
subject
{
described_class
.
new
(
user
.
id
,
project
.
id
,
'Project'
).
execute
}
context
'when project is private'
do
it
'removes todos for
a user who is not a memb
er'
do
it
'removes todos for
the provided us
er'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
3
).
to
(
1
)
expect
(
user
.
todos
).
to
be_empty
expect
(
project_memb
er
.
todos
).
to
match_array
([
todo_
conf_
issue_
memb
er
])
expect
(
us
er
2
.
todos
).
to
match_array
([
todo_issue_
us
er
2
])
end
end
context
'when project is not private'
do
before
do
group
.
update!
(
visibility_level:
Gitlab
::
VisibilityLevel
::
INTERNAL
)
project
.
update!
(
visibility_level:
Gitlab
::
VisibilityLevel
::
INTERNAL
)
end
context
'when a user is not an author of confidential issue'
do
before
do
group
.
update!
(
visibility_level:
Gitlab
::
VisibilityLevel
::
INTERNAL
)
project
.
update!
(
visibility_level:
Gitlab
::
VisibilityLevel
::
INTERNAL
)
issue
.
update!
(
confidential:
true
)
end
it
'removes only confidential issues todos'
do
...
...
@@ -42,10 +43,7 @@
context
'when a user is an author of confidential issue'
do
before
do
issue
.
update!
(
author:
user
)
group
.
update!
(
visibility_level:
Gitlab
::
VisibilityLevel
::
INTERNAL
)
project
.
update!
(
visibility_level:
Gitlab
::
VisibilityLevel
::
INTERNAL
)
issue
.
update!
(
author:
user
,
confidential:
true
)
end
it
'removes only confidential issues todos'
do
...
...
@@ -55,16 +53,26 @@
context
'when a user is an assignee of confidential issue'
do
before
do
issue
.
update!
(
confidential:
true
)
issue
.
assignees
<<
user
group
.
update!
(
visibility_level:
Gitlab
::
VisibilityLevel
::
INTERNAL
)
project
.
update!
(
visibility_level:
Gitlab
::
VisibilityLevel
::
INTERNAL
)
end
it
'removes only confidential issues todos'
do
expect
{
subject
}.
not_to
change
{
Todo
.
count
}
end
end
context
'feature visibility check'
do
context
'when issues are visible only to project members'
do
before
do
project
.
project_feature
.
update!
(
issues_access_level:
ProjectFeature
::
PRIVATE
)
end
it
'removes only users issue todos'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
3
).
to
(
2
)
end
end
end
end
end
...
...
@@ -72,33 +80,36 @@
subject
{
described_class
.
new
(
user
.
id
,
group
.
id
,
'Group'
).
execute
}
context
'when group is private'
do
it
'removes todos for
a user who is not a memb
er'
do
it
'removes todos for
the us
er'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
3
).
to
(
1
)
expect
(
user
.
todos
).
to
be_empty
expect
(
project_memb
er
.
todos
).
to
match_array
([
todo_
conf_
issue_
memb
er
])
expect
(
us
er
2
.
todos
).
to
match_array
([
todo_issue_
us
er
2
])
end
context
'with nested groups'
,
:nested_groups
do
let
(
:subgroup
)
{
create
(
:group
,
:private
,
parent:
group
)
}
let
(
:subproject
)
{
create
(
:project
,
group:
subgroup
)
}
let!
(
:todo_subproject_
non_memb
er
)
{
create
(
:todo
,
user:
user
,
project:
subproject
)
}
let!
(
:todo_subproject_
memb
er
)
{
create
(
:todo
,
user:
project_memb
er
,
project:
subproject
)
}
let!
(
:todo_subproject_
us
er
)
{
create
(
:todo
,
user:
user
,
project:
subproject
)
}
let!
(
:todo_subproject_
us
er
2
)
{
create
(
:todo
,
user:
us
er
2
,
project:
subproject
)
}
it
'removes todos for
a
user
who is not a member
'
do
it
'removes todos for
the
user
including subprojects todos
'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
5
).
to
(
2
)
expect
(
user
.
todos
).
to
be_empty
expect
(
project_memb
er
.
todos
)
.
to
match_array
([
todo_
conf_
issue_
memb
er
,
todo_subproject_
memb
er
])
expect
(
us
er
2
.
todos
)
.
to
match_array
([
todo_issue_
us
er
2
,
todo_subproject_
us
er
2
])
end
end
end
context
'when group is not private'
do
before
do
issue
.
update!
(
confidential:
true
)
group
.
update!
(
visibility_level:
Gitlab
::
VisibilityLevel
::
INTERNAL
)
project
.
update!
(
visibility_level:
Gitlab
::
VisibilityLevel
::
INTERNAL
)
end
it
'removes only confidential issues todos'
do
...
...
spec/services/todos/destroy/private_features_service_spec.rb
0 → 100644
View file @
1971e54b
require
'spec_helper'
describe
Todos
::
Destroy
::
PrivateFeaturesService
do
let
(
:project
)
{
create
(
:project
,
:public
)
}
let
(
:user
)
{
create
(
:user
)
}
let
(
:another_user
)
{
create
(
:user
)
}
let
(
:project_member
)
{
create
(
:user
)
}
let
(
:issue
)
{
create
(
:issue
,
project:
project
)
}
let
(
:mr
)
{
create
(
:merge_request
,
source_project:
project
)
}
let!
(
:todo_mr_non_member
)
{
create
(
:todo
,
user:
user
,
target:
mr
,
project:
project
)
}
let!
(
:todo_mr_non_member2
)
{
create
(
:todo
,
user:
another_user
,
target:
mr
,
project:
project
)
}
let!
(
:todo_mr_member
)
{
create
(
:todo
,
user:
project_member
,
target:
mr
,
project:
project
)
}
let!
(
:todo_issue_non_member
)
{
create
(
:todo
,
user:
user
,
target:
issue
,
project:
project
)
}
let!
(
:todo_issue_non_member2
)
{
create
(
:todo
,
user:
another_user
,
target:
issue
,
project:
project
)
}
let!
(
:todo_issue_member
)
{
create
(
:todo
,
user:
project_member
,
target:
issue
,
project:
project
)
}
let!
(
:commit_todo_non_member
)
{
create
(
:on_commit_todo
,
user:
user
,
project:
project
)
}
let!
(
:commit_todo_non_member2
)
{
create
(
:on_commit_todo
,
user:
another_user
,
project:
project
)
}
let!
(
:commit_todo_member
)
{
create
(
:on_commit_todo
,
user:
project_member
,
project:
project
)
}
before
do
project
.
add_developer
(
project_member
)
end
context
'when user_id is provided'
do
subject
{
described_class
.
new
(
project
.
id
,
user
.
id
).
execute
}
context
'when all feaures have same visibility as the project'
do
it
'removes only user issue todos'
do
expect
{
subject
}.
not_to
change
{
Todo
.
count
}
end
end
context
'when issues are visible only to project members but the user is a member'
do
before
do
project
.
project_feature
.
update!
(
issues_access_level:
ProjectFeature
::
PRIVATE
)
project
.
add_developer
(
user
)
end
it
'does not remove any todos'
do
expect
{
subject
}.
not_to
change
{
Todo
.
count
}
end
end
context
'when issues are visible only to project members'
do
before
do
project
.
project_feature
.
update!
(
issues_access_level:
ProjectFeature
::
PRIVATE
)
end
it
'removes only user issue todos'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
9
).
to
(
8
)
end
end
context
'when mrs, builds and repository are visible only to project members'
do
before
do
# builds and merge requests cannot have higher visibility than repository
project
.
project_feature
.
update!
(
merge_requests_access_level:
ProjectFeature
::
PRIVATE
)
project
.
project_feature
.
update!
(
builds_access_level:
ProjectFeature
::
PRIVATE
)
project
.
project_feature
.
update!
(
repository_access_level:
ProjectFeature
::
PRIVATE
)
end
it
'removes only user mr and commit todos'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
9
).
to
(
7
)
end
end
context
'when mrs are visible only to project members'
do
before
do
project
.
project_feature
.
update!
(
merge_requests_access_level:
ProjectFeature
::
PRIVATE
)
end
it
'removes only user merge request todo'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
9
).
to
(
8
)
end
end
context
'when mrs and issues are visible only to project members'
do
before
do
project
.
project_feature
.
update!
(
issues_access_level:
ProjectFeature
::
PRIVATE
)
project
.
project_feature
.
update!
(
merge_requests_access_level:
ProjectFeature
::
PRIVATE
)
end
it
'removes only user merge request and issue todos'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
9
).
to
(
7
)
end
end
end
context
'when user_id is not provided'
do
subject
{
described_class
.
new
(
project
.
id
).
execute
}
context
'when all feaures have same visibility as the project'
do
it
'does not remove any todos'
do
expect
{
subject
}.
not_to
change
{
Todo
.
count
}
end
end
context
'when issues are visible only to project members'
do
before
do
project
.
project_feature
.
update!
(
issues_access_level:
ProjectFeature
::
PRIVATE
)
end
it
'removes only non members issue todos'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
9
).
to
(
7
)
end
end
context
'when mrs, builds and repository are visible only to project members'
do
before
do
# builds and merge requests cannot have higher visibility than repository
project
.
project_feature
.
update!
(
merge_requests_access_level:
ProjectFeature
::
PRIVATE
)
project
.
project_feature
.
update!
(
builds_access_level:
ProjectFeature
::
PRIVATE
)
project
.
project_feature
.
update!
(
repository_access_level:
ProjectFeature
::
PRIVATE
)
end
it
'removes only non members mr and commit todos'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
9
).
to
(
5
)
end
end
context
'when mrs are visible only to project members'
do
before
do
project
.
project_feature
.
update!
(
merge_requests_access_level:
ProjectFeature
::
PRIVATE
)
end
it
'removes only non members merge request todos'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
9
).
to
(
7
)
end
end
context
'when mrs and issues are visible only to project members'
do
before
do
project
.
project_feature
.
update!
(
issues_access_level:
ProjectFeature
::
PRIVATE
)
project
.
project_feature
.
update!
(
merge_requests_access_level:
ProjectFeature
::
PRIVATE
)
end
it
'removes only non members merge request and issue todos'
do
expect
{
subject
}.
to
change
{
Todo
.
count
}.
from
(
9
).
to
(
5
)
end
end
end
end
spec/workers/todos_destroyer/private_features_worker_spec.rb
0 → 100644
View file @
1971e54b
require
'spec_helper'
describe
TodosDestroyer
::
PrivateFeaturesWorker
do
it
"calls the Todos::Destroy::PrivateFeaturesService with the params it was given"
do
service
=
double
expect
(
::
Todos
::
Destroy
::
PrivateFeaturesService
).
to
receive
(
:new
).
with
(
100
,
nil
).
and_return
(
service
)
expect
(
service
).
to
receive
(
:execute
)
described_class
.
new
.
perform
(
100
)
end
end
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
.
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment