diff options
147 files changed, 2554 insertions, 1134 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7f1da58fa9d..8596037afa3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,7 +36,7 @@ _This notice should stay as the first item in the CONTRIBUTING.md file._ - [Release Scoping labels](#release-scoping-labels) - [Priority labels](#priority-labels) - [Severity labels](#severity-labels) - - [Severity impact guidance](#severity-impact-guidance) + - [Severity impact guidance](#severity-impact-guidance) - [Label for community contributors](#label-for-community-contributors) - [Implement design & UI elements](#implement-design--ui-elements) - [Issue tracker](#issue-tracker) @@ -70,7 +70,7 @@ to contribute to GitLab in a way that is easy for everyone. For a first-time step-by-step guide to the contribution process, please see ["Contributing to GitLab"](https://about.gitlab.com/contributing/). -Looking for something to work on? Look for issues with the label [Accepting Merge Requests](#i-want-to-contribute). +Looking for something to work on? Look for issues in the [Backlog (Accepting merge requests) milestone](#i-want-to-contribute). GitLab comes into two flavors, GitLab Community Edition (CE) our free and open source edition, and GitLab Enterprise Edition (EE) which is our commercial @@ -151,8 +151,8 @@ the remaining issues on the GitHub issue tracker. ## I want to contribute! -If you want to contribute to GitLab [issues with the label `Accepting Merge Requests` and small weight][accepting-mrs-weight] -is a great place to start. Issues with a lower weight (1 or 2) are deemed +If you want to contribute to GitLab, [issues in the Backlog (Accepting merge requests)](https://gitlab.com/gitlab-org/gitlab-ce/issues?scope=all&utf8=✓&state=opened&assignee_id=0&milestone_title=Backlog%20(Accepting%20merge%20requests)) +are a great place to start. Issues with a lower weight (1 or 2) are deemed suitable for beginners. These issues will be of reasonable size and challenge, for anyone to start contributing to GitLab. If you have any questions or need help visit [Getting Help](https://about.gitlab.com/getting-help/#discussion) to learn how to communicate with GitLab. If you're looking for a Gitter or Slack channel diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 48b1190fd56..aa6a32fa84e 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -128,7 +128,7 @@ GEM coderay (1.1.2) coercible (1.0.0) descendants_tracker (~> 0.0.1) - commonmarker (0.17.8) + commonmarker (0.17.13) ruby-enum (~> 0.5) concord (0.1.5) adamantium (~> 0.2.0) diff --git a/PROCESS.md b/PROCESS.md index 583f36b820f..38ec01f9de0 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -74,14 +74,31 @@ star, smile, etc.). Some good tips about code reviews can be found in our ## Feature freeze on the 7th for the release on the 22nd -After 7th at 23:59 (Pacific Time Zone) of each month, RC1 of the upcoming release (to be shipped on the 22nd) is created and deployed to GitLab.com and the stable branch for this release is frozen, which means master is no longer merged into it. -Merge requests may still be merged into master during this period, -but they will go into the _next_ release, unless they are manually cherry-picked into the stable branch. +After 7th at 23:59 (Pacific Time Zone) of each month, RC1 of the upcoming +release (to be shipped on the 22nd) is created and deployed to GitLab.com and +the stable branch for this release is frozen, which means master is no longer +merged into it. Merge requests may still be merged into master during this +period, but they will go into the _next_ release, unless they are manually +cherry-picked into the stable branch. -By freezing the stable branches 2 weeks prior to a release, we reduce the risk of a last minute merge request potentially breaking things. +By freezing the stable branches 2 weeks prior to a release, we reduce the risk +of a last minute merge request potentially breaking things. -Any release candidate that gets created after this date can become a final release, -hence the name release candidate. +Any release candidate that gets created after this date can become a final +release, hence the name release candidate. + +### Feature flags + +Merge requests that make changes hidden behind a feature flag, or remove an +existing feature flag because a feature is deemed stable, may be merged (and +picked into the stable branches) up to the 19th of the month. Such merge +requests should have the ~"feature flag" label assigned, and don't require a +corresponding exception request to be created. + +While rare, release managers may decide to reject picking a change into a stable +branch, even when feature flags are used. This might be necessary if the changes +are deemed problematic, too invasive, or there simply isn't enough time to +properly test how the changes behave on GitLab.com. ### Between the 1st and the 7th @@ -223,36 +240,36 @@ Check [this guide](https://gitlab.com/gitlab-org/release/docs/blob/master/genera A ~bug is a defect, error, failure which causes the system to behave incorrectly or prevents it from fulfilling the product requirements. -The level of impact of a ~bug can vary from blocking a whole functionality -or a feature usability bug. A bug should always be linked to a severity level. +The level of impact of a ~bug can vary from blocking a whole functionality +or a feature usability bug. A bug should always be linked to a severity level. Refer to our [severity levels](../CONTRIBUTING.md#severity-labels) -Whether the bug is also a regression or not, the triage process should start as soon as possible. +Whether the bug is also a regression or not, the triage process should start as soon as possible. Ensure that the Engineering Manager and/or the Product Manager for the relative area is involved to prioritize the work as needed. ### Regressions A ~regression implies that a previously **verified working functionality** no longer works. Regressions are a subset of bugs. We use the ~regression label to imply that the defect caused the functionality to regress. -The label tells us that something worked before and it needs extra attention from Engineering and Product Managers to schedule/reschedule. +The label tells us that something worked before and it needs extra attention from Engineering and Product Managers to schedule/reschedule. -The regression label does not apply to ~bugs for new features for which functionality was **never verified as working**. -These, by definition, are not regressions. +The regression label does not apply to ~bugs for new features for which functionality was **never verified as working**. +These, by definition, are not regressions. A regression should always have the `regression:xx.x` label on it to designate when it was introduced. -Regressions should be considered high priority issues that should be solved as soon as possible, especially if they have severe impact on users. +Regressions should be considered high priority issues that should be solved as soon as possible, especially if they have severe impact on users. ### Managing bugs -**Prioritization:** We give higher priority to regressions on features that worked in the last recent monthly release and the current release candidates. -The two scenarios below can [bypass the exception request in the release process](https://gitlab.com/gitlab-org/release/docs/blob/master/general/exception-request/process.md#after-the-7th), where the affected regression version matches the current monthly release version. +**Prioritization:** We give higher priority to regressions on features that worked in the last recent monthly release and the current release candidates. +The two scenarios below can [bypass the exception request in the release process](https://gitlab.com/gitlab-org/release/docs/blob/master/general/exception-request/process.md#after-the-7th), where the affected regression version matches the current monthly release version. * A regression which worked in the **Last monthly release** * **Example:** In 11.0 we released a new `feature X` that is verified as working. Then in release 11.1 the feature no longer works, this is regression for 11.1. The issue should have the `regression:11.1` label. * *Note:* When we say `the last recent monthly release`, this can refer to either the version currently running on GitLab.com, or the most recent version available in the package repositories. * A regression which worked in the **Current release candidates** * **Example:** In 11.1-RC3 we shipped a new feature which has been verified as working. Then in 11.1-RC5 the feature no longer works, this is regression for 11.1. The issue should have the `regression:11.1` label. - * *Note:* Because GitLab.com runs release candidates of new releases, a regression can be reported in a release before its 'official' release date on the 22nd of the month. + * *Note:* Because GitLab.com runs release candidates of new releases, a regression can be reported in a release before its 'official' release date on the 22nd of the month. When a bug is found: 1. Create an issue describing the problem in the most detailed way possible. @@ -264,11 +281,11 @@ When a bug is found: The counterpart Product Manager is included to weigh-in on prioritization as needed. 1. If the ~bug is **NOT** a regression: 1. The Engineering Manager decides which milestone the bug will be fixed. The appropriate milestone is applied. -1. If the bug is a ~regression: +1. If the bug is a ~regression: 1. Determine the release that the regression affects and add the corresponding `regression:xx.x` label. 1. If the affected release version can't be determined, add the generic ~regression label for the time being. - 1. If the affected version `xx.x` in `regression:xx.x` is the **current release**, it's recommended to schedule the fix for the current milestone. - 1. This falls under regressions which worked in the last release and the current RCs. More detailed explanations in the **Prioritization** section above. + 1. If the affected version `xx.x` in `regression:xx.x` is the **current release**, it's recommended to schedule the fix for the current milestone. + 1. This falls under regressions which worked in the last release and the current RCs. More detailed explanations in the **Prioritization** section above. 1. If the affected version `xx.x` in `regression:xx.x` is older than the **current release** 1. If the regression is an ~S1 severity, it's recommended to schedule the fix for the current milestone. We would like to fix the highest severity regression as soon as we can. 1. If the regression is an ~S2, ~S3 or ~S4 severity, the regression may be scheduled for later milestones at the discretion of the Engineering Manager and Product Manager. diff --git a/app/assets/images/cluster_app_logos/elasticsearch.png b/app/assets/images/cluster_app_logos/elasticsearch.png Binary files differnew file mode 100644 index 00000000000..96e9e0ff934 --- /dev/null +++ b/app/assets/images/cluster_app_logos/elasticsearch.png diff --git a/app/assets/images/cluster_app_logos/gitlab.png b/app/assets/images/cluster_app_logos/gitlab.png Binary files differnew file mode 100644 index 00000000000..cb2195fc6a2 --- /dev/null +++ b/app/assets/images/cluster_app_logos/gitlab.png diff --git a/app/assets/images/cluster_app_logos/helm.png b/app/assets/images/cluster_app_logos/helm.png Binary files differnew file mode 100644 index 00000000000..2989cae7b93 --- /dev/null +++ b/app/assets/images/cluster_app_logos/helm.png diff --git a/app/assets/images/cluster_app_logos/jeager.png b/app/assets/images/cluster_app_logos/jeager.png Binary files differnew file mode 100644 index 00000000000..be5bf2a0c9c --- /dev/null +++ b/app/assets/images/cluster_app_logos/jeager.png diff --git a/app/assets/images/cluster_app_logos/jupyterhub.png b/app/assets/images/cluster_app_logos/jupyterhub.png Binary files differnew file mode 100644 index 00000000000..80c7343067f --- /dev/null +++ b/app/assets/images/cluster_app_logos/jupyterhub.png diff --git a/app/assets/images/cluster_app_logos/kubernetes.png b/app/assets/images/cluster_app_logos/kubernetes.png Binary files differnew file mode 100644 index 00000000000..4d774909c10 --- /dev/null +++ b/app/assets/images/cluster_app_logos/kubernetes.png diff --git a/app/assets/images/cluster_app_logos/meltano.png b/app/assets/images/cluster_app_logos/meltano.png Binary files differnew file mode 100644 index 00000000000..7a2d82fbe27 --- /dev/null +++ b/app/assets/images/cluster_app_logos/meltano.png diff --git a/app/assets/images/cluster_app_logos/prometheus.png b/app/assets/images/cluster_app_logos/prometheus.png Binary files differnew file mode 100644 index 00000000000..a8663449b88 --- /dev/null +++ b/app/assets/images/cluster_app_logos/prometheus.png diff --git a/app/assets/javascripts/clusters/components/application_row.vue b/app/assets/javascripts/clusters/components/application_row.vue index 651f3b50236..0452729d3ff 100644 --- a/app/assets/javascripts/clusters/components/application_row.vue +++ b/app/assets/javascripts/clusters/components/application_row.vue @@ -2,6 +2,7 @@ /* eslint-disable vue/require-default-prop */ import { s__, sprintf } from '../../locale'; import eventHub from '../event_hub'; + import identicon from '../../vue_shared/components/identicon.vue'; import loadingButton from '../../vue_shared/components/loading_button.vue'; import { APPLICATION_STATUS, @@ -13,6 +14,7 @@ export default { components: { loadingButton, + identicon, }, props: { id: { @@ -31,6 +33,16 @@ type: String, required: false, }, + logoUrl: { + type: String, + required: false, + default: null, + }, + disabled: { + type: Boolean, + required: false, + default: false, + }, status: { type: String, required: false, @@ -60,6 +72,18 @@ isKnownStatus() { return Object.values(APPLICATION_STATUS).includes(this.status); }, + isInstalled() { + return ( + this.status === APPLICATION_STATUS.INSTALLED || this.status === APPLICATION_STATUS.UPDATED + ); + }, + hasLogo() { + return !!this.logoUrl; + }, + identiconId() { + // generate a deterministic integer id for the identicon background + return this.id.charCodeAt(0); + }, rowJsClass() { return `js-cluster-application-row-${this.id}`; }, @@ -128,37 +152,81 @@ <template> <div - :class="rowJsClass" - class="gl-responsive-table-row gl-responsive-table-row-col-span" + :class="[ + rowJsClass, + isInstalled && 'cluster-application-installed', + disabled && 'cluster-application-disabled' + ]" + class="cluster-application-row gl-responsive-table-row gl-responsive-table-row-col-span" > <div class="gl-responsive-table-row-layout" role="row" > - <a - v-if="titleLink" - :href="titleLink" - target="blank" - rel="noopener noreferrer" + <div + class="table-section append-right-8 section-align-top" role="gridcell" - class="table-section section-15 section-align-top js-cluster-application-title" > - {{ title }} - </a> - <span - v-else - class="table-section section-15 section-align-top js-cluster-application-title" - > - {{ title }} - </span> + <img + v-if="hasLogo" + :src="logoUrl" + :alt="`${title} logo`" + class="cluster-application-logo avatar s40" + /> + <identicon + v-else + :entity-id="identiconId" + :entity-name="title" + size-class="s40" + /> + </div> <div - class="table-section section-wrap" + class="table-section cluster-application-description section-wrap" role="gridcell" > + <strong> + <a + v-if="titleLink" + :href="titleLink" + target="blank" + rel="noopener noreferrer" + class="js-cluster-application-title" + > + {{ title }} + </a> + <span + v-else + class="js-cluster-application-title" + > + {{ title }} + </span> + </strong> <slot name="description"></slot> + <div + v-if="hasError || isUnknownStatus" + class="cluster-application-error text-danger prepend-top-10" + > + <p class="js-cluster-application-general-error-message append-bottom-0"> + {{ generalErrorDescription }} + </p> + <ul v-if="statusReason || requestReason"> + <li + v-if="statusReason" + class="js-cluster-application-status-error-message" + > + {{ statusReason }} + </li> + <li + v-if="requestReason" + class="js-cluster-application-request-error-message" + > + {{ requestReason }} + </li> + </ul> + </div> </div> <div - :class="{ 'section-20': showManageButton, 'section-15': !showManageButton }" + :class="{ 'section-25': showManageButton, 'section-15': !showManageButton }" class="table-section table-button-footer section-align-top" role="gridcell" > @@ -168,6 +236,7 @@ > <a :href="manageLink" + :class="{ disabled: disabled }" class="btn" > {{ manageButtonLabel }} @@ -176,7 +245,7 @@ <div class="btn-group table-action-buttons"> <loading-button :loading="installButtonLoading" - :disabled="installButtonDisabled" + :disabled="disabled || installButtonDisabled" :label="installButtonLabel" class="js-cluster-application-install-button" @click="installClicked" @@ -184,35 +253,5 @@ </div> </div> </div> - <div - v-if="hasError || isUnknownStatus" - class="gl-responsive-table-row-layout" - role="row" - > - <div - class="alert alert-danger alert-block append-bottom-0 clusters-error-alert" - role="gridcell" - > - <div> - <p class="js-cluster-application-general-error-message"> - {{ generalErrorDescription }} - </p> - <ul v-if="statusReason || requestReason"> - <li - v-if="statusReason" - class="js-cluster-application-status-error-message" - > - {{ statusReason }} - </li> - <li - v-if="requestReason" - class="js-cluster-application-request-error-message" - > - {{ requestReason }} - </li> - </ul> - </div> - </div> - </div> </div> </template> diff --git a/app/assets/javascripts/clusters/components/applications.vue b/app/assets/javascripts/clusters/components/applications.vue index d708a9e595a..a1069985178 100644 --- a/app/assets/javascripts/clusters/components/applications.vue +++ b/app/assets/javascripts/clusters/components/applications.vue @@ -1,5 +1,14 @@ <script> import _ from 'underscore'; +import helmInstallIllustration from '@gitlab-org/gitlab-svgs/illustrations/kubernetes-installation.svg'; +import elasticsearchLogo from 'images/cluster_app_logos/elasticsearch.png'; +import gitlabLogo from 'images/cluster_app_logos/gitlab.png'; +import helmLogo from 'images/cluster_app_logos/helm.png'; +import jeagerLogo from 'images/cluster_app_logos/jeager.png'; +import jupyterhubLogo from 'images/cluster_app_logos/jupyterhub.png'; +import kubernetesLogo from 'images/cluster_app_logos/kubernetes.png'; +import meltanoLogo from 'images/cluster_app_logos/meltano.png'; +import prometheusLogo from 'images/cluster_app_logos/prometheus.png'; import { s__, sprintf } from '../../locale'; import applicationRow from './application_row.vue'; import clipboardButton from '../../vue_shared/components/clipboard_button.vue'; @@ -37,21 +46,21 @@ export default { default: '', }, }, + data: () => ({ + elasticsearchLogo, + gitlabLogo, + helmLogo, + jeagerLogo, + jupyterhubLogo, + kubernetesLogo, + meltanoLogo, + prometheusLogo, + }), computed: { - generalApplicationDescription() { - return sprintf( - _.escape( - s__( - `ClusterIntegration|Install applications on your Kubernetes cluster. - Read more about %{helpLink}`, - ), - ), - { - helpLink: `<a href="${this.helpPath}"> - ${_.escape(s__('ClusterIntegration|installing applications'))} - </a>`, - }, - false, + helmInstalled() { + return ( + this.applications.helm.status === APPLICATION_STATUS.INSTALLED || + this.applications.helm.status === APPLICATION_STATUS.UPDATED ); }, ingressId() { @@ -128,224 +137,240 @@ export default { return this.applications.jupyter.hostname; }, }, + created() { + this.helmInstallIllustration = helmInstallIllustration; + }, }; </script> <template> - <section - id="cluster-applications" - class="settings no-animate expanded" - > - <div class="settings-header"> - <h4> - {{ s__('ClusterIntegration|Applications') }} - </h4> - <p - class="append-bottom-0" - v-html="generalApplicationDescription" - > - </p> - </div> + <section id="cluster-applications"> + <h4> + {{ s__('ClusterIntegration|Applications') }} + </h4> + <p class="append-bottom-0"> + {{ s__(`ClusterIntegration|Choose which applications to install on your Kubernetes cluster. + Helm Tiller is required to install any of the following applications.`) }} + <a :href="helpPath"> + {{ __('More information') }} + </a> + </p> - <div class="settings-content"> - <div class="append-bottom-20"> - <application-row - id="helm" - :title="applications.helm.title" - :status="applications.helm.status" - :status-reason="applications.helm.statusReason" - :request-status="applications.helm.requestStatus" - :request-reason="applications.helm.requestReason" - title-link="https://docs.helm.sh/" - > - <div slot="description"> - {{ s__(`ClusterIntegration|Helm streamlines installing - and managing Kubernetes applications. - Tiller runs inside of your Kubernetes Cluster, - and manages releases of your charts.`) }} - </div> - </application-row> - <application-row - :id="ingressId" - :title="applications.ingress.title" - :status="applications.ingress.status" - :status-reason="applications.ingress.statusReason" - :request-status="applications.ingress.requestStatus" - :request-reason="applications.ingress.requestReason" - title-link="https://kubernetes.io/docs/concepts/services-networking/ingress/" + <div class="cluster-application-list prepend-top-10"> + <application-row + id="helm" + :logo-url="helmLogo" + :title="applications.helm.title" + :status="applications.helm.status" + :status-reason="applications.helm.statusReason" + :request-status="applications.helm.requestStatus" + :request-reason="applications.helm.requestReason" + class="rounded-top" + title-link="https://docs.helm.sh/" + > + <div slot="description"> + {{ s__(`ClusterIntegration|Helm streamlines installing + and managing Kubernetes applications. + Tiller runs inside of your Kubernetes Cluster, + and manages releases of your charts.`) }} + </div> + </application-row> + <div + v-show="!helmInstalled" + class="cluster-application-warning" + > + <div + class="svg-container" + v-html="helmInstallIllustration" > - <div slot="description"> - <p> - {{ s__(`ClusterIntegration|Ingress gives you a way to route - requests to services based on the request host or path, - centralizing a number of services into a single entrypoint.`) }} - </p> + </div> + {{ s__(`ClusterIntegration|You must first install Helm Tiller before + installing the applications below`) }} + </div> + <application-row + :id="ingressId" + :logo-url="kubernetesLogo" + :title="applications.ingress.title" + :status="applications.ingress.status" + :status-reason="applications.ingress.statusReason" + :request-status="applications.ingress.requestStatus" + :request-reason="applications.ingress.requestReason" + :disabled="!helmInstalled" + title-link="https://kubernetes.io/docs/concepts/services-networking/ingress/" + > + <div slot="description"> + <p> + {{ s__(`ClusterIntegration|Ingress gives you a way to route + requests to services based on the request host or path, + centralizing a number of services into a single entrypoint.`) }} + </p> - <template v-if="ingressInstalled"> - <div class="form-group"> - <label for="ingress-ip-address"> - {{ s__('ClusterIntegration|Ingress IP Address') }} - </label> - <div - v-if="ingressExternalIp" - class="input-group" - > - <input - id="ingress-ip-address" - :value="ingressExternalIp" - type="text" - class="form-control js-ip-address" - readonly - /> - <span class="input-group-append"> - <clipboard-button - :text="ingressExternalIp" - :title="s__('ClusterIntegration|Copy Ingress IP Address to clipboard')" - class="input-group-text js-clipboard-btn" - /> - </span> - </div> + <template v-if="ingressInstalled"> + <div class="form-group"> + <label for="ingress-ip-address"> + {{ s__('ClusterIntegration|Ingress IP Address') }} + </label> + <div + v-if="ingressExternalIp" + class="input-group" + > <input - v-else + id="ingress-ip-address" + :value="ingressExternalIp" type="text" class="form-control js-ip-address" readonly - value="?" /> + <span class="input-group-append"> + <clipboard-button + :text="ingressExternalIp" + :title="s__('ClusterIntegration|Copy Ingress IP Address to clipboard')" + class="input-group-text js-clipboard-btn" + /> + </span> </div> + <input + v-else + type="text" + class="form-control js-ip-address" + readonly + value="?" + /> + </div> - <p - v-if="!ingressExternalIp" - class="settings-message js-no-ip-message" - > - {{ s__(`ClusterIntegration|The IP address is in - the process of being assigned. Please check your Kubernetes - cluster or Quotas on Google Kubernetes Engine if it takes a long time.`) }} + <p + v-if="!ingressExternalIp" + class="settings-message js-no-ip-message" + > + {{ s__(`ClusterIntegration|The IP address is in + the process of being assigned. Please check your Kubernetes + cluster or Quotas on Google Kubernetes Engine if it takes a long time.`) }} - <a - :href="ingressHelpPath" - target="_blank" - rel="noopener noreferrer" - > - {{ __('More information') }} - </a> - </p> + <a + :href="ingressHelpPath" + target="_blank" + rel="noopener noreferrer" + > + {{ __('More information') }} + </a> + </p> - <p> - {{ s__(`ClusterIntegration|Point a wildcard DNS to this - generated IP address in order to access - your application after it has been deployed.`) }} - <a - :href="ingressDnsHelpPath" - target="_blank" - rel="noopener noreferrer" - > - {{ __('More information') }} - </a> - </p> + <p> + {{ s__(`ClusterIntegration|Point a wildcard DNS to this + generated IP address in order to access + your application after it has been deployed.`) }} + <a + :href="ingressDnsHelpPath" + target="_blank" + rel="noopener noreferrer" + > + {{ __('More information') }} + </a> + </p> - </template> - <div - v-else - v-html="ingressDescription" - > - </div> - </div> - </application-row> - <application-row - id="prometheus" - :title="applications.prometheus.title" - :manage-link="managePrometheusPath" - :status="applications.prometheus.status" - :status-reason="applications.prometheus.statusReason" - :request-status="applications.prometheus.requestStatus" - :request-reason="applications.prometheus.requestReason" - title-link="https://prometheus.io/docs/introduction/overview/" - > + </template> <div - slot="description" - v-html="prometheusDescription" + v-html="ingressDescription" > </div> - </application-row> - <application-row - id="runner" - :title="applications.runner.title" - :status="applications.runner.status" - :status-reason="applications.runner.statusReason" - :request-status="applications.runner.requestStatus" - :request-reason="applications.runner.requestReason" - title-link="https://docs.gitlab.com/runner/" - > - <div slot="description"> - {{ s__(`ClusterIntegration|GitLab Runner connects to this - project's repository and executes CI/CD jobs, - pushing results back and deploying, - applications to production.`) }} - </div> - </application-row> - <application-row - id="jupyter" - :title="applications.jupyter.title" - :status="applications.jupyter.status" - :status-reason="applications.jupyter.statusReason" - :request-status="applications.jupyter.requestStatus" - :request-reason="applications.jupyter.requestReason" - :install-application-request-params="{ hostname: applications.jupyter.hostname }" - title-link="https://jupyterhub.readthedocs.io/en/stable/" + </div> + </application-row> + <application-row + id="prometheus" + :logo-url="prometheusLogo" + :title="applications.prometheus.title" + :manage-link="managePrometheusPath" + :status="applications.prometheus.status" + :status-reason="applications.prometheus.statusReason" + :request-status="applications.prometheus.requestStatus" + :request-reason="applications.prometheus.requestReason" + :disabled="!helmInstalled" + title-link="https://prometheus.io/docs/introduction/overview/" + > + <div + slot="description" + v-html="prometheusDescription" > - <div slot="description"> - <p> - {{ s__(`ClusterIntegration|JupyterHub, a multi-user Hub, spawns, - manages, and proxies multiple instances of the single-user - Jupyter notebook server. JupyterHub can be used to serve - notebooks to a class of students, a corporate data science group, - or a scientific research group.`) }} - </p> + </div> + </application-row> + <application-row + id="runner" + :logo-url="gitlabLogo" + :title="applications.runner.title" + :status="applications.runner.status" + :status-reason="applications.runner.statusReason" + :request-status="applications.runner.requestStatus" + :request-reason="applications.runner.requestReason" + :disabled="!helmInstalled" + title-link="https://docs.gitlab.com/runner/" + > + <div slot="description"> + {{ s__(`ClusterIntegration|GitLab Runner connects to this + project's repository and executes CI/CD jobs, + pushing results back and deploying, + applications to production.`) }} + </div> + </application-row> + <application-row + id="jupyter" + :logo-url="jupyterhubLogo" + :title="applications.jupyter.title" + :status="applications.jupyter.status" + :status-reason="applications.jupyter.statusReason" + :request-status="applications.jupyter.requestStatus" + :request-reason="applications.jupyter.requestReason" + :install-application-request-params="{ hostname: applications.jupyter.hostname }" + :disabled="!helmInstalled" + class="hide-bottom-border rounded-bottom" + title-link="https://jupyterhub.readthedocs.io/en/stable/" + > + <div slot="description"> + <p> + {{ s__(`ClusterIntegration|JupyterHub, a multi-user Hub, spawns, + manages, and proxies multiple instances of the single-user + Jupyter notebook server. JupyterHub can be used to serve + notebooks to a class of students, a corporate data science group, + or a scientific research group.`) }} + </p> - <template v-if="ingressExternalIp"> - <div class="form-group"> - <label for="jupyter-hostname"> - {{ s__('ClusterIntegration|Jupyter Hostname') }} - </label> + <template v-if="ingressExternalIp"> + <div class="form-group"> + <label for="jupyter-hostname"> + {{ s__('ClusterIntegration|Jupyter Hostname') }} + </label> - <div class="input-group"> - <input - v-model="applications.jupyter.hostname" - :readonly="jupyterInstalled" - type="text" - class="form-control js-hostname" + <div class="input-group"> + <input + v-model="applications.jupyter.hostname" + :readonly="jupyterInstalled" + type="text" + class="form-control js-hostname" + /> + <span + class="input-group-btn" + > + <clipboard-button + :text="jupyterHostname" + :title="s__('ClusterIntegration|Copy Jupyter Hostname to clipboard')" + class="js-clipboard-btn" /> - <span - class="input-group-btn" - > - <clipboard-button - :text="jupyterHostname" - :title="s__('ClusterIntegration|Copy Jupyter Hostname to clipboard')" - class="js-clipboard-btn" - /> - </span> - </div> + </span> </div> - <p v-if="ingressInstalled"> - {{ s__(`ClusterIntegration|Replace this with your own hostname if you want. - If you do so, point hostname to Ingress IP Address from above.`) }} - <a - :href="ingressDnsHelpPath" - target="_blank" - rel="noopener noreferrer" - > - {{ __('More information') }} - </a> - </p> - </template> - </div> - </application-row> - <!-- - NOTE: Don't forget to update `clusters.scss` - min-height for this block and uncomment `application_spec` tests - --> - </div> + </div> + <p v-if="ingressInstalled"> + {{ s__(`ClusterIntegration|Replace this with your own hostname if you want. + If you do so, point hostname to Ingress IP Address from above.`) }} + <a + :href="ingressDnsHelpPath" + target="_blank" + rel="noopener noreferrer" + > + {{ __('More information') }} + </a> + </p> + </template> + </div> + </application-row> </div> </section> </template> diff --git a/app/assets/javascripts/filtered_search/admin_runners_filtered_search_token_keys.js b/app/assets/javascripts/filtered_search/admin_runners_filtered_search_token_keys.js index 1f9c3f41e52..b4588cc1318 100644 --- a/app/assets/javascripts/filtered_search/admin_runners_filtered_search_token_keys.js +++ b/app/assets/javascripts/filtered_search/admin_runners_filtered_search_token_keys.js @@ -5,7 +5,7 @@ const tokenKeys = [{ type: 'string', param: 'status', symbol: '', - icon: 'signal', + icon: 'messages', tag: 'status', }]; diff --git a/app/assets/javascripts/filtered_search/dropdown_hint.js b/app/assets/javascripts/filtered_search/dropdown_hint.js index 184b34b7b5e..8aecf9725e6 100644 --- a/app/assets/javascripts/filtered_search/dropdown_hint.js +++ b/app/assets/javascripts/filtered_search/dropdown_hint.js @@ -62,7 +62,7 @@ export default class DropdownHint extends FilteredSearchDropdown { renderContent() { const dropdownData = this.tokenKeys.get() .map(tokenKey => ({ - icon: `fa-${tokenKey.icon}`, + icon: `${gon.sprite_icons}#${tokenKey.icon}`, hint: tokenKey.key, tag: `:${tokenKey.tag}`, type: tokenKey.type, diff --git a/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js b/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js index cce2c07dc3e..cc7291c9f59 100644 --- a/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js +++ b/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js @@ -1,6 +1,6 @@ import FilteredSearchTokenKeys from './filtered_search_token_keys'; -const tokenKeys = [{ +export const tokenKeys = [{ key: 'author', type: 'string', param: 'username', @@ -19,14 +19,14 @@ const tokenKeys = [{ type: 'string', param: 'title', symbol: '%', - icon: 'clock-o', + icon: 'clock', tag: '%milestone', }, { key: 'label', type: 'array', param: 'name[]', symbol: '~', - icon: 'tag', + icon: 'labels', tag: '~label', }]; @@ -37,19 +37,19 @@ if (gon.current_user_id) { type: 'string', param: 'emoji', symbol: '', - icon: 'thumbs-up', + icon: 'thumb-up', tag: 'emoji', }); } -const alternativeTokenKeys = [{ +export const alternativeTokenKeys = [{ key: 'label', type: 'string', param: 'name', symbol: '~', }]; -const conditions = [{ +export const conditions = [{ url: 'assignee_id=0', tokenKey: 'assignee', value: 'none', diff --git a/app/assets/javascripts/ide/components/file_row_extra.vue b/app/assets/javascripts/ide/components/file_row_extra.vue new file mode 100644 index 00000000000..44a360ab909 --- /dev/null +++ b/app/assets/javascripts/ide/components/file_row_extra.vue @@ -0,0 +1,104 @@ +<script> +import { mapGetters } from 'vuex'; +import { n__, __, sprintf } from '~/locale'; +import tooltip from '~/vue_shared/directives/tooltip'; +import Icon from '~/vue_shared/components/icon.vue'; +import NewDropdown from './new_dropdown/index.vue'; +import ChangedFileIcon from './changed_file_icon.vue'; +import MrFileIcon from './mr_file_icon.vue'; + +export default { + name: 'FileRowExtra', + directives: { + tooltip, + }, + components: { + Icon, + NewDropdown, + ChangedFileIcon, + MrFileIcon, + }, + props: { + file: { + type: Object, + required: true, + }, + mouseOver: { + type: Boolean, + required: true, + }, + }, + computed: { + ...mapGetters([ + 'getChangesInFolder', + 'getUnstagedFilesCountForPath', + 'getStagedFilesCountForPath', + ]), + folderUnstagedCount() { + return this.getUnstagedFilesCountForPath(this.file.path); + }, + folderStagedCount() { + return this.getStagedFilesCountForPath(this.file.path); + }, + changesCount() { + return this.getChangesInFolder(this.file.path); + }, + folderChangesTooltip() { + if (this.changesCount === 0) return undefined; + + if (this.folderUnstagedCount > 0 && this.folderStagedCount === 0) { + return n__('%d unstaged change', '%d unstaged changes', this.folderUnstagedCount); + } else if (this.folderUnstagedCount === 0 && this.folderStagedCount > 0) { + return n__('%d staged change', '%d staged changes', this.folderStagedCount); + } + + return sprintf(__('%{unstaged} unstaged and %{staged} staged changes'), { + unstaged: this.folderUnstagedCount, + staged: this.folderStagedCount, + }); + }, + showTreeChangesCount() { + return this.file.type === 'tree' && this.changesCount > 0 && !this.file.opened; + }, + showChangedFileIcon() { + return this.file.changed || this.file.tempFile || this.file.staged; + }, + }, +}; +</script> + +<template> + <div class="float-right ide-file-icon-holder"> + <mr-file-icon + v-if="file.mrChange" + /> + <span + v-if="showTreeChangesCount" + class="ide-tree-changes" + > + {{ changesCount }} + <icon + v-tooltip + :title="folderChangesTooltip" + :size="12" + data-container="body" + data-placement="right" + name="file-modified" + css-classes="prepend-left-5 ide-file-modified" + /> + </span> + <changed-file-icon + v-else-if="showChangedFileIcon" + :file="file" + :show-tooltip="true" + :show-staged-icon="true" + :force-modified-icon="true" + /> + <new-dropdown + :type="file.type" + :path="file.path" + :mouse-over="mouseOver" + class="prepend-left-8" + /> + </div> +</template> diff --git a/app/assets/javascripts/ide/components/ide_tree_list.vue b/app/assets/javascripts/ide/components/ide_tree_list.vue index 00ae5ea2c15..e658d1bf956 100644 --- a/app/assets/javascripts/ide/components/ide_tree_list.vue +++ b/app/assets/javascripts/ide/components/ide_tree_list.vue @@ -2,15 +2,16 @@ import { mapActions, mapGetters, mapState } from 'vuex'; import Icon from '~/vue_shared/components/icon.vue'; import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; -import RepoFile from './repo_file.vue'; +import FileRow from '~/vue_shared/components/file_row.vue'; import NavDropdown from './nav_dropdown.vue'; +import FileRowExtra from './file_row_extra.vue'; export default { components: { Icon, - RepoFile, SkeletonLoadingContainer, NavDropdown, + FileRow, }, props: { viewerType: { @@ -34,8 +35,9 @@ export default { this.updateViewer(this.viewerType); }, methods: { - ...mapActions(['updateViewer']), + ...mapActions(['updateViewer', 'toggleTreeOpen']), }, + FileRowExtra, }; </script> @@ -63,11 +65,13 @@ export default { <div class="ide-tree-body h-100" > - <repo-file + <file-row v-for="file in currentTree.tree" :key="file.key" :file="file" :level="0" + :extra-component="$options.FileRowExtra" + @toggleTreeOpen="toggleTreeOpen" /> </div> </template> diff --git a/app/assets/javascripts/ide/components/repo_file.vue b/app/assets/javascripts/ide/components/repo_file.vue deleted file mode 100644 index 110eda83bb4..00000000000 --- a/app/assets/javascripts/ide/components/repo_file.vue +++ /dev/null @@ -1,227 +0,0 @@ -<script> -import { mapActions, mapGetters } from 'vuex'; -import { n__, __, sprintf } from '~/locale'; -import tooltip from '~/vue_shared/directives/tooltip'; -import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; -import Icon from '~/vue_shared/components/icon.vue'; -import FileIcon from '~/vue_shared/components/file_icon.vue'; -import router from '../ide_router'; -import NewDropdown from './new_dropdown/index.vue'; -import FileStatusIcon from './repo_file_status_icon.vue'; -import ChangedFileIcon from './changed_file_icon.vue'; -import MrFileIcon from './mr_file_icon.vue'; - -export default { - name: 'RepoFile', - directives: { - tooltip, - }, - components: { - SkeletonLoadingContainer, - NewDropdown, - FileStatusIcon, - FileIcon, - ChangedFileIcon, - MrFileIcon, - Icon, - }, - props: { - file: { - type: Object, - required: true, - }, - level: { - type: Number, - required: true, - }, - }, - data() { - return { - mouseOver: false, - }; - }, - computed: { - ...mapGetters([ - 'getChangesInFolder', - 'getUnstagedFilesCountForPath', - 'getStagedFilesCountForPath', - ]), - folderUnstagedCount() { - return this.getUnstagedFilesCountForPath(this.file.path); - }, - folderStagedCount() { - return this.getStagedFilesCountForPath(this.file.path); - }, - changesCount() { - return this.getChangesInFolder(this.file.path); - }, - folderChangesTooltip() { - if (this.changesCount === 0) return undefined; - - if (this.folderUnstagedCount > 0 && this.folderStagedCount === 0) { - return n__('%d unstaged change', '%d unstaged changes', this.folderUnstagedCount); - } else if (this.folderUnstagedCount === 0 && this.folderStagedCount > 0) { - return n__('%d staged change', '%d staged changes', this.folderStagedCount); - } - - return sprintf(__('%{unstaged} unstaged and %{staged} staged changes'), { - unstaged: this.folderUnstagedCount, - staged: this.folderStagedCount, - }); - }, - isTree() { - return this.file.type === 'tree'; - }, - isBlob() { - return this.file.type === 'blob'; - }, - levelIndentation() { - return { - marginLeft: `${this.level * 16}px`, - }; - }, - fileClass() { - return { - 'file-open': this.isBlob && this.file.opened, - 'file-active': this.isBlob && this.file.active, - folder: this.isTree, - 'is-open': this.file.opened, - }; - }, - showTreeChangesCount() { - return this.isTree && this.changesCount > 0 && !this.file.opened; - }, - showChangedFileIcon() { - return this.file.changed || this.file.tempFile || this.file.staged; - }, - }, - watch: { - 'file.active': function fileActiveWatch(active) { - if (this.file.type === 'blob' && active) { - this.scrollIntoView(); - } - }, - }, - mounted() { - if (this.hasPathAtCurrentRoute()) { - this.scrollIntoView(true); - } - }, - methods: { - ...mapActions(['toggleTreeOpen']), - clickFile() { - // Manual Action if a tree is selected/opened - if (this.isTree && this.hasUrlAtCurrentRoute()) { - this.toggleTreeOpen(this.file.path); - } - - router.push(`/project${this.file.url}`); - }, - scrollIntoView(isInit = false) { - const block = isInit && this.isTree ? 'center' : 'nearest'; - - this.$el.scrollIntoView({ - behavior: 'smooth', - block, - }); - }, - hasPathAtCurrentRoute() { - if (!this.$router || !this.$router.currentRoute) { - return false; - } - - // - strip route up to "/-/" and ending "/" - const routePath = this.$router.currentRoute.path - .replace(/^.*?[/]-[/]/g, '') - .replace(/[/]$/g, ''); - - // - strip ending "/" - const filePath = this.file.path.replace(/[/]$/g, ''); - - return filePath === routePath; - }, - hasUrlAtCurrentRoute() { - return this.$router.currentRoute.path === `/project${this.file.url}`; - }, - toggleHover(over) { - this.mouseOver = over; - }, - }, -}; -</script> - -<template> - <div> - <div - :class="fileClass" - class="file" - role="button" - @click="clickFile" - @mouseover="toggleHover(true)" - @mouseout="toggleHover(false)" - > - <div - class="file-name" - > - <span - :style="levelIndentation" - class="ide-file-name str-truncated" - > - <file-icon - :file-name="file.name" - :loading="file.loading" - :folder="isTree" - :opened="file.opened" - :size="16" - /> - {{ file.name }} - <file-status-icon - :file="file" - /> - </span> - <span class="float-right ide-file-icon-holder"> - <mr-file-icon - v-if="file.mrChange" - /> - <span - v-if="showTreeChangesCount" - class="ide-tree-changes" - > - {{ changesCount }} - <icon - v-tooltip - :title="folderChangesTooltip" - :size="12" - data-container="body" - data-placement="right" - name="file-modified" - css-classes="prepend-left-5 ide-file-modified" - /> - </span> - <changed-file-icon - v-else-if="showChangedFileIcon" - :file="file" - :show-tooltip="true" - :show-staged-icon="true" - :force-modified-icon="true" - class="float-right" - /> - </span> - <new-dropdown - :type="file.type" - :path="file.path" - :mouse-over="mouseOver" - class="float-right prepend-left-8" - /> - </div> - </div> - <template v-if="file.opened"> - <repo-file - v-for="childFile in file.tree" - :key="childFile.key" - :file="childFile" - :level="level + 1" - /> - </template> - </div> -</template> diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue new file mode 100644 index 00000000000..6f7bdbc2c4d --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -0,0 +1,210 @@ +<script> +import Icon from '~/vue_shared/components/icon.vue'; +import FileIcon from '~/vue_shared/components/file_icon.vue'; + +export default { + name: 'FileRow', + components: { + FileIcon, + Icon, + }, + props: { + file: { + type: Object, + required: true, + }, + level: { + type: Number, + required: true, + }, + extraComponent: { + type: Object, + required: false, + default: null, + }, + }, + data() { + return { + mouseOver: false, + }; + }, + computed: { + isTree() { + return this.file.type === 'tree'; + }, + isBlob() { + return this.file.type === 'blob'; + }, + levelIndentation() { + return { + marginLeft: `${this.level * 16}px`, + }; + }, + fileClass() { + return { + 'file-open': this.isBlob && this.file.opened, + 'is-active': this.isBlob && this.file.active, + folder: this.isTree, + 'is-open': this.file.opened, + }; + }, + }, + watch: { + 'file.active': function fileActiveWatch(active) { + if (this.file.type === 'blob' && active) { + this.scrollIntoView(); + } + }, + }, + mounted() { + if (this.hasPathAtCurrentRoute()) { + this.scrollIntoView(true); + } + }, + methods: { + toggleTreeOpen(path) { + this.$emit('toggleTreeOpen', path); + }, + clickFile() { + // Manual Action if a tree is selected/opened + if (this.isTree && this.hasUrlAtCurrentRoute()) { + this.toggleTreeOpen(this.file.path); + } + + if (this.$router) this.$router.push(`/project${this.file.url}`); + }, + scrollIntoView(isInit = false) { + const block = isInit && this.isTree ? 'center' : 'nearest'; + + this.$el.scrollIntoView({ + behavior: 'smooth', + block, + }); + }, + hasPathAtCurrentRoute() { + if (!this.$router || !this.$router.currentRoute) { + return false; + } + + // - strip route up to "/-/" and ending "/" + const routePath = this.$router.currentRoute.path + .replace(/^.*?[/]-[/]/g, '') + .replace(/[/]$/g, ''); + + // - strip ending "/" + const filePath = this.file.path.replace(/[/]$/g, ''); + + return filePath === routePath; + }, + hasUrlAtCurrentRoute() { + if (!this.$router || !this.$router.currentRoute) return true; + + return this.$router.currentRoute.path === `/project${this.file.url}`; + }, + toggleHover(over) { + this.mouseOver = over; + }, + }, +}; +</script> + +<template> + <div> + <div + :class="fileClass" + class="file-row" + role="button" + @click="clickFile" + @mouseover="toggleHover(true)" + @mouseout="toggleHover(false)" + > + <div + class="file-row-name-container" + > + <span + :style="levelIndentation" + class="file-row-name str-truncated" + > + <file-icon + :file-name="file.name" + :loading="file.loading" + :folder="isTree" + :opened="file.opened" + :size="16" + /> + {{ file.name }} + </span> + <component + v-if="extraComponent" + :is="extraComponent" + :file="file" + :mouse-over="mouseOver" + /> + </div> + </div> + <template v-if="file.opened"> + <file-row + v-for="childFile in file.tree" + :key="childFile.key" + :file="childFile" + :level="level + 1" + :extra-component="extraComponent" + @toggleTreeOpen="toggleTreeOpen" + /> + </template> + </div> +</template> + +<style> +.file-row { + display: flex; + align-items: center; + height: 32px; + padding: 4px 8px; + margin-left: -8px; + margin-right: -8px; + border-radius: 3px; + text-align: left; + cursor: pointer; +} + +.file-row:hover, +.file-row:focus { + background: #f2f2f2; +} + +.file-row:active { + background: #dfdfdf; +} + +.file-row.is-active { + background: #f2f2f2; +} + +.file-row-name-container { + display: flex; + width: 100%; + align-items: center; + overflow: visible; +} + +.file-row-name { + display: inline-block; + flex: 1; + max-width: inherit; + height: 18px; + line-height: 16px; + text-overflow: ellipsis; + white-space: nowrap; +} + +.file-row-name svg { + margin-right: 2px; + vertical-align: middle; +} + +.file-row-name .loading-container { + display: inline-block; + margin-right: 4px; +} +</style> diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index e9b074236cc..d5693a5d1a1 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -389,9 +389,8 @@ .btn { text-overflow: ellipsis; - .fa { - width: 15px; - line-height: $line-height-base; + svg { + margin-right: $gl-padding-8; } .dropdown-label-box { diff --git a/app/assets/stylesheets/page_bundles/ide.scss b/app/assets/stylesheets/page_bundles/ide.scss index 45df8391f9a..65f0a0d18e2 100644 --- a/app/assets/stylesheets/page_bundles/ide.scss +++ b/app/assets/stylesheets/page_bundles/ide.scss @@ -53,83 +53,9 @@ $ide-commit-header-height: 48px; flex: 1; min-height: 0; // firefox fix - .file { - height: 32px; - cursor: pointer; - - &.file-active { - background: $theme-gray-100; - } - - .ide-file-name { - flex: 1; - white-space: nowrap; - text-overflow: ellipsis; - max-width: inherit; - line-height: 16px; - display: inline-block; - height: 18px; - - svg { - vertical-align: middle; - margin-right: 2px; - } - - .loading-container { - margin-right: 4px; - display: inline-block; - } - } - - .ide-file-icon-holder { - display: flex; - align-items: center; - color: $theme-gray-700; - } - - .ide-file-changed-icon { - margin-left: auto; - - > svg { - display: block; - } - } - - .ide-new-btn { - display: none; - - .btn { - padding: 2px 5px; - } - } - - &:hover, - &:focus { - .ide-new-btn { - display: block; - } - } - - .folder-icon { - fill: $gl-text-color-secondary; - } - } - a { color: $gl-text-color; } - - th { - position: sticky; - top: 0; - } -} - -.file-name { - display: flex; - overflow: visible; - align-items: center; - width: 100%; } .multi-file-loading-container { @@ -625,8 +551,7 @@ $ide-commit-header-height: 48px; } } -.multi-file-commit-list-path, -.ide-file-list .file { +.multi-file-commit-list-path { display: flex; align-items: center; margin-left: -$grid-size; @@ -634,28 +559,14 @@ $ide-commit-header-height: 48px; padding: $grid-size / 2 $grid-size; border-radius: $border-radius-default; text-align: left; - - &:hover, - &:focus { - background: $theme-gray-100; - } - - &:active { - background: $theme-gray-200; - } -} - -.multi-file-commit-list-path { cursor: pointer; height: $ide-commit-row-height; padding-right: 0; - &.is-active { - background-color: $white-normal; - } - &:hover, &:focus { + background: $theme-gray-100; + outline: 0; .multi-file-discard-btn { @@ -665,6 +576,14 @@ $ide-commit-header-height: 48px; } } + &:active { + background: $theme-gray-200; + } + + &.is-active { + background-color: $white-normal; + } + svg { min-width: 16px; vertical-align: middle; @@ -1398,9 +1317,17 @@ $ide-commit-header-height: 48px; } } -.ide-new-btn .dropdown.show .ide-entry-dropdown-toggle { - color: $white-normal; - background-color: $blue-500; +.ide-new-btn { + display: none; + + .btn { + padding: 2px 5px; + } + + .dropdown.show .ide-entry-dropdown-toggle { + color: $white-normal; + background-color: $blue-500; + } } .ide-preview-header { @@ -1465,3 +1392,28 @@ $ide-commit-header-height: 48px; width: $ide-commit-row-height; height: $ide-commit-row-height; } + +.ide-file-icon-holder { + display: flex; + align-items: center; + color: $theme-gray-700; +} + +.ide-file-changed-icon { + margin-left: auto; + + > svg { + display: block; + } +} + +.file-row:hover, +.file-row:focus { + .ide-new-btn { + display: block; + } + + .folder-icon { + fill: $gl-text-color-secondary; + } +} diff --git a/app/assets/stylesheets/pages/clusters.scss b/app/assets/stylesheets/pages/clusters.scss index 0f22fe21143..71a3fd544f2 100644 --- a/app/assets/stylesheets/pages/clusters.scss +++ b/app/assets/stylesheets/pages/clusters.scss @@ -4,9 +4,60 @@ } } -.cluster-applications-table { - // Wait for the Vue to kick-in and render the applications block - min-height: 628px; +.cluster-application-row { + background: $gray-lighter; + + &.cluster-application-installed { + background: none; + } + + .settings-message { + padding: $gl-vert-padding $gl-padding-8; + } +} + +@media (min-width: map-get($grid-breakpoints, md)) { + .cluster-application-list { + border: 1px solid $border-color; + border-radius: $border-radius-default; + } + + .cluster-application-row { + border-bottom: 1px solid $border-color; + padding: $gl-padding; + } +} + +.cluster-application-logo { + border: 3px solid $white-light; + box-shadow: 0 0 0 1px $gray-normal; + + &.avatar:hover { + border-color: $white-light; + } +} + +.cluster-application-warning { + font-weight: bold; + text-align: center; + padding: $gl-padding; + border-bottom: 1px solid $white-normal; + + .svg-container { + display: inline-block; + vertical-align: middle; + margin-right: $gl-padding-8; + width: 40px; + height: 40px; + } +} + +.cluster-application-description { + flex: 1; +} + +.cluster-application-disabled { + opacity: 0.5; } .clusters-dropdown-menu { diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 17f34319050..caa839c32a5 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -279,6 +279,10 @@ table.u2f-registrations { } } +.codes { + padding-top: 14px; +} + .oauth-application-show { .scope-name { font-weight: $gl-font-weight-bold; diff --git a/app/finders/members_finder.rb b/app/finders/members_finder.rb index 48777838d60..f90a7868102 100644 --- a/app/finders/members_finder.rb +++ b/app/finders/members_finder.rb @@ -18,7 +18,7 @@ class MembersFinder group_members = GroupMembersFinder.new(group).execute(include_descendants: include_descendants) # rubocop: disable CodeReuse/Finder group_members = group_members.non_invite - union = Gitlab::SQL::Union.new([project_members, group_members], remove_duplicates: false) + union = Gitlab::SQL::Union.new([project_members, group_members], remove_duplicates: false) # rubocop: disable Gitlab/Union sql = distinct_on(union) diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index 715dffb972f..3528e4228b2 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -89,9 +89,7 @@ class SnippetsFinder < UnionFinder # We use a UNION here instead of OR clauses since this results in better # performance. - union = Gitlab::SQL::Union.new([authorized_projects.select('projects.id'), visible_projects.select('projects.id')]) - - Project.from("(#{union.to_sql}) AS #{Project.table_name}") + Project.from_union([authorized_projects, visible_projects]) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 0d9f16fdce7..74baf79e4f2 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -164,16 +164,13 @@ class TodosFinder # rubocop: disable CodeReuse/ActiveRecord def by_group(items) - if group? - groups = group.self_and_descendants - project_todos = items.where(project_id: Project.where(group: groups).select(:id)) - group_todos = items.where(group_id: groups.select(:id)) + return items unless group? - union = Gitlab::SQL::Union.new([project_todos, group_todos]) - items = Todo.from("(#{union.to_sql}) #{Todo.table_name}") - end + groups = group.self_and_descendants + project_todos = items.where(project_id: Project.where(group: groups).select(:id)) + group_todos = items.where(group_id: groups.select(:id)) - items + Todo.from_union([project_todos, group_todos]) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/finders/union_finder.rb b/app/finders/union_finder.rb index 798c3eba0f3..c3b02f7e52f 100644 --- a/app/finders/union_finder.rb +++ b/app/finders/union_finder.rb @@ -1,15 +1,15 @@ # frozen_string_literal: true class UnionFinder - # rubocop: disable CodeReuse/ActiveRecord def find_union(segments, klass) - if segments.length > 1 - union = Gitlab::SQL::Union.new(segments.map { |s| s.select(:id) }) + unless klass < FromUnion + raise TypeError, "#{klass.inspect} must include the FromUnion module" + end - klass.where("#{klass.table_name}.id IN (#{union.to_sql})") + if segments.length > 1 + klass.from_union(segments) else segments.first end end - # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index bb401b03709..ef4560023af 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -13,7 +13,7 @@ module ApplicationHelper # Check if a particular controller is the current one # - # args - One or more controller names to check + # args - One or more controller names to check (using path notation when inside namespaces) # # Examples # @@ -21,6 +21,11 @@ module ApplicationHelper # current_controller?(:tree) # => true # current_controller?(:commits) # => false # current_controller?(:commits, :tree) # => true + # + # # On Admin::ApplicationController + # current_controller?(:application) # => true + # current_controller?('admin/application') # => true + # current_controller?('gitlab/application') # => false def current_controller?(*args) args.any? do |v| v.to_s.downcase == controller.controller_name || v.to_s.downcase == controller.controller_path diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index f8d36dce45d..136772e1ec3 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -123,11 +123,6 @@ module CiStatusHelper render_status_with_link('pipeline', pipeline.status, path, tooltip_placement: tooltip_placement) end - def no_runners_for_project?(project) - project.runners.blank? && - Ci::Runner.instance_type.blank? - end - def render_status_with_link(type, status, path = nil, tooltip_placement: 'left', cssclass: '', container: 'body', icon_size: 16) klass = "ci-status-link ci-status-icon-#{status.dasherize} #{cssclass}" title = "#{type.titleize}: #{ci_label_for_status(status)}" diff --git a/app/helpers/tab_helper.rb b/app/helpers/tab_helper.rb index e310fda51d7..d91f0f78db7 100644 --- a/app/helpers/tab_helper.rb +++ b/app/helpers/tab_helper.rb @@ -8,7 +8,7 @@ module TabHelper # element is the value passed to the block. # # options - The options hash used to determine if the element is "active" (default: {}) - # :controller - One or more controller names to check (optional). + # :controller - One or more controller names to check, use path notation when namespaced (optional). # :action - One or more action names to check (optional). # :path - A shorthand path, such as 'dashboard#index', to check (optional). # :html_options - Extra options to be passed to the list element (optional). @@ -42,6 +42,20 @@ module TabHelper # nav_link(controller: :tree, html_options: {class: 'home'}) { "Hello" } # # => '<li class="home active">Hello</li>' # + # # For namespaced controllers like Admin::AppearancesController#show + # + # # Controller and namespace matches + # nav_link(controller: 'admin/appearances') { "Hello" } + # # => '<li class="active">Hello</li>' + # + # # Controller and namespace matches but action doesn't + # nav_link(controller: 'admin/appearances', action: :edit) { "Hello" } + # # => '<li>Hello</li>' + # + # # Shorthand path with namespace + # nav_link(path: 'admin/appearances#show') { "Hello"} + # # => '<li class="active">Hello</li>' + # # Returns a list item element String def nav_link(options = {}, &block) klass = active_nav_link?(options) ? 'active' : '' diff --git a/app/models/badge.rb b/app/models/badge.rb index 7e3b6b659e4..f016654206b 100644 --- a/app/models/badge.rb +++ b/app/models/badge.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Badge < ActiveRecord::Base + include FromUnion + # This structure sets the placeholders that the urls # can have. This hash also sets which action to ask when # the placeholder is found. diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index eabb41c29d7..3e815937f4b 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -7,6 +7,7 @@ module Ci include IgnorableColumn include RedisCacheable include ChronicDurationAttribute + include FromUnion RUNNER_QUEUE_EXPIRY_TIME = 60.minutes ONLINE_CONTACT_TIMEOUT = 1.hour @@ -57,18 +58,26 @@ module Ci } scope :owned_or_instance_wide, -> (project_id) do - union = Gitlab::SQL::Union.new( - [belonging_to_project(project_id), belonging_to_parent_group_of_project(project_id), instance_type], + from_union( + [ + belonging_to_project(project_id), + belonging_to_parent_group_of_project(project_id), + instance_type + ], remove_duplicates: false ) - from("(#{union.to_sql}) ci_runners") end scope :assignable_for, ->(project) do # FIXME: That `to_sql` is needed to workaround a weird Rails bug. # Without that, placeholders would miss one and couldn't match. + # + # We use "unscoped" here so that any current Ci::Runner filters don't + # apply to the inner query, which is not necessary. + exclude_runners = unscoped { project.runners.select(:id) }.to_sql + where(locked: false) - .where.not("ci_runners.id IN (#{project.runners.select(:id).to_sql})") + .where.not("ci_runners.id IN (#{exclude_runners})") .project_type end diff --git a/app/models/concerns/from_union.rb b/app/models/concerns/from_union.rb new file mode 100644 index 00000000000..9b8595b1211 --- /dev/null +++ b/app/models/concerns/from_union.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module FromUnion + extend ActiveSupport::Concern + + class_methods do + # Produces a query that uses a FROM to select data using a UNION. + # + # Using a FROM for a UNION has in the past lead to better query plans. As + # such, we generally recommend this pattern instead of using a WHERE IN. + # + # Example: + # users = User.from_union([User.where(id: 1), User.where(id: 2)]) + # + # This would produce the following SQL query: + # + # SELECT * + # FROM ( + # SELECT * + # FROM users + # WHERE id = 1 + # + # UNION + # + # SELECT * + # FROM users + # WHERE id = 2 + # ) users; + # + # members - An Array of ActiveRecord::Relation objects to use in the UNION. + # + # remove_duplicates - A boolean indicating if duplicate entries should be + # removed. Defaults to true. + # + # alias_as - The alias to use for the sub query. Defaults to the name of the + # table of the current model. + # rubocop: disable Gitlab/Union + def from_union(members, remove_duplicates: true, alias_as: table_name) + union = Gitlab::SQL::Union + .new(members, remove_duplicates: remove_duplicates) + .to_sql + + # This pattern is necessary as a bug in Rails 4 can cause the use of + # `from("string here").includes(:foo)` to break ActiveRecord. This is + # fixed in https://github.com/rails/rails/pull/25374, which is released as + # part of Rails 5. + from([Arel.sql("(#{union}) #{alias_as}")]) + end + # rubocop: enable Gitlab/Union + end +end diff --git a/app/models/dashboard_group_milestone.rb b/app/models/dashboard_group_milestone.rb index 067e14dda1c..32e8104125c 100644 --- a/app/models/dashboard_group_milestone.rb +++ b/app/models/dashboard_group_milestone.rb @@ -13,7 +13,11 @@ class DashboardGroupMilestone < GlobalMilestone end def self.build_collection(groups) - MilestonesFinder.new(group_ids: groups.select(:id)).execute.map { |m| new(m) } # rubocop: disable CodeReuse/Finder + Milestone.of_groups(groups.select(:id)) + .reorder_by_due_date_asc + .order_by_name_asc + .active + .map { |m| new(m) } end override :group_milestone? diff --git a/app/models/event.rb b/app/models/event.rb index 041dac6941b..596155a9525 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -3,6 +3,7 @@ class Event < ActiveRecord::Base include Sortable include IgnorableColumn + include FromUnion default_scope { reorder(nil) } CREATED = 1 diff --git a/app/models/group.rb b/app/models/group.rb index 024e77188b8..62af20d2142 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -304,14 +304,12 @@ class Group < Namespace # 3. They belong to a sub-group or project in such sub-group # 4. They belong to an ancestor group def direct_and_indirect_users - union = Gitlab::SQL::Union.new([ + User.from_union([ User .where(id: direct_and_indirect_members.select(:user_id)) .reorder(nil), project_users_with_descendants ]) - - User.from("(#{union.to_sql}) #{User.table_name}") end # Returns all users that are members of projects diff --git a/app/models/label.rb b/app/models/label.rb index 8dc7ded53ad..9ef57a05b3e 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -7,6 +7,7 @@ class Label < ActiveRecord::Base include Gitlab::SQL::Pattern include OptionallySearch include Sortable + include FromUnion # Represents a "No Label" state used for filtering Issues and Merge # Requests that have no label assigned. diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index e19bf62dcd0..dd5d494997d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -14,6 +14,7 @@ class MergeRequest < ActiveRecord::Base include Gitlab::Utils::StrongMemoize include LabelEventable include ReactiveCaching + include FromUnion self.reactive_cache_key = ->(model) { [model.project.id, model.iid] } self.reactive_cache_refresh_interval = 10.minutes @@ -237,11 +238,10 @@ class MergeRequest < ActiveRecord::Base def self.in_projects(relation) # unscoping unnecessary conditions that'll be applied # when executing `where("merge_requests.id IN (#{union.to_sql})")` - source = unscoped.where(source_project_id: relation).select(:id) - target = unscoped.where(target_project_id: relation).select(:id) - union = Gitlab::SQL::Union.new([source, target]) + source = unscoped.where(source_project_id: relation) + target = unscoped.where(target_project_id: relation) - where("merge_requests.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection + from_union([source, target]) end # This is used after project import, to reset the IDs to the correct @@ -740,11 +740,8 @@ class MergeRequest < ActiveRecord::Base # compared to using OR statements. We're using UNION ALL since the queries # used won't produce any duplicates (e.g. a note for a commit can't also be # a note for an MR). - union = Gitlab::SQL::Union - .new([notes, commit_notes], remove_duplicates: false) - .to_sql - - Note.from("(#{union}) #{Note.table_name}") + Note + .from_union([notes, commit_notes], remove_duplicates: false) .includes(:noteable) end diff --git a/app/models/milestone.rb b/app/models/milestone.rb index cb1def1b422..892a680f221 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -46,6 +46,9 @@ class Milestone < ActiveRecord::Base where(conditions.reduce(:or)) end + scope :order_by_name_asc, -> { order(Arel::Nodes::Ascending.new(arel_table[:title].lower)) } + scope :reorder_by_due_date_asc, -> { reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) } + validates :group, presence: true, unless: :project validates :project, presence: true, unless: :group @@ -149,7 +152,7 @@ class Milestone < ActiveRecord::Base sorted = case method.to_s when 'due_date_asc' - reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) + reorder_by_due_date_asc when 'due_date_desc' reorder(Gitlab::Database.nulls_last_order('due_date', 'DESC')) when 'name_asc' diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 76920c3c039..0289f29211d 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -11,6 +11,7 @@ class Namespace < ActiveRecord::Base include Gitlab::SQL::Pattern include IgnorableColumn include FeatureGate + include FromUnion ignore_column :deleted_at diff --git a/app/models/note.rb b/app/models/note.rb index 4429c1dcb07..bea02d69b65 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -17,6 +17,7 @@ class Note < ActiveRecord::Base include Editable include Gitlab::SQL::Pattern include ThrottledTouch + include FromUnion module SpecialRole FIRST_TIME_CONTRIBUTOR = :first_time_contributor diff --git a/app/models/project.rb b/app/models/project.rb index c37915e111f..9e4c7f7a2d0 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -29,6 +29,7 @@ class Project < ActiveRecord::Base include BatchDestroyDependentAssociations include FeatureGate include OptionallySearch + include FromUnion extend Gitlab::Cache::RequestCache extend Gitlab::ConfigHelper @@ -1493,8 +1494,7 @@ class Project < ActiveRecord::Base end def all_runners - union = Gitlab::SQL::Union.new([runners, group_runners, shared_runners]) - Ci::Runner.from("(#{union.to_sql}) ci_runners") + Ci::Runner.from_union([runners, group_runners, shared_runners]) end def active_runners @@ -2022,12 +2022,10 @@ class Project < ActiveRecord::Base def badges return project_badges unless group - group_badges_rel = GroupBadge.where(group: group.self_and_ancestors) - - union = Gitlab::SQL::Union.new([project_badges.select(:id), - group_badges_rel.select(:id)]) - - Badge.where("id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection + Badge.from_union([ + project_badges, + GroupBadge.where(group: group.self_and_ancestors) + ]) end def merge_requests_allowing_push_to_user(user) diff --git a/app/models/project_authorization.rb b/app/models/project_authorization.rb index 746bb4584c9..2c590008db2 100644 --- a/app/models/project_authorization.rb +++ b/app/models/project_authorization.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ProjectAuthorization < ActiveRecord::Base + include FromUnion + belongs_to :user belongs_to :project @@ -8,9 +10,9 @@ class ProjectAuthorization < ActiveRecord::Base validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true validates :user, uniqueness: { scope: [:project, :access_level] }, presence: true - def self.select_from_union(union) - select(['project_id', 'MAX(access_level) AS access_level']) - .from("(#{union.to_sql}) #{ProjectAuthorization.table_name}") + def self.select_from_union(relations) + from_union(relations) + .select(['project_id', 'MAX(access_level) AS access_level']) .group(:project_id) end diff --git a/app/models/project_services/chat_message/merge_message.rb b/app/models/project_services/chat_message/merge_message.rb index 58631e09538..cc88d57faf8 100644 --- a/app/models/project_services/chat_message/merge_message.rb +++ b/app/models/project_services/chat_message/merge_message.rb @@ -48,7 +48,7 @@ module ChatMessage end def merge_request_message - "#{user_combined_name} #{state} #{merge_request_link} in #{project_link}: #{title}" + "#{user_combined_name} #{state_or_action_text} #{merge_request_link} in #{project_link}" end def merge_request_link @@ -62,5 +62,10 @@ module ChatMessage def merge_request_url "#{project_url}/merge_requests/#{merge_request_iid}" end + + # overridden in EE + def state_or_action_text + state + end end end diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index f4b3421f04b..761359b3c9f 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -184,11 +184,12 @@ class ProjectWiki def commit_details(action, message = nil, title = nil) commit_message = message || default_message(action, title) + git_user = Gitlab::Git::User.from_gitlab(@user) Gitlab::Git::Wiki::CommitDetails.new(@user.id, - @user.username, - @user.name, - @user.email, + git_user.username, + git_user.name, + git_user.email, commit_message) end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 5b394e3fa79..e9533ee7c77 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -12,6 +12,7 @@ class Snippet < ActiveRecord::Base include Spammable include Editable include Gitlab::SQL::Pattern + include FromUnion cache_markdown_field :title, pipeline: :single_line cache_markdown_field :description diff --git a/app/models/todo.rb b/app/models/todo.rb index 48d92ad04b3..265fb932f7c 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -2,6 +2,7 @@ class Todo < ActiveRecord::Base include Sortable + include FromUnion ASSIGNED = 1 MENTIONED = 2 diff --git a/app/models/user.rb b/app/models/user.rb index d68108a8e8e..eeac87e2e52 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -20,6 +20,7 @@ class User < ActiveRecord::Base include BlocksJsonSerialization include WithUploads include OptionallySearch + include FromUnion DEFAULT_NOTIFICATION_LEVEL = :participating @@ -286,11 +287,9 @@ class User < ActiveRecord::Base # user_id - The ID of the user to include. def self.union_with_user(user_id = nil) if user_id.present? - union = Gitlab::SQL::Union.new([all, User.unscoped.where(id: user_id)]) - # We use "unscoped" here so that any inner conditions are not repeated for # the outer query, which would be redundant. - User.unscoped.from("(#{union.to_sql}) #{User.table_name}") + User.unscoped.from_union([all, User.unscoped.where(id: user_id)]) else all end @@ -354,9 +353,8 @@ class User < ActiveRecord::Base emails = joins(:emails).where(emails: { email: email }) emails = emails.confirmed if confirmed - union = Gitlab::SQL::Union.new([users, emails]) - from("(#{union.to_sql}) #{table_name}") + from_union([users, emails]) end def filter(filter_name) @@ -635,7 +633,7 @@ class User < ActiveRecord::Base # possibility of the commit_email column not existing. def commit_email - return unless has_attribute?(:commit_email) + return self.email unless has_attribute?(:commit_email) # The commit email is the same as the primary email if undefined super.presence || self.email @@ -676,10 +674,10 @@ class User < ActiveRecord::Base # Returns the groups a user has access to, either through a membership or a project authorization def authorized_groups - union = Gitlab::SQL::Union - .new([groups.select(:id), authorized_projects.select(:namespace_id)]) - - Group.where("namespaces.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection + Group.from_union([ + groups, + authorized_projects.joins(:namespace).select('namespaces.*') + ]) end # Returns the groups a user is a member of, either directly or through a parent group @@ -744,7 +742,15 @@ class User < ActiveRecord::Base end def owned_projects - @owned_projects ||= Project.from("(#{owned_projects_union.to_sql}) AS projects") + @owned_projects ||= Project.from_union( + [ + Project.where(namespace: namespace), + Project.joins(:project_authorizations) + .where("projects.namespace_id <> ?", namespace.id) + .where(project_authorizations: { user_id: id, access_level: Gitlab::Access::OWNER }) + ], + remove_duplicates: false + ) end # Returns projects which user can admin issues on (for example to move an issue to that project). @@ -1138,17 +1144,17 @@ class User < ActiveRecord::Base def ci_owned_runners @ci_owned_runners ||= begin - project_runner_ids = Ci::RunnerProject + project_runners = Ci::RunnerProject .where(project: authorized_projects(Gitlab::Access::MAINTAINER)) - .select(:runner_id) + .joins(:runner) + .select('ci_runners.*') - group_runner_ids = Ci::RunnerNamespace + group_runners = Ci::RunnerNamespace .where(namespace_id: owned_or_maintainers_groups.select(:id)) - .select(:runner_id) + .joins(:runner) + .select('ci_runners.*') - union = Gitlab::SQL::Union.new([project_runner_ids, group_runner_ids]) - - Ci::Runner.where("ci_runners.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection + Ci::Runner.from_union([project_runners, group_runners]) end end @@ -1176,13 +1182,13 @@ class User < ActiveRecord::Base def assigned_open_merge_requests_count(force: false) Rails.cache.fetch(['users', id, 'assigned_open_merge_requests_count'], force: force, expires_in: 20.minutes) do - MergeRequestsFinder.new(self, assignee_id: self.id, state: 'opened').execute.count + MergeRequestsFinder.new(self, assignee_id: self.id, state: 'opened', non_archived: true).execute.count end end def assigned_open_issues_count(force: false) Rails.cache.fetch(['users', id, 'assigned_open_issues_count'], force: force, expires_in: 20.minutes) do - IssuesFinder.new(self, assignee_id: self.id, state: 'opened').execute.count + IssuesFinder.new(self, assignee_id: self.id, state: 'opened', non_archived: true).execute.count end end @@ -1380,15 +1386,6 @@ class User < ActiveRecord::Base Gitlab::CurrentSettings.usage_stats_set_by_user_id == self.id end - def owned_projects_union - Gitlab::SQL::Union.new([ - Project.where(namespace: namespace), - Project.joins(:project_authorizations) - .where("projects.namespace_id <> ?", namespace.id) - .where(project_authorizations: { user_id: id, access_level: Gitlab::Access::OWNER }) - ], remove_duplicates: false) - end - # Added according to https://github.com/plataformatec/devise/blob/7df57d5081f9884849ca15e4fde179ef164a575f/README.md#activejob-integration def send_devise_notification(notification, *args) return true unless can?(:receive_notifications) diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index ab9861c58c4..00a441a9a1e 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -79,6 +79,20 @@ class BuildDetailsEntity < JobEntity expose :trigger_variables, as: :variables, using: TriggerVariableEntity end + expose :runners do + expose :online do |build| + build.any_runners_online? + end + + expose :available do |build| + project.any_runners? + end + + expose :settings_path, if: -> (*) { can_admin_build? } do |build| + project_runners_path(project) + end + end + private def build_failed_issue_options @@ -97,4 +111,8 @@ class BuildDetailsEntity < JobEntity def can_create_build_terminal? can?(current_user, :create_build_terminal, build) && build.has_terminal? end + + def can_admin_build? + can?(request.current_user, :admin_build, project) + end end diff --git a/app/serializers/runner_entity.rb b/app/serializers/runner_entity.rb index 04ec80e0efa..97e5b336a35 100644 --- a/app/serializers/runner_entity.rb +++ b/app/serializers/runner_entity.rb @@ -5,8 +5,7 @@ class RunnerEntity < Grape::Entity expose :id, :description - expose :edit_path, - if: -> (*) { can?(request.current_user, :admin_build, project) && runner.project_type? } do |runner| + expose :edit_path, if: -> (*) { can_edit_runner? } do |runner| edit_project_runner_path(project, runner) end @@ -17,4 +16,8 @@ class RunnerEntity < Grape::Entity def project request.project end + + def can_edit_runner? + can?(request.current_user, :update_runner, runner) && runner.project_type? + end end diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb index 4e352f2dc63..0b69661bbd0 100644 --- a/app/services/boards/issues/list_service.rb +++ b/app/services/boards/issues/list_service.rb @@ -56,6 +56,7 @@ module Boards set_parent set_state set_scope + set_non_archived params end @@ -76,6 +77,10 @@ module Boards params[:include_subgroups] = board.group_board? end + def set_non_archived + params[:non_archived] = parent.is_a?(Group) + end + # rubocop: disable CodeReuse/ActiveRecord def board_label_ids @board_label_ids ||= board.lists.movable.pluck(:label_id) diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index fc7b236f7da..39e614d6569 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -7,8 +7,10 @@ module Files def initialize(*args) super - @author_email = params[:author_email] || current_user&.email - @author_name = params[:author_name] || current_user&.name + git_user = Gitlab::Git::User.from_gitlab(current_user) if current_user.present? + + @author_email = params[:author_email] || git_user&.email + @author_name = params[:author_name] || git_user&.name @commit_message = params[:commit_message] @last_commit_sha = params[:last_commit_sha] diff --git a/app/services/labels/transfer_service.rb b/app/services/labels/transfer_service.rb index aec0282b31b..52360f775dc 100644 --- a/app/services/labels/transfer_service.rb +++ b/app/services/labels/transfer_service.rb @@ -34,13 +34,13 @@ module Labels # rubocop: disable CodeReuse/ActiveRecord def labels_to_transfer - label_ids = [] - label_ids << group_labels_applied_to_issues.select(:id) - label_ids << group_labels_applied_to_merge_requests.select(:id) - - union = Gitlab::SQL::Union.new(label_ids) - - Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq # rubocop:disable GitlabSecurity/SqlInjection + Label + .from_union([ + group_labels_applied_to_issues, + group_labels_applied_to_merge_requests + ]) + .reorder(nil) + .uniq end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb index faaf1283078..216acf79cbd 100644 --- a/app/validators/url_validator.rb +++ b/app/validators/url_validator.rb @@ -41,12 +41,13 @@ class UrlValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) @record = record - if value.present? - value.strip! - else + unless value.present? record.errors.add(attribute, 'must be a valid URL') + return end + value = strip_value!(record, attribute, value) + Gitlab::UrlBlocker.validate!(value, blocker_args) rescue Gitlab::UrlBlocker::BlockedUrlError => e record.errors.add(attribute, "is blocked: #{e.message}") @@ -54,6 +55,13 @@ class UrlValidator < ActiveModel::EachValidator private + def strip_value!(record, attribute, value) + new_value = value.strip + return value if new_value == value + + record.public_send("#{attribute}=", new_value) # rubocop:disable GitlabSecurity/PublicSend + end + def default_options # By default the validator doesn't block any url based on the ip address { diff --git a/app/views/admin/runners/index.html.haml b/app/views/admin/runners/index.html.haml index 4dc076c95c5..07333d63f2c 100644 --- a/app/views/admin/runners/index.html.haml +++ b/app/views/admin/runners/index.html.haml @@ -69,7 +69,7 @@ %ul{ data: { dropdown: true } } %li.filter-dropdown-item{ data: { action: 'submit' } } = button_tag class: %w[btn btn-link] do - = icon('search') + = sprite_icon('search') %span = _('Press Enter or click to search') %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } @@ -77,7 +77,8 @@ = button_tag class: %w[btn btn-link] do -# Encapsulate static class name `{{icon}}` inside #{} to bypass -# haml lint's ClassAttributeWithStaticValue - %i.fa{ class: "#{'{{icon}}'}" } + %svg + %use{ 'xlink:href': "#{'{{icon}}'}" } %span.js-filter-hint {{hint}} %span.js-filter-tag.dropdown-light-content diff --git a/app/views/devise/passwords/edit.html.haml b/app/views/devise/passwords/edit.html.haml index 35dafb3e980..4b8ad5acd5b 100644 --- a/app/views/devise/passwords/edit.html.haml +++ b/app/views/devise/passwords/edit.html.haml @@ -7,12 +7,12 @@ = f.hidden_field :reset_password_token .form-group = f.label 'New password', for: "user_password" - = f.password_field :password, class: "form-control top", required: true, title: 'This field is required' + = f.password_field :password, class: "form-control top qa-password-field", required: true, title: 'This field is required' .form-group = f.label 'Confirm new password', for: "user_password_confirmation" - = f.password_field :password_confirmation, class: "form-control bottom", title: 'This field is required', required: true + = f.password_field :password_confirmation, class: "form-control bottom qa-password-confirmation", title: 'This field is required', required: true .clearfix - = f.submit "Change your password", class: "btn btn-primary" + = f.submit "Change your password", class: "btn btn-primary qa-change-password-button" .clearfix.prepend-top-20 %p diff --git a/app/views/devise/sessions/_new_base.html.haml b/app/views/devise/sessions/_new_base.html.haml index 17a9c8df872..492c36081d8 100644 --- a/app/views/devise/sessions/_new_base.html.haml +++ b/app/views/devise/sessions/_new_base.html.haml @@ -1,10 +1,10 @@ = form_for(resource, as: resource_name, url: session_path(resource_name), html: { class: 'new_user gl-show-field-errors', 'aria-live' => 'assertive'}) do |f| .form-group = f.label "Username or email", for: "user_login", class: 'label-bold' - = f.text_field :login, class: "form-control top", autofocus: "autofocus", autocapitalize: "off", autocorrect: "off", required: true, title: "This field is required." + = f.text_field :login, class: "form-control top qa-login-field", autofocus: "autofocus", autocapitalize: "off", autocorrect: "off", required: true, title: "This field is required." .form-group = f.label :password, class: 'label-bold' - = f.password_field :password, class: "form-control bottom", required: true, title: "This field is required." + = f.password_field :password, class: "form-control bottom qa-password-field", required: true, title: "This field is required." - if devise_mapping.rememberable? .remember-me %label{ for: "user_remember_me" } @@ -17,4 +17,4 @@ = recaptcha_tags .submit-container.move-submit-down - = f.submit "Sign in", class: "btn btn-save" + = f.submit "Sign in", class: "btn btn-save qa-sign-in-button" diff --git a/app/views/devise/sessions/_new_ldap.html.haml b/app/views/devise/sessions/_new_ldap.html.haml index 6bf7349f602..6b04f69fe61 100644 --- a/app/views/devise/sessions/_new_ldap.html.haml +++ b/app/views/devise/sessions/_new_ldap.html.haml @@ -1,13 +1,13 @@ = form_tag(omniauth_callback_path(:user, server['provider_name']), id: 'new_ldap_user', class: "gl-show-field-errors") do .form-group = label_tag :username, "#{server['label']} Username" - = text_field_tag :username, nil, { class: "form-control top", title: "This field is required.", autofocus: "autofocus", required: true } + = text_field_tag :username, nil, { class: "form-control top qa-username-field", title: "This field is required.", autofocus: "autofocus", required: true } .form-group = label_tag :password - = password_field_tag :password, nil, { class: "form-control bottom", title: "This field is required.", required: true } + = password_field_tag :password, nil, { class: "form-control bottom qa-password-field", title: "This field is required.", required: true } - if devise_mapping.rememberable? .remember-me %label{ for: "remember_me" } = check_box_tag :remember_me, '1', false, id: 'remember_me' %span Remember me - = submit_tag "Sign in", class: "btn-save btn" + = submit_tag "Sign in", class: "btn-save btn qa-sign-in-button" diff --git a/app/views/devise/shared/_tabs_ldap.html.haml b/app/views/devise/shared/_tabs_ldap.html.haml index 58c585a29ff..3764e86dd8b 100644 --- a/app/views/devise/shared/_tabs_ldap.html.haml +++ b/app/views/devise/shared/_tabs_ldap.html.haml @@ -4,10 +4,10 @@ = link_to "Crowd", "#crowd", class: 'nav-link active', 'data-toggle' => 'tab' - @ldap_servers.each_with_index do |server, i| %li.nav-item - = link_to server['label'], "##{server['provider_name']}", class: "nav-link #{active_when(i.zero? && !crowd_enabled?)}", 'data-toggle' => 'tab' + = link_to server['label'], "##{server['provider_name']}", class: "nav-link #{active_when(i.zero? && !crowd_enabled?)} qa-ldap-tab", 'data-toggle' => 'tab' - if password_authentication_enabled_for_web? %li.nav-item - = link_to 'Standard', '#login-pane', class: 'nav-link', 'data-toggle' => 'tab' + = link_to 'Standard', '#login-pane', class: 'nav-link qa-standard-tab', 'data-toggle' => 'tab' - if allow_signup? %li.nav-item = link_to 'Register', '#register-pane', class: 'nav-link', 'data-toggle' => 'tab' diff --git a/app/views/devise/shared/_tabs_normal.html.haml b/app/views/devise/shared/_tabs_normal.html.haml index 284d4fa1b89..8745a4e9d3e 100644 --- a/app/views/devise/shared/_tabs_normal.html.haml +++ b/app/views/devise/shared/_tabs_normal.html.haml @@ -1,6 +1,6 @@ %ul.nav-links.new-session-tabs.nav-tabs.nav{ role: 'tablist' } %li.nav-item{ role: 'presentation' } - %a.nav-link.active{ href: '#login-pane', data: { toggle: 'tab' }, role: 'tab' } Sign in + %a.nav-link.qa-sign-in-tab.active{ href: '#login-pane', data: { toggle: 'tab' }, role: 'tab' } Sign in - if allow_signup? %li.nav-item{ role: 'presentation' } - %a.nav-link{ href: '#register-pane', data: { toggle: 'tab' }, role: 'tab' } Register + %a.nav-link.qa-register-tab{ href: '#register-pane', data: { toggle: 'tab' }, role: 'tab' } Register diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 30e0e9fca27..7239780871c 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -158,7 +158,7 @@ - if project_nav_tab? :pipelines = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :artifacts]) do - = link_to project_pipelines_path(@project), class: 'shortcuts-pipelines' do + = link_to project_pipelines_path(@project), class: 'shortcuts-pipelines qa-link-pipelines' do .nav-icon-container = sprite_icon('rocket') %span.nav-item-name diff --git a/app/views/profiles/keys/_form.html.haml b/app/views/profiles/keys/_form.html.haml index 5207921d6fe..700c22e1652 100644 --- a/app/views/profiles/keys/_form.html.haml +++ b/app/views/profiles/keys/_form.html.haml @@ -5,10 +5,10 @@ .form-group = f.label :key, class: 'label-bold' %p= _("Paste your public SSH key, which is usually contained in the file '~/.ssh/id_rsa.pub' and begins with 'ssh-rsa'. Don't use your private SSH key.") - = f.text_area :key, class: "form-control js-add-ssh-key-validation-input", rows: 8, required: true, placeholder: s_('Profiles|Typically starts with "ssh-rsa …"') + = f.text_area :key, class: "form-control js-add-ssh-key-validation-input qa-key-public-key-field", rows: 8, required: true, placeholder: s_('Profiles|Typically starts with "ssh-rsa …"') .form-group = f.label :title, class: 'label-bold' - = f.text_field :title, class: "form-control input-lg", required: true, placeholder: s_('Profiles|e.g. My MacBook key') + = f.text_field :title, class: "form-control input-lg qa-key-title-field", required: true, placeholder: s_('Profiles|e.g. My MacBook key') %p.form-text.text-muted= _('Name your individual key via a title') .js-add-ssh-key-validation-warning.hide @@ -19,4 +19,4 @@ %button.btn.btn-create.js-add-ssh-key-validation-confirm-submit= _("Yes, add it") .prepend-top-default - = f.submit s_('Profiles|Add key'), class: "btn btn-create js-add-ssh-key-validation-original-submit" + = f.submit s_('Profiles|Add key'), class: "btn btn-create js-add-ssh-key-validation-original-submit qa-add-key-button" diff --git a/app/views/profiles/keys/_key_details.html.haml b/app/views/profiles/keys/_key_details.html.haml index 2ac514d3f6f..88473c7f72d 100644 --- a/app/views/profiles/keys/_key_details.html.haml +++ b/app/views/profiles/keys/_key_details.html.haml @@ -24,4 +24,4 @@ = @key.key .col-md-12 .float-right - = link_to 'Remove', path_to_key(@key, is_admin), data: {confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove delete-key" + = link_to 'Remove', path_to_key(@key, is_admin), data: {confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove delete-key qa-delete-key-button" diff --git a/app/views/profiles/two_factor_auths/_codes.html.haml b/app/views/profiles/two_factor_auths/_codes.html.haml index 93722d7b034..fb4fff12027 100644 --- a/app/views/profiles/two_factor_auths/_codes.html.haml +++ b/app/views/profiles/two_factor_auths/_codes.html.haml @@ -10,4 +10,6 @@ %li %span.monospace= code -= link_to 'Proceed', profile_account_path, class: 'btn btn-success' +.d-flex + = link_to 'Proceed', profile_account_path, class: 'btn btn-success append-right-10' + = link_to 'Download codes', "data:text/plain;charset=utf-8,#{CGI.escape(@codes.join("\n"))}", download: "gitlab-recovery-codes.txt", class: 'btn btn-default' diff --git a/app/views/projects/environments/terminal.html.haml b/app/views/projects/environments/terminal.html.haml index 5b680189bc8..e40d631a1a1 100644 --- a/app/views/projects/environments/terminal.html.haml +++ b/app/views/projects/environments/terminal.html.haml @@ -2,7 +2,7 @@ - page_title "Terminal for environment", @environment.name - content_for :page_specific_javascripts do - = stylesheet_link_tag "xterm/xterm" + = stylesheet_link_tag "xterm.css" %div{ class: container_class } .top-area diff --git a/app/views/projects/jobs/show.html.haml b/app/views/projects/jobs/show.html.haml index 078f40c4477..cf8d42976f8 100644 --- a/app/views/projects/jobs/show.html.haml +++ b/app/views/projects/jobs/show.html.haml @@ -10,7 +10,7 @@ - unless @build.any_runners_online? .bs-callout.bs-callout-warning.js-build-stuck %p - - if no_runners_for_project?(@build.project) + - if @project.any_runners? This job is stuck, because the project doesn't have any runners online assigned to it. - elsif @build.tags.any? This job is stuck, because you don't have any active runners online with any of these tags assigned to them: diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index 9ce7f6fe269..659e03fd67d 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -34,7 +34,7 @@ %ul{ data: { dropdown: true } } %li.filter-dropdown-item{ data: { action: 'submit' } } %button.btn.btn-link - = icon('search') + = sprite_icon('search') %span Press Enter or click to search %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } @@ -42,7 +42,8 @@ %button.btn.btn-link -# Encapsulate static class name `{{icon}}` inside #{} to bypass -# haml lint's ClassAttributeWithStaticValue - %i.fa{ class: "#{'{{icon}}'}" } + %svg + %use{ 'xlink:href': "#{'{{icon}}'}" } %span.js-filter-hint {{hint}} %span.js-filter-tag.dropdown-light-content diff --git a/changelogs/unreleased/41729-enable-auto-devops-instance-wide-for-everyone.yml b/changelogs/unreleased/41729-enable-auto-devops-instance-wide-for-everyone.yml new file mode 100644 index 00000000000..ff925c4fc55 --- /dev/null +++ b/changelogs/unreleased/41729-enable-auto-devops-instance-wide-for-everyone.yml @@ -0,0 +1,5 @@ +--- +title: Enable Auto DevOps Instance Wide Default +merge_request: 21157 +author: +type: changed diff --git a/changelogs/unreleased/44596-double-title-merge-request-message.yml b/changelogs/unreleased/44596-double-title-merge-request-message.yml new file mode 100644 index 00000000000..714d16977fb --- /dev/null +++ b/changelogs/unreleased/44596-double-title-merge-request-message.yml @@ -0,0 +1,5 @@ +--- +title: Fix double title in merge request chat messages. +merge_request: 21670 +author: Kukovskii Vladimir +type: fixed diff --git a/changelogs/unreleased/45754-issue-mr-and-archived-projects.yml b/changelogs/unreleased/45754-issue-mr-and-archived-projects.yml new file mode 100644 index 00000000000..d81f47d9654 --- /dev/null +++ b/changelogs/unreleased/45754-issue-mr-and-archived-projects.yml @@ -0,0 +1,5 @@ +--- +title: Issue and MR count now ignores archived projects +merge_request: 21721 +author: +type: fixed diff --git a/changelogs/unreleased/45754-open-issues-from-archived-project-listed-in-group-issue-board.yml b/changelogs/unreleased/45754-open-issues-from-archived-project-listed-in-group-issue-board.yml new file mode 100644 index 00000000000..34394396020 --- /dev/null +++ b/changelogs/unreleased/45754-open-issues-from-archived-project-listed-in-group-issue-board.yml @@ -0,0 +1,5 @@ +--- +title: No longer show open issues from archived projects in group issue board +merge_request: 21721 +author: +type: fixed diff --git a/changelogs/unreleased/46733-move-filter-dropdown-from-font-awesome-to-our-own-icons.yml b/changelogs/unreleased/46733-move-filter-dropdown-from-font-awesome-to-our-own-icons.yml new file mode 100644 index 00000000000..07549781330 --- /dev/null +++ b/changelogs/unreleased/46733-move-filter-dropdown-from-font-awesome-to-our-own-icons.yml @@ -0,0 +1,5 @@ +--- +title: Updated icons used in filtered search dropdowns +merge_request: 21694 +author: +type: changed diff --git a/changelogs/unreleased/50111-improve-design-of-cluster-apps-to-handle-larger-quantity.yml b/changelogs/unreleased/50111-improve-design-of-cluster-apps-to-handle-larger-quantity.yml new file mode 100644 index 00000000000..438c847327a --- /dev/null +++ b/changelogs/unreleased/50111-improve-design-of-cluster-apps-to-handle-larger-quantity.yml @@ -0,0 +1,5 @@ +--- +title: Improve install flow of Kubernetes cluster apps +merge_request: 21567 +author: +type: changed diff --git a/changelogs/unreleased/50956-web-terminal.yml b/changelogs/unreleased/50956-web-terminal.yml new file mode 100644 index 00000000000..73317b0224c --- /dev/null +++ b/changelogs/unreleased/50956-web-terminal.yml @@ -0,0 +1,5 @@ +--- +title: Include correct CSS file for xterm in environments page +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/51273-expose-runners-for-build-in-job-api.yml b/changelogs/unreleased/51273-expose-runners-for-build-in-job-api.yml new file mode 100644 index 00000000000..df43f1dfbae --- /dev/null +++ b/changelogs/unreleased/51273-expose-runners-for-build-in-job-api.yml @@ -0,0 +1,5 @@ +--- +title: Expose project runners in job API +merge_request: 21618 +author: +type: other diff --git a/changelogs/unreleased/51564-fix-commit-email-usage.yml b/changelogs/unreleased/51564-fix-commit-email-usage.yml new file mode 100644 index 00000000000..2f1b042ae8a --- /dev/null +++ b/changelogs/unreleased/51564-fix-commit-email-usage.yml @@ -0,0 +1,5 @@ +--- +title: Respect the user commit email in more places +merge_request: 21773 +author: +type: fixed diff --git a/changelogs/unreleased/add-2fa-button.yml b/changelogs/unreleased/add-2fa-button.yml new file mode 100644 index 00000000000..6cb71d52781 --- /dev/null +++ b/changelogs/unreleased/add-2fa-button.yml @@ -0,0 +1,5 @@ +--- +title: Add button to download 2FA codes +merge_request: +author: Luke Picciau +type: added diff --git a/changelogs/unreleased/osw-gitaly-diff-stats-client.yml b/changelogs/unreleased/osw-gitaly-diff-stats-client.yml new file mode 100644 index 00000000000..9f280162409 --- /dev/null +++ b/changelogs/unreleased/osw-gitaly-diff-stats-client.yml @@ -0,0 +1,5 @@ +--- +title: Add Gitaly diff stats RPC client +merge_request: 21732 +author: +type: changed diff --git a/danger/commit_messages/Dangerfile b/danger/commit_messages/Dangerfile index e982e8ed7c5..a152ff837a9 100644 --- a/danger/commit_messages/Dangerfile +++ b/danger/commit_messages/Dangerfile @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'json' + # rubocop: disable Style/SignalException # rubocop: disable Metrics/CyclomaticComplexity # rubocop: disable Metrics/PerceivedComplexity @@ -8,6 +10,30 @@ # https://github.com/jonallured/danger-commit_lint because its output is not # very helpful, and it doesn't offer the means of ignoring merge commits. +class EmojiChecker + DIGESTS = File.expand_path('../../fixtures/emojis/digests.json', __dir__) + ALIASES = File.expand_path('../../fixtures/emojis/aliases.json', __dir__) + + # A regex that indicates a piece of text _might_ include an Emoji. The regex + # alone is not enough, as we'd match `:foo:bar:baz`. Instead, we use this + # regex to save us from having to check for all possible emoji names when we + # know one definitely is not included. + LIKELY_EMOJI = /:[\+a-z0-9_\-]+:/ + + def initialize + names = JSON.parse(File.read(DIGESTS)).keys + + JSON.parse(File.read(ALIASES)).keys + + @emoji = names.map { |name| ":#{name}:" } + end + + def includes_emoji?(text) + return false unless text.match?(LIKELY_EMOJI) + + @emoji.any? { |emoji| text.include?(emoji) } + end +end + def fail_commit(commit, message) fail("#{commit.sha}: #{message}") end @@ -33,6 +59,7 @@ end def lint_commits(commits) failures = false + emoji_checker = EmojiChecker.new unicode_emoji_regex = %r(( [\u{1F300}-\u{1F5FF}] | @@ -117,7 +144,7 @@ def lint_commits(commits) failures = true end - if commit.message.match?(/:[\+a-z0-9_\-]+:/) + if emoji_checker.includes_emoji?(commit.message) fail_commit( commit, 'Avoid the use of Markdown Emoji such as `:+1:`. ' \ diff --git a/db/migrate/20180813101999_change_default_of_auto_devops_instance_wide.rb b/db/migrate/20180813101999_change_default_of_auto_devops_instance_wide.rb new file mode 100644 index 00000000000..05d1124f5c4 --- /dev/null +++ b/db/migrate/20180813101999_change_default_of_auto_devops_instance_wide.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class ChangeDefaultOfAutoDevopsInstanceWide < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + change_column_default :application_settings, :auto_devops_enabled, true + end + + def down + change_column_default :application_settings, :auto_devops_enabled, false + end +end diff --git a/db/migrate/20180813102000_enable_auto_devops_instance_wide_for_everyone.rb b/db/migrate/20180813102000_enable_auto_devops_instance_wide_for_everyone.rb new file mode 100644 index 00000000000..21fb62806b3 --- /dev/null +++ b/db/migrate/20180813102000_enable_auto_devops_instance_wide_for_everyone.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class EnableAutoDevopsInstanceWideForEveryone < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + execute "UPDATE application_settings SET auto_devops_enabled = true" + end + + def down + # No way to know here what their previous setting was... + end +end diff --git a/db/schema.rb b/db/schema.rb index 95df735cd37..b299cde4898 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -141,7 +141,7 @@ ActiveRecord::Schema.define(version: 20180907015926) do t.integer "performance_bar_allowed_group_id" t.boolean "hashed_storage_enabled", default: false, null: false t.boolean "project_export_enabled", default: true, null: false - t.boolean "auto_devops_enabled", default: false, null: false + t.boolean "auto_devops_enabled", default: true, null: false t.integer "circuitbreaker_failure_count_threshold", default: 3 t.integer "circuitbreaker_failure_reset_time", default: 1800 t.integer "circuitbreaker_storage_timeout", default: 15 diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 41de9a50efc..31a065bc196 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1231,13 +1231,16 @@ rspec: ``` The collected JUnit reports will be uploaded to GitLab as an artifact and will -be automatically [shown in merge requests](../junit_test_reports.md). +be automatically shown in merge requests. + +For more examples, see [JUnit test reports](../junit_test_reports.md). NOTE: **Note:** In case the JUnit tool you use exports to multiple XML files, you can specify -multiple test report paths within a single job -(`junit: [rspec-1.xml, rspec-2.xml, rspec-3.xml]`) and they will be automatically -concatenated into a single file. +multiple test report paths within a single job and they will be automatically +concatenated into a single file. Use a filename pattern (`junit: rspec-*.xml`), +an array of filenames (`junit: [rspec-1.xml, rspec-2.xml, rspec-3.xml]`), or a +combination thereof (`junit: [rspec.xml, test-results/TEST-*.xml]`). ## `dependencies` diff --git a/doc/development/README.md b/doc/development/README.md index efe37b8ba0b..d8dbc993442 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -53,11 +53,14 @@ description: 'Learn how to contribute to GitLab.' ## Performance guides -- [Instrumentation](instrumentation.md) -- [Performance guidelines](performance.md) +- [Instrumentation](instrumentation.md) for Ruby code running in production + environments +- [Performance guidelines](performance.md) for writing code, benchmarks, and + certain patterns to avoid - [Merge request performance guidelines](merge_request_performance_guidelines.md) for ensuring merge requests do not negatively impact GitLab performance -- [Profiling](profiling.md) for profiling a URL +- [Profiling](profiling.md) a URL, measuring performance using Sherlock, or + tracking down N+1 queries using Bullet ## Database guides diff --git a/doc/development/feature_flags.md b/doc/development/feature_flags.md index 6f757f1ce7b..417298205f5 100644 --- a/doc/development/feature_flags.md +++ b/doc/development/feature_flags.md @@ -65,13 +65,18 @@ In the rare case that you need the feature flag to be on automatically, use Feature.enabled?(:feature_flag, project, default_enabled: true) ``` +For more information about rolling out changes using feature flags, refer to the +[Rolling out changes using feature flags](rolling_out_changes_using_feature_flags.md) +guide. + ### Specs In the test environment `Feature.enabled?` is stubbed to always respond to `true`, so we make sure behavior under feature flag doesn't go untested in some non-specific contexts. -If you need to test the feature flag in a different state, you need to stub it with: +Whenever a feature flag is present, make sure to test _both_ states of the +feature flag. You can stub a feature flag as follows: ```ruby stub_feature_flags(my_feature_flag: false) diff --git a/doc/development/rolling_out_changes_using_feature_flags.md b/doc/development/rolling_out_changes_using_feature_flags.md new file mode 100644 index 00000000000..905aa26a40b --- /dev/null +++ b/doc/development/rolling_out_changes_using_feature_flags.md @@ -0,0 +1,153 @@ +# Rolling out changes using feature flags + +[Feature flags](feature_flags.md) can be used to gradually roll out changes, be +it a new feature, or a performance improvement. By using feature flags, we can +comfortably measure the impact of our changes, while still being able to easily +disable those changes, without having to revert an entire release. + +## When to use feature flags + +Starting with GitLab 11.4, developers are required to use feature flags for +non-trivial changes. Such changes include: + +* New features (e.g. a new merge request widget, epics, etc). +* Complex performance improvements that may require additional testing in + production, such as rewriting complex queries. +* Invasive changes to the user interface, such as a new navigation bar or the + removal of a sidebar. +* Adding support for importing projects from a third-party service. + +In all cases, those working on the changes can best decide if a feature flag is +necessary. For example, changing the color of a button doesn't need a feature +flag, while changing the navigation bar definitely needs one. In case you are +uncertain if a feature flag is necessary, simply ask about this in the merge +request, and those reviewing the changes will likely provide you with an answer. + +When using a feature flag for UI elements, make sure to _also_ use a feature +flag for the underlying backend code, if there is any. This ensures there is +absolutely no way to use the feature until it is enabled. + +## The cost of feature flags + +When reading the above, one might be tempted to think this procedure is going to +add a lot of work. Fortunately, this is not the case, and we'll show why. For +this example we'll specify the cost of the work to do as a number, ranging from +0 to infinity. The greater the number, the more expensive the work is. The cost +does _not_ translate to time, it's just a way of measuring complexity of one +change relative to another. + +Let's say we are building a new feature, and we have determined that the cost of +this is 10. We have also determined that the cost of adding a feature flag check +in a variety of places is 1. If we do not use feature flags, and our feature +works as intended, our total cost is 10. This however is the best case scenario. +Optimising for the best case scenario is guaranteed to lead to trouble, whereas +optimising for the worst case scenario is almost always better. + +To illustrate this, let's say our feature causes an outage, and there's no +immediate way to resolve it. This means we'd have to take the following steps to +resolve the outage: + +1. Revert the release. +1. Perform any cleanups that might be necessary, depending on the changes that + were made. +1. Revert the commit, ensuring the "master" branch remains stable. This is + especially necessary if solving the problem can take days or even weeks. +1. Pick the revert commit into the appropriate stable branches, ensuring we + don't block any future releases until the problem is resolved. + +As history has shown, these steps are time consuming, complex, often involve +many developers, and worst of all: our users will have a bad experience using +GitLab.com until the problem is resolved. + +Now let's say that all of this has an associated cost of 10. This means that in +the worst case scenario, which we should optimise for, our total cost is now 20. + +If we had used a feature flag, things would have been very different. We don't +need to revert a release, and because feature flags are disabled by default we +don't need to revert and pick any Git commits. In fact, all we have to do is +disable the feature, and _maybe_ perform some cleanup. Let's say that the cost +of this is 1. In this case, our best case cost is 11: 10 to build the feature, +and 1 to add the feature flag. The worst case cost is now 12: 10 to build the +feature, 1 to add the feature flag, and 1 to disable it. + +Here we can see that in the best case scenario the work necessary is only a tiny +bit more compared to not using a feature flag. Meanwhile, the process of +reverting our changes has been made significantly cheaper, to the point of being +trivial. + +In other words, feature flags do not slow down the development process. Instead, +they speed up the process as managing incidents now becomes _much_ easier. Once +continuous deployments are easier to perform, the time to iterate on a feature +is reduced even further, as you no longer need to wait weeks before your changes +are available on GitLab.com. + +## Rolling out changes + +The procedure of using feature flags is straightforward, and similar to not +using them. You add the necessary tests (make sure to test both the on and off +states of your feature flag(s)), make sure they all pass, have the code +reviewed, etc. You then submit your merge request, and add the ~"feature flag" +label. This label is used to signal to release managers that your changes are +hidden behind a feature flag and that it is safe to pick the MR into a stable +branch, without the need for an exception request. + +When the changes are deployed it is time to start rolling out the feature to our +users. The exact procedure of rolling out a change is unspecified, as this can +vary from change to change. However, in general we recommend rolling out changes +incrementally, instead of enabling them for everybody right away. We also +recommend you to _not_ enable a feature _before_ the code is being deployed. +This allows you to separate rolling out a feature from a deploy, making it +easier to measure the impact of both separately. + +GitLab's feature library (using +[Flipper](https://github.com/jnunemaker/flipper), and covered in the [Feature +Flags](feature_flags.md) guide) supports rolling out changes to a percentage of +users. This in turn can be controlled using [GitLab +chatops](https://docs.gitlab.com/ee/ci/chatops/). + +For example, to enable a feature for 25% of all users, run the following in +Slack: + +``` +/chatops run feature set new_navigation_bar 25 +``` + +This will enable the feature for GitLab.com, with `new_navigation_bar` being the +name of the feature. We can also enable the feature for <https://dev.gitlab.org> +or <https://staging.gitlab.com>: + +``` +/chatops run feature set new_navigation_bar 25 --dev +/chatops run feature set new_navigation_bar 25 --staging +``` + +If you are not certain what percentages to use, simply use the following steps: + +1. 25% +1. 50% +1. 75% +1. 100% + +Between every step you'll want to wait a little while and monitor the +appropriate graphs on <https://dashboards.gitlab.net>. The exact time to wait +may differ. For some features a few minutes is enough, while for others you may +want to wait several hours or even days. This is entirely up to you, just make +sure it is clearly communicated to your team, and the Production team if you +anticipate any potential problems. + +Once a change is deemed stable, submit a new merge request to remove the +feature flag. This ensures the change is available to all users and self-hosted +instances. Make sure to add the ~"feature flag" label to this merge request so +release managers are aware the changes are hidden behind a feature flag. If the +merge request has to be picked into a stable branch (e.g. after the 7th), make +sure to also add the appropriate "Pick into X" label (e.g. "Pick into 11.4"). + +One might be tempted to think this will delay the release of a feature by at +least one month (= one release). This is not the case. A feature flag does not +have to stick around for a specific amount of time (e.g. at least one release), +instead they should stick around until the feature is deemed stable. Stable +means it works on GitLab.com without causing any problems, such as outages. In +most cases this will translate to a feature (with a feature flag) being shipped +in RC1, followed by the feature flag being removed in RC2. This in turn means +the feature will be stable by the time we publish a stable package around the +22nd of the month. diff --git a/doc/install/requirements.md b/doc/install/requirements.md index 5531dcde4e9..df59ce9837c 100644 --- a/doc/install/requirements.md +++ b/doc/install/requirements.md @@ -27,7 +27,7 @@ Please see the [installation from source guide](installation.md) and the [instal ### Non-Unix operating systems such as Windows -GitLab is developed for Unix operating systems. +GitLab is developed for Unix operating systems. It does **not** run on Windows, and we have no plans to support it in the near future. For the latest development status view this [issue](https://gitlab.com/gitlab-org/gitlab-ce/issues/46567). Please consider using a virtual machine to run GitLab. @@ -103,19 +103,21 @@ features of GitLab work with MySQL/MariaDB: 1. MySQL support for subgroups was [dropped with GitLab 9.3][post]. See [issue #30472][30472] for more information. -1. GitLab Geo does [not support MySQL](https://docs.gitlab.com/ee/gitlab-geo/database.html#mysql-replication). -1. [Zero downtime migrations][zero] do not work with MySQL +1. Geo **[STARTER ONLY]** does [not support MySQL](https://docs.gitlab.com/ee/gitlab-geo/database.html#mysql-replication). This means no supported Disaster Recovery solution if using MySQL. +1. [Zero downtime migrations][../update/README.md#upgrading-without-downtime] do not work with MySQL. 1. GitLab [optimizes the loading of dashboard events](https://gitlab.com/gitlab-org/gitlab-ce/issues/31806) using [PostgreSQL LATERAL JOINs](https://blog.heapanalytics.com/postgresqls-powerful-new-join-type-lateral/). 1. In general, SQL optimized for PostgreSQL may run much slower in MySQL due to differences in query planners. For example, subqueries that work well in PostgreSQL - may not be [performant in MySQL](https://dev.mysql.com/doc/refman/5.7/en/optimizing-subqueries.html) + may not be [performant in MySQL](https://dev.mysql.com/doc/refman/5.7/en/optimizing-subqueries.html). +1. Binary column index length is limited to 20 bytes. This is accomplished with [a hack](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/initializers/mysql_set_length_for_binary_indexes.rb). +1. MySQL requires a variety of hacks to increase limits on various columns, [for example](https://gitlab.com/gitlab-org/gitlab-ce/issues/49583). +1. [The milestone filter runs slower queries on MySQL](https://gitlab.com/gitlab-org/gitlab-ce/issues/51173#note_99391731). 1. We expect this list to grow over time. Existing users using GitLab with MySQL/MariaDB are advised to [migrate to PostgreSQL](../update/mysql_to_postgresql.md) instead. [30472]: https://gitlab.com/gitlab-org/gitlab-ce/issues/30472 -[zero]: ../update/README.md#upgrading-without-downtime [post]: https://about.gitlab.com/2017/06/22/gitlab-9-3-released/#dropping-support-for-subgroups-in-mysql ### PostgreSQL Requirements diff --git a/lib/gitlab/contributions_calendar.rb b/lib/gitlab/contributions_calendar.rb index 3236abfa43f..1ffc2639237 100644 --- a/lib/gitlab/contributions_calendar.rb +++ b/lib/gitlab/contributions_calendar.rb @@ -30,8 +30,9 @@ module Gitlab note_events = event_counts(date_from, :merge_requests) .having(action: [Event::COMMENTED]) - union = Gitlab::SQL::Union.new([repo_events, issue_events, mr_events, note_events]) - events = Event.find_by_sql(union.to_sql).map(&:attributes) + events = Event + .from_union([repo_events, issue_events, mr_events, note_events]) + .map(&:attributes) @activity_dates = events.each_with_object(Hash.new {|h, k| h[k] = 0 }) do |event, activities| activities[event["date"]] += event["total_amount"] diff --git a/lib/gitlab/database/grant.rb b/lib/gitlab/database/grant.rb index d32837f5793..7d334a79009 100644 --- a/lib/gitlab/database/grant.rb +++ b/lib/gitlab/database/grant.rb @@ -2,6 +2,8 @@ module Gitlab module Database # Model that can be used for querying permissions of a SQL user. class Grant < ActiveRecord::Base + include FromUnion + self.table_name = if Database.postgresql? 'information_schema.role_table_grants' @@ -42,9 +44,7 @@ module Gitlab .where("GRANTEE = CONCAT('\\'', REPLACE(CURRENT_USER(), '@', '\\'@\\''), '\\'')") ] - union = SQL::Union.new(queries).to_sql - - Grant.from("(#{union}) privs").any? + Grant.from_union(queries, alias_as: 'privs').any? end end end diff --git a/lib/gitlab/git/diff_stats_collection.rb b/lib/gitlab/git/diff_stats_collection.rb new file mode 100644 index 00000000000..84d9e46f98e --- /dev/null +++ b/lib/gitlab/git/diff_stats_collection.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Gitlab + module Git + class DiffStatsCollection + include Enumerable + + def initialize(diff_stats) + @collection = diff_stats + end + + def each(&block) + @collection.each(&block) + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 1b8d320ff3b..fa22294ac51 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -438,6 +438,16 @@ module Gitlab Gitlab::Git::DiffCollection.new(iterator, options) end + def diff_stats(left_id, right_id) + stats = wrapped_gitaly_errors do + gitaly_commit_client.diff_stats(left_id, right_id) + end + + Gitlab::Git::DiffStatsCollection.new(stats) + rescue CommandError + Gitlab::Git::DiffStatsCollection.new([]) + end + # Returns a RefName for a given SHA def ref_name_for_sha(ref_path, sha) raise ArgumentError, "sha can't be empty" unless sha.present? @@ -581,10 +591,6 @@ module Gitlab end end - def user_to_committer(user) - Gitlab::Git.committer_hash(email: user.email, name: user.name) - end - # Delete the specified branch from the repository def delete_branch(branch_name) wrapped_gitaly_errors do diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index f65d7383dc7..6c95abdcb4b 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -172,6 +172,17 @@ module Gitlab consume_commits_response(response) end + def diff_stats(left_commit_sha, right_commit_sha) + request = Gitaly::DiffStatsRequest.new( + repository: @gitaly_repo, + left_commit_id: left_commit_sha, + right_commit_id: right_commit_sha + ) + + response = GitalyClient.call(@repository.storage, :diff_service, :diff_stats, request, timeout: GitalyClient.medium_timeout) + response.flat_map(&:stats) + end + def find_all_commits(opts = {}) request = Gitaly::FindAllCommitsRequest.new( repository: @gitaly_repo, diff --git a/lib/gitlab/group_hierarchy.rb b/lib/gitlab/group_hierarchy.rb index b74e7b10448..8fbfa1a86bf 100644 --- a/lib/gitlab/group_hierarchy.rb +++ b/lib/gitlab/group_hierarchy.rb @@ -89,14 +89,14 @@ module Gitlab ancestors_table = ancestors.alias_to(groups_table) descendants_table = descendants.alias_to(groups_table) - union = SQL::Union.new([model.unscoped.from(ancestors_table), - model.unscoped.from(descendants_table)]) - relation = model .unscoped .with .recursive(ancestors.to_arel, descendants.to_arel) - .from("(#{union.to_sql}) #{model.table_name}") + .from_union([ + model.unscoped.from(ancestors_table), + model.unscoped.from(descendants_table) + ]) read_only(relation) end diff --git a/lib/gitlab/project_authorizations/with_nested_groups.rb b/lib/gitlab/project_authorizations/with_nested_groups.rb index e3da1634fa5..448c3f3a7d8 100644 --- a/lib/gitlab/project_authorizations/with_nested_groups.rb +++ b/lib/gitlab/project_authorizations/with_nested_groups.rb @@ -49,13 +49,11 @@ module Gitlab .where('p_ns.share_with_group_lock IS FALSE') ] - union = Gitlab::SQL::Union.new(relations) - ProjectAuthorization .unscoped .with .recursive(cte.to_arel) - .select_from_union(union) + .select_from_union(relations) end private diff --git a/lib/gitlab/project_authorizations/without_nested_groups.rb b/lib/gitlab/project_authorizations/without_nested_groups.rb index 7d0c00c7f36..ed2287dcc7e 100644 --- a/lib/gitlab/project_authorizations/without_nested_groups.rb +++ b/lib/gitlab/project_authorizations/without_nested_groups.rb @@ -24,11 +24,9 @@ module Gitlab user.groups.joins(:shared_projects).select_for_project_authorization ] - union = Gitlab::SQL::Union.new(relations) - ProjectAuthorization .unscoped - .select_from_union(union) + .select_from_union(relations) end end end diff --git a/lib/gitlab/user_extractor.rb b/lib/gitlab/user_extractor.rb index 09652a4fb5e..bd0d24e4369 100644 --- a/lib/gitlab/user_extractor.rb +++ b/lib/gitlab/user_extractor.rb @@ -18,7 +18,7 @@ module Gitlab def users return User.none unless @text.present? - @users ||= User.from("(#{union.to_sql}) users") + @users ||= User.from_union(union_relations) end # rubocop: enable CodeReuse/ActiveRecord @@ -43,13 +43,13 @@ module Gitlab private - def union + def union_relations relations = [] relations << User.by_any_email(emails) if emails.any? relations << User.by_username(usernames) if usernames.any? - Gitlab::SQL::Union.new(relations) + relations end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index db372811db3..3e6baeae87a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1353,6 +1353,9 @@ msgstr "" msgid "ClusterIntegration|Certificate Authority bundle (PEM format)" msgstr "" +msgid "ClusterIntegration|Choose which applications to install on your Kubernetes cluster. Helm Tiller is required to install any of the following applications." +msgstr "" + msgid "ClusterIntegration|Choose which of your environments will use this cluster." msgstr "" @@ -1443,9 +1446,6 @@ msgstr "" msgid "ClusterIntegration|Install" msgstr "" -msgid "ClusterIntegration|Install applications on your Kubernetes cluster. Read more about %{helpLink}" -msgstr "" - msgid "ClusterIntegration|Installed" msgstr "" @@ -1653,6 +1653,9 @@ msgstr "" msgid "ClusterIntegration|With a Kubernetes cluster associated to this project, you can use review apps, deploy your applications, run your pipelines, and much more in an easy way." msgstr "" +msgid "ClusterIntegration|You must first install Helm Tiller before installing the applications below" +msgstr "" + msgid "ClusterIntegration|Your account must have %{link_to_kubernetes_engine}" msgstr "" @@ -1671,9 +1674,6 @@ msgstr "" msgid "ClusterIntegration|help page" msgstr "" -msgid "ClusterIntegration|installing applications" -msgstr "" - msgid "ClusterIntegration|meets the requirements" msgstr "" @@ -57,6 +57,7 @@ module QA autoload :Wiki, 'qa/factory/resource/wiki' autoload :File, 'qa/factory/resource/file' autoload :Fork, 'qa/factory/resource/fork' + autoload :SSHKey, 'qa/factory/resource/ssh_key' end module Repository @@ -217,6 +218,7 @@ module QA module Profile autoload :PersonalAccessTokens, 'qa/page/profile/personal_access_tokens' + autoload :SSHKeys, 'qa/page/profile/ssh_keys' end module Issuable diff --git a/qa/qa/factory/repository/project_push.rb b/qa/qa/factory/repository/project_push.rb index 4f78098d348..167f47c9141 100644 --- a/qa/qa/factory/repository/project_push.rb +++ b/qa/qa/factory/repository/project_push.rb @@ -11,7 +11,9 @@ module QA factory.output end - product(:project) { |factory| factory.project } + product :project do |factory| + factory.project + end def initialize @file_name = 'file.txt' @@ -21,8 +23,8 @@ module QA @new_branch = true end - def repository_uri - @repository_uri ||= begin + def repository_http_uri + @repository_http_uri ||= begin project.visit! Page::Project::Show.act do choose_repository_clone_http @@ -30,6 +32,16 @@ module QA end end end + + def repository_ssh_uri + @repository_ssh_uri ||= begin + project.visit! + Page::Project::Show.act do + choose_repository_clone_ssh + repository_location.uri + end + end + end end end end diff --git a/qa/qa/factory/repository/push.rb b/qa/qa/factory/repository/push.rb index 5b7ebf6c41f..6c5088f1da5 100644 --- a/qa/qa/factory/repository/push.rb +++ b/qa/qa/factory/repository/push.rb @@ -5,8 +5,8 @@ module QA module Repository class Push < Factory::Base attr_accessor :file_name, :file_content, :commit_message, - :branch_name, :new_branch, :output, :repository_uri, - :user + :branch_name, :new_branch, :output, :repository_http_uri, + :repository_ssh_uri, :ssh_key, :user attr_writer :remote_branch @@ -16,7 +16,8 @@ module QA @commit_message = "This is a test commit" @branch_name = 'master' @new_branch = true - @repository_uri = "" + @repository_http_uri = "" + @ssh_key = nil end def remote_branch @@ -31,9 +32,14 @@ module QA def fabricate! Git::Repository.perform do |repository| - repository.uri = repository_uri + if ssh_key + repository.uri = repository_ssh_uri + repository.use_ssh_key(ssh_key) + else + repository.uri = repository_http_uri + repository.use_default_credentials + end - repository.use_default_credentials username = 'GitLab QA' email = 'root@gitlab.com' @@ -63,6 +69,8 @@ module QA repository.commit(commit_message) @output = repository.push_changes("#{branch_name}:#{remote_branch}") + + repository.delete_ssh_key end end end diff --git a/qa/qa/factory/repository/wiki_push.rb b/qa/qa/factory/repository/wiki_push.rb index fb7c2bb660d..ecc6cc18c88 100644 --- a/qa/qa/factory/repository/wiki_push.rb +++ b/qa/qa/factory/repository/wiki_push.rb @@ -16,8 +16,8 @@ module QA @new_branch = false end - def repository_uri - @repository_uri ||= begin + def repository_http_uri + @repository_http_uri ||= begin wiki.visit! Page::Project::Wiki::Show.act do go_to_clone_repository diff --git a/qa/qa/factory/resource/project.rb b/qa/qa/factory/resource/project.rb index 7fff22b5468..90db26ab3ab 100644 --- a/qa/qa/factory/resource/project.rb +++ b/qa/qa/factory/resource/project.rb @@ -20,6 +20,13 @@ module QA end end + product :repository_http_location do + Page::Project::Show.act do + choose_repository_clone_http + repository_location + end + end + def initialize @description = 'My awesome project' end diff --git a/qa/qa/factory/resource/ssh_key.rb b/qa/qa/factory/resource/ssh_key.rb new file mode 100644 index 00000000000..6c872f32d16 --- /dev/null +++ b/qa/qa/factory/resource/ssh_key.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module QA + module Factory + module Resource + class SSHKey < Factory::Base + extend Forwardable + + attr_accessor :title + attr_reader :private_key, :public_key, :fingerprint + def_delegators :key, :private_key, :public_key, :fingerprint + + product :private_key do |factory| + factory.private_key + end + + product :title do |factory| + factory.title + end + + product :fingerprint do |factory| + factory.fingerprint + end + + def key + @key ||= Runtime::Key::RSA.new + end + + def fabricate! + Page::Menu::Main.act { go_to_profile_settings } + Page::Menu::Profile.act { click_ssh_keys } + + Page::Profile::SSHKeys.perform do |page| + page.add_key(public_key, title) + end + end + end + end + end +end diff --git a/qa/qa/git/repository.rb b/qa/qa/git/repository.rb index faecacd45ec..14cb8125fdb 100644 --- a/qa/qa/git/repository.rb +++ b/qa/qa/git/repository.rb @@ -7,6 +7,10 @@ module QA class Repository include Scenario::Actable + def initialize + @ssh_cmd = "" + end + def self.perform(*args) Dir.mktmpdir do |dir| Dir.chdir(dir) { super } @@ -38,7 +42,7 @@ module QA end def clone(opts = '') - run_and_redact_credentials("git clone #{opts} #{@uri} ./") + run_and_redact_credentials(build_git_command("git clone #{opts} #{@uri} ./")) end def checkout(branch_name) @@ -58,6 +62,10 @@ module QA `git config user.email #{email}` end + def configure_ssh_command(command) + @ssh_cmd = "GIT_SSH_COMMAND='#{command}'" + end + def commit_file(name, contents, message) add_file(name, contents) commit(message) @@ -74,7 +82,7 @@ module QA end def push_changes(branch = 'master') - output, _ = run_and_redact_credentials("git push #{@uri} #{branch}") + output, _ = run_and_redact_credentials(build_git_command("git push #{@uri} #{branch}")) output end @@ -83,6 +91,31 @@ module QA `git log --oneline`.split("\n") end + def use_ssh_key(key) + @private_key_file = Tempfile.new("id_#{SecureRandom.hex(8)}") + File.binwrite(@private_key_file, key.private_key) + File.chmod(0700, @private_key_file) + + @known_hosts_file = Tempfile.new("known_hosts_#{SecureRandom.hex(8)}") + keyscan_params = ['-H'] + keyscan_params << "-p #{@uri.port}" if @uri.port + keyscan_params << @uri.host + run_and_redact_credentials("ssh-keyscan #{keyscan_params.join(' ')} >> #{@known_hosts_file.path}") + + configure_ssh_command("ssh -i #{@private_key_file.path} -o UserKnownHostsFile=#{@known_hosts_file.path}") + end + + def delete_ssh_key + return unless @private_key_file + + @private_key_file.close(true) + @known_hosts_file.close(true) + end + + def build_git_command(command_str) + [@ssh_cmd, command_str].compact.join(' ') + end + private # Since the remote URL contains the credentials, and git occasionally diff --git a/qa/qa/page/main/login.rb b/qa/qa/page/main/login.rb index 08cf8da34fd..e9e49964e63 100644 --- a/qa/qa/page/main/login.rb +++ b/qa/qa/page/main/login.rb @@ -3,31 +3,31 @@ module QA module Main class Login < Page::Base view 'app/views/devise/passwords/edit.html.haml' do - element :password_field, 'password_field :password' - element :password_confirmation, 'password_field :password_confirmation' - element :change_password_button, 'submit "Change your password"' + element :password_field + element :password_confirmation + element :change_password_button end view 'app/views/devise/sessions/_new_base.html.haml' do - element :login_field, 'text_field :login' - element :password_field, 'password_field :password' - element :sign_in_button, 'submit "Sign in"' + element :login_field + element :password_field + element :sign_in_button end view 'app/views/devise/sessions/_new_ldap.html.haml' do - element :username_field, 'text_field_tag :username' - element :password_field, 'password_field_tag :password' - element :sign_in_button, 'submit_tag "Sign in"' + element :username_field + element :password_field + element :sign_in_button end view 'app/views/devise/shared/_tabs_ldap.html.haml' do - element :ldap_tab, "link_to server['label']" - element :standard_tab, "link_to 'Standard'" + element :ldap_tab + element :standard_tab end view 'app/views/devise/shared/_tabs_normal.html.haml' do - element :sign_in_tab, /nav-link.*login-pane.*Sign in/ - element :register_tab, /nav-link.*register-pane.*Register/ + element :sign_in_tab + element :register_tab end def initialize @@ -66,6 +66,8 @@ module QA end using_wait_time 0 do + set_initial_password_if_present + sign_in_using_gitlab_credentials(admin) end @@ -85,19 +87,19 @@ module QA end def switch_to_sign_in_tab - click_on 'Sign in' + click_element :sign_in_tab end def switch_to_register_tab - click_on 'Register' + click_element :register_tab end def switch_to_ldap_tab - click_on 'LDAP' + click_element :ldap_tab end def switch_to_standard_tab - click_on 'Standard' + click_element :standard_tab end private @@ -105,26 +107,26 @@ module QA def sign_in_using_ldap_credentials switch_to_ldap_tab - fill_in :username, with: Runtime::User.ldap_username - fill_in :password, with: Runtime::User.ldap_password - click_button 'Sign in' + fill_element :username_field, Runtime::User.ldap_username + fill_element :password_field, Runtime::User.ldap_password + click_element :sign_in_button end def sign_in_using_gitlab_credentials(user) switch_to_sign_in_tab unless sign_in_tab? switch_to_standard_tab if ldap_tab? - fill_in :user_login, with: user.username - fill_in :user_password, with: user.password - click_button 'Sign in' + fill_element :login_field, user.username + fill_element :password_field, user.password + click_element :sign_in_button end def set_initial_password_if_present return unless page.has_content?('Change your password') - fill_in :user_password, with: Runtime::User.password - fill_in :user_password_confirmation, with: Runtime::User.password - click_button 'Change your password' + fill_element :password_field, Runtime::User.password + fill_element :password_confirmation, Runtime::User.password + click_element :change_password_button end end end diff --git a/qa/qa/page/menu/profile.rb b/qa/qa/page/menu/profile.rb index 95e88d863e4..7e24fa85c33 100644 --- a/qa/qa/page/menu/profile.rb +++ b/qa/qa/page/menu/profile.rb @@ -6,6 +6,7 @@ module QA element :access_token_link, 'link_to profile_personal_access_tokens_path' element :access_token_title, 'Access Tokens' element :top_level_items, '.sidebar-top-level-items' + element :ssh_keys, 'SSH Keys' end def click_access_tokens @@ -14,6 +15,12 @@ module QA end end + def click_ssh_keys + within_sidebar do + click_link('SSH Keys') + end + end + private def within_sidebar diff --git a/qa/qa/page/menu/side.rb b/qa/qa/page/menu/side.rb index 354ccec2a5a..a1eedfea42e 100644 --- a/qa/qa/page/menu/side.rb +++ b/qa/qa/page/menu/side.rb @@ -6,6 +6,7 @@ module QA element :settings_item element :settings_link, 'link_to edit_project_path' element :repository_link, "title: _('Repository')" + element :link_pipelines element :pipelines_settings_link, "title: _('CI / CD')" element :operations_kubernetes_link, "title: _('Kubernetes')" element :issues_link, /link_to.*shortcuts-issues/ @@ -49,7 +50,7 @@ module QA def click_ci_cd_pipelines within_sidebar do - click_link('CI / CD') + click_element :link_pipelines end end diff --git a/qa/qa/page/profile/ssh_keys.rb b/qa/qa/page/profile/ssh_keys.rb new file mode 100644 index 00000000000..ce1813b14d0 --- /dev/null +++ b/qa/qa/page/profile/ssh_keys.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module QA + module Page + module Profile + class SSHKeys < Page::Base + view 'app/views/profiles/keys/_form.html.haml' do + element :key_title_field + element :key_public_key_field + element :add_key_button + end + + view 'app/views/profiles/keys/_key_details.html.haml' do + element :delete_key_button + end + + def add_key(public_key, title) + fill_element :key_public_key_field, public_key + fill_element :key_title_field, title + + click_element :add_key_button + end + + def remove_key(title) + click_link(title) + + accept_alert do + click_element :delete_key_button + end + end + end + end + end +end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/add_ssh_key_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/add_ssh_key_spec.rb new file mode 100644 index 00000000000..84f663c4866 --- /dev/null +++ b/qa/qa/specs/features/browser_ui/3_create/repository/add_ssh_key_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module QA + context :create do + describe 'SSH keys support' do + let(:key_title) { "key for ssh tests #{Time.now.to_f}" } + + it 'user adds and then removes an SSH key' do + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.act { sign_in_using_credentials } + + key = Factory::Resource::SSHKey.fabricate! do |resource| + resource.title = key_title + end + + expect(page).to have_content("Title: #{key_title}") + expect(page).to have_content(key.fingerprint) + + Page::Menu::Main.act { go_to_profile_settings } + Page::Menu::Profile.act { click_ssh_keys } + + Page::Profile::SSHKeys.perform do |ssh_keys| + ssh_keys.remove_key(key_title) + end + + expect(page).not_to have_content("Title: #{key_title}") + expect(page).not_to have_content(key.fingerprint) + end + end + end +end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/use_ssh_key_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/use_ssh_key_spec.rb new file mode 100644 index 00000000000..7c989bfd8cc --- /dev/null +++ b/qa/qa/specs/features/browser_ui/3_create/repository/use_ssh_key_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module QA + context :create do + describe 'SSH key support' do + # Note: If you run this test against GDK make sure you've enabled sshd + # See: https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/run_qa_against_gdk.md + + let(:key_title) { "key for ssh tests #{Time.now.to_f}" } + + it 'user adds an ssh key and pushes code to the repository' do + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.act { sign_in_using_credentials } + + key = Factory::Resource::SSHKey.fabricate! do |resource| + resource.title = key_title + end + + Factory::Repository::ProjectPush.fabricate! do |push| + push.ssh_key = key + push.file_name = 'README.md' + push.file_content = '# Test Use SSH Key' + push.commit_message = 'Add README.md' + end + + Page::Project::Show.act { wait_for_push } + + expect(page).to have_content('README.md') + expect(page).to have_content('Test Use SSH Key') + + Page::Menu::Main.act { go_to_profile_settings } + Page::Menu::Profile.act { click_ssh_keys } + + Page::Profile::SSHKeys.perform do |ssh_keys| + ssh_keys.remove_key(key_title) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/union.rb b/rubocop/cop/gitlab/union.rb new file mode 100644 index 00000000000..09541d8af3b --- /dev/null +++ b/rubocop/cop/gitlab/union.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true +require_relative '../../spec_helpers' + +module RuboCop + module Cop + module Gitlab + # Cop that disallows the use of `Gitlab::SQL::Union`, in favour of using + # the `FromUnion` module. + class Union < RuboCop::Cop::Cop + include SpecHelpers + + MSG = 'Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly' + + def_node_matcher :raw_union?, <<~PATTERN + (send (const (const (const nil? :Gitlab) :SQL) :Union) :new ...) + PATTERN + + def on_send(node) + return unless raw_union?(node) + return if in_spec?(node) + + add_offense(node, location: :expression) + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 46bd7d3bec6..9d6cc73fc3b 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -3,6 +3,7 @@ require_relative 'cop/gitlab/module_with_instance_variables' require_relative 'cop/gitlab/predicate_memoization' require_relative 'cop/gitlab/httparty' require_relative 'cop/gitlab/finder_with_find_by' +require_relative 'cop/gitlab/union' require_relative 'cop/include_sidekiq_worker' require_relative 'cop/avoid_return_from_blocks' require_relative 'cop/avoid_break_from_strong_memoize' diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 706f801a285..c82c85970dc 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -229,6 +229,114 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do expect(json_response['deployment_status']["environment"]).not_to be_nil end end + + context 'when user can edit runner' do + context 'that belongs to the project' do + let(:runner) { create(:ci_runner, :project, projects: [project]) } + let(:job) { create(:ci_build, :success, pipeline: pipeline, runner: runner) } + + before do + project.add_maintainer(user) + sign_in(user) + + get_show(id: job.id, format: :json) + end + + it 'user can edit runner' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runner']).to have_key('edit_path') + end + end + + context 'that belongs to group' do + let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } + let(:job) { create(:ci_build, :success, pipeline: pipeline, runner: runner) } + let(:user) { create(:user, :admin) } + + before do + project.add_maintainer(user) + sign_in(user) + + get_show(id: job.id, format: :json) + end + + it 'user can not edit runner' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runner']).not_to have_key('edit_path') + end + end + + context 'that belongs to instance' do + let(:runner) { create(:ci_runner, :instance) } + let(:job) { create(:ci_build, :success, pipeline: pipeline, runner: runner) } + let(:user) { create(:user, :admin) } + + before do + project.add_maintainer(user) + sign_in(user) + + get_show(id: job.id, format: :json) + end + + it 'user can not edit runner' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runner']).not_to have_key('edit_path') + end + end + end + + context 'when no runners are available' do + let(:runner) { create(:ci_runner, :instance, active: false) } + let(:job) { create(:ci_build, :pending, pipeline: pipeline, runner: runner) } + + it 'exposes needed information' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runners']['online']).to be false + expect(json_response['runners']['available']).to be false + end + end + + context 'when no runner is online' do + let(:runner) { create(:ci_runner, :instance) } + let(:job) { create(:ci_build, :pending, pipeline: pipeline, runner: runner) } + + it 'exposes needed information' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runners']['online']).to be false + expect(json_response['runners']['available']).to be true + end + end + + context 'settings_path' do + context 'when user is developer' do + it 'settings_path is not available' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runners']).not_to have_key('settings_path') + end + end + + context 'when user is maintainer' do + let(:user) { create(:user, :admin) } + + before do + project.add_maintainer(user) + sign_in(user) + end + + it 'settings_path is available' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runners']['settings_path']).to match(/runners/) + end + end + end end context 'when requesting JSON job is triggered' do diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 59db8cdc34b..a47bd7cafca 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -58,6 +58,14 @@ FactoryBot.define do project_view :readme end + trait :commit_email do + after(:create) do |user, evaluator| + additional = create(:email, :confirmed, user: user, email: "commit-#{user.email}") + + user.update!(commit_email: additional.email) + end + end + factory :omniauth_user do transient do extern_uid '123456' diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 2ba4d4918ff..e2f9e7e9cc5 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -533,7 +533,7 @@ describe 'File blob', :js do expect(page).to have_content('This project is licensed under the MIT License.') # shows a learn more link - expect(page).to have_link('Learn more', 'http://choosealicense.com/licenses/mit/') + expect(page).to have_link('Learn more', href: 'http://choosealicense.com/licenses/mit/') end end end @@ -566,10 +566,10 @@ describe 'File blob', :js do expect(page).to have_content('This project manages its dependencies using RubyGems and defines a gem named activerecord.') # shows a link to the gem - expect(page).to have_link('activerecord', 'https://rubygems.org/gems/activerecord') + expect(page).to have_link('activerecord', href: 'https://rubygems.org/gems/activerecord') # shows a learn more link - expect(page).to have_link('Learn more', 'http://choosealicense.com/licenses/mit/') + expect(page).to have_link('Learn more', href: 'https://rubygems.org/') end end end diff --git a/spec/features/projects/clusters/user_spec.rb b/spec/features/projects/clusters/user_spec.rb index 00941efc8c8..2b4998ed5ac 100644 --- a/spec/features/projects/clusters/user_spec.rb +++ b/spec/features/projects/clusters/user_spec.rb @@ -86,7 +86,6 @@ describe 'User Cluster', :js do context 'when user disables the cluster' do before do page.find(:css, '.js-cluster-enable-toggle-area .js-project-feature-toggle').click - fill_in 'cluster_name', with: 'dev-cluster' page.within('#cluster-integration') { click_button 'Save changes' } end diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb index 26a92f14787..41822babbc9 100644 --- a/spec/features/projects/pipelines/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -9,6 +9,7 @@ describe 'Pipelines', :js do before do sign_in(user) project.add_developer(user) + project.update!(auto_devops_attributes: { enabled: false }) end describe 'GET /:project/pipelines' do @@ -641,6 +642,7 @@ describe 'Pipelines', :js do context 'when user is not logged in' do before do + project.update!(auto_devops_attributes: { enabled: false }) visit project_pipelines_path(project) end diff --git a/spec/features/projects/show/user_sees_setup_shortcut_buttons_spec.rb b/spec/features/projects/show/user_sees_setup_shortcut_buttons_spec.rb index b8326edd4fd..6fe21579e8e 100644 --- a/spec/features/projects/show/user_sees_setup_shortcut_buttons_spec.rb +++ b/spec/features/projects/show/user_sees_setup_shortcut_buttons_spec.rb @@ -28,8 +28,6 @@ describe 'Projects > Show > User sees setup shortcut buttons' do end it '"Auto DevOps enabled" button not linked' do - project.create_auto_devops!(enabled: true) - visit project_path(project) page.within('.project-stats') do @@ -65,19 +63,23 @@ describe 'Projects > Show > User sees setup shortcut buttons' do end describe 'Auto DevOps button' do - it '"Enable Auto DevOps" button linked to settings page' do - page.within('.project-stats') do - expect(page).to have_link('Enable Auto DevOps', href: project_settings_ci_cd_path(project, anchor: 'autodevops-settings')) + context 'when Auto DevOps is enabled' do + it '"Auto DevOps enabled" anchor linked to settings page' do + visit project_path(project) + + page.within('.project-stats') do + expect(page).to have_link('Auto DevOps enabled', href: project_settings_ci_cd_path(project, anchor: 'autodevops-settings')) + end end end - it '"Auto DevOps enabled" anchor linked to settings page' do - project.create_auto_devops!(enabled: true) - - visit project_path(project) + context 'when Auto DevOps is not enabled' do + let(:project) { create(:project, :public, :empty_repo, auto_devops_attributes: { enabled: false }) } - page.within('.project-stats') do - expect(page).to have_link('Auto DevOps enabled', href: project_settings_ci_cd_path(project, anchor: 'autodevops-settings')) + it '"Enable Auto DevOps" button linked to settings page' do + page.within('.project-stats') do + expect(page).to have_link('Enable Auto DevOps', href: project_settings_ci_cd_path(project, anchor: 'autodevops-settings')) + end end end end @@ -113,27 +115,31 @@ describe 'Projects > Show > User sees setup shortcut buttons' do visit project_path(project) end - it 'no Auto DevOps button if can not manage pipelines' do - page.within('.project-stats') do - expect(page).not_to have_link('Enable Auto DevOps') - expect(page).not_to have_link('Auto DevOps enabled') + context 'when Auto DevOps is enabled' do + it '"Auto DevOps enabled" button not linked' do + visit project_path(project) + + page.within('.project-stats') do + expect(page).to have_text('Auto DevOps enabled') + end end end - it '"Auto DevOps enabled" button not linked' do - project.create_auto_devops!(enabled: true) - - visit project_path(project) + context 'when Auto DevOps is not enabled' do + let(:project) { create(:project, :public, :repository, auto_devops_attributes: { enabled: false }) } - page.within('.project-stats') do - expect(page).to have_text('Auto DevOps enabled') + it 'no Auto DevOps button if can not manage pipelines' do + page.within('.project-stats') do + expect(page).not_to have_link('Enable Auto DevOps') + expect(page).not_to have_link('Auto DevOps enabled') + end end - end - it 'no Kubernetes cluster button if can not manage clusters' do - page.within('.project-stats') do - expect(page).not_to have_link('Add Kubernetes cluster') - expect(page).not_to have_link('Kubernetes configured') + it 'no Kubernetes cluster button if can not manage clusters' do + page.within('.project-stats') do + expect(page).not_to have_link('Add Kubernetes cluster') + expect(page).not_to have_link('Kubernetes configured') + end end end end @@ -222,97 +228,105 @@ describe 'Projects > Show > User sees setup shortcut buttons' do end describe 'GitLab CI configuration button' do - it '"Set up CI/CD" button linked to new file populated for a .gitlab-ci.yml' do - visit project_path(project) - - expect(project.repository.gitlab_ci_yml).to be_nil + context 'when Auto DevOps is enabled' do + it 'no "Set up CI/CD" button if the project has Auto DevOps enabled' do + visit project_path(project) - page.within('.project-stats') do - expect(page).to have_link('Set up CI/CD', href: presenter.add_ci_yml_path) + page.within('.project-stats') do + expect(page).not_to have_link('Set up CI/CD') + end end end - it 'no "Set up CI/CD" button if the project already has a .gitlab-ci.yml' do - Files::CreateService.new( - project, - project.creator, - start_branch: 'master', - branch_name: 'master', - commit_message: "Add .gitlab-ci.yml", - file_path: '.gitlab-ci.yml', - file_content: File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) - ).execute + context 'when Auto DevOps is not enabled' do + let(:project) { create(:project, :public, :repository, auto_devops_attributes: { enabled: false }) } - expect(project.repository.gitlab_ci_yml).not_to be_nil + it '"Set up CI/CD" button linked to new file populated for a .gitlab-ci.yml' do + visit project_path(project) - visit project_path(project) + expect(project.repository.gitlab_ci_yml).to be_nil - page.within('.project-stats') do - expect(page).not_to have_link('Set up CI/CD') + page.within('.project-stats') do + expect(page).to have_link('Set up CI/CD', href: presenter.add_ci_yml_path) + end end - end - it 'no "Set up CI/CD" button if the project has Auto DevOps enabled' do - project.create_auto_devops!(enabled: true) + it 'no "Set up CI/CD" button if the project already has a .gitlab-ci.yml' do + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add .gitlab-ci.yml", + file_path: '.gitlab-ci.yml', + file_content: File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) + ).execute - visit project_path(project) + expect(project.repository.gitlab_ci_yml).not_to be_nil - page.within('.project-stats') do - expect(page).not_to have_link('Set up CI/CD') + visit project_path(project) + + page.within('.project-stats') do + expect(page).not_to have_link('Set up CI/CD') + end end end end describe 'Auto DevOps button' do - it '"Enable Auto DevOps" button linked to settings page' do - visit project_path(project) + context 'when Auto DevOps is enabled' do + it '"Auto DevOps enabled" anchor linked to settings page' do + visit project_path(project) - page.within('.project-stats') do - expect(page).to have_link('Enable Auto DevOps', href: project_settings_ci_cd_path(project, anchor: 'autodevops-settings')) + page.within('.project-stats') do + expect(page).to have_link('Auto DevOps enabled', href: project_settings_ci_cd_path(project, anchor: 'autodevops-settings')) + end end end - it '"Enable Auto DevOps" button linked to settings page' do - project.create_auto_devops!(enabled: true) + context 'when Auto DevOps is not enabled' do + let(:project) { create(:project, :public, :repository, auto_devops_attributes: { enabled: false }) } - visit project_path(project) + it '"Enable Auto DevOps" button linked to settings page' do + visit project_path(project) - page.within('.project-stats') do - expect(page).to have_link('Auto DevOps enabled', href: project_settings_ci_cd_path(project, anchor: 'autodevops-settings')) + page.within('.project-stats') do + expect(page).to have_link('Enable Auto DevOps', href: project_settings_ci_cd_path(project, anchor: 'autodevops-settings')) + end end - end - it 'no Auto DevOps button if Auto DevOps callout is shown' do - allow_any_instance_of(AutoDevopsHelper).to receive(:show_auto_devops_callout?).and_return(true) + it 'no Auto DevOps button if Auto DevOps callout is shown' do + allow_any_instance_of(AutoDevopsHelper).to receive(:show_auto_devops_callout?).and_return(true) - visit project_path(project) + visit project_path(project) - expect(page).to have_selector('.js-autodevops-banner') + expect(page).to have_selector('.js-autodevops-banner') - page.within('.project-stats') do - expect(page).not_to have_link('Enable Auto DevOps') - expect(page).not_to have_link('Auto DevOps enabled') + page.within('.project-stats') do + expect(page).not_to have_link('Enable Auto DevOps') + expect(page).not_to have_link('Auto DevOps enabled') + end end - end - it 'no "Enable Auto DevOps" button when .gitlab-ci.yml already exists' do - Files::CreateService.new( - project, - project.creator, - start_branch: 'master', - branch_name: 'master', - commit_message: "Add .gitlab-ci.yml", - file_path: '.gitlab-ci.yml', - file_content: File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) - ).execute + it 'no "Enable Auto DevOps" button when .gitlab-ci.yml already exists' do + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add .gitlab-ci.yml", + file_path: '.gitlab-ci.yml', + file_content: File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) + ).execute - expect(project.repository.gitlab_ci_yml).not_to be_nil + expect(project.repository.gitlab_ci_yml).not_to be_nil - visit project_path(project) + visit project_path(project) - page.within('.project-stats') do - expect(page).not_to have_link('Enable Auto DevOps') - expect(page).not_to have_link('Auto DevOps enabled') + page.within('.project-stats') do + expect(page).not_to have_link('Enable Auto DevOps') + expect(page).not_to have_link('Auto DevOps enabled') + end end end end diff --git a/spec/fixtures/api/schemas/job/job_details.json b/spec/fixtures/api/schemas/job/job_details.json index ab0ade55bbe..cd67d3e4160 100644 --- a/spec/fixtures/api/schemas/job/job_details.json +++ b/spec/fixtures/api/schemas/job/job_details.json @@ -7,6 +7,8 @@ "artifact": { "$ref": "artifact.json" }, "terminal_path": { "type": "string" }, "trigger": { "$ref": "trigger.json" }, - "deployment_status": { "$ref": "deployment_status.json" } + "deployment_status": { "$ref": "deployment_status.json" }, + "runner": { "$ref": "runner.json" }, + "runners": { "type": "runners.json" } } } diff --git a/spec/fixtures/api/schemas/job/runner.json b/spec/fixtures/api/schemas/job/runner.json new file mode 100644 index 00000000000..acfeeeeb808 --- /dev/null +++ b/spec/fixtures/api/schemas/job/runner.json @@ -0,0 +1,17 @@ +{ + "oneOf": [ + { "type": "null" }, + { + "type": "object", + "required": [ + "id", + "description" + ], + "properties": { + "id": { "type": "integer" }, + "description": { "type": "string" }, + "edit_path": { "type": "string" } + } + } + ] +} diff --git a/spec/fixtures/api/schemas/job/runners.json b/spec/fixtures/api/schemas/job/runners.json new file mode 100644 index 00000000000..bebb0c88652 --- /dev/null +++ b/spec/fixtures/api/schemas/job/runners.json @@ -0,0 +1,13 @@ +{ + "type": "object", + "required": [ + "online", + "available" + ], + "properties": { + "online": { "type": "boolean" }, + "available": { "type": "boolean" }, + "settings_path": { "type": "string" } + }, + "additionalProperties": false +} diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 14297a1a544..1238cfbd1e7 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -3,48 +3,66 @@ require 'spec_helper' describe ApplicationHelper do describe 'current_controller?' do - it 'returns true when controller matches argument' do + before do stub_controller_name('foo') + end - expect(helper.current_controller?(:foo)).to eq true + it 'returns true when controller matches argument' do + expect(helper.current_controller?(:foo)).to be_truthy end it 'returns false when controller does not match argument' do - stub_controller_name('foo') - - expect(helper.current_controller?(:bar)).to eq false + expect(helper.current_controller?(:bar)).to be_falsey end it 'takes any number of arguments' do - stub_controller_name('foo') + expect(helper.current_controller?(:baz, :bar)).to be_falsey + expect(helper.current_controller?(:baz, :bar, :foo)).to be_truthy + end + + context 'when namespaced' do + before do + stub_controller_path('bar/foo') + end + + it 'returns true when controller matches argument' do + expect(helper.current_controller?(:foo)).to be_truthy + end - expect(helper.current_controller?(:baz, :bar)).to eq false - expect(helper.current_controller?(:baz, :bar, :foo)).to eq true + it 'returns true when controller and namespace matches argument in path notation' do + expect(helper.current_controller?('bar/foo')).to be_truthy + end + + it 'returns false when namespace doesnt match' do + expect(helper.current_controller?('foo/foo')).to be_falsey + end end def stub_controller_name(value) allow(helper.controller).to receive(:controller_name).and_return(value) end + + def stub_controller_path(value) + allow(helper.controller).to receive(:controller_path).and_return(value) + end end describe 'current_action?' do - it 'returns true when action matches' do + before do stub_action_name('foo') + end - expect(helper.current_action?(:foo)).to eq true + it 'returns true when action matches' do + expect(helper.current_action?(:foo)).to be_truthy end it 'returns false when action does not match' do - stub_action_name('foo') - - expect(helper.current_action?(:bar)).to eq false + expect(helper.current_action?(:bar)).to be_falsey end it 'takes any number of arguments' do - stub_action_name('foo') - - expect(helper.current_action?(:baz, :bar)).to eq false - expect(helper.current_action?(:baz, :bar, :foo)).to eq true + expect(helper.current_action?(:baz, :bar)).to be_falsey + expect(helper.current_action?(:baz, :bar, :foo)).to be_truthy end def stub_action_name(value) @@ -100,8 +118,7 @@ describe ApplicationHelper do end it 'accepts a custom html_class' do - expect(element(html_class: 'custom_class').attr('class')) - .to eq 'js-timeago custom_class' + expect(element(html_class: 'custom_class').attr('class')).to eq 'js-timeago custom_class' end it 'accepts a custom tooltip placement' do @@ -114,6 +131,7 @@ describe ApplicationHelper do it 'add class for the short format' do timeago_element = element(short_format: 'short') + expect(timeago_element.attr('class')).to eq 'js-short-timeago' expect(timeago_element.next_element).to eq nil end @@ -128,11 +146,9 @@ describe ApplicationHelper do context 'when alternate support url is specified' do let(:alternate_url) { 'http://company.example.com/getting-help' } - before do + it 'returns the alternate support url' do stub_application_setting(help_page_support_url: alternate_url) - end - it 'returns the alternate support url' do expect(helper.support_url).to eq(alternate_url) end end @@ -155,9 +171,12 @@ describe ApplicationHelper do describe '#autocomplete_data_sources' do let(:project) { create(:project) } let(:noteable_type) { Issue } + it 'returns paths for autocomplete_sources_controller' do sources = helper.autocomplete_data_sources(project, noteable_type) + expect(sources.keys).to match_array([:members, :issues, :mergeRequests, :labels, :milestones, :commands]) + sources.keys.each do |key| expect(sources[key]).not_to be_nil end diff --git a/spec/helpers/auto_devops_helper_spec.rb b/spec/helpers/auto_devops_helper_spec.rb index 1950c2b129b..75c30dbfe48 100644 --- a/spec/helpers/auto_devops_helper_spec.rb +++ b/spec/helpers/auto_devops_helper_spec.rb @@ -16,7 +16,15 @@ describe AutoDevopsHelper do subject { helper.show_auto_devops_callout?(project) } - context 'when all conditions are met' do + context 'when auto devops is implicitly enabled' do + it { is_expected.to eq(false) } + end + + context 'when auto devops is not implicitly enabled' do + before do + Gitlab::CurrentSettings.update!(auto_devops_enabled: false) + end + it { is_expected.to eq(true) } end diff --git a/spec/helpers/tab_helper_spec.rb b/spec/helpers/tab_helper_spec.rb index b473c0a7416..9abf63d4bd4 100644 --- a/spec/helpers/tab_helper_spec.rb +++ b/spec/helpers/tab_helper_spec.rb @@ -9,31 +9,71 @@ describe TabHelper do allow(self).to receive(:action_name).and_return('foo') end - it "captures block output" do - expect(nav_link { "Testing Blocks" }).to match(/Testing Blocks/) + context 'with the content of the li' do + it "captures block output" do + expect(nav_link { "Testing Blocks" }).to match(/Testing Blocks/) + end end - it "performs checks on the current controller" do - expect(nav_link(controller: :foo)).to match(/<li class="active">/) - expect(nav_link(controller: :bar)).not_to match(/active/) - expect(nav_link(controller: [:foo, :bar])).to match(/active/) - end + context 'with controller param' do + it "performs checks on the current controller" do + expect(nav_link(controller: :foo)).to match(/<li class="active">/) + expect(nav_link(controller: :bar)).not_to match(/active/) + expect(nav_link(controller: [:foo, :bar])).to match(/active/) + end + + context 'with action param' do + it "performs checks on both controller and action when both are present" do + expect(nav_link(controller: :bar, action: :foo)).not_to match(/active/) + expect(nav_link(controller: :foo, action: :bar)).not_to match(/active/) + expect(nav_link(controller: :foo, action: :foo)).to match(/active/) + end + end + + context 'with namespace in path notation' do + before do + allow(controller).to receive(:controller_path).and_return('bar/foo') + end - it "performs checks on the current action" do - expect(nav_link(action: :foo)).to match(/<li class="active">/) - expect(nav_link(action: :bar)).not_to match(/active/) - expect(nav_link(action: [:foo, :bar])).to match(/active/) + it 'performs checks on both controller and namespace' do + expect(nav_link(controller: 'foo/foo')).not_to match(/active/) + expect(nav_link(controller: 'bar/foo')).to match(/active/) + end + + context 'with action param' do + it "performs checks on both namespace, controller and action when they are all present" do + expect(nav_link(controller: 'foo/foo', action: :foo)).not_to match(/active/) + expect(nav_link(controller: 'bar/foo', action: :bar)).not_to match(/active/) + expect(nav_link(controller: 'bar/foo', action: :foo)).to match(/active/) + end + end + end end - it "performs checks on both controller and action when both are present" do - expect(nav_link(controller: :bar, action: :foo)).not_to match(/active/) - expect(nav_link(controller: :foo, action: :bar)).not_to match(/active/) - expect(nav_link(controller: :foo, action: :foo)).to match(/active/) + context 'with action param' do + it "performs checks on the current action" do + expect(nav_link(action: :foo)).to match(/<li class="active">/) + expect(nav_link(action: :bar)).not_to match(/active/) + expect(nav_link(action: [:foo, :bar])).to match(/active/) + end end - it "accepts a path shorthand" do - expect(nav_link(path: 'foo#bar')).not_to match(/active/) - expect(nav_link(path: 'foo#foo')).to match(/active/) + context 'with path param' do + it "accepts a path shorthand" do + expect(nav_link(path: 'foo#bar')).not_to match(/active/) + expect(nav_link(path: 'foo#foo')).to match(/active/) + end + + context 'with namespace' do + before do + allow(controller).to receive(:controller_path).and_return('bar/foo') + end + + it 'accepts a path shorthand with namespace' do + expect(nav_link(path: 'bar/foo#foo')).to match(/active/) + expect(nav_link(path: 'foo/foo#foo')).not_to match(/active/) + end + end end it "passes extra html options to the list element" do diff --git a/spec/javascripts/ide/components/file_row_extra_spec.js b/spec/javascripts/ide/components/file_row_extra_spec.js new file mode 100644 index 00000000000..60dabe28045 --- /dev/null +++ b/spec/javascripts/ide/components/file_row_extra_spec.js @@ -0,0 +1,159 @@ +import Vue from 'vue'; +import { createStore } from '~/ide/stores'; +import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import FileRowExtra from '~/ide/components/file_row_extra.vue'; +import { file, resetStore } from '../helpers'; + +describe('IDE extra file row component', () => { + let Component; + let vm; + let unstagedFilesCount = 0; + let stagedFilesCount = 0; + let changesCount = 0; + + beforeAll(() => { + Component = Vue.extend(FileRowExtra); + }); + + beforeEach(() => { + vm = createComponentWithStore(Component, createStore(), { + file: { + ...file('test'), + }, + mouseOver: false, + }); + + spyOnProperty(vm, 'getUnstagedFilesCountForPath').and.returnValue(() => unstagedFilesCount); + spyOnProperty(vm, 'getStagedFilesCountForPath').and.returnValue(() => stagedFilesCount); + spyOnProperty(vm, 'getChangesInFolder').and.returnValue(() => changesCount); + + vm.$mount(); + }); + + afterEach(() => { + vm.$destroy(); + resetStore(vm.$store); + + stagedFilesCount = 0; + unstagedFilesCount = 0; + changesCount = 0; + }); + + describe('folderChangesTooltip', () => { + it('returns undefined when changes count is 0', () => { + expect(vm.folderChangesTooltip).toBe(undefined); + }); + + it('returns unstaged changes text', () => { + changesCount = 1; + unstagedFilesCount = 1; + + expect(vm.folderChangesTooltip).toBe('1 unstaged change'); + }); + + it('returns staged changes text', () => { + changesCount = 1; + stagedFilesCount = 1; + + expect(vm.folderChangesTooltip).toBe('1 staged change'); + }); + + it('returns staged and unstaged changes text', () => { + changesCount = 1; + stagedFilesCount = 1; + unstagedFilesCount = 1; + + expect(vm.folderChangesTooltip).toBe('1 unstaged and 1 staged changes'); + }); + }); + + describe('show tree changes count', () => { + it('does not show for blobs', () => { + vm.file.type = 'blob'; + + expect(vm.$el.querySelector('.ide-tree-changes')).toBe(null); + }); + + it('does not show when changes count is 0', () => { + vm.file.type = 'tree'; + + expect(vm.$el.querySelector('.ide-tree-changes')).toBe(null); + }); + + it('does not show when tree is open', done => { + vm.file.type = 'tree'; + vm.file.opened = true; + changesCount = 1; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ide-tree-changes')).toBe(null); + + done(); + }); + }); + + it('shows for trees with changes', done => { + vm.file.type = 'tree'; + vm.file.opened = false; + changesCount = 1; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ide-tree-changes')).not.toBe(null); + + done(); + }); + }); + }); + + describe('changes file icon', () => { + it('hides when file is not changed', () => { + expect(vm.$el.querySelector('.ide-file-changed-icon')).toBe(null); + }); + + it('shows when file is changed', done => { + vm.file.changed = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ide-file-changed-icon')).not.toBe(null); + + done(); + }); + }); + + it('shows when file is staged', done => { + vm.file.staged = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ide-file-changed-icon')).not.toBe(null); + + done(); + }); + }); + + it('shows when file is a tempFile', done => { + vm.file.tempFile = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ide-file-changed-icon')).not.toBe(null); + + done(); + }); + }); + }); + + describe('merge request icon', () => { + it('hides when not a merge request change', () => { + expect(vm.$el.querySelector('.ic-git-merge')).toBe(null); + }); + + it('shows when a merge request change', done => { + vm.file.mrChange = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ic-git-merge')).not.toBe(null); + + done(); + }); + }); + }); +}); diff --git a/spec/javascripts/ide/components/repo_file_spec.js b/spec/javascripts/ide/components/repo_file_spec.js deleted file mode 100644 index fc639a672e2..00000000000 --- a/spec/javascripts/ide/components/repo_file_spec.js +++ /dev/null @@ -1,145 +0,0 @@ -import Vue from 'vue'; -import store from '~/ide/stores'; -import repoFile from '~/ide/components/repo_file.vue'; -import router from '~/ide/ide_router'; -import { createComponentWithStore } from '../../helpers/vue_mount_component_helper'; -import { file } from '../helpers'; - -describe('RepoFile', () => { - let vm; - - function createComponent(propsData) { - const RepoFile = Vue.extend(repoFile); - - vm = createComponentWithStore(RepoFile, store, propsData); - - vm.$mount(); - } - - afterEach(() => { - vm.$destroy(); - }); - - it('renders link, icon and name', () => { - createComponent({ - file: file('t4'), - level: 0, - }); - - const name = vm.$el.querySelector('.ide-file-name'); - - expect(name.href).toMatch(''); - expect(name.textContent.trim()).toEqual(vm.file.name); - }); - - it('fires clickFile when the link is clicked', done => { - spyOn(router, 'push'); - createComponent({ - file: file('t3'), - level: 0, - }); - - vm.$el.querySelector('.file-name').click(); - - setTimeout(() => { - expect(router.push).toHaveBeenCalledWith(`/project${vm.file.url}`); - - done(); - }); - }); - - describe('folder', () => { - it('renders changes count inside folder', () => { - const f = { - ...file('folder'), - path: 'testing', - type: 'tree', - branchId: 'master', - projectId: 'project', - }; - - store.state.changedFiles.push({ - ...file('fileName'), - path: 'testing/fileName', - }); - - createComponent({ - file: f, - level: 0, - }); - - const treeChangesEl = vm.$el.querySelector('.ide-tree-changes'); - - expect(treeChangesEl).not.toBeNull(); - expect(treeChangesEl.textContent).toContain('1'); - }); - - it('renders action dropdown', done => { - createComponent({ - file: { - ...file('t4'), - type: 'tree', - branchId: 'master', - projectId: 'project', - }, - level: 0, - }); - - setTimeout(() => { - expect(vm.$el.querySelector('.ide-new-btn')).not.toBeNull(); - - done(); - }); - }); - }); - - describe('locked file', () => { - let f; - - beforeEach(() => { - f = file('locked file'); - f.file_lock = { - user: { - name: 'testuser', - updated_at: new Date(), - }, - }; - - createComponent({ - file: f, - level: 0, - }); - }); - - it('renders lock icon', () => { - expect(vm.$el.querySelector('.file-status-icon')).not.toBeNull(); - }); - - it('renders a tooltip', () => { - expect( - vm.$el.querySelector('.ide-file-name span:nth-child(2)').dataset.originalTitle, - ).toContain('Locked by testuser'); - }); - }); - - it('calls scrollIntoView if made active', done => { - createComponent({ - file: { - ...file(), - type: 'blob', - active: false, - }, - level: 0, - }); - - spyOn(vm, 'scrollIntoView'); - - vm.file.active = true; - - vm.$nextTick(() => { - expect(vm.scrollIntoView).toHaveBeenCalled(); - - done(); - }); - }); -}); diff --git a/spec/javascripts/vue_shared/components/file_row_spec.js b/spec/javascripts/vue_shared/components/file_row_spec.js new file mode 100644 index 00000000000..9914c0b70f3 --- /dev/null +++ b/spec/javascripts/vue_shared/components/file_row_spec.js @@ -0,0 +1,74 @@ +import Vue from 'vue'; +import FileRow from '~/vue_shared/components/file_row.vue'; +import { file } from 'spec/ide/helpers'; +import mountComponent from '../../helpers/vue_mount_component_helper'; + +describe('RepoFile', () => { + let vm; + + function createComponent(propsData) { + const FileRowComponent = Vue.extend(FileRow); + + vm = mountComponent(FileRowComponent, propsData); + } + + afterEach(() => { + vm.$destroy(); + }); + + it('renders name', () => { + createComponent({ + file: file('t4'), + level: 0, + }); + + const name = vm.$el.querySelector('.file-row-name'); + + expect(name.textContent.trim()).toEqual(vm.file.name); + }); + + it('emits toggleTreeOpen on click', () => { + createComponent({ + file: { + ...file('t3'), + type: 'tree', + }, + level: 0, + }); + spyOn(vm, '$emit').and.stub(); + + vm.$el.querySelector('.file-row').click(); + + expect(vm.$emit).toHaveBeenCalledWith('toggleTreeOpen', vm.file.path); + }); + + it('calls scrollIntoView if made active', done => { + createComponent({ + file: { + ...file(), + type: 'blob', + active: false, + }, + level: 0, + }); + + spyOn(vm, 'scrollIntoView').and.stub(); + + vm.file.active = true; + + vm.$nextTick(() => { + expect(vm.scrollIntoView).toHaveBeenCalled(); + + done(); + }); + }); + + it('indents row based on level', () => { + createComponent({ + file: file('t4'), + level: 2, + }); + + expect(vm.$el.querySelector('.file-row-name').style.marginLeft).toBe('32px'); + }); +}); diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index ce4caf0c27d..58c260ee1f0 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1112,6 +1112,32 @@ describe Gitlab::Git::Repository, :seed_helper do end end + describe '#diff_stats' do + let(:left_commit_id) { 'feature' } + let(:right_commit_id) { 'master' } + + it 'returns a DiffStatsCollection' do + collection = repository.diff_stats(left_commit_id, right_commit_id) + + expect(collection).to be_a(Gitlab::Git::DiffStatsCollection) + expect(collection).to be_a(Enumerable) + end + + it 'yields Gitaly::DiffStats objects' do + collection = repository.diff_stats(left_commit_id, right_commit_id) + + expect(collection.to_a).to all(be_a(Gitaly::DiffStats)) + end + + it 'returns no Gitaly::DiffStats when SHAs are invalid' do + collection = repository.diff_stats('foo', 'bar') + + expect(collection).to be_a(Gitlab::Git::DiffStatsCollection) + expect(collection).to be_a(Enumerable) + expect(collection.to_a).to be_empty + end + end + describe "#ls_files" do let(:master_file_paths) { repository.ls_files("master") } let(:utf8_file_paths) { repository.ls_files("ls-files-utf8") } diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index bcdf12a00a0..d7bd757149d 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -118,6 +118,22 @@ describe Gitlab::GitalyClient::CommitService do end end + describe '#diff_stats' do + let(:left_commit_id) { 'master' } + let(:right_commit_id) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } + + it 'sends an RPC request' do + request = Gitaly::DiffStatsRequest.new(repository: repository_message, + left_commit_id: left_commit_id, + right_commit_id: right_commit_id) + + expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:diff_stats) + .with(request, kind_of(Hash)).and_return([]) + + described_class.new(repository).diff_stats(left_commit_id, right_commit_id) + end + end + describe '#tree_entries' do let(:path) { '/' } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 2216705c032..4755702c0e9 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1151,7 +1151,11 @@ describe Ci::Pipeline, :mailer do end describe '#set_config_source' do - context 'when pipelines does not contain needed data' do + context 'when pipelines does not contain needed data and auto devops is disabled' do + before do + stub_application_setting(auto_devops_enabled: false) + end + it 'defines source to be unknown' do pipeline.set_config_source @@ -1196,7 +1200,6 @@ describe Ci::Pipeline, :mailer do context 'auto devops enabled' do before do - stub_application_setting(auto_devops_enabled: true) allow(project).to receive(:ci_config_path) { 'custom' } end diff --git a/spec/models/concerns/from_union_spec.rb b/spec/models/concerns/from_union_spec.rb new file mode 100644 index 00000000000..ee427a667c6 --- /dev/null +++ b/spec/models/concerns/from_union_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe FromUnion do + describe '.from_union' do + let(:model) do + Class.new(ActiveRecord::Base) do + self.table_name = 'users' + + include FromUnion + end + end + + it 'selects from the results of the UNION' do + query = model.from_union([model.where(id: 1), model.where(id: 2)]) + + expect(query.to_sql).to match(/FROM \(SELECT.+UNION.+SELECT.+\) users/m) + end + + it 'supports the use of a custom alias for the sub query' do + query = model.from_union( + [model.where(id: 1), model.where(id: 2)], + alias_as: 'kittens' + ) + + expect(query.to_sql).to match(/FROM \(SELECT.+UNION.+SELECT.+\) kittens/m) + end + + it 'supports keeping duplicate rows' do + query = model.from_union( + [model.where(id: 1), model.where(id: 2)], + remove_duplicates: false + ) + + expect(query.to_sql) + .to match(/FROM \(SELECT.+UNION ALL.+SELECT.+\) users/m) + end + end +end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 55b984faecf..27d4e622710 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -95,6 +95,24 @@ describe Milestone do end end + describe '.order_by_name_asc' do + it 'sorts by name ascending' do + milestone1 = create(:milestone, title: 'Foo') + milestone2 = create(:milestone, title: 'Bar') + + expect(described_class.order_by_name_asc).to eq([milestone2, milestone1]) + end + end + + describe '.reorder_by_due_date_asc' do + it 'reorders the input relation' do + milestone1 = create(:milestone, due_date: Date.new(2018, 9, 30)) + milestone2 = create(:milestone, due_date: Date.new(2018, 10, 20)) + + expect(described_class.reorder_by_due_date_asc).to eq([milestone1, milestone2]) + end + end + describe "#percent_complete" do it "does not count open issues" do milestone.issues << issue diff --git a/spec/models/project_services/chat_message/merge_message_spec.rb b/spec/models/project_services/chat_message/merge_message_spec.rb index 184a07ae0f9..96496295825 100644 --- a/spec/models/project_services/chat_message/merge_message_spec.rb +++ b/spec/models/project_services/chat_message/merge_message_spec.rb @@ -33,7 +33,7 @@ describe ChatMessage::MergeMessage do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) opened <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*') + 'Test User (test.user) opened <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>') expect(subject.attachments).to be_empty end end @@ -44,7 +44,7 @@ describe ChatMessage::MergeMessage do end it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) closed <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*') + 'Test User (test.user) closed <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>') expect(subject.attachments).to be_empty end end @@ -58,7 +58,7 @@ describe ChatMessage::MergeMessage do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) opened [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*') + 'Test User (test.user) opened [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com)') expect(subject.attachments).to be_empty expect(subject.activity).to eq({ title: 'Merge Request opened by Test User (test.user)', @@ -76,7 +76,7 @@ describe ChatMessage::MergeMessage do it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'Test User (test.user) closed [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*') + 'Test User (test.user) closed [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com)') expect(subject.attachments).to be_empty expect(subject.activity).to eq({ title: 'Merge Request closed by Test User (test.user)', diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index dfe2de71a76..909012b7789 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3229,17 +3229,17 @@ describe Project do expect(repository).to receive(:gitlab_ci_yml) { nil } end - it "CI is not available" do - expect(project).not_to have_ci + it "CI is available" do + expect(project).to have_ci end - context 'when auto devops is enabled' do + context 'when auto devops is disabled' do before do - stub_application_setting(auto_devops_enabled: true) + stub_application_setting(auto_devops_enabled: false) end - it "CI is available" do - expect(project).to have_ci + it "CI is not available" do + expect(project).not_to have_ci end end end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 528f5b610d7..f38fc191943 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -2,12 +2,13 @@ require "spec_helper" describe ProjectWiki do - let(:project) { create(:project, :wiki_repo) } + let(:user) { create(:user, :commit_email) } + let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } let(:repository) { project.repository } - let(:user) { project.owner } let(:gitlab_shell) { Gitlab::Shell.new } let(:project_wiki) { described_class.new(project, user) } let(:raw_repository) { Gitlab::Git::Repository.new(project.repository_storage, subject.disk_path + '.git', 'foo') } + let(:commit) { project_wiki.repository.head_commit } subject { project_wiki } @@ -276,6 +277,14 @@ describe ProjectWiki do expect(subject.pages.first.page.version.message).to eq("commit message") end + it 'sets the correct commit email' do + subject.create_page('test page', 'content') + + expect(user.commit_email).not_to eq(user.email) + expect(commit.author_email).to eq(user.commit_email) + expect(commit.committer_email).to eq(user.commit_email) + end + it 'updates project activity' do subject.create_page('Test Page', 'This is content') @@ -320,6 +329,12 @@ describe ProjectWiki do expect(@page.version.message).to eq("updated page") end + it 'sets the correct commit email' do + expect(user.commit_email).not_to eq(user.email) + expect(commit.author_email).to eq(user.commit_email) + expect(commit.committer_email).to eq(user.commit_email) + end + it 'updates project activity' do subject.update_page( @gitlab_git_wiki_page, @@ -347,6 +362,14 @@ describe ProjectWiki do expect(subject.pages.count).to eq(0) end + it 'sets the correct commit email' do + subject.delete_page(@page) + + expect(user.commit_email).not_to eq(user.email) + expect(commit.author_email).to eq(user.commit_email) + expect(commit.committer_email).to eq(user.commit_email) + end + it 'updates project activity' do subject.delete_page(@page) @@ -420,7 +443,7 @@ describe ProjectWiki do end def commit_details - Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.email, "test commit") + Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.commit_email, "test commit") end def create_page(name, content) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 27aec49e348..99d17f563d9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2543,6 +2543,34 @@ describe User do end end + describe '#assigned_open_merge_requests_count' do + it 'returns number of open merge requests from non-archived projects' do + user = create(:user) + project = create(:project, :public) + archived_project = create(:project, :public, :archived) + + create(:merge_request, source_project: project, author: user, assignee: user) + create(:merge_request, :closed, source_project: project, author: user, assignee: user) + create(:merge_request, source_project: archived_project, author: user, assignee: user) + + expect(user.assigned_open_merge_requests_count(force: true)).to eq 1 + end + end + + describe '#assigned_open_issues_count' do + it 'returns number of open issues from non-archived projects' do + user = create(:user) + project = create(:project, :public) + archived_project = create(:project, :public, :archived) + + create(:issue, project: project, author: user, assignees: [user]) + create(:issue, :closed, project: project, author: user, assignees: [user]) + create(:issue, project: archived_project, author: user, assignees: [user]) + + expect(user.assigned_open_issues_count(force: true)).to eq 1 + end + end + describe '#personal_projects_count' do it 'returns the number of personal projects using a single query' do user = build(:user) diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb index 342a97b6a69..f0e1992bccd 100644 --- a/spec/requests/api/pipelines_spec.rb +++ b/spec/requests/api/pipelines_spec.rb @@ -370,12 +370,18 @@ describe API::Pipelines do end context 'without gitlab-ci.yml' do - it 'fails to create pipeline' do - post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch + context 'without auto devops enabled' do + before do + project.update!(auto_devops_attributes: { enabled: false }) + end - expect(response).to have_gitlab_http_status(400) - expect(json_response['message']['base'].first).to eq 'Missing .gitlab-ci.yml file' - expect(json_response).not_to be_an Array + it 'fails to create pipeline' do + post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']['base'].first).to eq 'Missing .gitlab-ci.yml file' + expect(json_response).not_to be_an Array + end end end end diff --git a/spec/rubocop/cop/gitlab/union_spec.rb b/spec/rubocop/cop/gitlab/union_spec.rb new file mode 100644 index 00000000000..5b06f30b25f --- /dev/null +++ b/spec/rubocop/cop/gitlab/union_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/union' + +describe RuboCop::Cop::Gitlab::Union do + include CopHelper + + subject(:cop) { described_class.new } + + it 'flags the use of Gitlab::SQL::Union.new' do + expect_offense(<<~SOURCE) + Gitlab::SQL::Union.new([foo]) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly + SOURCE + end + + it 'does not flag the use of Gitlab::SQL::Union in a spec' do + allow(cop).to receive(:in_spec?).and_return(true) + + expect_no_offenses('Gitlab::SQL::Union.new([foo])') + end +end diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb index 27a7bf0e605..010679b5360 100644 --- a/spec/services/boards/issues/list_service_spec.rb +++ b/spec/services/boards/issues/list_service_spec.rb @@ -24,7 +24,7 @@ describe Boards::Issues::ListService do let!(:opened_issue1) { create(:labeled_issue, project: project, milestone: m1, title: 'Issue 1', labels: [bug]) } let!(:opened_issue2) { create(:labeled_issue, project: project, milestone: m2, title: 'Issue 2', labels: [p2]) } - let!(:reopened_issue1) { create(:issue, :opened, project: project, title: 'Issue 3' ) } + let!(:reopened_issue1) { create(:issue, :opened, project: project, title: 'Reopened Issue 1' ) } let!(:list1_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [p2, development]) } let!(:list1_issue2) { create(:labeled_issue, project: project, milestone: m2, labels: [development]) } @@ -44,12 +44,19 @@ describe Boards::Issues::ListService do end it_behaves_like 'issues list service' + + context 'when project is archived' do + let(:project) { create(:project, :archived) } + + it_behaves_like 'issues list service' + end end context 'when parent is a group' do let(:user) { create(:user) } let(:project) { create(:project, :empty_repo, namespace: group) } let(:project1) { create(:project, :empty_repo, namespace: group) } + let(:project_archived) { create(:project, :empty_repo, :archived, namespace: group) } let(:m1) { create(:milestone, group: group) } let(:m2) { create(:milestone, group: group) } @@ -77,7 +84,8 @@ describe Boards::Issues::ListService do let!(:opened_issue1) { create(:labeled_issue, project: project, milestone: m1, title: 'Issue 1', labels: [bug]) } let!(:opened_issue2) { create(:labeled_issue, project: project, milestone: m2, title: 'Issue 2', labels: [p2, p2_project]) } - let!(:reopened_issue1) { create(:issue, state: 'opened', project: project, title: 'Issue 3', closed_at: Time.now ) } + let!(:opened_issue3) { create(:labeled_issue, project: project_archived, milestone: m1, title: 'Issue 3', labels: [bug]) } + let!(:reopened_issue1) { create(:issue, state: 'opened', project: project, title: 'Reopened Issue 1', closed_at: Time.now ) } let!(:list1_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [p2, p2_project, development]) } let!(:list1_issue2) { create(:labeled_issue, project: project, milestone: m2, labels: [development]) } diff --git a/spec/services/files/create_service_spec.rb b/spec/services/files/create_service_spec.rb index 30d94e4318d..751b7160276 100644 --- a/spec/services/files/create_service_spec.rb +++ b/spec/services/files/create_service_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Files::CreateService do let(:project) { create(:project, :repository) } let(:repository) { project.repository } - let(:user) { create(:user) } + let(:user) { create(:user, :commit_email) } let(:file_content) { 'Test file content' } let(:branch_name) { project.default_branch } let(:start_branch) { branch_name } @@ -20,6 +20,8 @@ describe Files::CreateService do } end + let(:commit) { repository.head_commit } + subject { described_class.new(project, user, commit_params) } before do @@ -75,4 +77,16 @@ describe Files::CreateService do end end end + + context 'commit attribute' do + let(:file_path) { 'test-commit-attributes.txt' } + + it 'uses the commit email' do + subject.execute + + expect(user.commit_email).not_to eq(user.email) + expect(commit.author_email).to eq(user.commit_email) + expect(commit.committer_email).to eq(user.commit_email) + end + end end diff --git a/spec/services/files/delete_service_spec.rb b/spec/services/files/delete_service_spec.rb index 73566afe8c8..309802ce733 100644 --- a/spec/services/files/delete_service_spec.rb +++ b/spec/services/files/delete_service_spec.rb @@ -4,10 +4,11 @@ describe Files::DeleteService do subject { described_class.new(project, user, commit_params) } let(:project) { create(:project, :repository) } - let(:user) { create(:user) } + let(:user) { create(:user, :commit_email) } let(:file_path) { 'files/ruby/popen.rb' } let(:branch_name) { project.default_branch } let(:last_commit_sha) { nil } + let(:commit) { project.repository.head_commit } let(:commit_params) do { @@ -34,6 +35,14 @@ describe Files::DeleteService do expect(blob).to be_nil end + + it 'uses the commit email' do + subject.execute + + expect(user.commit_email).not_to eq(user.email) + expect(commit.author_email).to eq(user.commit_email) + expect(commit.committer_email).to eq(user.commit_email) + end end before do diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb index e01fe487ffa..23db35c2418 100644 --- a/spec/services/files/update_service_spec.rb +++ b/spec/services/files/update_service_spec.rb @@ -4,11 +4,12 @@ describe Files::UpdateService do subject { described_class.new(project, user, commit_params) } let(:project) { create(:project, :repository) } - let(:user) { create(:user) } + let(:user) { create(:user, :commit_email) } let(:file_path) { 'files/ruby/popen.rb' } let(:new_contents) { 'New Content' } let(:branch_name) { project.default_branch } let(:last_commit_sha) { nil } + let(:commit) { project.repository.commit } let(:commit_params) do { @@ -54,6 +55,14 @@ describe Files::UpdateService do expect(results.data).to eq(new_contents) end + + it 'uses the commit email' do + subject.execute + + expect(user.commit_email).not_to eq(user.email) + expect(commit.author_email).to eq(user.commit_email) + expect(commit.committer_email).to eq(user.commit_email) + end end context "when the last_commit_sha is not supplied" do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index d4528256640..45ef26aebbd 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -246,13 +246,15 @@ describe GitPushService, services: true do describe 'system hooks' do let!(:push_data) { push_data_from_service(project, user, oldrev, newrev, ref) } - let(:system_hooks_service) { SystemHooksService.new } + let!(:system_hooks_service) { SystemHooksService.new } it "sends a system hook after pushing a branch" do - expect(SystemHooksService).to receive(:new).and_return(system_hooks_service) - expect(system_hooks_service).to receive(:execute_hooks).with(push_data, :push_hooks) + allow(SystemHooksService).to receive(:new).and_return(system_hooks_service) + allow(system_hooks_service).to receive(:execute_hooks) execute_service(project, user, oldrev, newrev, ref) + + expect(system_hooks_service).to have_received(:execute_hooks).with(push_data, :push_hooks) end end end diff --git a/spec/support/helpers/markdown_feature.rb b/spec/support/helpers/markdown_feature.rb index 346f5b1cc4d..96401379cf0 100644 --- a/spec/support/helpers/markdown_feature.rb +++ b/spec/support/helpers/markdown_feature.rb @@ -10,6 +10,12 @@ class MarkdownFeature include FactoryBot::Syntax::Methods + attr_reader :fixture_path + + def initialize(fixture_path = Rails.root.join('spec/fixtures/markdown.md.erb')) + @fixture_path = fixture_path + end + def user @user ||= create(:user) end @@ -122,7 +128,7 @@ class MarkdownFeature end def raw_markdown - markdown = File.read(Rails.root.join('spec/fixtures/markdown.md.erb')) + markdown = File.read(fixture_path) ERB.new(markdown).result(binding) end end diff --git a/spec/validators/url_validator_spec.rb b/spec/validators/url_validator_spec.rb index 93fe013d11c..ab6100509a6 100644 --- a/spec/validators/url_validator_spec.rb +++ b/spec/validators/url_validator_spec.rb @@ -24,6 +24,21 @@ describe UrlValidator do expect(badge.errors.empty?).to be true end + + it 'strips urls' do + badge.link_url = "\n\r\n\nhttps://127.0.0.1\r\n\r\n\n\n\n" + + # It's unusual for a validator to modify its arguments. Some extensions, + # such as attr_encrypted, freeze the string to signal that modifications + # will not be persisted, so freeze this string to ensure the scheme is + # compatible with them. + badge.link_url.freeze + + subject + + expect(badge.errors).to be_empty + expect(badge.link_url).to eq('https://127.0.0.1') + end end context 'when allow_localhost is set to false' do diff --git a/spec/views/help/index.html.haml_spec.rb b/spec/views/help/index.html.haml_spec.rb index 836d452304c..4b4de540d9e 100644 --- a/spec/views/help/index.html.haml_spec.rb +++ b/spec/views/help/index.html.haml_spec.rb @@ -21,7 +21,7 @@ describe 'help/index' do render expect(rendered).to match '8.0.2' - expect(rendered).to have_link('abcdefg', 'https://gitlab.com/gitlab-org/gitlab-ce/commits/abcdefg') + expect(rendered).to have_link('abcdefg', href: 'https://gitlab.com/gitlab-org/gitlab-ce/commits/abcdefg') end end @@ -29,7 +29,7 @@ describe 'help/index' do it 'is visible to guests' do render - expect(rendered).to have_link(nil, help_instance_configuration_url) + expect(rendered).to have_link(nil, href: help_instance_configuration_url) end end |
