diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2016-07-20 21:13:02 +0300 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2016-07-29 13:51:18 +0300 |
commit | 002ad215818450d2cbbc5fa065850a953dc7ada8 (patch) | |
tree | 9ac1703e56bd460fd24c7a5c93a5c465a83127b7 /spec | |
parent | 9b0e131b83cfc44d3132bddfefb6cbd4bff7d253 (diff) |
Method for returning issues readable by a user
The method Ability.issues_readable_by_user takes a list of users and an
optional user and returns an Array of issues readable by said user. This
method in turn is used by
Banzai::ReferenceParser::IssueParser#nodes_visible_to_user so this
method no longer needs to get all the available abilities just to check
if a user has the "read_issue" ability.
To test this I benchmarked an issue with 222 comments on my development
environment. Using these changes the time spent in nodes_visible_to_user
was reduced from around 120 ms to around 40 ms.
Diffstat (limited to 'spec')
-rw-r--r-- | spec/lib/banzai/reference_parser/issue_parser_spec.rb | 12 | ||||
-rw-r--r-- | spec/models/ability_spec.rb | 48 | ||||
-rw-r--r-- | spec/models/concerns/mentionable_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/issue_spec.rb | 253 |
4 files changed, 312 insertions, 7 deletions
diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb index 514c752546d..85cfe728b6a 100644 --- a/spec/lib/banzai/reference_parser/issue_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb @@ -16,17 +16,17 @@ describe Banzai::ReferenceParser::IssueParser, lib: true do end it 'returns the nodes when the user can read the issue' do - expect(Ability.abilities).to receive(:allowed?). - with(user, :read_issue, issue). - and_return(true) + expect(Ability).to receive(:issues_readable_by_user). + with([issue], user). + and_return([issue]) expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) end it 'returns an empty Array when the user can not read the issue' do - expect(Ability.abilities).to receive(:allowed?). - with(user, :read_issue, issue). - and_return(false) + expect(Ability).to receive(:issues_readable_by_user). + with([issue], user). + and_return([]) expect(subject.nodes_visible_to_user(user, [link])).to eq([]) end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index cd5f40fe3d2..853f6943cef 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -170,4 +170,52 @@ describe Ability, lib: true do end end end + + describe '.issues_readable_by_user' do + context 'with an admin user' do + it 'returns all given issues' do + user = build(:user, admin: true) + issue = build(:issue) + + expect(described_class.issues_readable_by_user([issue], user)). + to eq([issue]) + end + end + + context 'with a regular user' do + it 'returns the issues readable by the user' do + user = build(:user) + issue = build(:issue) + + expect(issue).to receive(:readable_by?).with(user).and_return(true) + + expect(described_class.issues_readable_by_user([issue], user)). + to eq([issue]) + end + + it 'returns an empty Array when no issues are readable' do + user = build(:user) + issue = build(:issue) + + expect(issue).to receive(:readable_by?).with(user).and_return(false) + + expect(described_class.issues_readable_by_user([issue], user)).to eq([]) + end + end + + context 'without a regular user' do + it 'returns issues that are publicly visible' do + hidden_issue = build(:issue) + visible_issue = build(:issue) + + expect(hidden_issue).to receive(:publicly_visible?).and_return(false) + expect(visible_issue).to receive(:publicly_visible?).and_return(true) + + issues = described_class. + issues_readable_by_user([hidden_issue, visible_issue]) + + expect(issues).to eq([visible_issue]) + end + end + end end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 5e652660e2c..549b0042038 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -68,7 +68,7 @@ describe Issue, "Mentionable" do describe '#create_cross_references!' do let(:project) { create(:project) } - let(:author) { double('author') } + let(:author) { build(:user) } let(:commit) { project.commit } let(:commit2) { project.commit } @@ -88,6 +88,10 @@ describe Issue, "Mentionable" do let(:author) { create(:author) } let(:issues) { create_list(:issue, 2, project: project, author: author) } + before do + project.team << [author, Gitlab::Access::DEVELOPER] + end + context 'before changes are persisted' do it 'ignores pre-existing references' do issue = create_issue(description: issues[0].to_reference) diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 6a897c96690..3259f795296 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -306,4 +306,257 @@ describe Issue, models: true do expect(user2.assigned_open_issues_count).to eq(1) end end + + describe '#visible_to_user?' do + context 'with a user' do + let(:user) { build(:user) } + let(:issue) { build(:issue) } + + it 'returns true when the issue is readable' do + expect(issue).to receive(:readable_by?).with(user).and_return(true) + + expect(issue.visible_to_user?(user)).to eq(true) + end + + it 'returns false when the issue is not readable' do + expect(issue).to receive(:readable_by?).with(user).and_return(false) + + expect(issue.visible_to_user?(user)).to eq(false) + end + end + + context 'without a user' do + let(:issue) { build(:issue) } + + it 'returns true when the issue is publicly visible' do + expect(issue).to receive(:publicly_visible?).and_return(true) + + expect(issue.visible_to_user?).to eq(true) + end + + it 'returns false when the issue is not publicly visible' do + expect(issue).to receive(:publicly_visible?).and_return(false) + + expect(issue.visible_to_user?).to eq(false) + end + end + end + + describe '#readable_by?' do + describe 'with a regular user that is not a team member' do + let(:user) { create(:user) } + + context 'using a public project' do + let(:project) { create(:empty_project, :public) } + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, project: project, confidential: true) + + expect(issue).not_to be_readable_by(user) + end + end + + context 'using an internal project' do + let(:project) { create(:empty_project, :internal) } + + context 'using an internal user' do + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + end + + context 'using an external user' do + before do + allow(user).to receive(:external?).and_return(true) + end + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + end + end + + context 'using a private project' do + let(:project) { create(:empty_project, :private) } + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_readable_by(user) + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + + context 'when the user is the project owner' do + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_readable_by(user) + end + end + end + end + + context 'with a regular user that is a team member' do + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public) } + + context 'using a public project' do + before do + project.team << [user, Gitlab::Access::DEVELOPER] + end + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + + context 'using an internal project' do + let(:project) { create(:empty_project, :internal) } + + before do + project.team << [user, Gitlab::Access::DEVELOPER] + end + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + + context 'using a private project' do + let(:project) { create(:empty_project, :private) } + + before do + project.team << [user, Gitlab::Access::DEVELOPER] + end + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + end + + context 'with an admin user' do + let(:project) { create(:empty_project) } + let(:user) { create(:user, admin: true) } + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_readable_by(user) + end + + it 'returns true for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).to be_readable_by(user) + end + end + end + + describe '#publicly_visible?' do + context 'using a public project' do + let(:project) { create(:empty_project, :public) } + + it 'returns true for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).to be_publicly_visible + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_publicly_visible + end + end + + context 'using an internal project' do + let(:project) { create(:empty_project, :internal) } + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_publicly_visible + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_publicly_visible + end + end + + context 'using a private project' do + let(:project) { create(:empty_project, :private) } + + it 'returns false for a regular issue' do + issue = build(:issue, project: project) + + expect(issue).not_to be_publicly_visible + end + + it 'returns false for a confidential issue' do + issue = build(:issue, :confidential, project: project) + + expect(issue).not_to be_publicly_visible + end + end + end end |