Commit 345b3d4b72d3e450a65ec92806be019be512da54
1 parent
645e8d47
Exists in
spb-stable
and in
3 other branches
Update tests and fix Finders readme
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Showing
6 changed files
with
149 additions
and
137 deletions
Show diff stats
app/finders/README.md
| @@ -3,7 +3,7 @@ | @@ -3,7 +3,7 @@ | ||
| 3 | This type of classes responsible for collectiong items based on different conditions. | 3 | This type of classes responsible for collectiong items based on different conditions. |
| 4 | To prevent lookup methods in models like this: | 4 | To prevent lookup methods in models like this: |
| 5 | 5 | ||
| 6 | -``` | 6 | +```ruby |
| 7 | class Project | 7 | class Project |
| 8 | def issues_for_user_filtered_by(user, filter) | 8 | def issues_for_user_filtered_by(user, filter) |
| 9 | # A lot of logic not related to project model itself | 9 | # A lot of logic not related to project model itself |
| @@ -15,10 +15,8 @@ issues = project.issues_for_user_filtered_by(user, params) | @@ -15,10 +15,8 @@ issues = project.issues_for_user_filtered_by(user, params) | ||
| 15 | 15 | ||
| 16 | Better use this: | 16 | Better use this: |
| 17 | 17 | ||
| 18 | -``` | ||
| 19 | -selector = Finders::Issues.new | ||
| 20 | - | ||
| 21 | -issues = selector.execute(project, user, filter) | 18 | +```ruby |
| 19 | +issues = IssuesFinder.new.execute(project, user, filter) | ||
| 22 | ``` | 20 | ``` |
| 23 | 21 | ||
| 24 | It will help keep models thiner | 22 | It will help keep models thiner |
| @@ -0,0 +1,58 @@ | @@ -0,0 +1,58 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe IssuesFinder do | ||
| 4 | + let(:user) { create :user } | ||
| 5 | + let(:user2) { create :user } | ||
| 6 | + let(:project1) { create(:project) } | ||
| 7 | + let(:project2) { create(:project) } | ||
| 8 | + let(:issue1) { create(:issue, assignee: user, project: project1) } | ||
| 9 | + let(:issue2) { create(:issue, assignee: user, project: project2) } | ||
| 10 | + let(:issue3) { create(:issue, assignee: user2, project: project2) } | ||
| 11 | + | ||
| 12 | + before do | ||
| 13 | + project1.team << [user, :master] | ||
| 14 | + project2.team << [user, :developer] | ||
| 15 | + project2.team << [user2, :developer] | ||
| 16 | + end | ||
| 17 | + | ||
| 18 | + describe :execute do | ||
| 19 | + before :each do | ||
| 20 | + issue1 | ||
| 21 | + issue2 | ||
| 22 | + issue3 | ||
| 23 | + end | ||
| 24 | + | ||
| 25 | + it 'should filter by all' do | ||
| 26 | + params = { scope: "all", state: 'opened' } | ||
| 27 | + issues = IssuesFinder.new.execute(user, params) | ||
| 28 | + issues.size.should == 3 | ||
| 29 | + end | ||
| 30 | + | ||
| 31 | + it 'should filter by assignee' do | ||
| 32 | + params = { scope: "assigned-to-me", state: 'opened' } | ||
| 33 | + issues = IssuesFinder.new.execute(user, params) | ||
| 34 | + issues.size.should == 2 | ||
| 35 | + end | ||
| 36 | + | ||
| 37 | + it 'should filter by project' do | ||
| 38 | + params = { scope: "assigned-to-me", state: 'opened', project_id: project1.id } | ||
| 39 | + issues = IssuesFinder.new.execute(user, params) | ||
| 40 | + issues.size.should == 1 | ||
| 41 | + end | ||
| 42 | + | ||
| 43 | + it 'should be empty for unauthorized user' do | ||
| 44 | + params = { scope: "all", state: 'opened' } | ||
| 45 | + issues = IssuesFinder.new.execute(nil, params) | ||
| 46 | + issues.size.should be_zero | ||
| 47 | + end | ||
| 48 | + | ||
| 49 | + it 'should not include unauthorized issues' do | ||
| 50 | + params = { scope: "all", state: 'opened' } | ||
| 51 | + issues = IssuesFinder.new.execute(user2, params) | ||
| 52 | + issues.size.should == 2 | ||
| 53 | + issues.should_not include(issue1) | ||
| 54 | + issues.should include(issue2) | ||
| 55 | + issues.should include(issue3) | ||
| 56 | + end | ||
| 57 | + end | ||
| 58 | +end |
| @@ -0,0 +1,37 @@ | @@ -0,0 +1,37 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe MergeRequestsFinder do | ||
| 4 | + let(:user) { create :user } | ||
| 5 | + let(:user2) { create :user } | ||
| 6 | + let(:project1) { create(:project) } | ||
| 7 | + let(:project2) { create(:project) } | ||
| 8 | + let(:merge_request1) { create(:merge_request, author: user, source_project: project1, target_project: project2) } | ||
| 9 | + let(:merge_request2) { create(:merge_request, author: user, source_project: project2, target_project: project1) } | ||
| 10 | + let(:merge_request3) { create(:merge_request, author: user, source_project: project2, target_project: project2) } | ||
| 11 | + | ||
| 12 | + before do | ||
| 13 | + project1.team << [user, :master] | ||
| 14 | + project2.team << [user, :developer] | ||
| 15 | + project2.team << [user2, :developer] | ||
| 16 | + end | ||
| 17 | + | ||
| 18 | + describe :execute do | ||
| 19 | + before :each do | ||
| 20 | + merge_request1 | ||
| 21 | + merge_request2 | ||
| 22 | + merge_request3 | ||
| 23 | + end | ||
| 24 | + | ||
| 25 | + it 'should filter by scope' do | ||
| 26 | + params = { scope: 'authored', state: 'opened' } | ||
| 27 | + merge_requests = MergeRequestsFinder.new.execute(user, params) | ||
| 28 | + merge_requests.size.should == 3 | ||
| 29 | + end | ||
| 30 | + | ||
| 31 | + it 'should filter by project' do | ||
| 32 | + params = { project_id: project1.id, scope: 'authored', state: 'opened' } | ||
| 33 | + merge_requests = MergeRequestsFinder.new.execute(user, params) | ||
| 34 | + merge_requests.size.should == 1 | ||
| 35 | + end | ||
| 36 | + end | ||
| 37 | +end |
| @@ -0,0 +1,51 @@ | @@ -0,0 +1,51 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe ProjectsFinder do | ||
| 4 | + let(:user) { create :user } | ||
| 5 | + let(:group) { create :group } | ||
| 6 | + | ||
| 7 | + let(:project1) { create(:empty_project, group: group, visibility_level: Project::PUBLIC) } | ||
| 8 | + let(:project2) { create(:empty_project, group: group, visibility_level: Project::INTERNAL) } | ||
| 9 | + let(:project3) { create(:empty_project, group: group, visibility_level: Project::PRIVATE) } | ||
| 10 | + let(:project4) { create(:empty_project, group: group, visibility_level: Project::PRIVATE) } | ||
| 11 | + | ||
| 12 | + context 'non authenticated' do | ||
| 13 | + subject { ProjectsFinder.new.execute(nil, group: group) } | ||
| 14 | + | ||
| 15 | + it { should include(project1) } | ||
| 16 | + it { should_not include(project2) } | ||
| 17 | + it { should_not include(project3) } | ||
| 18 | + it { should_not include(project4) } | ||
| 19 | + end | ||
| 20 | + | ||
| 21 | + context 'authenticated' do | ||
| 22 | + subject { ProjectsFinder.new.execute(user, group: group) } | ||
| 23 | + | ||
| 24 | + it { should include(project1) } | ||
| 25 | + it { should include(project2) } | ||
| 26 | + it { should_not include(project3) } | ||
| 27 | + it { should_not include(project4) } | ||
| 28 | + end | ||
| 29 | + | ||
| 30 | + context 'authenticated, project member' do | ||
| 31 | + before { project3.team << [user, :developer] } | ||
| 32 | + | ||
| 33 | + subject { ProjectsFinder.new.execute(user, group: group) } | ||
| 34 | + | ||
| 35 | + it { should include(project1) } | ||
| 36 | + it { should include(project2) } | ||
| 37 | + it { should include(project3) } | ||
| 38 | + it { should_not include(project4) } | ||
| 39 | + end | ||
| 40 | + | ||
| 41 | + context 'authenticated, group member' do | ||
| 42 | + before { group.add_user(user, Gitlab::Access::DEVELOPER) } | ||
| 43 | + | ||
| 44 | + subject { ProjectsFinder.new.execute(user, group: group) } | ||
| 45 | + | ||
| 46 | + it { should include(project1) } | ||
| 47 | + it { should include(project2) } | ||
| 48 | + it { should include(project3) } | ||
| 49 | + it { should include(project4) } | ||
| 50 | + end | ||
| 51 | +end |
spec/services/filtering_service_spec.rb
| @@ -1,81 +0,0 @@ | @@ -1,81 +0,0 @@ | ||
| 1 | -require 'spec_helper' | ||
| 2 | - | ||
| 3 | -describe FilteringService do | ||
| 4 | - let(:user) { create :user } | ||
| 5 | - let(:user2) { create :user } | ||
| 6 | - let(:project1) { create(:project) } | ||
| 7 | - let(:project2) { create(:project) } | ||
| 8 | - let(:merge_request1) { create(:merge_request, author: user, source_project: project1, target_project: project2) } | ||
| 9 | - let(:merge_request2) { create(:merge_request, author: user, source_project: project2, target_project: project1) } | ||
| 10 | - let(:merge_request3) { create(:merge_request, author: user, source_project: project2, target_project: project2) } | ||
| 11 | - let(:issue1) { create(:issue, assignee: user, project: project1) } | ||
| 12 | - let(:issue2) { create(:issue, assignee: user, project: project2) } | ||
| 13 | - let(:issue3) { create(:issue, assignee: user2, project: project2) } | ||
| 14 | - | ||
| 15 | - before do | ||
| 16 | - project1.team << [user, :master] | ||
| 17 | - project2.team << [user, :developer] | ||
| 18 | - project2.team << [user2, :developer] | ||
| 19 | - end | ||
| 20 | - | ||
| 21 | - describe 'merge requests' do | ||
| 22 | - before :each do | ||
| 23 | - merge_request1 | ||
| 24 | - merge_request2 | ||
| 25 | - merge_request3 | ||
| 26 | - end | ||
| 27 | - | ||
| 28 | - it 'should filter by scope' do | ||
| 29 | - params = { scope: 'authored', state: 'opened' } | ||
| 30 | - merge_requests = FilteringService.new.execute(MergeRequest, user, params) | ||
| 31 | - merge_requests.size.should == 3 | ||
| 32 | - end | ||
| 33 | - | ||
| 34 | - it 'should filter by project' do | ||
| 35 | - params = { project_id: project1.id, scope: 'authored', state: 'opened' } | ||
| 36 | - merge_requests = FilteringService.new.execute(MergeRequest, user, params) | ||
| 37 | - merge_requests.size.should == 1 | ||
| 38 | - end | ||
| 39 | - end | ||
| 40 | - | ||
| 41 | - describe 'issues' do | ||
| 42 | - before :each do | ||
| 43 | - issue1 | ||
| 44 | - issue2 | ||
| 45 | - issue3 | ||
| 46 | - end | ||
| 47 | - | ||
| 48 | - it 'should filter by all' do | ||
| 49 | - params = { scope: "all", state: 'opened' } | ||
| 50 | - issues = FilteringService.new.execute(Issue, user, params) | ||
| 51 | - issues.size.should == 3 | ||
| 52 | - end | ||
| 53 | - | ||
| 54 | - it 'should filter by assignee' do | ||
| 55 | - params = { scope: "assigned-to-me", state: 'opened' } | ||
| 56 | - issues = FilteringService.new.execute(Issue, user, params) | ||
| 57 | - issues.size.should == 2 | ||
| 58 | - end | ||
| 59 | - | ||
| 60 | - it 'should filter by project' do | ||
| 61 | - params = { scope: "assigned-to-me", state: 'opened', project_id: project1.id } | ||
| 62 | - issues = FilteringService.new.execute(Issue, user, params) | ||
| 63 | - issues.size.should == 1 | ||
| 64 | - end | ||
| 65 | - | ||
| 66 | - it 'should be empty for unauthorized user' do | ||
| 67 | - params = { scope: "all", state: 'opened' } | ||
| 68 | - issues = FilteringService.new.execute(Issue, nil, params) | ||
| 69 | - issues.size.should be_zero | ||
| 70 | - end | ||
| 71 | - | ||
| 72 | - it 'should not include unauthorized issues' do | ||
| 73 | - params = { scope: "all", state: 'opened' } | ||
| 74 | - issues = FilteringService.new.execute(Issue, user2, params) | ||
| 75 | - issues.size.should == 2 | ||
| 76 | - issues.should_not include(issue1) | ||
| 77 | - issues.should include(issue2) | ||
| 78 | - issues.should include(issue3) | ||
| 79 | - end | ||
| 80 | - end | ||
| 81 | -end |
spec/services/projects_collect_service_spec.rb
| @@ -1,51 +0,0 @@ | @@ -1,51 +0,0 @@ | ||
| 1 | -require 'spec_helper' | ||
| 2 | - | ||
| 3 | -describe Projects::CollectService do | ||
| 4 | - let(:user) { create :user } | ||
| 5 | - let(:group) { create :group } | ||
| 6 | - | ||
| 7 | - let(:project1) { create(:empty_project, group: group, visibility_level: Project::PUBLIC) } | ||
| 8 | - let(:project2) { create(:empty_project, group: group, visibility_level: Project::INTERNAL) } | ||
| 9 | - let(:project3) { create(:empty_project, group: group, visibility_level: Project::PRIVATE) } | ||
| 10 | - let(:project4) { create(:empty_project, group: group, visibility_level: Project::PRIVATE) } | ||
| 11 | - | ||
| 12 | - context 'non authenticated' do | ||
| 13 | - subject { Projects::CollectService.new.execute(nil, group: group) } | ||
| 14 | - | ||
| 15 | - it { should include(project1) } | ||
| 16 | - it { should_not include(project2) } | ||
| 17 | - it { should_not include(project3) } | ||
| 18 | - it { should_not include(project4) } | ||
| 19 | - end | ||
| 20 | - | ||
| 21 | - context 'authenticated' do | ||
| 22 | - subject { Projects::CollectService.new.execute(user, group: group) } | ||
| 23 | - | ||
| 24 | - it { should include(project1) } | ||
| 25 | - it { should include(project2) } | ||
| 26 | - it { should_not include(project3) } | ||
| 27 | - it { should_not include(project4) } | ||
| 28 | - end | ||
| 29 | - | ||
| 30 | - context 'authenticated, project member' do | ||
| 31 | - before { project3.team << [user, :developer] } | ||
| 32 | - | ||
| 33 | - subject { Projects::CollectService.new.execute(user, group: group) } | ||
| 34 | - | ||
| 35 | - it { should include(project1) } | ||
| 36 | - it { should include(project2) } | ||
| 37 | - it { should include(project3) } | ||
| 38 | - it { should_not include(project4) } | ||
| 39 | - end | ||
| 40 | - | ||
| 41 | - context 'authenticated, group member' do | ||
| 42 | - before { group.add_user(user, Gitlab::Access::DEVELOPER) } | ||
| 43 | - | ||
| 44 | - subject { Projects::CollectService.new.execute(user, group: group) } | ||
| 45 | - | ||
| 46 | - it { should include(project1) } | ||
| 47 | - it { should include(project2) } | ||
| 48 | - it { should include(project3) } | ||
| 49 | - it { should include(project4) } | ||
| 50 | - end | ||
| 51 | -end |