GitLab merge requestshttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests2017-10-04T15:23:30Zhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2931[EE] Fix the default branches sorting to actually be 'Last updated'2017-10-04T15:23:30Zusername-removed-128633[EE] Fix the default branches sorting to actually be 'Last updated'EE MR for https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14295EE MR for https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1429510.1Robert SpeicherRobert Speicherhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2966Add push_rules is_sample partial index2017-10-04T09:37:08ZGregory StarkAdd push_rules is_sample partial indexAdds a an index on `push_rules.is_sample where is_sample` to address https://gitlab.com/gitlab-org/gitlab-ee/issues/3425. This index is already present in production.
This index is needed to avoid sequential scans on one of the top cons...Adds a an index on `push_rules.is_sample where is_sample` to address https://gitlab.com/gitlab-org/gitlab-ee/issues/3425. This index is already present in production.
This index is needed to avoid sequential scans on one of the top consumers of I/O in the database. This is similar to the `labels.template` case where we have a few (or in the case of gitlab.com, zero) special "template" or "sample" records in an otherwise large table and are doing a huge sequential scan to pick out the few template rows. I would prefer to store these in a separate table for various reasons but as described in the issue that isn't happening soon so to stop the pain create an index today.
```
== 20170920091408 AddIndexForPushrulesIsSample: migrating =====================
-- index_exists?(:push_rules, :is_sample)
-> 0.0016s
-- transaction_open?()
-> 0.0000s
-- execute("SET statement_timeout TO 0")
-> 0.0002s
-- add_index(:push_rules, :is_sample, {:where=>"is_sample", :algorithm=>:concurrently})
-> 0.0593s
== 20170920091408 AddIndexForPushrulesIsSample: migrated (0.0613s) ============
```
On staging (the index was already created in production):
```
gitlabhq_production=# explain analyze SELECT "push_rules".* FROM "push_rules" WHERE "push_rules"."is_sample" = 't' LIMIT 1;
QUERY PLAN
------------------------------------------------------------------------------------------------------------------
Limit (cost=0.00..49222.89 rows=1 width=193) (actual time=20.464..20.465 rows=1 loops=1)
-> Seq Scan on push_rules (cost=0.00..49222.89 rows=1 width=193) (actual time=20.461..20.461 rows=1 loops=1)
Filter: is_sample
Rows Removed by Filter: 173287
Planning time: 0.055 ms
Execution time: 20.488 ms
(6 rows)
gitlabhq_production=# begin;
BEGIN
gitlabhq_production=# create index push_rules_on_is_sample on push_rules (is_sample) where is_sample;
CREATE INDEX
gitlabhq_production=# explain analyze SELECT "push_rules".* FROM "push_rules" WHERE "push_rules"."is_sample" = 't' LIMIT 1;
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------
Limit (cost=0.12..2.14 rows=1 width=193) (actual time=0.019..0.019 rows=1 loops=1)
-> Index Scan using push_rules_on_is_sample on push_rules (cost=0.12..2.14 rows=1 width=193) (actual time=0.017..0.017 rows=1 loops=1)
Planning time: 0.156 ms
Execution time: 0.044 ms
(4 rows)
```
## Database Checklist
When adding migrations:
- [x] Updated `db/schema.rb`
- [x] Added a `down` method so the migration can be reverted
- [x] Added the output of the migration(s) to the MR body
- [x] Added the execution time of the migration(s) to the MR body
- [ ] Added tests for the migration in `spec/migrations` if necessary (e.g. when
migrating data)
- [x] Made sure the migration won't interfere with a running GitLab cluster,
for example by disabling transactions for long running migrations
When adding indexes:
- [x] Described the need for these indexes in the MR body
- [x] Made sure existing indexes can not be reused instead
When removing columns, tables, indexes or other structures:
- [x] Removed these in a post-deployment migration
- [x] Made sure the application no longer uses (or ignores) these structures
## General Checklist
- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added, if necessary
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- [x] Tests added for this feature/bug
- Review
- [x] Has been reviewed by Backend
- [x] Has been reviewed by Database
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
10.1yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/3059GitLab QA fix: Adjust license key add for GitLab 10.0 changes2017-10-04T06:39:56ZStan HuGitLab QA fix: Adjust license key add for GitLab 10.0 changesGitLab 10.0 changed a few text/buttons/links for adding a license. This MR adjusts them to work again.GitLab 10.0 changed a few text/buttons/links for adding a license. This MR adjusts them to work again.10.0Grzegorz BizonGrzegorz Bizonhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/3015Update CI parallelization description and move section about EE-specific test...2017-09-27T14:13:23Zusername-removed-128633Update CI parallelization description and move section about EE-specific tests to its correct placeEE MR for https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14503.EE MR for https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14503.10.1Achilleas PipinellisAchilleas Pipinellishttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/3018Improve the IssuableFinder#SCALAR_PARAMS array formatting and add EE_SCALAR_P...2017-09-26T18:24:54Zusername-removed-128633Improve the IssuableFinder#SCALAR_PARAMS array formatting and add EE_SCALAR_PARAMSThis "upport" what suggested in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14304#note_41425132.This "upport" what suggested in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14304#note_41425132.10.1Grzegorz BizonGrzegorz Bizonhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2953Refactor spec/policies/project_policy_spec.rb to minimize the diff with CE2017-09-25T07:47:49Zusername-removed-128633Refactor spec/policies/project_policy_spec.rb to minimize the diff with CE## What does this MR do?
I discovered the discrepancy while reviewing https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14257/diffs#d458b7cdebc6dde2dcb0e3a6c2b68503219b28d9.
The diff was as follows:
```diff
diff --git a/spec/poli...## What does this MR do?
I discovered the discrepancy while reviewing https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14257/diffs#d458b7cdebc6dde2dcb0e3a6c2b68503219b28d9.
The diff was as follows:
```diff
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 4dbaf7fb025..4e7bbaf83ca 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -6,13 +6,14 @@ describe ProjectPolicy do
let(:dev) { create(:user) }
let(:master) { create(:user) }
let(:owner) { create(:user) }
+ let(:auditor) { create(:user, :auditor) }
let(:admin) { create(:admin) }
let(:project) { create(:project, :public, namespace: owner.namespace) }
let(:guest_permissions) do
%i[
read_project read_board read_list read_wiki read_issue read_label
- read_milestone read_project_snippet read_project_member
+ read_issue_link read_milestone read_project_snippet read_project_member
read_note create_project create_issue create_note
upload_file
]
@@ -21,7 +22,7 @@ describe ProjectPolicy do
let(:reporter_permissions) do
%i[
download_code fork_project create_project_snippet update_issue
- admin_issue admin_label admin_list read_commit_status read_build
+ admin_issue admin_label admin_issue_link admin_list read_commit_status read_build
read_container_image read_pipeline read_environment read_deployment
read_merge_request download_wiki_code
]
@@ -43,7 +44,8 @@ describe ProjectPolicy do
let(:master_permissions) do
%i[
- delete_protected_branch update_project_snippet update_environment
+ push_code_to_protected_branches delete_protected_branch
+ update_project_snippet update_environment
update_deployment admin_milestone admin_project_snippet
admin_project_member admin_note admin_wiki admin_project
admin_commit_status admin_build admin_container_image
@@ -66,6 +68,16 @@ describe ProjectPolicy do
]
end
+ let(:auditor_permissions) do
+ %i[
+ download_code download_wiki_code read_project read_board read_list
+ read_wiki read_issue read_label read_issue_link read_milestone read_project_snippet
+ read_project_member read_note read_cycle_analytics read_pipeline
+ read_build read_commit_status read_container_image read_environment
+ read_deployment read_merge_request read_pages
+ ]
+ end
+
before do
project.team << [guest, :guest]
project.team << [master, :master]
@@ -268,5 +280,33 @@ describe ProjectPolicy do
expect_allowed(*owner_permissions)
end
end
+
+ context 'auditor' do
+ let(:current_user) { auditor }
+
+ context 'not a team member' do
+ it do
+ is_expected.to be_disallowed(*developer_permissions)
+ is_expected.to be_disallowed(*master_permissions)
+ is_expected.to be_disallowed(*owner_permissions)
+ is_expected.to be_disallowed(*(guest_permissions - auditor_permissions))
+ is_expected.to be_allowed(*auditor_permissions)
+ end
+ end
+
+ context 'team member' do
+ before do
+ project.team << [auditor, :guest]
+ end
+
+ it do
+ is_expected.to be_disallowed(*developer_permissions)
+ is_expected.to be_disallowed(*master_permissions)
+ is_expected.to be_disallowed(*owner_permissions)
+ is_expected.to be_allowed(*(guest_permissions - auditor_permissions))
+ is_expected.to be_allowed(*auditor_permissions)
+ end
+ end
+ end
end
end
```10.1Douwe MaanDouwe Maanhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/3000Do not clone the repo when running the review-docs jobs2017-09-22T11:24:21ZAchilleas PipinellisDo not clone the repo when running the review-docs jobs## What does this MR do?
Port from CE https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14420## What does this MR do?
Port from CE https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1442010.1username-removed-128633username-removed-128633https://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2927Enable ee_compat_check for forks, but not EE2017-09-20T11:31:11Zusername-removed-423915Enable ee_compat_check for forks, but not EEEE counterpart of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14189
## What does this MR do?
Enable ee_compat_check for forks, but not EE
## Are there points in the code the reviewer needs to double check?
EE of course doe...EE counterpart of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14189
## What does this MR do?
Enable ee_compat_check for forks, but not EE
## Are there points in the code the reviewer needs to double check?
EE of course doesn't need this, but could we do this without having conflicts?10.1username-removed-128633username-removed-128633https://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2719[EE] Greatly reduce test duration for git_access_spec2017-09-20T10:06:43ZRobert Speicher[EE] Greatly reduce test duration for git_access_specEE version of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13675
This spec is slow -- 20+ minutes on CI. Numbers below are local.
### Baseline
```
Factory usage counts:
name total avg coun...EE version of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13675
This spec is slow -- 20+ minutes on CI. Numbers below are local.
### Baseline
```
Factory usage counts:
name total avg count
project-repository 121.871 0.116 1049
user 78.694 0.031 2569
merge_request 60.41 0.135 448
namespace 40.125 0.038 1065
protected_branch 3.396 0.008 432
group 2.442 0.013 192
Finished in 12 minutes 10 seconds (files took 15.44 seconds to load)
1067 examples, 0 failures
```
### Post cherry-pick of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13675
```
Factory usage counts:
name total avg count
project-repository 36.574 0.108 338
merge_request 22.779 0.136 168
user 20.53 0.028 746
namespace 12.03 0.034 354
group 2.455 0.013 192
deploy_key 1.825 0.13 14
protected_branch 1.608 0.007 222
Finished in 3 minutes 51.1 seconds (files took 14.22 seconds to load)
356 examples, 0 failures
```
### Refactor `run_group_permission_checks`
This uses the same thinking as the earlier `run_permission_checks`
refactor, to run all of the checks for the matrix in a single `it` block
to avoid repeated setup.
```
Factory usage counts:
name total avg count
project-repository 19.041 0.112 170
user 8.969 0.03 298
merge_request 7.754 0.138 56
namespace 6.641 0.036 186
Finished in 1 minute 44.65 seconds (files took 0.91818 seconds to load)
188 examples, 0 failures
```
:tada:
### Fix invalid GitAccess specs for License and secondary Geo node
Due to a logic error, these specs weren't actually doing anything -- we
were running the checks on `Hash.new(Hash.new(false))`, which is just
`{}`.
### Refactor `push_rule_check` GitAccess specs
- Reduces duplication of long ref strings
- Passes push rule attributes directly to `create_push_rule`, rather
than creating one and then updating it.9.5Douwe MaanDouwe Maanhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2655Use rspec-parameterized for table-based tests2017-09-12T20:35:46ZNick ThomasUse rspec-parameterized for table-based tests## What does this MR do?
Adds the `rspec-parameterized` gem, documents it, and converts a number of existing ad-hoc table-based tests to use the framework as a proof of concept.
## Are there points in the code the reviewer needs to dou...## What does this MR do?
Adds the `rspec-parameterized` gem, documents it, and converts a number of existing ad-hoc table-based tests to use the framework as a proof of concept.
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
Action point from %9.4 retrospective
## Screenshots (if relevant)
## Does this MR meet the acceptance criteria?
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/doc/development/doc_styleguide.md)
- Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ee/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Related to https://gitlab.com/gitlab-org/gitlab-ce/3580410.0Robert SpeicherRobert Speicherhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2843[EE] Port changes from gitlab-org/gitlab-ce!13972 to EE2017-09-07T16:17:27Zusername-removed-128633[EE] Port changes from gitlab-org/gitlab-ce!13972 to EE## What does this MR do?
This ports changes from gitlab-org/gitlab-ce!13972 to EE.
## Are there points in the code the reviewer needs to double check?
I had to change a bit of logic in `Ci::CreatePipelineService#execute` and `Ci::Crea...## What does this MR do?
This ports changes from gitlab-org/gitlab-ce!13972 to EE.
## Are there points in the code the reviewer needs to double check?
I had to change a bit of logic in `Ci::CreatePipelineService#execute` and `Ci::CreatePipelineService#validate_project_and_git_items`.
## Does this MR meet the acceptance criteria?
- [ ] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added, if necessary
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
- [ ] Added for this feature/bug
- [ ] All builds are passing
- [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ee/development/merge_request_performance_guidelines.html)
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
/cc @innerwhisper10.0Robert SpeicherRobert Speicherhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2849[EE] Update capybara 2.6.2 -> 2.15.12017-09-07T09:31:57ZRobert Speicher[EE] Update capybara 2.6.2 -> 2.15.1EE version of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13976EE version of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1397610.0username-removed-128633username-removed-128633https://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2847reset namespace columns in migration2017-09-06T18:12:54ZJarka Kadlecovajarka@gitlab.comreset namespace columns in migration## What does this MR do?
It tries to fix the broken build https://gitlab.com/gitlab-org/gitlab-ee/-/jobs/31484051
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
Builds are failing due t...## What does this MR do?
It tries to fix the broken build https://gitlab.com/gitlab-org/gitlab-ee/-/jobs/31484051
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
Builds are failing due to `undefined local variable or method `parent_id' for #<Namespace:0x00000006bdc8c8>` error although this column exists.
## Screenshots (if relevant)
## Does this MR meet the acceptance criteria?
- [ ] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added, if necessary
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
- [ ] Added for this feature/bug
- [ ] All builds are passing
- [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ee/development/merge_request_performance_guidelines.html)
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
#335610.0Robert SpeicherRobert Speicherhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2248Extend documentation on writing EE features2017-09-06T09:31:26ZToon ClaesExtend documentation on writing EE features## What does this MR do?
Add some more context about how EE should act as CE when no valid
license is provided.
## What are the relevant issue numbers?
#2335## What does this MR do?
Add some more context about how EE should act as CE when no valid
license is provided.
## What are the relevant issue numbers?
#233510.0Douwe MaanDouwe Maanhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2770Prevent transient Gitlab::LDAP::Access spec failure2017-09-04T16:18:16ZRobert SpeicherPrevent transient Gitlab::LDAP::Access spec failureWe're unconcerned with ordering here, so just make sure the right values
are present.
See !2757We're unconcerned with ordering here, so just make sure the right values
are present.
See !275710.0username-removed-443319username-removed-443319https://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/963Document a new gotcha when using `prepend`2017-09-04T12:28:15Zusername-removed-128633Document a new gotcha when using `prepend`8.15username-removed-283999douglas@gitlab.comusername-removed-283999douglas@gitlab.comhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2773Remove skipped examples in filtered issues weight feature spec2017-09-01T20:23:44ZRobert SpeicherRemove skipped examples in filtered issues weight feature specEE-specific changes related to https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13845EE-specific changes related to https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/138459.5Douwe MaanDouwe Maanhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2673Move EE specific files (_ee) to the EE directory2017-08-30T17:53:11Zusername-removed-423915Move EE specific files (_ee) to the EE directory## What does this MR do?
Move EE specific files (_ee) to the EE directory
## Are there points in the code the reviewer needs to double check?
Does karma still run for those EE JavaScript tests?
## What are the relevant issue numbers?...## What does this MR do?
Move EE specific files (_ee) to the EE directory
## Are there points in the code the reviewer needs to double check?
Does karma still run for those EE JavaScript tests?
## What are the relevant issue numbers?
See #310710.0Douwe MaanDouwe Maanhttps://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2483Move EE-specific files to a standalone directory2017-08-21T10:09:45Zusername-removed-423915Move EE-specific files to a standalone directory## What does this MR do?
Move EE-specific files to a standalone directory
* [x] app/{models, controllers, etc}
* [x] lib
* [x] app/views
* [x] app/views/admin/monitoring/ee
* [x] app/views/projects/protected_branches/ee
* [x] app/views...## What does this MR do?
Move EE-specific files to a standalone directory
* [x] app/{models, controllers, etc}
* [x] lib
* [x] app/views
* [x] app/views/admin/monitoring/ee
* [x] app/views/projects/protected_branches/ee
* [x] app/views/projects/protected_tags/ee
* [x] app/views/projects/settings/ee
* [x] app/assets/javascripts/vue_merge_request_widget/ee
* [x] app/assets/javascripts/protected_branches/ee
* [x] app/assets/javascripts/protected_tags/ee
* [x] spec
* [ ] qa
## Why was this MR needed?
To make it more clear which codes are EE-specific
## Does this MR meet the acceptance criteria?
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/doc/development/doc_styleguide.md)
## What are the relevant issue numbers?
Closes #29029.5username-removed-128633username-removed-128633https://staging.gitlab.com/gitlab-org/gitlab/-/merge_requests/2662[EE] Whitelist or fix additional `Gitlab/PublicSend` cop violations2017-08-16T11:25:46ZRobert Speicher[EE] Whitelist or fix additional `Gitlab/PublicSend` cop violationsEE version of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13467EE version of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1346710.0username-removed-128633username-removed-128633