Add latest changes from gitlab-org/gitlab@master

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline_container.vue

@@ -4,6 +4,7 @@ import ArtifactsApp from './artifacts_list_app.vue';
 import Deployment from './deployment.vue';
 import MrWidgetContainer from './mr_widget_container.vue';
 import MrWidgetPipeline from './mr_widget_pipeline.vue';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 /**
  * Renders the pipeline and related deployments from the store.
@@ -23,6 +24,7 @@ export default {
     MergeTrainPositionIndicator: () =>
       import('ee_component/vue_merge_request_widget/components/merge_train_position_indicator.vue'),
   },
+  mixins: [glFeatureFlagsMixin()],
   props: {
     mr: {
       type: Object,
@@ -62,7 +64,7 @@ export default {
       return this.isPostMerge ? this.mr.mergePipeline : this.mr.pipeline;
     },
     showVisualReviewAppLink() {
-      return this.mr.visualReviewAppAvailable;
+      return this.mr.visualReviewAppAvailable && this.glFeatures.anonymousVisualReviewFeedback;
     },
     showMergeTrainPositionIndicator() {
       return _.isNumber(this.mr.mergeTrainIndex);

app/assets/javascripts/vue_merge_request_widget/components/review_app_link.vue

@@ -18,7 +18,14 @@ export default {
 };
 </script>
 <template>
-  <a :href="link" target="_blank" rel="noopener noreferrer nofollow" :class="cssClass">
+  <a
+    :href="link"
+    target="_blank"
+    rel="noopener noreferrer nofollow"
+    :class="cssClass"
+    data-track-event="open_review_app"
+    data-track-label="review_app"
+  >
     {{ __('View app') }} <icon class="fgray" name="external-link" />
   </a>
 </template>

app/models/project.rb

@@ -1773,7 +1773,6 @@ class Project < ApplicationRecord
     InternalId.flush_records!(project: self)
     import_state.finish
-    import_state.remove_jid
     update_project_counter_caches
     after_create_default_branch
     join_pool_repository

app/models/project_import_state.rb

@@ -42,6 +42,14 @@ class ProjectImportState < ApplicationRecord
       end
     end
+    after_transition any => :finished do |state, _|
+      if state.jid.present?
+        Gitlab::SidekiqStatus.unset(state.jid)
+
+        state.update_column(:jid, nil)
+      end
+    end
+
     after_transition started: :finished do |state, _|
       project = state.project
@@ -81,14 +89,6 @@ class ProjectImportState < ApplicationRecord
     status == 'started' && project.import?
   end
-  def remove_jid
-    return unless jid
-
-    Gitlab::SidekiqStatus.unset(jid)
-
-    update_column(:jid, nil)
-  end
-
   # Refreshes the expiration time of the associated import job ID.
   #
   # This method can be used by asynchronous importers to refresh the status,

app/presenters/clusters/cluster_presenter.rb

@@ -18,8 +18,20 @@ module Clusters
       end
     end
-    def gke_cluster_url
-      "https://console.cloud.google.com/kubernetes/clusters/details/#{provider.zone}/#{name}" if gcp?
+    def provider_label
+      if aws?
+        s_('ClusterIntegration|Elastic Kubernetes Service')
+      elsif gcp?
+        s_('ClusterIntegration|Google Kubernetes Engine')
+      end
+    end
+
+    def provider_management_url
+      if aws?
+        "https://console.aws.amazon.com/eks/home?region=#{provider.region}\#/clusters/#{name}"
+      elsif gcp?
+        "https://console.cloud.google.com/kubernetes/clusters/details/#{provider.zone}/#{name}"
+      end
     end
     def can_read_cluster?

app/views/clusters/clusters/_advanced_settings.html.haml

@@ -8,10 +8,10 @@
   - unless @cluster.provided_by_user?
     .append-bottom-20
       %label.append-bottom-10
-        = s_('ClusterIntegration|Google Kubernetes Engine')
+        = @cluster.provider_label
       %p
-        - link_gke = link_to(s_('ClusterIntegration|Google Kubernetes Engine'), @cluster.gke_cluster_url, target: '_blank', rel: 'noopener noreferrer')
-        = s_('ClusterIntegration|Manage your Kubernetes cluster by visiting %{link_gke}').html_safe % { link_gke: link_gke }
+        - provider_link = link_to(@cluster.provider_label, @cluster.provider_management_url, target: '_blank', rel: 'noopener noreferrer')
+        = s_('ClusterIntegration|Manage your Kubernetes cluster by visiting %{provider_link}').html_safe % { provider_link: provider_link }
   = form_for @cluster, url: clusterable.cluster_path(@cluster), as: :cluster, html: { class: 'cluster_management_form' } do |field|

changelogs/unreleased/22392-update-cluster-settings-provider-link.yml

+---
+title: Update external link to provider in cluster settings
+merge_request: 20425
+author:
+type: fixed

changelogs/unreleased/29067-vrt-no-auth.yml

+---
+title: Remove authentication step from visual review tools instructions
+merge_request:
+author:
+type: changed

changelogs/unreleased/33596-package-ci-metadata.yml

+---
+title: Improve job tokens and provide access helper
+merge_request: 19793
+author:
+type: other

doc/administration/integration/plantuml.md

@@ -60,11 +60,11 @@ mvn package
 The above sequence of commands will generate a WAR file that can be deployed
 using Tomcat:
-```sh
-sudo apt-get install tomcat7
-sudo cp target/plantuml.war /var/lib/tomcat7/webapps/plantuml.war
-sudo chown tomcat7:tomcat7 /var/lib/tomcat7/webapps/plantuml.war
-sudo service tomcat7 restart
+```shell
+sudo apt-get install tomcat8
+sudo cp target/plantuml.war /var/lib/tomcat8/webapps/plantuml.war
+sudo chown tomcat8:tomcat8 /var/lib/tomcat8/webapps/plantuml.war
+sudo service tomcat8 restart
 ```
 Once the Tomcat service restarts the PlantUML service will be ready and
@@ -74,7 +74,7 @@ listening for requests on port 8080:
 http://localhost:8080/plantuml
 ```
-you can change these defaults by editing the `/etc/tomcat7/server.xml` file.
+you can change these defaults by editing the `/etc/tomcat8/server.xml` file.
 Note that the default URL is different than when using the Docker-based image,
 where the service is available at the root of URL with no relative path. Adjust

doc/ci/review_apps/index.md

@@ -163,6 +163,13 @@ that spawned the Review App.
 ### Configuring Visual Reviews
+Ensure that the `anonymous_visual_review_feedback` feature flag is enabled.
+Administrators can enable with a Rails console as follows:
+
+```ruby
+Feature.enabled(:anonymous_visual_review_feedback)
+```
+
 The feedback form is served through a script you add to pages in your Review App.
 If you have [Developer permissions](../../user/permissions.md) to the project,
 you can access it by clicking the **Review** button in the **Pipeline** section
@@ -221,6 +228,19 @@ NOTE: **Note:**
 Future enhancements [are planned](https://gitlab.com/gitlab-org/gitlab/issues/11322)
 to make this process even easier.
+### Determining merge request ID
+
+The visual review tools retrieve the merge request ID from the `data-merge-request-id`
+data attribute included in the `script` HTML tag used to add the visual review tools
+to your review app.
+
+​After determining the ID for the merge request to link to a visual review app, you
+can supply the ID by either:​​
+
+- Hardcoding it in the script tag via the data attribute `data-merge-request-id` of the app.
+- Dynamically adding the `data-merge-request-id` value during the build of the app.
+- Supplying it manually through the visual review form in the app.
+
 ### Using Visual Reviews
 After Visual Reviews has been [enabled](#configuring-visual-reviews) for the
@@ -231,25 +251,15 @@ the bottom-right corner.
 To use the feedback form:
-1. Create a [personal access token](../../user/profile/personal_access_tokens.md)
-   with the API scope selected.
-1. Paste the token into the feedback box when prompted. If you select **Remember me**,
-   your browser stores the token so that future visits to Review Apps at the same URL
-   will not require you to re-enter the token. To clear the token, click **Log out**.
 1. Make a comment on the visual review. You can make use of all the
    [Markdown annotations](../../user/markdown.md) that are also available in
    merge request comments.
+1. Submit your feedback anonymously or add your name.
 1. Finally, click **Send feedback**.
 After you make and submit a comment in the visual review box, it will appear
 automatically in the respective merge request.
-TIP: **Tip:**
-Because tokens must be entered on a per-domain basis and they can only be accessed
-once, different review apps will not remember your token. You can save the token
-to your password manager specifically for the purpose of Visual Reviews. This way,
-you will not need to create additional tokens for each merge request.
-
 ## Limitations
 Review App limitations are the same as [environments limitations](../environments.md#limitations).

doc/user/clusters/management_project.md

@@ -55,7 +55,7 @@ To select a cluster management project to use:
 ### Configuring your pipeline
 After designating a project as the management project for the cluster,
-write a [`.gitlab-ci,yml`](../../ci/yaml/README.md) in that project. For example:
+write a [`.gitlab-ci.yml`](../../ci/yaml/README.md) in that project. For example:
 ```yaml
 configure cluster:

doc/user/project/labels.md

@@ -12,7 +12,7 @@ requests are located.
 In GitLab, you can create project and group labels:
-- **Project labels** can be assigned to epics, issues and merge requests in that project only.
+- **Project labels** can be assigned to issues and merge requests in that project only.
 - **Group labels** can be assigned to any epics, issue and merge request in any project in
   that group, or any subgroups of the group.

locale/gitlab.pot

@@ -3673,6 +3673,9 @@ msgstr ""
 msgid "ClusterIntegration|Did you know?"
 msgstr ""
+msgid "ClusterIntegration|Elastic Kubernetes Service"
+msgstr ""
+
 msgid "ClusterIntegration|Elastic Stack"
 msgstr ""
@@ -3904,7 +3907,7 @@ msgstr ""
 msgid "ClusterIntegration|Make sure your account %{link_to_requirements} to create Kubernetes clusters"
 msgstr ""
-msgid "ClusterIntegration|Manage your Kubernetes cluster by visiting %{link_gke}"
+msgid "ClusterIntegration|Manage your Kubernetes cluster by visiting %{provider_link}"
 msgstr ""
 msgid "ClusterIntegration|No IAM Roles found"
@@ -19327,13 +19330,10 @@ msgstr ""
 msgid "VisualReviewApp|%{stepStart}Step 2%{stepEnd}. Add it to the %{headTags} tags of every page of your application, ensuring the merge request ID is set or not set as required. "
 msgstr ""
-msgid "VisualReviewApp|%{stepStart}Step 3%{stepEnd}. Open the Review App and provide a %{linkStart}personal access token%{linkEnd}."
-msgstr ""
-
-msgid "VisualReviewApp|%{stepStart}Step 4%{stepEnd}. If not previously %{linkStart}configured%{linkEnd} by a developer, enter the merge request ID for the review when prompted. The ID of this merge request is %{stepStart}%{mrId}%{stepStart}."
+msgid "VisualReviewApp|%{stepStart}Step 3%{stepEnd}. If not previously %{linkStart}configured%{linkEnd} by a developer, enter the merge request ID for the review when prompted. The ID of this merge request is %{stepStart}%{mrId}%{stepStart}."
 msgstr ""
-msgid "VisualReviewApp|%{stepStart}Step 5%{stepEnd}. Leave feedback in the Review App."
+msgid "VisualReviewApp|%{stepStart}Step 4%{stepEnd}. Leave feedback in the Review App."
 msgstr ""
 msgid "VisualReviewApp|Copy merge request ID"

spec/javascripts/vue_mr_widget/components/review_app_link_spec.js

 import Vue from 'vue';
 import component from '~/vue_merge_request_widget/components/review_app_link.vue';
+import { mockTracking, triggerEvent } from 'spec/helpers/tracking_helper';
 import mountComponent from '../../helpers/vue_mount_component_helper';
 describe('review app link', () => {
@@ -35,4 +36,13 @@ describe('review app link', () => {
   it('renders svg icon', () => {
     expect(el.querySelector('svg')).not.toBeNull();
   });
+
+  it('tracks an event when clicked', () => {
+    const spy = mockTracking('_category_', el, spyOn);
+    triggerEvent(el);
+
+    expect(spy).toHaveBeenCalledWith('_category_', 'open_review_app', {
+      label: 'review_app',
+    });
+  });
 });

spec/javascripts/vue_mr_widget/mock_data.js

@@ -284,7 +284,20 @@ export const mockStore = {
   targetBranch: 'target-branch',
   sourceBranch: 'source-branch',
   sourceBranchLink: 'source-branch-link',
-  deployments: [{ id: 0, name: 'bogus' }, { id: 1, name: 'bogus-docs' }],
+  deployments: [
+    {
+      id: 0,
+      name: 'bogus',
+      external_url: 'https://fake.com',
+      external_url_formatted: 'https://fake.com',
+    },
+    {
+      id: 1,
+      name: 'bogus-docs',
+      external_url: 'https://fake.com',
+      external_url_formatted: 'https://fake.com',
+    },
+  ],
   postMergeDeployments: [{ id: 0, name: 'prod' }, { id: 1, name: 'prod-docs' }],
   troubleshootingDocsPath: 'troubleshooting-docs-path',
   ciStatus: 'ci-status',

spec/models/project_import_state_spec.rb

@@ -95,30 +95,28 @@ describe ProjectImportState, type: :model do
     end
   end
-  describe '#remove_jid', :clean_gitlab_redis_cache do
-    let(:project) {  }
-
+  describe 'clearing `jid` after finish', :clean_gitlab_redis_cache do
     context 'without an JID' do
       it 'does nothing' do
-        import_state = create(:import_state)
+        import_state = create(:import_state, :started)
         expect(Gitlab::SidekiqStatus)
           .not_to receive(:unset)
-        import_state.remove_jid
+        import_state.finish!
       end
     end
     context 'with an JID' do
       it 'unsets the JID' do
-        import_state = create(:import_state, jid: '123')
+        import_state = create(:import_state, :started, jid: '123')
         expect(Gitlab::SidekiqStatus)
           .to receive(:unset)
           .with('123')
           .and_call_original
-        import_state.remove_jid
+        import_state.finish!
         expect(import_state.jid).to be_nil
       end

spec/models/project_spec.rb

@@ -4335,7 +4335,6 @@ describe Project do
       expect(project.wiki.repository).to receive(:after_import)
       expect(import_state).to receive(:finish)
       expect(project).to receive(:update_project_counter_caches)
-      expect(import_state).to receive(:remove_jid)
       expect(project).to receive(:after_create_default_branch)
       expect(project).to receive(:refresh_markdown_cache!)
       expect(InternalId).to receive(:flush_records!).with(project: project)

spec/presenters/clusters/cluster_presenter_spec.rb

@@ -153,11 +153,42 @@ describe Clusters::ClusterPresenter do
     end
   end
-  describe '#gke_cluster_url' do
-    subject { described_class.new(cluster).gke_cluster_url }
-    it { is_expected.to include(cluster.provider.zone) }
-    it { is_expected.to include(cluster.name) }
+  describe '#provider_label' do
+    let(:cluster) { create(:cluster, provider_type: provider_type) }
+    subject { described_class.new(cluster).provider_label }
+
+    context 'AWS provider' do
+      let(:provider_type) { :aws }
+
+      it { is_expected.to eq('Elastic Kubernetes Service') }
+    end
+
+    context 'GCP provider' do
+      let(:provider_type) { :gcp }
+
+      it { is_expected.to eq('Google Kubernetes Engine') }
+    end
+  end
+
+  describe '#provider_management_url' do
+    let(:cluster) { provider.cluster }
+
+    subject { described_class.new(cluster).provider_management_url }
+
+    context 'AWS provider' do
+      let(:provider) { create(:cluster_provider_aws) }
+
+      it { is_expected.to include(provider.region) }
+      it { is_expected.to include(cluster.name) }
+    end
+
+    context 'GCP provider' do
+      let(:provider) { create(:cluster_provider_gcp) }
+
+      it { is_expected.to include(provider.zone) }
+      it { is_expected.to include(cluster.name) }
+    end
   end
   describe '#cluster_type_description' do