diff options
| author | Tiago <tiagonbotelho@hotmail.com> | 2017-12-20 18:19:46 +0000 |
|---|---|---|
| committer | Tiago <tiagonbotelho@hotmail.com> | 2017-12-20 18:19:46 +0000 |
| commit | d525614556e713bce5ebd975db1b2c7017bd9629 (patch) | |
| tree | ae7c58ce85e22387f674d1aa35b6a7c258093b56 | |
| parent | 6e725a32319daf8c46bbdb0cfd91a3573868e0ba (diff) | |
| parent | 1229fe8d8720564535c15644fa00a9d5b26ef43f (diff) | |
| download | gitlab-ce-d525614556e713bce5ebd975db1b2c7017bd9629.tar.gz | |
Merge branch '10-3-stable-prepare-rc3' into '10-3-stable'
Prepare 10.3 RC4 release (initially planned to be RC3)
See merge request gitlab-org/gitlab-ce!16000
57 files changed, 549 insertions, 284 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js index 99f5c305df5..5fa838baba3 100644 --- a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js +++ b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js @@ -6,7 +6,7 @@ Vue.use(VueResource); export default class MRWidgetService { constructor(endpoints) { this.mergeResource = Vue.resource(endpoints.mergePath); - this.mergeCheckResource = Vue.resource(endpoints.statusPath); + this.mergeCheckResource = Vue.resource(`${endpoints.statusPath}?serializer=widget`); this.cancelAutoMergeResource = Vue.resource(endpoints.cancelAutoMergePath); this.removeWIPResource = Vue.resource(endpoints.removeWIPPath); this.removeSourceBranchResource = Vue.resource(endpoints.sourceBranchPath); diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index e7b3b73024b..6b59c8461a3 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -131,7 +131,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo .new(project, current_user, wip_event: 'unwip') .execute(@merge_request) - render json: serializer.represent(@merge_request) + render json: serialize_widget(@merge_request) end def commit_change_content @@ -147,7 +147,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo .new(@project, current_user) .cancel(@merge_request) - render json: serializer.represent(@merge_request) + render json: serialize_widget(@merge_request) end def merge @@ -304,6 +304,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end end + def serialize_widget(merge_request) + serializer.represent(merge_request, serializer: 'widget') + end + def serializer MergeRequestSerializer.new(current_user: current_user, project: merge_request.project) end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 4c60f4b0cd0..b4ca0e68e0b 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -32,7 +32,7 @@ module IssuablesHelper end end - def serialize_issuable(issuable) + def serialize_issuable(issuable, serializer: nil) serializer_klass = case issuable when Issue IssueSerializer @@ -42,7 +42,7 @@ module IssuablesHelper serializer_klass .new(current_user: current_user, project: issuable.project) - .represent(issuable) + .represent(issuable, serializer: serializer) .to_json end diff --git a/app/models/blob.rb b/app/models/blob.rb index 29e762724e3..19ad110db58 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -77,9 +77,15 @@ class Blob < SimpleDelegator end def self.lazy(project, commit_id, path) - BatchLoader.for(commit_id: commit_id, path: path).batch do |items, loader| - project.repository.blobs_at(items.map(&:values)).each do |blob| - loader.call({ commit_id: blob.commit_id, path: blob.path }, blob) if blob + BatchLoader.for({ project: project, commit_id: commit_id, path: path }).batch do |items, loader| + items_by_project = items.group_by { |i| i[:project] } + + items_by_project.each do |project, items| + items = items.map { |i| i.values_at(:commit_id, :path) } + + project.repository.blobs_at(items).each do |blob| + loader.call({ project: blob.project, commit_id: blob.commit_id, path: blob.path }, blob) if blob + end end end end diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index 4a65738214b..d67b16584a4 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -22,12 +22,9 @@ class DiffDiscussion < Discussion def merge_request_version_params return unless for_merge_request? - return {} if active? - if on_merge_request_commit? - { commit_id: commit_id } - else - noteable.version_params_for(position.diff_refs) + version_params.tap do |params| + params[:commit_id] = commit_id if on_merge_request_commit? end end @@ -37,4 +34,12 @@ class DiffDiscussion < Discussion position: position.to_json ) end + + private + + def version_params + return {} if active? + + noteable.version_params_for(position.diff_refs) + end end diff --git a/app/models/identity.rb b/app/models/identity.rb index ff811e19f8a..99d99bc6deb 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -14,11 +14,11 @@ class Identity < ActiveRecord::Base end def ldap? - provider.starts_with?('ldap') + Gitlab::OAuth::Provider.ldap_provider?(provider) end def self.normalize_uid(provider, uid) - if provider.to_s.starts_with?('ldap') + if Gitlab::OAuth::Provider.ldap_provider?(provider) Gitlab::LDAP::Person.normalize_dn(uid) else uid.to_s diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index fc9018e65a1..fb17ee96f03 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -8,6 +8,7 @@ class MergeRequest < ActiveRecord::Base include ManualInverseAssociation include EachBatch include ThrottledTouch + include Gitlab::Utils::StrongMemoize ignore_column :locked_at, :ref_fetched @@ -52,6 +53,7 @@ class MergeRequest < ActiveRecord::Base serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize after_create :ensure_merge_request_diff, unless: :importing? + after_update :clear_memoized_shas after_update :reload_diff_if_branch_changed # When this attribute is true some MR validation is ignored @@ -395,13 +397,17 @@ class MergeRequest < ActiveRecord::Base end def source_branch_head - return unless source_project - - source_project.repository.commit(source_branch_ref) if source_branch_ref + strong_memoize(:source_branch_head) do + if source_project && source_branch_ref + source_project.repository.commit(source_branch_ref) + end + end end def target_branch_head - target_project.repository.commit(target_branch_ref) + strong_memoize(:target_branch_head) do + target_project.repository.commit(target_branch_ref) + end end def branch_merge_base_commit @@ -533,6 +539,13 @@ class MergeRequest < ActiveRecord::Base end end + def clear_memoized_shas + @target_branch_sha = @source_branch_sha = nil + + clear_memoization(:source_branch_head) + clear_memoization(:target_branch_head) + end + def reload_diff_if_branch_changed if (source_branch_changed? || target_branch_changed?) && (source_branch_head && target_branch_head) diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index c37aa0a594b..e35de9b97ee 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -104,19 +104,19 @@ class MergeRequestDiff < ActiveRecord::Base def base_commit return unless base_commit_sha - project.commit(base_commit_sha) + project.commit_by(oid: base_commit_sha) end def start_commit return unless start_commit_sha - project.commit(start_commit_sha) + project.commit_by(oid: start_commit_sha) end def head_commit return unless head_commit_sha - project.commit(head_commit_sha) + project.commit_by(oid: head_commit_sha) end def commit_shas diff --git a/app/models/user.rb b/app/models/user.rb index ccf0e043aed..47d00dd1294 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -738,7 +738,7 @@ class User < ActiveRecord::Base def ldap_user? if identities.loaded? - identities.find { |identity| identity.provider.start_with?('ldap') && !identity.extern_uid.nil? } + identities.find { |identity| Gitlab::OAuth::Provider.ldap_provider?(identity.provider) && !identity.extern_uid.nil? } else identities.exists?(["provider LIKE ? AND extern_uid IS NOT NULL", "ldap%"]) end diff --git a/app/models/user_synced_attributes_metadata.rb b/app/models/user_synced_attributes_metadata.rb index 9f374304164..548b99b69d9 100644 --- a/app/models/user_synced_attributes_metadata.rb +++ b/app/models/user_synced_attributes_metadata.rb @@ -6,11 +6,11 @@ class UserSyncedAttributesMetadata < ActiveRecord::Base SYNCABLE_ATTRIBUTES = %i[name email location].freeze def read_only?(attribute) - Gitlab.config.omniauth.sync_profile_from_provider && synced?(attribute) + sync_profile_from_provider? && synced?(attribute) end def read_only_attributes - return [] unless Gitlab.config.omniauth.sync_profile_from_provider + return [] unless sync_profile_from_provider? SYNCABLE_ATTRIBUTES.select { |key| synced?(key) } end @@ -22,4 +22,10 @@ class UserSyncedAttributesMetadata < ActiveRecord::Base def set_attribute_synced(attribute, value) write_attribute("#{attribute}_synced", value) end + + private + + def sync_profile_from_provider? + Gitlab::OAuth::Provider.sync_profile_from_provider?(provider) + end end diff --git a/app/serializers/issuable_entity.rb b/app/serializers/issuable_entity.rb index 3b5a4fd4f79..6f31fbd6b7c 100644 --- a/app/serializers/issuable_entity.rb +++ b/app/serializers/issuable_entity.rb @@ -3,14 +3,6 @@ class IssuableEntity < Grape::Entity expose :id expose :iid - expose :author_id expose :description - expose :lock_version - expose :milestone_id expose :title - expose :updated_by_id - expose :created_at - expose :updated_at - expose :milestone, using: API::Entities::Milestone - expose :labels, using: LabelEntity end diff --git a/app/serializers/issuable_sidebar_entity.rb b/app/serializers/issuable_sidebar_entity.rb index ff23d8bf0c7..29138c803df 100644 --- a/app/serializers/issuable_sidebar_entity.rb +++ b/app/serializers/issuable_sidebar_entity.rb @@ -1,4 +1,5 @@ class IssuableSidebarEntity < Grape::Entity + include TimeTrackableEntity include RequestAwareEntity expose :participants, using: ::API::Entities::UserBasic do |issuable| @@ -8,9 +9,4 @@ class IssuableSidebarEntity < Grape::Entity expose :subscribed do |issuable| issuable.subscribed?(request.current_user, issuable.project) end - - expose :time_estimate - expose :total_time_spent - expose :human_time_estimate - expose :human_total_time_spent end diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb index 9d52b8d9752..0bdd4d7a272 100644 --- a/app/serializers/issue_entity.rb +++ b/app/serializers/issue_entity.rb @@ -2,7 +2,15 @@ class IssueEntity < IssuableEntity include TimeTrackableEntity expose :state + expose :milestone_id + expose :updated_by_id + expose :created_at + expose :updated_at expose :deleted_at + expose :milestone, using: API::Entities::Milestone + expose :labels, using: LabelEntity + expose :lock_version + expose :author_id expose :confidential expose :discussion_locked expose :assignees, using: API::Entities::UserBasic diff --git a/app/serializers/merge_request_serializer.rb b/app/serializers/merge_request_serializer.rb index e9d98d8baca..52eb30d688a 100644 --- a/app/serializers/merge_request_serializer.rb +++ b/app/serializers/merge_request_serializer.rb @@ -1,14 +1,14 @@ class MergeRequestSerializer < BaseSerializer # This overrided method takes care of which entity should be used - # to serialize the `merge_request` based on `basic` key in `opts` param. + # to serialize the `merge_request` based on `serializer` key in `opts` param. # Hence, `entity` doesn't need to be declared on the class scope. def represent(merge_request, opts = {}) entity = case opts[:serializer] when 'basic', 'sidebar' MergeRequestBasicEntity - else - MergeRequestEntity + when 'widget' + MergeRequestWidgetEntity end super(merge_request, opts, entity) diff --git a/app/serializers/merge_request_entity.rb b/app/serializers/merge_request_widget_entity.rb index eece9445dca..f8e59b2ffd7 100644 --- a/app/serializers/merge_request_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -1,8 +1,5 @@ -class MergeRequestEntity < IssuableEntity - include TimeTrackableEntity - +class MergeRequestWidgetEntity < IssuableEntity expose :state - expose :deleted_at expose :in_progress_merge_commit_sha expose :merge_commit_sha expose :merge_error diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index abff702fd9d..8740c6895df 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -20,7 +20,7 @@ -# haml-lint:disable InlineJavaScript :javascript window.gl = window.gl || {}; - window.gl.mrWidgetData = #{serialize_issuable(@merge_request)} + window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget')} #js-vue-mr-widget.mr-widget diff --git a/changelogs/unreleased/dm-ldap-email-readonly.yml b/changelogs/unreleased/dm-ldap-email-readonly.yml new file mode 100644 index 00000000000..744b21f901e --- /dev/null +++ b/changelogs/unreleased/dm-ldap-email-readonly.yml @@ -0,0 +1,5 @@ +--- +title: Make sure user email is read only when synced with LDAP +merge_request: 15915 +author: +type: fixed diff --git a/changelogs/unreleased/osw-isolate-mr-widget-exposed-attributes.yml b/changelogs/unreleased/osw-isolate-mr-widget-exposed-attributes.yml new file mode 100644 index 00000000000..6b05713d1a1 --- /dev/null +++ b/changelogs/unreleased/osw-isolate-mr-widget-exposed-attributes.yml @@ -0,0 +1,5 @@ +--- +title: Stop sending milestone and labels data over the wire for MR widget requests +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/zj-memoization-mr-commits.yml b/changelogs/unreleased/zj-memoization-mr-commits.yml new file mode 100644 index 00000000000..59dfc6d6049 --- /dev/null +++ b/changelogs/unreleased/zj-memoization-mr-commits.yml @@ -0,0 +1,5 @@ +--- +title: Cache commits for MergeRequest diffs +merge_request: +author: +type: performance diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index c8b6018bc1b..f2f05b3eeb2 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -383,6 +383,7 @@ production: &base # Sync user's profile from the specified Omniauth providers every time the user logs in (default: empty). # Define the allowed providers using an array, e.g. ["cas3", "saml", "twitter"], # or as true/false to allow all providers or none. + # When authenticating using LDAP, the user's email is always synced. # sync_profile_from_provider: [] # Select which info to sync from the providers above. (default: email). diff --git a/doc/administration/reply_by_email.md b/doc/administration/reply_by_email.md index 1304476e678..3a2cced37bf 100644 --- a/doc/administration/reply_by_email.md +++ b/doc/administration/reply_by_email.md @@ -89,9 +89,11 @@ email address in order to sign up. If you also host a public-facing GitLab instance at `hooli.com` and set your incoming email domain to `hooli.com`, an attacker could abuse the "Create new -issue by email" feature by using a project's unique address as the email when -signing up for Slack, which would send a confirmation email, which would create -a new issue on the project owned by the attacker, allowing them to click the +issue by email" or +"[Create new merge request by email](../user/project/merge_requests/index.md#create-new-merge-requests-by-email)" +features by using a project's unique address as the email when signing up for +Slack, which would send a confirmation email, which would create a new issue or +merge request on the project owned by the attacker, allowing them to click the confirmation link and validate their account on your company's private Slack instance. diff --git a/doc/development/README.md b/doc/development/README.md index 6892838be7f..f205cfb778b 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -80,10 +80,9 @@ comments: false ## Documentation guides -- [Documentation styleguide](doc_styleguide.md): Use this styleguide if you are - contributing to the documentation. - [Writing documentation](writing_documentation.md) - - [Distinction between general documentation and technical articles](writing_documentation.md#distinction-between-general-documentation-and-technical-articles) +- [Documentation styleguide](doc_styleguide.md) +- [Markdown](../user/markdown.md) ## Internationalization (i18n) guides diff --git a/doc/development/writing_documentation.md b/doc/development/writing_documentation.md index b6def7ef541..43a79ffcaa5 100644 --- a/doc/development/writing_documentation.md +++ b/doc/development/writing_documentation.md @@ -1,75 +1,14 @@ # Writing documentation - **General Documentation**: written by the [developers responsible by creating features](#contributing-to-docs). Should be submitted in the same merge request containing code. Feature proposals (by GitLab contributors) should also be accompanied by its respective documentation. They can be later improved by PMs and Technical Writers. - - **Technical Articles**: written by any [GitLab Team](https://about.gitlab.com/team/) member, GitLab contributors, or [Community Writers](https://about.gitlab.com/handbook/product/technical-writing/community-writers/). + - **[Technical Articles](#technical-articles)**: written by any [GitLab Team](https://about.gitlab.com/team/) member, GitLab contributors, or [Community Writers](https://about.gitlab.com/handbook/product/technical-writing/community-writers/). - **Indexes per topic**: initially prepared by the Technical Writing Team, and kept up-to-date by developers and PMs in the same merge request containing code. They gather all resources for that topic in a single page (user and admin documentation, articles, and third-party docs). -## Distinction between General Documentation and Technical Articles - -### General documentation - -General documentation is categorized by _User_, _Admin_, and _Contributor_, and describe what that feature is, what it does, and its available settings. - -### Technical Articles - -Technical articles replace technical content that once lived in the [GitLab Blog](https://about.gitlab.com/blog/), where they got out-of-date and weren't easily found. - -They are topic-related documentation, written with an user-friendly approach and language, aiming to provide the community with guidance on specific processes to achieve certain objectives. - -A technical article guides users and/or admins to achieve certain objectives (within guides and tutorials), or provide an overview of that particular topic or feature (within technical overviews). It can also describe the use, implementation, or integration of third-party tools with GitLab. - -They live under `doc/articles/article-title/index.md`, and their images should be placed under `doc/articles/article-title/img/`. Find a list of existing [technical articles](../articles/index.md) here. - -#### Types of Technical Articles - -- **User guides**: technical content to guide regular users from point A to point B -- **Admin guides**: technical content to guide administrators of GitLab instances from point A to point B -- **Technical Overviews**: technical content describing features, solutions, and third-party integrations -- **Tutorials**: technical content provided step-by-step on how to do things, or how to reach very specific objectives - -#### Understanding guides, tutorials, and technical overviews - -Suppose there's a process to go from point A to point B in 5 steps: `(A) 1 > 2 > 3 > 4 > 5 (B)`. - -A **guide** can be understood as a description of certain processes to achieve a particular objective. A guide brings you from A to B describing the characteristics of that process, but not necessarily going over each step. It can mention, for example, steps 2 and 3, but does not necessarily explain how to accomplish them. - -- Live example: "GitLab Pages from A to Z - [Part 1](../user/project/pages/getting_started_part_one.md) to [Part 4](../user/project/pages/getting_started_part_four.md)" - -A **tutorial** requires a clear **step-by-step** guidance to achieve a singular objective. It brings you from A to B, describing precisely all the necessary steps involved in that process, showing each of the 5 steps to go from A to B. -It does not only describes steps 2 and 3, but also shows you how to accomplish them. - -- Live example (on the blog): [Hosting on GitLab.com with GitLab Pages](https://about.gitlab.com/2016/04/07/gitlab-pages-setup/) - -A **technical overview** is a description of what a certain feature is, and what it does, but does not walk -through the process of how to use it systematically. - -- Live example (on the blog): [GitLab Workflow, an overview](https://about.gitlab.com/2016/10/25/gitlab-workflow-an-overview/) - -#### Special format - -Every **Technical Article** contains, in the very beginning, a blockquote with the following information: - -- A reference to the **type of article** (user guide, admin guide, tech overview, tutorial) -- A reference to the **knowledge level** expected from the reader to be able to follow through (beginner, intermediate, advanced) -- A reference to the **author's name** and **GitLab.com handle** -- A reference of the **publication date** - -```md -> **Article [Type](../../development/writing_documentation.html#types-of-technical-articles):** tutorial || -> **Level:** intermediary || -> **Author:** [Name Surname](https://gitlab.com/username) || -> **Publication date:** AAAA/MM/DD -``` - -#### Technical Articles - Writing Method - -Use the [writing method](https://about.gitlab.com/handbook/product/technical-writing/#writing-method) defined by the Technical Writing team. - ## Documentation style guidelines All the docs follow the same [styleguide](doc_styleguide.md). -### Contributing to docs +## Contributing to docs Whenever a feature is changed, updated, introduced, or deprecated, the merge request introducing these changes must be accompanied by the documentation @@ -118,13 +57,31 @@ and for every **major** feature present in Community Edition. Currently GitLab docs use Redcarpet as [markdown](../user/markdown.md) engine, but there's an [open discussion](https://gitlab.com/gitlab-com/gitlab-docs/issues/50) for implementing Kramdown in the near future. -## Testing +### Previewing locally -We try to treat documentation as code, thus have implemented some testing. +To preview your changes to documentation locally, please follow +this [development guide](https://gitlab.com/gitlab-com/gitlab-docs/blob/master/README.md#development). + +### Testing + +We treat documentation as code, thus have implemented some testing. Currently, the following tests are in place: 1. `docs lint`: Check that all internal (relative) links work correctly and - that all cURL examples in API docs use the full switches. + that all cURL examples in API docs use the full switches. It's recommended + to [check locally](#previewing-locally) before pushing to GitLab by executing the command + `bundle exec nanoc check internal_links` on your local + [`gitlab-docs`](https://gitlab.com/gitlab-com/gitlab-docs) directory. +1. [`ee_compat_check`](https://docs.gitlab.com/ee/development/automatic_ce_ee_merge.html#avoiding-ce-gt-ee-merge-conflicts-beforehand) (runs on CE only): + When you submit a merge request to GitLab Community Edition (CE), + there is this additional job that runs against Enterprise Edition (EE) + and checks if your changes can apply cleanly to the EE codebase. + If that job fails, read the instructions in the job log for what to do next. + As CE is merged into EE once a day, it's important to avoid merge conflicts. + Submitting an EE-equivalent merge request cherry-picking all commits from CE to EE is + essential to avoid them. + +### Branch naming If your contribution contains **only** documentation changes, you can speed up the CI process by following some branch naming conventions. You have three @@ -139,17 +96,7 @@ choices: If your branch name matches any of the above, it will run only the docs tests. If it doesn't, the whole test suite will run (including docs). ---- - -When you submit a merge request to GitLab Community Edition (CE), there is an -additional job called `rake ee_compat_check` that runs against Enterprise -Edition (EE) and checks if your changes can apply cleanly to the EE codebase. -If that job fails, read the instructions in the job log for what to do next. -Contributors do not need to submit their changes to EE, GitLab Inc. employees -on the other hand need to make sure that their changes apply cleanly to both -CE and EE. - -## Previewing the changes live +### Previewing the changes live If you want to preview the doc changes of your merge request live, you can use the manual `review-docs-deploy` job in your merge request. You will need at @@ -164,7 +111,7 @@ You will need to push a branch to those repositories, it doesn't work for forks. TIP: **Tip:** If your branch contains only documentation changes, you can use -[special branch names](#testing) to avoid long running pipelines. +[special branch names](#branch-naming) to avoid long running pipelines. In the mini pipeline graph, you should see an `>>` icon. Clicking on it will reveal the `review-docs-deploy` job. Hit the play button for the job to start. @@ -209,12 +156,12 @@ working on. If you don't, the remote docs branch won't be removed either, and the server where the Review Apps are hosted will eventually be out of disk space. -### Behind the scenes +#### Technical aspects If you want to know the hot details, here's what's really happening: 1. You manually run the `review-docs-deploy` job in a CE/EE merge request. -1. The job runs the [`scirpts/trigger-build-docs`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/scripts/trigger-build-docs) +1. The job runs the [`scripts/trigger-build-docs`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/scripts/trigger-build-docs) script with the `deploy` flag, which in turn: 1. Takes your branch name and applies the following: - The slug of the branch name is used to avoid special characters since @@ -243,3 +190,65 @@ The following GitLab features are used among others: - [Review Apps](../ci/review_apps/index.md) - [Artifacts](../ci/yaml/README.md#artifacts) - [Specific Runner](../ci/runners/README.md#locking-a-specific-runner-from-being-enabled-for-other-projects) + +## General Documentation vs Technical Articles + +### General documentation + +General documentation is categorized by _User_, _Admin_, and _Contributor_, and describe what that feature is, what it does, and its available settings. + +### Technical Articles + +Technical articles replace technical content that once lived in the [GitLab Blog](https://about.gitlab.com/blog/), where they got out-of-date and weren't easily found. + +They are topic-related documentation, written with an user-friendly approach and language, aiming to provide the community with guidance on specific processes to achieve certain objectives. + +A technical article guides users and/or admins to achieve certain objectives (within guides and tutorials), or provide an overview of that particular topic or feature (within technical overviews). It can also describe the use, implementation, or integration of third-party tools with GitLab. + +They should be placed in a new directory named `/article-title/index.md` under a topic-related folder, and their images should be placed in `/article-title/img/`. For example, a new article on GitLab Pages should be placed in `doc/user/project/pages/article-title/` and a new article on GitLab CI/CD should be placed in `doc/ci/article-title/`. + +#### Types of Technical Articles + +- **User guides**: technical content to guide regular users from point A to point B +- **Admin guides**: technical content to guide administrators of GitLab instances from point A to point B +- **Technical Overviews**: technical content describing features, solutions, and third-party integrations +- **Tutorials**: technical content provided step-by-step on how to do things, or how to reach very specific objectives + +#### Understanding guides, tutorials, and technical overviews + +Suppose there's a process to go from point A to point B in 5 steps: `(A) 1 > 2 > 3 > 4 > 5 (B)`. + +A **guide** can be understood as a description of certain processes to achieve a particular objective. A guide brings you from A to B describing the characteristics of that process, but not necessarily going over each step. It can mention, for example, steps 2 and 3, but does not necessarily explain how to accomplish them. + +- Live example: "GitLab Pages from A to Z - [Part 1](../user/project/pages/getting_started_part_one.md) to [Part 4](../user/project/pages/getting_started_part_four.md)" + +A **tutorial** requires a clear **step-by-step** guidance to achieve a singular objective. It brings you from A to B, describing precisely all the necessary steps involved in that process, showing each of the 5 steps to go from A to B. +It does not only describes steps 2 and 3, but also shows you how to accomplish them. + +- Live example (on the blog): [Hosting on GitLab.com with GitLab Pages](https://about.gitlab.com/2016/04/07/gitlab-pages-setup/) + +A **technical overview** is a description of what a certain feature is, and what it does, but does not walk +through the process of how to use it systematically. + +- Live example (on the blog): [GitLab Workflow, an overview](https://about.gitlab.com/2016/10/25/gitlab-workflow-an-overview/) + +#### Special format + +Every **Technical Article** contains, in the very beginning, a blockquote with the following information: + +- A reference to the **type of article** (user guide, admin guide, tech overview, tutorial) +- A reference to the **knowledge level** expected from the reader to be able to follow through (beginner, intermediate, advanced) +- A reference to the **author's name** and **GitLab.com handle** +- A reference of the **publication date** + +```md +> **[Article Type](../../development/writing_documentation.html#types-of-technical-articles):** tutorial || +> **Level:** intermediary || +> **Author:** [Name Surname](https://gitlab.com/username) || +> **Publication date:** AAAA-MM-DD +``` + +#### Technical Articles - Writing Method + +Use the [writing method](https://about.gitlab.com/handbook/product/technical-writing/#writing-method) defined by the Technical Writing team. + diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md index 0e20b8096e9..20087a981f9 100644 --- a/doc/integration/omniauth.md +++ b/doc/integration/omniauth.md @@ -229,16 +229,18 @@ In order to enable/disable an OmniAuth provider, go to Admin Area -> Settings -> ## Keep OmniAuth user profiles up to date You can enable profile syncing from selected OmniAuth providers and for all or for specific user information. - + +When authenticating using LDAP, the user's email is always synced. + ```ruby gitlab_rails['sync_profile_from_provider'] = ['twitter', 'google_oauth2'] gitlab_rails['sync_profile_attributes'] = ['name', 'email', 'location'] ``` - + **For installations from source** - + ```yaml omniauth: sync_profile_from_provider: ['twitter', 'google_oauth2'] - sync_profile_claims_from_provider: ['email', 'location'] - ```
\ No newline at end of file + sync_profile_attributes: ['email', 'location'] + ``` diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index d100b431721..0b48596006d 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -19,6 +19,7 @@ project in an easy and automatic way: 1. [Auto Build](#auto-build) 1. [Auto Test](#auto-test) 1. [Auto Code Quality](#auto-code-quality) +1. [Auto SAST (Static Application Security Testing)](#auto-sast) 1. [Auto Review Apps](#auto-review-apps) 1. [Auto Deploy](#auto-deploy) 1. [Auto Monitoring](#auto-monitoring) @@ -147,6 +148,10 @@ has a `.gitlab-ci.yml` or not: do that in a branch to test Auto DevOps before committing to `master`. NOTE: **Note:** +Starting with GitLab 10.3, when enabling Auto DevOps, a pipeline is +automatically run on the default branch. + +NOTE: **Note:** If you are a GitLab Administrator, you can enable Auto DevOps instance wide in **Admin Area > Settings > Continuous Integration and Deployment**. Doing that, all the projects that haven't explicitly set an option will have Auto DevOps @@ -198,6 +203,18 @@ out. In GitLab Enterprise Edition Starter, differences between the source and target branches are [shown in the merge request widget](https://docs.gitlab.com/ee/user/project/merge_requests/code_quality_diff.html). +### Auto SAST + +> Introduced in [GitLab Enterprise Edition Ultimate][ee] 10.3. + +Static Application Security Testing (SAST) uses the +[gl-sast Docker image](https://gitlab.com/gitlab-org/gl-sast) to run static +analysis on the current code and checks for potential security issues. Once the +report is created, it's uploaded as an artifact which you can later download and +check out. + +Any security warnings are also [shown in the merge request widget](https://docs.gitlab.com/ee/user/project/merge_requests/sast.html). + ### Auto Review Apps NOTE: **Note:** @@ -536,3 +553,4 @@ curl --data "value=true" --header "PRIVATE-TOKEN: personal_access_token" https:/ [postgresql]: https://www.postgresql.org/ [Auto DevOps template]: https://gitlab.com/gitlab-org/gitlab-ci-yml/blob/master/Auto-DevOps.gitlab-ci.yml [GitLab Omnibus Helm Chart]: ../../install/kubernetes/gitlab_omnibus.md +[ee]: https://about.gitlab.com/gitlab-ee/ diff --git a/doc/user/discussions/img/commit_comment_mr_context.png b/doc/user/discussions/img/commit_comment_mr_context.png Binary files differnew file mode 100644 index 00000000000..b363e0035e8 --- /dev/null +++ b/doc/user/discussions/img/commit_comment_mr_context.png diff --git a/doc/user/discussions/img/commit_comment_mr_discussions_tab.png b/doc/user/discussions/img/commit_comment_mr_discussions_tab.png Binary files differnew file mode 100644 index 00000000000..2b06cdcc055 --- /dev/null +++ b/doc/user/discussions/img/commit_comment_mr_discussions_tab.png diff --git a/doc/user/discussions/img/merge_request_commits_tab.png b/doc/user/discussions/img/merge_request_commits_tab.png Binary files differnew file mode 100644 index 00000000000..41a3648f390 --- /dev/null +++ b/doc/user/discussions/img/merge_request_commits_tab.png diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index 2206b2860f4..eacfe2baa27 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -32,6 +32,43 @@ hide discussions that are no longer relevant. Comments and discussions can be resolved by anyone with at least Developer access to the project or the author of the merge request. +### Commit discussions in the context of a merge request + +> [Introduced][ce-31847] in GitLab 10.3. + +For reviewers with commit-based workflow, it may be useful to add discussions to +specific commit diffs in the context of a merge request. These discussions will +persist through a commit ID change when: + +- force-pushing after a rebase +- amending a commit + +To create a commit diff discussion: + +1. Navigate to the merge request **Commits** tab. A list of commits that + constitute the merge request will be shown. + +  + +1. Navigate to a specific commit, click on the **Changes** tab (where you + will only be presented diffs from the selected commit), and leave a comment. + +  + +1. Any discussions created this way will be shown in the merge request's + **Discussions** tab and are resolvable. + +  + +Discussions created this way will only appear in the original merge request +and not when navigating to that commit under your project's +**Repository > Commits** page. + +TIP: **Tip:** +When a link of a commit reference is found in a discussion inside a merge +request, it will be automatically converted to a link in the context of the +current merge request. + ### Jumping between unresolved discussions When a merge request has a large number of comments it can be difficult to track @@ -133,6 +170,15 @@ From now on, any discussions on a diff will be resolved by default if a push makes that diff section outdated. Discussions on lines that don't change and top-level resolvable discussions are not automatically resolved. +## Commit discussions + +You can add comments and discussion threads to a particular commit under your +project's **Repository > Commits**. + +CAUTION: **Attention:** +Discussions created this way will be lost if the commit ID changes after a +force push. + ## Threaded discussions > [Introduced][ce-7527] in GitLab 9.1. @@ -229,6 +275,7 @@ edit existing comments. Non-team members are restricted from adding or editing c [ce-14053]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14053 [ce-14061]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061 [ce-14531]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14531 +[ce-31847]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31847 [resolve-discussion-button]: img/resolve_discussion_button.png [resolve-comment-button]: img/resolve_comment_button.png [discussion-view]: img/discussion_view.png diff --git a/doc/user/group/index.md b/doc/user/group/index.md index a1671f9dd91..1733017cbc0 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -197,11 +197,11 @@ username, you can create a new group and transfer projects to it. Changing a group's path can have unintended side effects. * Existing web URLs for the group and anything under it (i.e. projects) will -redirect to the new URLs -* Existing Git remote URLs for projects under the group will no longer work, but -Git responses will show an error with the new remote URL -* The original namespace can be claimed again by any group or user, which will -destroy web redirects and Git remote warnings +redirect to the new URLs. +* Existing Git remote URLs for projects under the group will redirect to the new remote URL, and they +will show a warning with the new remote URL. +* The redirect to the new URL is permanent, that implies the original namespace +can't be claimed again by any group or user. * If you are vacating the path so it can be claimed by another group or user, you may need to rename the group name as well since both names and paths must be unique diff --git a/doc/user/profile/index.md b/doc/user/profile/index.md index 5fcc0501dc1..0219e5d57f2 100644 --- a/doc/user/profile/index.md +++ b/doc/user/profile/index.md @@ -21,11 +21,10 @@ Alternatively, you can follow [this detailed procedure from the GitLab Team Hand Changing your username can have unintended side effects. * Existing web URLs for the user and anything under it (i.e. projects) will -redirect to the new URLs -* Existing Git remote URLs for projects under the user will no longer work, but -Git responses will show an error with the new remote URL -* The original namespace can be claimed again by any group or user, which will -destroy any web redirects and Git remote warnings +redirect to the new URLs. +* Existing Git remote URLs for projects under the user will redirect to the new remote URL. Git responses +will show a warning with the new remote URL. +* The redirect to the new URL is permanent, that implies the original namespace can't be claimed again by any group or user. > It is currently not possible to rename a namespace if it contains a project with container registry tags, because the project cannot be moved. diff --git a/doc/user/project/merge_requests/img/create_from_email.png b/doc/user/project/merge_requests/img/create_from_email.png Binary files differnew file mode 100644 index 00000000000..71eb4bf267d --- /dev/null +++ b/doc/user/project/merge_requests/img/create_from_email.png diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index b5c3f74a113..7037d7f5989 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -27,7 +27,7 @@ With GitLab merge requests, you can: - [Resolve merge conflicts from the UI](#resolve-conflicts) - Enable [fast-forward merge requests](#fast-forward-merge-requests) - Enable [semi-linear history merge requests](#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch -- [Create new merge requests by email](#create_by_email) +- [Create new merge requests by email](#create-new-merge-requests-by-email) With **[GitLab Enterprise Edition][ee]**, you can also: @@ -139,7 +139,12 @@ address. The address can be obtained on the merge requests page by clicking on a **Email a new merge request to this project** button. The subject will be used as the source branch name for the new merge request and the target branch will be the default branch for the project. The message body (if not empty) -will be used as the merge request description. +will be used as the merge request description. You need +["Reply by email"](../../../administration/reply_by_email.md) enabled to use +this feature. If it's not enabled to your instance, you may ask your GitLab +administrator to do so. + + ## Revert changes diff --git a/doc/user/project/settings/index.md b/doc/user/project/settings/index.md index a234a647b77..2b6fde1e2a5 100644 --- a/doc/user/project/settings/index.md +++ b/doc/user/project/settings/index.md @@ -50,3 +50,9 @@ Here you can run housekeeping, archive, rename, transfer, or remove a project. It's possible to mark a project as archived via the Project Settings. An archived project will be hidden by default in the project listings. An archived project can be fully restored and will therefore retain it's repository and all associated resources whilst in an archived state. + +#### Renaming a project + +>**Note:** Only Project Owners and Admin users have the permission to rename a project + +It's possible to rename a project from "Rename repository" or "Transfer project" sections. When doing so, you will need to update your local repositories to point to the new location, otherwise Git operations will be rejected. diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index fb28e80ff73..b9099ce256a 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -19,6 +19,8 @@ module Gitlab commit_message: commit_message || default_commit_message } resolver.resolve_conflicts(user, files, args) + ensure + @merge_request.clear_memoized_shas end def files diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb index 3945df27eed..84ee94e38e4 100644 --- a/lib/gitlab/ldap/user.rb +++ b/lib/gitlab/ldap/user.rb @@ -36,10 +36,6 @@ module Gitlab ldap_config.block_auto_created_users end - def sync_profile_from_provider? - true - end - def allowed? Gitlab::LDAP::Access.allowed?(gl_user) end diff --git a/lib/gitlab/metrics/method_call.rb b/lib/gitlab/metrics/method_call.rb index 65d55576ac2..9112164f22e 100644 --- a/lib/gitlab/metrics/method_call.rb +++ b/lib/gitlab/metrics/method_call.rb @@ -1,7 +1,11 @@ +# rubocop:disable Style/ClassVars + module Gitlab module Metrics # Class for tracking timing information about method calls class MethodCall + @@measurement_enabled_cache = Concurrent::AtomicBoolean.new(false) + @@measurement_enabled_cache_expires_at = Concurrent::AtomicFixnum.new(Time.now.to_i) MUTEX = Mutex.new BASE_LABELS = { module: nil, method: nil }.freeze attr_reader :real_time, :cpu_time, :call_count, :labels @@ -18,6 +22,10 @@ module Gitlab end end + def self.measurement_enabled_cache_expires_at + @@measurement_enabled_cache_expires_at + end + # name - The full name of the method (including namespace) such as # `User#sign_in`. # @@ -72,7 +80,14 @@ module Gitlab end def call_measurement_enabled? - Feature.get(:prometheus_metrics_method_instrumentation).enabled? + expires_at = @@measurement_enabled_cache_expires_at.value + if expires_at < Time.now.to_i + if @@measurement_enabled_cache_expires_at.compare_and_set(expires_at, 1.minute.from_now.to_i) + @@measurement_enabled_cache.value = Feature.get(:prometheus_metrics_method_instrumentation).enabled? + end + end + + @@measurement_enabled_cache.value end end end diff --git a/lib/gitlab/o_auth/provider.rb b/lib/gitlab/o_auth/provider.rb index ac9d66c836d..657db29c85a 100644 --- a/lib/gitlab/o_auth/provider.rb +++ b/lib/gitlab/o_auth/provider.rb @@ -19,6 +19,18 @@ module Gitlab name.to_s.start_with?('ldap') end + def self.sync_profile_from_provider?(provider) + return true if ldap_provider?(provider) + + providers = Gitlab.config.omniauth.sync_profile_from_provider + + if providers.is_a?(Array) + providers.include?(provider) + else + providers + end + end + def self.config_for(name) name = name.to_s if ldap_provider?(name) diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 552133234a3..d33f33d192f 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -12,7 +12,7 @@ module Gitlab def initialize(auth_hash) self.auth_hash = auth_hash - update_profile if sync_profile_from_provider? + update_profile add_or_update_user_identities end @@ -195,29 +195,31 @@ module Gitlab end def sync_profile_from_provider? - providers = Gitlab.config.omniauth.sync_profile_from_provider - - if providers.is_a?(Array) - providers.include?(auth_hash.provider) - else - providers - end + Gitlab::OAuth::Provider.sync_profile_from_provider?(auth_hash.provider) end def update_profile - user_synced_attributes_metadata = gl_user.user_synced_attributes_metadata || gl_user.build_user_synced_attributes_metadata - - UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES.each do |key| - if auth_hash.has_attribute?(key) && gl_user.sync_attribute?(key) - gl_user[key] = auth_hash.public_send(key) # rubocop:disable GitlabSecurity/PublicSend - user_synced_attributes_metadata.set_attribute_synced(key, true) - else - user_synced_attributes_metadata.set_attribute_synced(key, false) + return unless sync_profile_from_provider? || creating_linked_ldap_user? + + metadata = gl_user.user_synced_attributes_metadata || gl_user.build_user_synced_attributes_metadata + + if sync_profile_from_provider? + UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES.each do |key| + if auth_hash.has_attribute?(key) && gl_user.sync_attribute?(key) + gl_user[key] = auth_hash.public_send(key) # rubocop:disable GitlabSecurity/PublicSend + metadata.set_attribute_synced(key, true) + else + metadata.set_attribute_synced(key, false) + end end + + metadata.provider = auth_hash.provider end - user_synced_attributes_metadata.provider = auth_hash.provider - gl_user.user_synced_attributes_metadata = user_synced_attributes_metadata + if creating_linked_ldap_user? && gl_user.email == ldap_person.email.first + metadata.set_attribute_synced(:email, true) + metadata.provider = ldap_person.provider + end end def log diff --git a/lib/gitlab/utils/strong_memoize.rb b/lib/gitlab/utils/strong_memoize.rb index a2ac9285b56..fe091f4611b 100644 --- a/lib/gitlab/utils/strong_memoize.rb +++ b/lib/gitlab/utils/strong_memoize.rb @@ -11,6 +11,8 @@ module Gitlab # # We could write it like: # + # include Gitlab::Utils::StrongMemoize + # # def trigger_from_token # strong_memoize(:trigger) do # Ci::Trigger.find_by_token(params[:token].to_s) @@ -18,14 +20,22 @@ module Gitlab # end # def strong_memoize(name) - ivar_name = "@#{name}" - - if instance_variable_defined?(ivar_name) - instance_variable_get(ivar_name) + if instance_variable_defined?(ivar(name)) + instance_variable_get(ivar(name)) else - instance_variable_set(ivar_name, yield) + instance_variable_set(ivar(name), yield) end end + + def clear_memoization(name) + remove_instance_variable(ivar(name)) if instance_variable_defined?(ivar(name)) + end + + private + + def ivar(name) + "@#{name}" + end end end end diff --git a/qa/qa/page/group/show.rb b/qa/qa/page/group/show.rb index 8080deda675..100e71ae157 100644 --- a/qa/qa/page/group/show.rb +++ b/qa/qa/page/group/show.rb @@ -6,7 +6,13 @@ module QA click_link name end + def filter_by_name(name) + fill_in 'Filter by name...', with: name + end + def has_subgroup?(name) + filter_by_name(name) + page.has_link?(name) end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 51d5d6a52b3..d03ecefe47f 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -91,11 +91,11 @@ describe Projects::MergeRequestsController do end end - context 'without basic serializer param' do - it 'renders the merge request in the json format' do - go(format: :json) + context 'with widget serializer param' do + it 'renders widget MR entity as json' do + go(serializer: 'widget', format: :json) - expect(response).to match_response_schema('entities/merge_request') + expect(response).to match_response_schema('entities/merge_request_widget') end end end diff --git a/spec/features/help_pages_spec.rb b/spec/features/help_pages_spec.rb index 93be3b066ee..ab896a310be 100644 --- a/spec/features/help_pages_spec.rb +++ b/spec/features/help_pages_spec.rb @@ -37,7 +37,7 @@ describe 'Help Pages' do context 'in a production environment with version check enabled', :js do before do allow(Rails.env).to receive(:production?) { true } - allow_any_instance_of(ApplicationSetting).to receive(:version_check_enabled) { true } + stub_application_setting(version_check_enabled: true) allow_any_instance_of(VersionCheck).to receive(:url) { '/version-check-url' } sign_in(create(:user)) @@ -56,9 +56,9 @@ describe 'Help Pages' do describe 'when help page is customized' do before do - allow_any_instance_of(ApplicationSetting).to receive(:help_page_hide_commercial_content?) { true } - allow_any_instance_of(ApplicationSetting).to receive(:help_page_text) { "My Custom Text" } - allow_any_instance_of(ApplicationSetting).to receive(:help_page_support_url) { "http://example.com/help" } + stub_application_setting(help_page_hide_commercial_content: true, + help_page_text: 'My Custom Text', + help_page_support_url: 'http://example.com/help') sign_in(create(:user)) visit help_path diff --git a/spec/features/merge_requests/mini_pipeline_graph_spec.rb b/spec/features/merge_requests/mini_pipeline_graph_spec.rb index 93c5e945453..a7e7c0eeff6 100644 --- a/spec/features/merge_requests/mini_pipeline_graph_spec.rb +++ b/spec/features/merge_requests/mini_pipeline_graph_spec.rb @@ -15,8 +15,8 @@ feature 'Mini Pipeline Graph', :js do visit_merge_request end - def visit_merge_request(format = :html) - visit project_merge_request_path(project, merge_request, format: format) + def visit_merge_request(format: :html, serializer: nil) + visit project_merge_request_path(project, merge_request, format: format, serializer: serializer) end it 'should display a mini pipeline graph' do @@ -33,12 +33,12 @@ feature 'Mini Pipeline Graph', :js do end it 'avoids repeated database queries' do - before = ActiveRecord::QueryRecorder.new { visit_merge_request(:json) } + before = ActiveRecord::QueryRecorder.new { visit_merge_request(format: :json, serializer: 'widget') } create(:ci_build, pipeline: pipeline, legacy_artifacts_file: artifacts_file2) create(:ci_build, pipeline: pipeline, when: 'manual') - after = ActiveRecord::QueryRecorder.new { visit_merge_request(:json) } + after = ActiveRecord::QueryRecorder.new { visit_merge_request(format: :json, serializer: 'widget') } expect(before.count).to eq(after.count) expect(before.cached_count).to eq(after.cached_count) diff --git a/spec/fixtures/api/schemas/entities/merge_request.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index ba094ba1657..342890c3dee 100644 --- a/spec/fixtures/api/schemas/entities/merge_request.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -80,15 +80,15 @@ "target_branch_tree_path": { "type": "string" }, "source_branch_path": { "type": "string" }, "conflict_resolution_path": { "type": ["string", "null"] }, - "cancel_merge_when_pipeline_succeeds_path": { "type": "string" }, - "create_issue_to_resolve_discussions_path": { "type": "string" }, - "merge_path": { "type": "string" }, + "cancel_merge_when_pipeline_succeeds_path": { "type": ["string", "null"] }, + "create_issue_to_resolve_discussions_path": { "type": ["string", "null"] }, + "merge_path": { "type": ["string", "null"] }, "cherry_pick_in_fork_path": { "type": ["string", "null"] }, "revert_in_fork_path": { "type": ["string", "null"] }, "email_patches_path": { "type": "string" }, "plain_diff_path": { "type": "string" }, "status_path": { "type": "string" }, - "new_blob_path": { "type": "string" }, + "new_blob_path": { "type": ["string", "null"] }, "merge_check_path": { "type": "string" }, "ci_environments_status_path": { "type": "string" }, "merge_commit_message_with_description": { "type": "string" }, diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb index cd15e27b497..36a44f8567a 100644 --- a/spec/helpers/notes_helper_spec.rb +++ b/spec/helpers/notes_helper_spec.rb @@ -41,6 +41,7 @@ describe NotesHelper do describe '#discussion_path' do let(:project) { create(:project, :repository) } + let(:anchor) { discussion.line_code } context 'for a merge request discusion' do let(:merge_request) { create(:merge_request, source_project: project, target_project: project, importing: true) } @@ -151,6 +152,15 @@ describe NotesHelper do expect(helper.discussion_path(discussion)).to be_nil end end + + context 'for a contextual commit discussion' do + let(:commit) { merge_request.commits.last } + let(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, commit_id: commit.id).to_discussion } + + it 'returns the merge request diff discussion scoped in the commit' do + expect(helper.discussion_path(discussion)).to eq(diffs_project_merge_request_path(project, merge_request, commit_id: commit.id, anchor: anchor)) + end + end end context 'for a commit discussion' do @@ -160,7 +170,7 @@ describe NotesHelper do let(:discussion) { create(:diff_note_on_commit, project: project).to_discussion } it 'returns the commit path with the line code' do - expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: discussion.line_code)) + expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: anchor)) end end @@ -168,7 +178,7 @@ describe NotesHelper do let(:discussion) { create(:legacy_diff_note_on_commit, project: project).to_discussion } it 'returns the commit path with the line code' do - expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: discussion.line_code)) + expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: anchor)) end end diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb index 260df6e4dae..048caa38fcf 100644 --- a/spec/lib/gitlab/ldap/user_spec.rb +++ b/spec/lib/gitlab/ldap/user_spec.rb @@ -38,7 +38,6 @@ describe Gitlab::LDAP::User do it "does not mark existing ldap user as changed" do create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', provider: 'ldapmain') - ldap_user.gl_user.user_synced_attributes_metadata(provider: 'ldapmain', email: true) expect(ldap_user.changed?).to be_falsey end end @@ -144,11 +143,15 @@ describe Gitlab::LDAP::User do expect(ldap_user.gl_user.email).to eq(info[:email]) end - it "has user_synced_attributes_metadata email set to true" do + it "has email set as synced" do expect(ldap_user.gl_user.user_synced_attributes_metadata.email_synced).to be_truthy end - it "has synced_attribute_provider set to ldapmain" do + it "has email set as read-only" do + expect(ldap_user.gl_user.read_only_attribute?(:email)).to be_truthy + end + + it "has synced attributes provider set to ldapmain" do expect(ldap_user.gl_user.user_synced_attributes_metadata.provider).to eql 'ldapmain' end end @@ -162,9 +165,13 @@ describe Gitlab::LDAP::User do expect(ldap_user.gl_user.temp_oauth_email?).to be_truthy end - it "has synced attribute email set to false" do + it "has email set as not synced" do expect(ldap_user.gl_user.user_synced_attributes_metadata.email_synced).to be_falsey end + + it "does not have email set as read-only" do + expect(ldap_user.gl_user.read_only_attribute?(:email)).to be_falsey + end end end diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index 5341addf911..78767d06462 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -20,9 +20,39 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is enabled' do before do + allow(Feature.get(:prometheus_metrics_method_instrumentation)).to receive(:enabled?).and_call_original + described_class.measurement_enabled_cache_expires_at.value = Time.now.to_i - 1 Feature.get(:prometheus_metrics_method_instrumentation).enable end + around do |example| + Timecop.freeze do + example.run + end + end + + it 'caches subsequent invocations of feature check' do + 10.times do + method_call.measure { 'foo' } + end + + expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).once + end + + it 'expires feature check cache after 1 minute' do + method_call.measure { 'foo' } + + Timecop.travel(1.minute.from_now) do + method_call.measure { 'foo' } + end + + Timecop.travel(1.minute.from_now + 1.second) do + method_call.measure { 'foo' } + end + + expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).twice + end + it 'observes the performance of the supplied block' do expect(described_class.call_duration_histogram) .to receive(:observe) @@ -34,6 +64,8 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is disabled' do before do + described_class.measurement_enabled_cache_expires_at.value = Time.now.to_i - 1 + Feature.get(:prometheus_metrics_method_instrumentation).disable end diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 2f19fb7312d..6334bcd0156 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -202,11 +202,13 @@ describe Gitlab::OAuth::User do end context "and no account for the LDAP user" do - it "creates a user with dual LDAP and omniauth identities" do + before do allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) oauth_user.save + end + it "creates a user with dual LDAP and omniauth identities" do expect(gl_user).to be_valid expect(gl_user.username).to eql uid expect(gl_user.email).to eql 'johndoe@example.com' @@ -219,6 +221,18 @@ describe Gitlab::OAuth::User do ] ) end + + it "has email set as synced" do + expect(gl_user.user_synced_attributes_metadata.email_synced).to be_truthy + end + + it "has email set as read-only" do + expect(gl_user.read_only_attribute?(:email)).to be_truthy + end + + it "has synced attributes provider set to ldapmain" do + expect(gl_user.user_synced_attributes_metadata.provider).to eql 'ldapmain' + end end context "and LDAP user has an account already" do @@ -440,11 +454,15 @@ describe Gitlab::OAuth::User do expect(gl_user.email).to eq(info_hash[:email]) end - it "has external_attributes set to true" do - expect(gl_user.user_synced_attributes_metadata).not_to be_nil + it "has email set as synced" do + expect(gl_user.user_synced_attributes_metadata.email_synced).to be_truthy + end + + it "has email set as read-only" do + expect(gl_user.read_only_attribute?(:email)).to be_truthy end - it "has attributes_provider set to my-provider" do + it "has synced attributes provider set to my-provider" do expect(gl_user.user_synced_attributes_metadata.provider).to eql 'my-provider' end end @@ -458,10 +476,13 @@ describe Gitlab::OAuth::User do expect(gl_user.email).not_to eq(info_hash[:email]) end - it "has user_synced_attributes_metadata set to nil" do - expect(gl_user.user_synced_attributes_metadata.provider).to eql 'my-provider' + it "has email set as not synced" do expect(gl_user.user_synced_attributes_metadata.email_synced).to be_falsey end + + it "does not have email set as read-only" do + expect(gl_user.read_only_attribute?(:email)).to be_falsey + end end end @@ -508,11 +529,15 @@ describe Gitlab::OAuth::User do expect(gl_user.email).to eq(info_hash[:email]) end - it "has email_synced_attribute set to true" do + it "has email set as synced" do expect(gl_user.user_synced_attributes_metadata.email_synced).to be(true) end - it "has my-provider as attributes_provider" do + it "has email set as read-only" do + expect(gl_user.read_only_attribute?(:email)).to be_truthy + end + + it "has synced attributes provider set to my-provider" do expect(gl_user.user_synced_attributes_metadata.provider).to eql 'my-provider' end end @@ -524,7 +549,14 @@ describe Gitlab::OAuth::User do it "does not update the user email" do expect(gl_user.email).not_to eq(info_hash[:email]) - expect(gl_user.user_synced_attributes_metadata.email_synced).to be(false) + end + + it "has email set as not synced" do + expect(gl_user.user_synced_attributes_metadata.email_synced).to be_falsey + end + + it "does not have email set as read-only" do + expect(gl_user.read_only_attribute?(:email)).to be_falsey end end end diff --git a/spec/lib/gitlab/utils/strong_memoize_spec.rb b/spec/lib/gitlab/utils/strong_memoize_spec.rb index 4a104ab6d97..473f8100771 100644 --- a/spec/lib/gitlab/utils/strong_memoize_spec.rb +++ b/spec/lib/gitlab/utils/strong_memoize_spec.rb @@ -49,4 +49,16 @@ describe Gitlab::Utils::StrongMemoize do end end end + + describe '#clear_memoization' do + let(:value) { 'mepmep' } + + it 'removes the instance variable' do + object.method_name + + object.clear_memoization(:method_name) + + expect(object.instance_variable_defined?(:@method_name)).to be(false) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 0603fc37715..bb63abd167b 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -124,6 +124,7 @@ describe MergeRequest do context 'when the target branch does not exist' do before do project.repository.rm_branch(subject.author, subject.target_branch) + subject.clear_memoized_shas end it 'returns nil' do @@ -600,30 +601,30 @@ describe MergeRequest do end describe '#can_remove_source_branch?' do - let(:user) { create(:user) } - let(:user2) { create(:user) } + set(:user) { create(:user) } + set(:merge_request) { create(:merge_request, :simple) } - before do - subject.source_project.team << [user, :master] + subject { merge_request } - subject.source_branch = "feature" - subject.target_branch = "master" - subject.save! + before do + subject.source_project.add_master(user) end it "can't be removed when its a protected branch" do allow(ProtectedBranch).to receive(:protected?).and_return(true) + expect(subject.can_remove_source_branch?(user)).to be_falsey end it "can't remove a root ref" do - subject.source_branch = "master" - subject.target_branch = "feature" + subject.update(source_branch: 'master', target_branch: 'feature') expect(subject.can_remove_source_branch?(user)).to be_falsey end it "is unable to remove the source branch for a project the user cannot push to" do + user2 = create(:user) + expect(subject.can_remove_source_branch?(user2)).to be_falsey end @@ -634,6 +635,7 @@ describe MergeRequest do end it "cannot be removed if the last commit is not also the head of the source branch" do + subject.clear_memoized_shas subject.source_branch = "lfs" expect(subject.can_remove_source_branch?(user)).to be_falsey @@ -733,7 +735,7 @@ describe MergeRequest do before do project.repository.raw_repository.delete_branch(subject.target_branch) - subject.reload + subject.clear_memoized_shas end it 'does not crash' do @@ -1404,6 +1406,16 @@ describe MergeRequest do subject.reload_diff end + + context 'when using the after_update hook to update' do + context 'when the branches are updated' do + it 'uses the new heads to generate the diff' do + expect { subject.update!(source_branch: subject.target_branch, target_branch: subject.source_branch) } + .to change { subject.merge_request_diff.start_commit_sha } + .and change { subject.merge_request_diff.head_commit_sha } + end + end + end end describe '#update_diff_discussion_positions' do @@ -1468,6 +1480,7 @@ describe MergeRequest do context 'when the target branch does not exist' do before do subject.project.repository.rm_branch(subject.author, subject.target_branch) + subject.clear_memoized_shas end it 'returns nil' do diff --git a/spec/serializers/merge_request_serializer_spec.rb b/spec/serializers/merge_request_serializer_spec.rb index e3abefa6d63..1ad974c774b 100644 --- a/spec/serializers/merge_request_serializer_spec.rb +++ b/spec/serializers/merge_request_serializer_spec.rb @@ -1,37 +1,43 @@ require 'spec_helper' describe MergeRequestSerializer do - let(:user) { build_stubbed(:user) } - let(:merge_request) { build_stubbed(:merge_request) } - - let(:serializer) do + let(:user) { create(:user) } + let(:resource) { create(:merge_request) } + let(:json_entity) do described_class.new(current_user: user) + .represent(resource, serializer: serializer) + .with_indifferent_access end - describe '#represent' do - let(:opts) { { serializer: serializer_entity } } - subject { serializer.represent(merge_request, serializer: serializer_entity) } + context 'widget merge request serialization' do + let(:serializer) { 'widget' } - context 'when passing basic serializer param' do - let(:serializer_entity) { 'basic' } + it 'matches issue json schema' do + expect(json_entity).to match_schema('entities/merge_request_widget') + end + end - it 'calls super class #represent with correct params' do - expect_any_instance_of(BaseSerializer).to receive(:represent) - .with(merge_request, opts, MergeRequestBasicEntity) + context 'sidebar merge request serialization' do + let(:serializer) { 'sidebar' } - subject - end + it 'matches basic merge request json schema' do + expect(json_entity).to match_schema('entities/merge_request_basic') end + end - context 'when serializer param is falsy' do - let(:serializer_entity) { nil } + context 'basic merge request serialization' do + let(:serializer) { 'basic' } + + it 'matches basic merge request json schema' do + expect(json_entity).to match_schema('entities/merge_request_basic') + end + end - it 'calls super class #represent with correct params' do - expect_any_instance_of(BaseSerializer).to receive(:represent) - .with(merge_request, opts, MergeRequestEntity) + context 'no serializer' do + let(:serializer) { nil } - subject - end + it 'raises an error' do + expect { json_entity }.to raise_error(NoMethodError) end end end diff --git a/spec/serializers/merge_request_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb index 1ad672fd355..a5924a8589c 100644 --- a/spec/serializers/merge_request_entity_spec.rb +++ b/spec/serializers/merge_request_widget_entity_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe MergeRequestEntity do +describe MergeRequestWidgetEntity do let(:project) { create :project, :repository } let(:resource) { create(:merge_request, source_project: project, target_project: project) } let(:user) { create(:user) } @@ -35,33 +35,6 @@ describe MergeRequestEntity do end end - it 'includes issues_links' do - issues_links = subject[:issues_links] - - expect(issues_links).to include(:closing, :mentioned_but_not_closing, - :assign_to_closing) - end - - it 'has Issuable attributes' do - expect(subject).to include(:id, :iid, :author_id, :description, :lock_version, :milestone_id, - :title, :updated_by_id, :created_at, :updated_at, :milestone, :labels) - end - - it 'has time estimation attributes' do - expect(subject).to include(:time_estimate, :total_time_spent, :human_time_estimate, :human_total_time_spent) - end - - it 'has important MergeRequest attributes' do - expect(subject).to include(:state, :deleted_at, :diff_head_sha, :merge_commit_message, - :has_conflicts, :has_ci, :merge_path, - :conflict_resolution_path, - :cancel_merge_when_pipeline_succeeds_path, - :create_issue_to_resolve_discussions_path, - :source_branch_path, :target_branch_commits_path, - :target_branch_tree_path, :commits_count, :merge_ongoing, - :ff_only_enabled) - end - it 'has email_patches_path' do expect(subject[:email_patches_path]) .to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}.patch") @@ -116,18 +89,6 @@ describe MergeRequestEntity do end end - it 'includes merge_event' do - create(:event, :merged, author: user, project: resource.project, target: resource) - - expect(subject[:merge_event]).to include(:author, :updated_at) - end - - it 'includes closed_event' do - create(:event, :closed, author: user, project: resource.project, target: resource) - - expect(subject[:closed_event]).to include(:author, :updated_at) - end - describe 'diverged_commits_count' do context 'when MR open and its diverging' do it 'returns diverged commits count' do diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index f86f1ac2443..c38ddf4612b 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -1,14 +1,14 @@ require 'spec_helper' describe MergeRequests::MergeService do - let(:user) { create(:user) } - let(:user2) { create(:user) } + set(:user) { create(:user) } + set(:user2) { create(:user) } let(:merge_request) { create(:merge_request, :simple, author: user2, assignee: user2) } let(:project) { merge_request.project } before do - project.team << [user, :master] - project.team << [user2, :developer] + project.add_master(user) + project.add_developer(user2) end describe '#execute' do diff --git a/spec/support/batch_loader.rb b/spec/support/batch_loader.rb new file mode 100644 index 00000000000..bb790e660a6 --- /dev/null +++ b/spec/support/batch_loader.rb @@ -0,0 +1,5 @@ +RSpec.configure do |config| + config.after do + BatchLoader::Executor.clear_current + end +end diff --git a/spec/support/stub_configuration.rb b/spec/support/stub_configuration.rb index b36cf3c544c..9f08c139322 100644 --- a/spec/support/stub_configuration.rb +++ b/spec/support/stub_configuration.rb @@ -7,6 +7,9 @@ module StubConfiguration allow_any_instance_of(ApplicationSetting).to receive_messages(to_settings(messages)) allow(Gitlab::CurrentSettings.current_application_settings) .to receive_messages(to_settings(messages)) + + # Ensure that we don't use the Markdown cache when stubbing these values + allow_any_instance_of(ApplicationSetting).to receive(:cached_html_up_to_date?).and_return(false) end def stub_not_protect_default_branch diff --git a/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml b/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml index da4d86b9a04..275487071f3 100644 --- a/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml +++ b/vendor/gitlab-ci-yml/Auto-DevOps.gitlab-ci.yml @@ -89,7 +89,7 @@ sast: POSTGRES_DB: "false" allow_failure: true script: - - /app/bin/run . + - sast . artifacts: paths: [gl-sast-report.json] @@ -232,6 +232,17 @@ production: docker run ${cc_opts} codeclimate/codeclimate:0.69.0 analyze -f json > codeclimate.json } + function sast() { + case "$CI_SERVER_VERSION" in + *-ee) + /app/bin/run "$@" + ;; + *) + echo "GitLab EE is required" + ;; + esac + } + function deploy() { track="${1-stable}" name="$CI_ENVIRONMENT_SLUG" |
