From 157f9a451a428f66666ba85d8b880df78aff6cc4 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 22 Nov 2019 00:06:08 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../mr_widget_pipeline_container.vue | 4 +- .../components/review_app_link.vue | 9 ++++- app/models/project.rb | 1 - app/models/project_import_state.rb | 16 ++++---- app/presenters/clusters/cluster_presenter.rb | 16 +++++++- .../clusters/_advanced_settings.html.haml | 6 +-- ...-update-cluster-settings-provider-link.yml | 5 +++ changelogs/unreleased/29067-vrt-no-auth.yml | 5 +++ .../unreleased/33596-package-ci-metadata.yml | 5 +++ doc/administration/integration/plantuml.md | 12 +++--- doc/ci/review_apps/index.md | 32 +++++++++------ doc/user/clusters/management_project.md | 2 +- doc/user/project/labels.md | 2 +- locale/gitlab.pot | 12 +++--- .../components/review_app_link_spec.js | 10 +++++ spec/javascripts/vue_mr_widget/mock_data.js | 15 ++++++- spec/models/project_import_state_spec.rb | 12 +++--- spec/models/project_spec.rb | 1 - .../clusters/cluster_presenter_spec.rb | 39 +++++++++++++++++-- 19 files changed, 150 insertions(+), 54 deletions(-) create mode 100644 changelogs/unreleased/22392-update-cluster-settings-provider-link.yml create mode 100644 changelogs/unreleased/29067-vrt-no-auth.yml create mode 100644 changelogs/unreleased/33596-package-ci-metadata.yml diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline_container.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline_container.vue index ffc3e0967d4..a297156ab10 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline_container.vue +++ b/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); diff --git a/app/assets/javascripts/vue_merge_request_widget/components/review_app_link.vue b/app/assets/javascripts/vue_merge_request_widget/components/review_app_link.vue index 75f557d05dd..d2d32492e6c 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/review_app_link.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/review_app_link.vue @@ -18,7 +18,14 @@ export default { }; diff --git a/app/models/project.rb b/app/models/project.rb index 9a208b0058b..0887e742d96 100644 --- a/app/models/project.rb +++ b/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 diff --git a/app/models/project_import_state.rb b/app/models/project_import_state.rb index bff00816e15..b79e3554926 100644 --- a/app/models/project_import_state.rb +++ b/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, diff --git a/app/presenters/clusters/cluster_presenter.rb b/app/presenters/clusters/cluster_presenter.rb index 1634d2479a0..97771d84031 100644 --- a/app/presenters/clusters/cluster_presenter.rb +++ b/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? diff --git a/app/views/clusters/clusters/_advanced_settings.html.haml b/app/views/clusters/clusters/_advanced_settings.html.haml index 493d7a00854..59cdf2016fb 100644 --- a/app/views/clusters/clusters/_advanced_settings.html.haml +++ b/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| diff --git a/changelogs/unreleased/22392-update-cluster-settings-provider-link.yml b/changelogs/unreleased/22392-update-cluster-settings-provider-link.yml new file mode 100644 index 00000000000..cd041ea491b --- /dev/null +++ b/changelogs/unreleased/22392-update-cluster-settings-provider-link.yml @@ -0,0 +1,5 @@ +--- +title: Update external link to provider in cluster settings +merge_request: 20425 +author: +type: fixed diff --git a/changelogs/unreleased/29067-vrt-no-auth.yml b/changelogs/unreleased/29067-vrt-no-auth.yml new file mode 100644 index 00000000000..5e3a81740c7 --- /dev/null +++ b/changelogs/unreleased/29067-vrt-no-auth.yml @@ -0,0 +1,5 @@ +--- +title: Remove authentication step from visual review tools instructions +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/33596-package-ci-metadata.yml b/changelogs/unreleased/33596-package-ci-metadata.yml new file mode 100644 index 00000000000..24fecc077c8 --- /dev/null +++ b/changelogs/unreleased/33596-package-ci-metadata.yml @@ -0,0 +1,5 @@ +--- +title: Improve job tokens and provide access helper +merge_request: 19793 +author: +type: other diff --git a/doc/administration/integration/plantuml.md b/doc/administration/integration/plantuml.md index 23803b82543..33ac925748f 100644 --- a/doc/administration/integration/plantuml.md +++ b/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 diff --git a/doc/ci/review_apps/index.md b/doc/ci/review_apps/index.md index b69562faad7..0da1228aa53 100644 --- a/doc/ci/review_apps/index.md +++ b/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). diff --git a/doc/user/clusters/management_project.md b/doc/user/clusters/management_project.md index 83b6f6fe300..57a1f46ac6e 100644 --- a/doc/user/clusters/management_project.md +++ b/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: diff --git a/doc/user/project/labels.md b/doc/user/project/labels.md index d8356abdd1c..e4264615488 100644 --- a/doc/user/project/labels.md +++ b/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. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a6b2c315eff..0ef14bf8ed1 100644 --- a/locale/gitlab.pot +++ b/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}." +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 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}." -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" diff --git a/spec/javascripts/vue_mr_widget/components/review_app_link_spec.js b/spec/javascripts/vue_mr_widget/components/review_app_link_spec.js index 68a65bd21c6..069bc14e01e 100644 --- a/spec/javascripts/vue_mr_widget/components/review_app_link_spec.js +++ b/spec/javascripts/vue_mr_widget/components/review_app_link_spec.js @@ -1,5 +1,6 @@ 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', + }); + }); }); diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 089ec08fbf9..40bec001ffe 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/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', diff --git a/spec/models/project_import_state_spec.rb b/spec/models/project_import_state_spec.rb index 0b4dcc62ff6..babde7b0670 100644 --- a/spec/models/project_import_state_spec.rb +++ b/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 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 815ab7aa166..4e6a4bd6b51 100644 --- a/spec/models/project_spec.rb +++ b/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) diff --git a/spec/presenters/clusters/cluster_presenter_spec.rb b/spec/presenters/clusters/cluster_presenter_spec.rb index 8bc5374f2db..6a1360807b7 100644 --- a/spec/presenters/clusters/cluster_presenter_spec.rb +++ b/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 } + describe '#provider_label' do + let(:cluster) { create(:cluster, provider_type: provider_type) } - it { is_expected.to include(cluster.provider.zone) } - it { is_expected.to include(cluster.name) } + 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