From 6b02127f03b1f8e1dbbecccfc2ba16e62aadcbed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 28 Jul 2016 19:28:56 +0200 Subject: New Members::RequestAccessService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/models/project_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 83f61f0af0a..f88636e1c19 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -942,6 +942,14 @@ describe Project, models: true do end end + describe '#request_access' do + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + + it { expect(project.request_access(user)).to be_a(ProjectMember) } + it { expect(project.request_access(user).user).to eq(user) } + end + describe '.search' do let(:project) { create(:project, description: 'kitten mittens') } -- cgit v1.2.1 From 178e2758f685ee124c06d2ce8961950a4d39f013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 9 Sep 2016 17:09:43 +0200 Subject: Fix specs that requires an access request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/models/project_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f88636e1c19..b8f268e95c3 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -68,7 +68,7 @@ describe Project, models: true do it { is_expected.to have_many(:forks).through(:forked_project_links) } describe '#members & #requesters' do - let(:project) { create(:project) } + let(:project) { create(:project, :public) } let(:requester) { create(:user) } let(:developer) { create(:user) } before do -- cgit v1.2.1 From 29cb161e925f787624e7c6b7bdfdeb4d2462c43a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 19 Sep 2016 15:15:01 +0200 Subject: Re-add the AccessRequestable concern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/models/project_spec.rb | 8 -------- 1 file changed, 8 deletions(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b8f268e95c3..98f5305a855 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -942,14 +942,6 @@ describe Project, models: true do end end - describe '#request_access' do - let(:project) { create(:empty_project, :public) } - let(:user) { create(:user) } - - it { expect(project.request_access(user)).to be_a(ProjectMember) } - it { expect(project.request_access(user).user).to eq(user) } - end - describe '.search' do let(:project) { create(:project, description: 'kitten mittens') } -- cgit v1.2.1 From ec0061a95cbba02286b2c143048c93d8f26ff5f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 16 Sep 2016 17:54:21 +0200 Subject: Allow Member.add_user to handle access requesters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes include: - Ensure Member.add_user is not called directly when not necessary - New GroupMember.add_users_to_group to have the same abstraction level as for Project - Refactor Member.add_user to take a source instead of an array of members - Fix Rubocop offenses - Always use Project#add_user instead of project.team.add_user - Factorize users addition as members in Member.add_users_to_source - Make access_level a keyword argument in GroupMember.add_users_to_group and ProjectMember.add_users_to_projects - Destroy any requester before adding them as a member - Improve the way we handle access requesters in Member.add_user Instead of removing the requester and creating a new member, we now simply accepts their access request. This way, they will receive a "access request granted" email. - Fix error that was previously silently ignored - Stop raising when access level is invalid in Member, let Rails validation do their work Signed-off-by: Rémy Coutable --- spec/models/project_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 83f61f0af0a..50423d027ac 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -836,7 +836,7 @@ describe Project, models: true do describe 'when a user has access to a project' do before do - project.team.add_user(user, Gitlab::Access::MASTER) + project.add_user(user, Gitlab::Access::MASTER) end it { is_expected.to eq([project]) } -- cgit v1.2.1 From 706737a004b67303f15ccbd1f8630b0d80f481e9 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 4 Oct 2016 13:21:26 +0200 Subject: Exclude system notes from Project.trending Having many system notes isn't really an indication of a project being trending. Including these notes would lead to projects with lots of commit cross references (for example) showing up in the trending projects list. --- spec/models/project_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ef854a25321..1a316176f63 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -826,6 +826,14 @@ describe Project, models: true do expect(subject).to eq([project2, project1]) end end + + it 'does not take system notes into account' do + 10.times do + create(:note_on_commit, project: project2, system: true) + end + + expect(described_class.trending.to_a).to eq([project1, project2]) + end end describe '.visible_to_user' do -- cgit v1.2.1 From c9bcfc631a79db97203d51d7c221a7549ba65adf Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 4 Oct 2016 17:22:58 +0200 Subject: Remove lease from Event#reset_project_activity Per GitLab.com's performance metrics this method could take up to 5 seconds of wall time to complete, while only taking 1-2 milliseconds of CPU time. Removing the Redis lease in favour of conditional updates allows us to work around this. A slight drawback is that this allows for multiple threads/processes to try and update the same row. However, only a single thread/process will ever win since the UPDATE query uses a WHERE condition to only update rows that were not updated in the last hour. Fixes gitlab-org/gitlab-ce#22473 --- spec/models/project_spec.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ef854a25321..3ab5ac78bba 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -308,8 +308,7 @@ describe Project, models: true do end describe 'last_activity methods' do - let(:timestamp) { Time.now - 2.hours } - let(:project) { create(:project, created_at: timestamp, updated_at: timestamp) } + let(:project) { create(:project, last_activity_at: 2.hours.ago) } describe 'last_activity' do it 'alias last_activity to last_event' do @@ -321,7 +320,6 @@ describe Project, models: true do describe 'last_activity_date' do it 'returns the creation date of the project\'s last event if present' do - expect_any_instance_of(Event).to receive(:try_obtain_lease).and_return(true) new_event = create(:event, project: project, created_at: Time.now) expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i) -- cgit v1.2.1 From e94cd6fdfe43d9128d37a539cf84f4388c5cf970 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Thu, 6 Oct 2016 22:17:11 +0100 Subject: Add markdown cache columns to the database, but don't use them yet This commit adds a number of _html columns and, with the exception of Note, starts updating them whenever the content of their partner fields changes. Note has a collision with the note_html attr_accessor; that will be fixed later A background worker for clearing these cache columns is also introduced - use `rake cache:clear` to set it off. You can clear the database or Redis caches separately by running `rake cache:clear:db` or `rake cache:clear:redis`, respectively. --- spec/models/project_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e52d4aaf884..381d14ed21a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -518,7 +518,7 @@ describe Project, models: true do end describe '#cache_has_external_issue_tracker' do - let(:project) { create(:project) } + let(:project) { create(:project, has_external_issue_tracker: nil) } it 'stores true if there is any external_issue_tracker' do services = double(:service, external_issue_trackers: [RedmineService.new]) -- cgit v1.2.1 From 1662640985b56390a4d22dab1fee7fd04ccd5bc8 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 6 Oct 2016 15:50:29 -0700 Subject: Fix Event#reset_project_activity updates !6678 removed the lease from Event#reset_project_activity, but it wasn't actually updating the project's last_activity_at timestamp properly. The WHERE clause would always return no matching projects. The spec passed occasionally because the created_at timestamp was automatically set to last_activity_at. --- spec/models/project_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e52d4aaf884..67a968aaf11 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -308,7 +308,9 @@ describe Project, models: true do end describe 'last_activity methods' do - let(:project) { create(:project, last_activity_at: 2.hours.ago) } + let(:timestamp) { 2.hours.ago } + # last_activity_at gets set to created_at upon creation + let(:project) { create(:project, created_at: timestamp, updated_at: timestamp) } describe 'last_activity' do it 'alias last_activity to last_event' do @@ -322,6 +324,7 @@ describe Project, models: true do it 'returns the creation date of the project\'s last event if present' do new_event = create(:event, project: project, created_at: Time.now) + project.reload expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i) end -- cgit v1.2.1 From 237c8f66e6608420629503280aaea555ee980022 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 7 Oct 2016 15:24:09 +0200 Subject: Precalculate trending projects This commit introduces a Sidekiq worker that precalculates the list of trending projects on a daily basis. The resulting set is stored in a database table that is then queried by Project.trending. This setup means that Unicorn workers no longer _may_ have to calculate the list of trending projects. Furthermore it supports filtering without any complex caching mechanisms. The data in the "trending_projects" table is inserted in the same order as the project ranking. This means that getting the projects in the correct order is simply a matter of: SELECT projects.* FROM projects INNER JOIN trending_projects ON trending_projects.project_id = projects.id ORDER BY trending_projects.id ASC; Such a query will only take a few milliseconds at most (as measured on GitLab.com), opposed to a few seconds for the query used for calculating the project ranks. The migration in this commit does not require downtime and takes care of populating an initial list of trending projects. --- spec/models/project_spec.rb | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8aadfcb439b..dae546a0cdc 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -800,32 +800,14 @@ describe Project, models: true do end create(:note_on_commit, project: project2) - end - - describe 'without an explicit start date' do - subject { described_class.trending.to_a } - it 'sorts Projects by the amount of notes in descending order' do - expect(subject).to eq([project1, project2]) - end + TrendingProject.refresh! end - describe 'with an explicit start date' do - let(:date) { 2.months.ago } + subject { described_class.trending.to_a } - subject { described_class.trending(date).to_a } - - before do - 2.times do - # Little fix for special issue related to Fractional Seconds support for MySQL. - # See: https://github.com/rails/rails/pull/14359/files - create(:note_on_commit, project: project2, created_at: date + 1) - end - end - - it 'sorts Projects by the amount of notes in descending order' do - expect(subject).to eq([project2, project1]) - end + it 'sorts projects by the amount of notes in descending order' do + expect(subject).to eq([project1, project2]) end it 'does not take system notes into account' do -- cgit v1.2.1 From fb5a4202062d07d2dbca544f4cfb475a65411716 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 5 Oct 2016 11:42:37 -0300 Subject: Allow projects to have many boards --- spec/models/project_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index dae546a0cdc..3748b1c7f5f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -24,7 +24,7 @@ describe Project, models: true do it { is_expected.to have_one(:slack_service).dependent(:destroy) } it { is_expected.to have_one(:pushover_service).dependent(:destroy) } it { is_expected.to have_one(:asana_service).dependent(:destroy) } - it { is_expected.to have_one(:board).dependent(:destroy) } + it { is_expected.to have_many(:boards).dependent(:destroy) } it { is_expected.to have_one(:campfire_service).dependent(:destroy) } it { is_expected.to have_one(:drone_ci_service).dependent(:destroy) } it { is_expected.to have_one(:emails_on_push_service).dependent(:destroy) } -- cgit v1.2.1 From 95a5cc9285a8583988ece697ebdb948730b5db55 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 6 Oct 2016 15:58:28 -0300 Subject: Restrict the number of permitted boards per project to one --- spec/models/project_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3748b1c7f5f..1b13f1be477 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -94,6 +94,15 @@ describe Project, models: true do end end end + + describe '#boards' do + it 'raises an error when attempting to add more than one board to the project' do + subject.boards.build + + expect { subject.boards.build }.to raise_error(StandardError, 'Number of permitted boards exceeded') + expect(subject.boards.size).to eq 1 + end + end end describe 'modules' do -- cgit v1.2.1 From 2ad531f5e279bcd278600d1f95ff9d4e4988b034 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 10 Oct 2016 11:22:30 -0300 Subject: Add Project::BoardLimitExcedeed error class --- spec/models/project_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1b13f1be477..308a00db9cd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -99,7 +99,7 @@ describe Project, models: true do it 'raises an error when attempting to add more than one board to the project' do subject.boards.build - expect { subject.boards.build }.to raise_error(StandardError, 'Number of permitted boards exceeded') + expect { subject.boards.build }.to raise_error(Project::BoardLimitExceeded, 'Number of permitted boards exceeded') expect(subject.boards.size).to eq 1 end end -- cgit v1.2.1 From bba47886264d0ca7650d2b4b202d69984650b0ae Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Mon, 10 Oct 2016 09:40:14 +0200 Subject: Extract project#update_merge_requests and SystemHooks to its own worker from GitPushService --- spec/models/project_spec.rb | 21 --------------------- 1 file changed, 21 deletions(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index dae546a0cdc..7435713d4c0 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -219,7 +219,6 @@ describe Project, models: true do describe 'Respond to' do it { is_expected.to respond_to(:url_to_repo) } it { is_expected.to respond_to(:repo_exists?) } - it { is_expected.to respond_to(:update_merge_requests) } it { is_expected.to respond_to(:execute_hooks) } it { is_expected.to respond_to(:owner) } it { is_expected.to respond_to(:path_with_namespace) } @@ -380,26 +379,6 @@ describe Project, models: true do end end - describe '#update_merge_requests' do - let(:project) { create(:project) } - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let(:key) { create(:key, user_id: project.owner.id) } - let(:prev_commit_id) { merge_request.commits.last.id } - let(:commit_id) { merge_request.commits.first.id } - - it 'closes merge request if last commit from source branch was pushed to target branch' do - project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.target_branch}", key.user) - merge_request.reload - expect(merge_request.merged?).to be_truthy - end - - it 'updates merge request commits with new one if pushed to source branch' do - project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.source_branch}", key.user) - merge_request.reload - expect(merge_request.diff_head_sha).to eq(commit_id) - end - end - describe '.find_with_namespace' do context 'with namespace' do before do -- cgit v1.2.1 From cfedd42badc6b84457d1de35cb31988777462d5a Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 20 Sep 2016 17:07:56 -0300 Subject: Add ProjectLabel model --- spec/models/project_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 67dbcc362f6..e6d98e25d0b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -56,7 +56,7 @@ describe Project, models: true do it { is_expected.to have_many(:runners) } it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:triggers) } - it { is_expected.to have_many(:labels).dependent(:destroy) } + it { is_expected.to have_many(:labels).class_name('ProjectLabel').dependent(:destroy) } it { is_expected.to have_many(:users_star_projects).dependent(:destroy) } it { is_expected.to have_many(:environments).dependent(:destroy) } it { is_expected.to have_many(:deployments).dependent(:destroy) } -- cgit v1.2.1 From ed9838cd292ce78ae98079f513d0d1648b7f49f0 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Fri, 14 Oct 2016 19:38:41 -0300 Subject: Create project feature when project is created --- spec/models/project_spec.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'spec/models/project_spec.rb') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e6d98e25d0b..f4dda1ee558 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -67,6 +67,14 @@ describe Project, models: true do it { is_expected.to have_many(:notification_settings).dependent(:destroy) } it { is_expected.to have_many(:forks).through(:forked_project_links) } + context 'after create' do + it "creates project feature" do + project = FactoryGirl.build(:project) + + expect { project.save }.to change{ project.project_feature.present? }.from(false).to(true) + end + end + describe '#members & #requesters' do let(:project) { create(:project, :public) } let(:requester) { create(:user) } @@ -531,9 +539,9 @@ describe Project, models: true do end describe '#has_wiki?' do - let(:no_wiki_project) { build(:project, wiki_enabled: false, has_external_wiki: false) } - let(:wiki_enabled_project) { build(:project) } - let(:external_wiki_project) { build(:project, has_external_wiki: true) } + let(:no_wiki_project) { create(:project, wiki_access_level: ProjectFeature::DISABLED, has_external_wiki: false) } + let(:wiki_enabled_project) { create(:project) } + let(:external_wiki_project) { create(:project, has_external_wiki: true) } it 'returns true if project is wiki enabled or has external wiki' do expect(wiki_enabled_project).to have_wiki -- cgit v1.2.1