Merge branch 'master' into 8-7-stable

CHANGELOG

@@ -12,6 +12,7 @@ v 8.7.0 (unreleased)
   - Allow back dating on issues when created through the API
   - Fix Error 500 after renaming a project path (Stan Hu)
   - Fix avatar stretching by providing a cropping feature
+  - API: Expose `subscribed` for issues and merge requests (Robert Schilling)
   - Allow SAML to handle external users based on user's information !3530
   - Add endpoints to archive or unarchive a project !3372
   - Add links to CI setup documentation from project settings and builds pages

CONTRIBUTING.md

@@ -448,7 +448,7 @@ merge request:
     - multi-line method chaining style **Option B**: dot `.` on previous line
     - string literal quoting style **Option A**: single quoted by default
 1.  [Rails](https://github.com/bbatsov/rails-style-guide)
-1.  [Testing](https://github.com/thoughtbot/guides/tree/master/style/testing)
+1.  [Testing](doc/development/testing.md)
 1.  [CoffeeScript](https://github.com/thoughtbot/guides/tree/master/style/coffeescript)
 1.  [SCSS styleguide][scss-styleguide]
 1.  [Shell commands](doc/development/shell_commands.md) created by GitLab

GITLAB_SHELL_VERSION

-2.7.0
+2.7.2

GITLAB_WORKHORSE_VERSION

-0.7.1
+0.7.2

app/assets/javascripts/todos.js.coffee

@@ -57,5 +57,10 @@ class @Todos
     $('.todos-pending .badge, .todos-pending-count').text data.count
     $('.todos-done .badge').text data.done_count
 
-  goToTodoUrl: ->
-    Turbolinks.visit($(this).data('url'))
+  goToTodoUrl: (e)->
+    todoLink = $(this).data('url')
+    if e.metaKey
+      e.preventDefault()
+      window.open(todoLink,'_blank')
+    else
+      Turbolinks.visit(todoLink)

doc/api/issues.md

@@ -76,8 +76,9 @@ Example response:
       "title" : "Consequatur vero maxime deserunt laboriosam est voluptas dolorem.",
       "created_at" : "2016-01-04T15:31:51.081Z",
       "iid" : 6,
-      "labels" : []
-   },
+      "labels" : [],
+      "subscribed" : false
+   }
 ]
 ```
 
@@ -152,7 +153,8 @@ Example response:
       "id" : 41,
       "title" : "Ut commodi ullam eos dolores perferendis nihil sunt.",
       "updated_at" : "2016-01-04T15:31:46.176Z",
-      "created_at" : "2016-01-04T15:31:46.176Z"
+      "created_at" : "2016-01-04T15:31:46.176Z",
+      "subscribed" : false
    }
 ]
 ```
@@ -213,7 +215,8 @@ Example response:
    "id" : 41,
    "title" : "Ut commodi ullam eos dolores perferendis nihil sunt.",
    "updated_at" : "2016-01-04T15:31:46.176Z",
-   "created_at" : "2016-01-04T15:31:46.176Z"
+   "created_at" : "2016-01-04T15:31:46.176Z",
+   "subscribed": false
 }
 ```
 
@@ -267,7 +270,8 @@ Example response:
    },
    "description" : null,
    "updated_at" : "2016-01-07T12:44:33.959Z",
-   "milestone" : null
+   "milestone" : null,
+   "subscribed" : true
 }
 ```
 
@@ -323,7 +327,8 @@ Example response:
    ],
    "id" : 85,
    "assignee" : null,
-   "milestone" : null
+   "milestone" : null,
+   "subscribed" : true
 }
 ```
 

doc/api/merge_requests.md

@@ -66,7 +66,8 @@ Parameters:
       "due_date": null
     },
     "merge_when_build_succeeds": true,
-    "merge_status": "can_be_merged"
+    "merge_status": "can_be_merged",
+    "subscribed" : false
   }
 ]
 ```
@@ -128,7 +129,8 @@ Parameters:
     "due_date": null
   },
   "merge_when_build_succeeds": true,
-  "merge_status": "can_be_merged"
+  "merge_status": "can_be_merged",
+  "subscribed" : true
 }
 ```
 
@@ -227,6 +229,7 @@ Parameters:
   },
   "merge_when_build_succeeds": true,
   "merge_status": "can_be_merged",
+  "subscribed" : true,
   "changes": [
     {
     "old_path": "VERSION",
@@ -304,7 +307,8 @@ Parameters:
     "due_date": null
   },
   "merge_when_build_succeeds": true,
-  "merge_status": "can_be_merged"
+  "merge_status": "can_be_merged",
+  "subscribed" : true
 }
 ```
 
@@ -373,7 +377,8 @@ Parameters:
     "due_date": null
   },
   "merge_when_build_succeeds": true,
-  "merge_status": "can_be_merged"
+  "merge_status": "can_be_merged",
+  "subscribed" : true
 }
 ```
 
@@ -466,7 +471,8 @@ Parameters:
     "due_date": null
   },
   "merge_when_build_succeeds": true,
-  "merge_status": "can_be_merged"
+  "merge_status": "can_be_merged",
+  "subscribed" : true
 }
 ```
 
@@ -530,7 +536,8 @@ Parameters:
     "due_date": null
   },
   "merge_when_build_succeeds": true,
-  "merge_status": "can_be_merged"
+  "merge_status": "can_be_merged",
+  "subscribed" : true
 }
 ```
 

doc/ci/ssh_keys/README.md

@@ -57,7 +57,7 @@ before_script:
   # WARNING: Use this only with the Docker executor, if you use it with shell
   # you will overwrite your user's SSH config.
   - mkdir -p ~/.ssh
-  - '[[ -f /.dockerinit ]] && echo -e "Host *\n\tStrictHostKeyChecking no\n\n" > ~/.ssh/config`
+  - '[[ -f /.dockerinit ]] && echo -e "Host *\n\tStrictHostKeyChecking no\n\n" > ~/.ssh/config'
 ```
 
 As a final step, add the _public_ key from the one you created earlier to the

doc/development/README.md

@@ -10,4 +10,5 @@
 - [Shell commands](shell_commands.md) in the GitLab codebase
 - [Sidekiq debugging](sidekiq_debugging.md)
 - [SQL guidelines](sql.md) for SQL guidelines
+- [Testing standards and style guidelines](testing.md)
 - [UI guide](ui_guide.md) for building GitLab with existing css styles and elements

doc/development/testing.md

+# Testing Standards and Style Guidelines
+
+This guide outlines standards and best practices for automated testing of GitLab
+CE and EE.
+
+It is meant to be an _extension_ of the [thoughtbot testing
+styleguide](https://github.com/thoughtbot/guides/tree/master/style/testing). If
+this guide defines a rule that contradicts the thoughtbot guide, this guide
+takes precedence. Some guidelines may be repeated verbatim to stress their
+importance.
+
+## Factories
+
+GitLab uses [factory_girl] as a test fixture replacement.
+
+- Factory definitions live in `spec/factories/`, named using the pluralization
+  of their corresponding model (`User` factories are defined in `users.rb`).
+- There should be only one top-level factory definition per file.
+- Make use of [Traits] to clean up definitions and usages.
+- When defining a factory, don't define attributes that are not required for the
+  resulting record to pass validation.
+- When instantiating from a factory, don't supply extraneous attributes that
+  aren't required by the test.
+- Factories don't have to be limited to `ActiveRecord` objects.
+  [See example](https://gitlab.com/gitlab-org/gitlab-ce/commit/0b8cefd3b2385a21cfed779bd659978c0402766d).
+
+[factory_girl]: https://github.com/thoughtbot/factory_girl
+[Traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits
+
+## JavaScript
+
+GitLab uses [Teaspoon] to run its [Jasmine] JavaScript specs. They can be run on
+the command line via `bundle exec teaspoon`, or via a web browser at
+`http://localhost:3000/teaspoon` when the Rails server is running.
+
+- JavaScript tests live in `spec/javascripts/`, matching the folder structure of
+  `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js.coffee` has a corresponding
+  `spec/javascripts/behaviors/autosize_spec.js.coffee` file.
+- Haml fixtures required for JavaScript tests live in
+  `spec/javascripts/fixtures`. They should contain the bare minimum amount of
+  markup necessary for the test.
+
+  > **Warning:** Keep in mind that a Rails view may change and
+  invalidate your test, but everything will still pass because your fixture
+  doesn't reflect the latest view.
+
+- Keep in mind that in a CI environment, these tests are run in a headless
+  browser and you will not have access to certain APIs, such as
+  [`Notification`](https://developer.mozilla.org/en-US/docs/Web/API/notification),
+  which will have to be stubbed.
+
+[Teaspoon]: https://github.com/modeset/teaspoon
+[Jasmine]: https://github.com/jasmine/jasmine
+
+## RSpec
+
+### General Guidelines
+
+- Use a single, top-level `describe ClassName` block.
+- Use `described_class` instead of repeating the class name being described.
+- Use `.method` to describe class methods and `#method` to describe instance
+  methods.
+- Use `context` to test branching logic.
+- Don't `describe` symbols (see [Gotchas](gotchas.md#dont-describe-symbols)).
+- Prefer `not_to` to `to_not`.
+- Try to match the ordering of tests to the ordering within the class.
+- Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines
+  to separate phases.
+
+[four-phase-test]: https://robots.thoughtbot.com/four-phase-test
+
+### `let` variables
+
+GitLab's RSpec suite has made extensive use of `let` variables to reduce
+duplication. However, this sometimes [comes at the cost of clarity][lets-not],
+so we need to set some guidelines for their use going forward:
+
+- `let` variables are preferable to instance variables. Local variables are
+  preferable to `let` variables.
+- Use `let` to reduce duplication throughout an entire spec file.
+- Don't use `let` to define variables used by a single test; define them as
+  local variables inside the test's `it` block.
+- Don't define a `let` variable inside the top-level `describe` block that's
+  only used in a more deeply-nested `context` or `describe` block. Keep the
+  definition as close as possible to where it's used.
+- Try to avoid overriding the definition of one `let` variable with another.
+- Don't define a `let` variable that's only used by the definition of another.
+  Use a helper method instead.
+
+[lets-not]: https://robots.thoughtbot.com/lets-not
+
+### Test speed
+
+GitLab has a massive test suite that, without parallelization, can take more
+than an hour to run. It's important that we make an effort to write tests that
+are accurate and effective _as well as_ fast.
+
+Here are some things to keep in mind regarding test performance:
+
+- `double` and `spy` are faster than `FactoryGirl.build(...)`
+- `FactoryGirl.build(...)` and `.build_stubbed` are faster than `.create`.
+- Don't `create` an object when `build`, `build_stubbed`, `attributes_for`,
+  `spy`, or `double` will do. Database persistence is slow!
+- Use `create(:empty_project)` instead of `create(:project)` when you don't need
+  the underlying repository. Filesystem operations are slow!
+- Don't mark a feature as requiring JavaScript (through `@javascript` in
+  Spinach or `js: true` in RSpec) unless it's _actually_ required for the test
+  to be valid. Headless browser testing is slow!
+
+### Features / Integration
+
+- Feature specs live in `spec/features/` and should be named
+  `ROLE_ACTION_spec.rb`, such as `user_changes_password_spec.rb`.
+- Use only one `feature` block per feature spec file.
+- Use scenario titles that describe the success and failure paths.
+- Avoid scenario titles that add no information, such as "successfully."
+- Avoid scenario titles that repeat the feature title.
+
+## Spinach (feature) tests
+
+GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426)
+for its feature/integration tests in September 2012.
+
+As of March 2016, we are [trying to avoid adding new Spinach
+tests](https://gitlab.com/gitlab-org/gitlab-ce/issues/14121) going forward,
+opting for [RSpec feature](#features-integration) specs.
+
+Adding new Spinach scenarios is acceptable _only if_ the new scenario requires
+no more than one new `step` definition. If more than that is required, the
+test should be re-implemented using RSpec instead.
+
+---
+
+[Return to Development documentation](README.md)

doc/development/ui_guide.md

 # UI Guide for building GitLab 
 
-## Best practices for creating new pages in GitLab
-
-TODO: write some best practices when develop GitLab features.
-
 ## GitLab UI development kit
 
 We created a page inside GitLab where you can check commonly used html and css elements.

doc/install/installation.md

@@ -352,7 +352,7 @@ GitLab Shell is an SSH access and repository management software developed speci
     cd /home/git
     sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-workhorse.git
     cd gitlab-workhorse
-    sudo -u git -H git checkout v0.7.1
+    sudo -u git -H git checkout v0.7.2
     sudo -u git -H make
 
 ### Initialize Database and Activate Advanced Features

doc/update/8.6-to-8.7.md

@@ -58,7 +58,7 @@ GitLab 8.1.
 ```bash
 cd /home/git/gitlab-workhorse
 sudo -u git -H git fetch --all
-sudo -u git -H git checkout v0.7.1
+sudo -u git -H git checkout v0.7.2
 sudo -u git -H make
 ```
 

lib/api/entities.rb

@@ -170,6 +170,10 @@ module API
       expose :label_names, as: :labels
       expose :milestone, using: Entities::Milestone
       expose :assignee, :author, using: Entities::UserBasic
+
+      expose :subscribed do |issue, options|
+        issue.subscribed?(options[:current_user])
+      end
     end
 
     class MergeRequest < ProjectEntity
@@ -183,6 +187,10 @@ module API
       expose :milestone, using: Entities::Milestone
       expose :merge_when_build_succeeds
       expose :merge_status
+
+      expose :subscribed do |merge_request, options|
+        merge_request.subscribed?(options[:current_user])
+      end
     end
 
     class MergeRequestChanges < MergeRequest

lib/api/issues.rb

@@ -55,7 +55,7 @@ module API
         issues = filter_issues_state(issues, params[:state]) unless params[:state].nil?
         issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil?
         issues.reorder(issuable_order_by => issuable_sort)
-        present paginate(issues), with: Entities::Issue
+        present paginate(issues), with: Entities::Issue, current_user: current_user
       end
     end
 
@@ -92,7 +92,7 @@ module API
         end
 
         issues.reorder(issuable_order_by => issuable_sort)
-        present paginate(issues), with: Entities::Issue
+        present paginate(issues), with: Entities::Issue, current_user: current_user
       end
 
       # Get a single project issue
@@ -105,7 +105,7 @@ module API
       get ":id/issues/:issue_id" do
         @issue = user_project.issues.find(params[:issue_id])
         not_found! unless can?(current_user, :read_issue, @issue)
-        present @issue, with: Entities::Issue
+        present @issue, with: Entities::Issue, current_user: current_user
       end
 
       # Create a new project issue
@@ -149,7 +149,7 @@ module API
             issue.add_labels_by_names(params[:labels].split(','))
           end
 
-          present issue, with: Entities::Issue
+          present issue, with: Entities::Issue, current_user: current_user
         else
           render_validation_error!(issue)
         end
@@ -189,7 +189,7 @@ module API
             issue.add_labels_by_names(params[:labels].split(','))
           end
 
-          present issue, with: Entities::Issue
+          present issue, with: Entities::Issue, current_user: current_user
         else
           render_validation_error!(issue)
         end

lib/api/merge_requests.rb

@@ -56,7 +56,7 @@ module API
           end
 
         merge_requests = merge_requests.reorder(issuable_order_by => issuable_sort)
-        present paginate(merge_requests), with: Entities::MergeRequest
+        present paginate(merge_requests), with: Entities::MergeRequest, current_user: current_user
       end
 
       # Create MR
@@ -94,7 +94,7 @@ module API
             merge_request.add_labels_by_names(params[:labels].split(","))
           end
 
-          present merge_request, with: Entities::MergeRequest
+          present merge_request, with: Entities::MergeRequest, current_user: current_user
         else
           handle_merge_request_errors! merge_request.errors
         end
@@ -130,7 +130,7 @@ module API
 
           authorize! :read_merge_request, merge_request
 
-          present merge_request, with: Entities::MergeRequest
+          present merge_request, with: Entities::MergeRequest, current_user: current_user
         end
 
         # Show MR commits
@@ -162,7 +162,7 @@ module API
           merge_request = user_project.merge_requests.
             find(params[:merge_request_id])
           authorize! :read_merge_request, merge_request
-          present merge_request, with: Entities::MergeRequestChanges
+          present merge_request, with: Entities::MergeRequestChanges, current_user: current_user
         end
 
         # Update MR
@@ -204,7 +204,7 @@ module API
               merge_request.add_labels_by_names(params[:labels].split(","))
             end
 
-            present merge_request, with: Entities::MergeRequest
+            present merge_request, with: Entities::MergeRequest, current_user: current_user
           else
             handle_merge_request_errors! merge_request.errors
           end
@@ -246,7 +246,7 @@ module API
               execute(merge_request)
           end
 
-          present merge_request, with: Entities::MergeRequest
+          present merge_request, with: Entities::MergeRequest, current_user: current_user
         end
 
         # Cancel Merge if Merge When build succeeds is enabled
@@ -325,7 +325,7 @@ module API
         get "#{path}/closes_issues" do
           merge_request = user_project.merge_requests.find(params[:merge_request_id])
           issues = ::Kaminari.paginate_array(merge_request.closes_issues(current_user))
-          present paginate(issues), with: Entities::Issue
+          present paginate(issues), with: Entities::Issue, current_user: current_user
         end
       end
     end

lib/api/milestones.rb

@@ -103,7 +103,7 @@ module API
         authorize! :read_milestone, user_project
 
         @milestone = user_project.milestones.find(params[:milestone_id])
-        present paginate(@milestone.issues), with: Entities::Issue
+        present paginate(@milestone.issues), with: Entities::Issue, current_user: current_user
       end
 
     end

lib/gitlab/metrics/metric.rb

@@ -2,6 +2,8 @@ module Gitlab
   module Metrics
     # Class for storing details of a single metric (label, value, etc).
     class Metric
+      JITTER_RANGE = 0.000001..0.001
+
       attr_reader :series, :values, :tags, :created_at
 
       # series - The name of the series (as a String) to store the metric in.
@@ -16,11 +18,29 @@ module Gitlab
 
       # Returns a Hash in a format that can be directly written to InfluxDB.
       def to_hash
+        # InfluxDB overwrites an existing point if a new point has the same
+        # series, tag set, and timestamp. In a highly concurrent environment
+        # this means that using the number of seconds since the Unix epoch is
+        # inevitably going to collide with another timestamp. For example, two
+        # Rails requests processed by different processes may end up generating
+        # metrics using the _exact_ same timestamp (in seconds).
+        #
+        # Due to the way InfluxDB is set up there's no solution to this problem,
+        # all we can do is lower the amount of collisions. We do this by using
+        # Time#to_f which returns the seconds as a Float providing greater
+        # accuracy. We then add a small random value that is large enough to
+        # distinguish most timestamps but small enough to not alter the amount
+        # of seconds.
+        #
+        # See https://gitlab.com/gitlab-com/operations/issues/175 for more
+        # information.
+        time = @created_at.to_f + rand(JITTER_RANGE)
+
         {
           series:    @series,
           tags:      @tags,
           values:    @values,
-          timestamp: @created_at.to_i * 1_000_000_000
+          timestamp: (time * 1_000_000_000).to_i
         }
       end
     end