Add latest changes from gitlab-org/gitlab@master

6 files changed, 189 insertions, 42 deletions

diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue
index 32fe841f16e..316408adfb2 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue
@@ -65,12 +65,6 @@ export default {
     modalActionText() {
       return this.variableBeingEdited ? __('Update variable') : __('Add variable');
     },
-    primaryAction() {
-      return {
-        text: this.modalActionText,
-        attributes: { variant: 'success', disabled: !this.canSubmit },
-      };
-    },
     maskedFeedback() {
       return __('This variable can not be masked');
     },
@@ -120,6 +114,8 @@ export default {
     ref="modal"
     :modal-id="$options.modalId"
     :title="modalActionText"
+    static
+    lazy
     @hidden="resetModalHandler"
   >
     <form>
@@ -127,7 +123,7 @@ export default {
         <gl-form-input
           id="ci-variable-key"
           v-model="variableData.key"
-          data-qa-selector="variable_key"
+          data-qa-selector="ci_variable_key_field"
         />
       </gl-form-group>
 
@@ -142,7 +138,7 @@ export default {
           v-model="variableData.secret_value"
           rows="3"
           max-rows="6"
-          data-qa-selector="variable_value"
+          data-qa-selector="ci_variable_value_field"
         />
       </gl-form-group>
 
@@ -189,7 +185,7 @@ export default {
         <gl-form-checkbox
           ref="masked-ci-variable"
           v-model="variableData.masked"
-          data-qa-selector="variable_masked"
+          data-qa-selector="ci_variable_masked_checkbox"
         >
           {{ __('Mask variable') }}
           <gl-link href="/help/ci/variables/README#masked-variables">
@@ -218,6 +214,7 @@ export default {
         ref="deleteCiVariable"
         category="secondary"
         variant="danger"
+        data-qa-selector="ci_variable_delete_button"
         @click="deleteVarAndClose"
         >{{ __('Delete variable') }}</gl-deprecated-button
       >
@@ -225,6 +222,7 @@ export default {
         ref="updateOrAddVariable"
         :disabled="!canSubmit"
         variant="success"
+        data-qa-selector="ci_variable_save_button"
         @click="updateOrAddVariable"
         >{{ modalActionText }}
       </gl-deprecated-button>
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue
index b374d950c1f..7eb791f97e4 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue
@@ -26,7 +26,6 @@ export default {
     {
       key: 'value',
       label: s__('CiVariables|Value'),
-      tdClass: 'qa-ci-variable-input-value',
       customStyle: { width: '40%' },
     },
     {
@@ -89,6 +88,7 @@ export default {
       :fields="fields"
       :items="variables"
       tbody-tr-class="js-ci-variable-row"
+      data-qa-selector="ci_variable_table_content"
       sort-by="key"
       sort-direction="asc"
       stacked="lg"
@@ -150,6 +150,7 @@ export default {
         <gl-deprecated-button
           ref="edit-ci-variable"
           v-gl-modal-directive="$options.modalId"
+          data-qa-selector="edit_ci_variable_button"
           @click="editVariable(item)"
         >
           <gl-icon :size="$options.iconSize" name="pencil" />
@@ -168,7 +169,7 @@ export default {
       <gl-deprecated-button
         v-if="tableIsNotEmpty"
         ref="secret-value-reveal-button"
-        data-qa-selector="reveal_ci_variable_value"
+        data-qa-selector="reveal_ci_variable_value_button"
         class="append-right-8"
         @click="toggleValues(!valuesHidden)"
         >{{ valuesButtonText }}</gl-deprecated-button
@@ -176,7 +177,7 @@ export default {
       <gl-deprecated-button
         ref="add-ci-variable"
         v-gl-modal-directive="$options.modalId"
-        data-qa-selector="add_ci_variable"
+        data-qa-selector="add_ci_variable_button"
         variant="success"
         >{{ __('Add Variable') }}</gl-deprecated-button
       >
diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js
index f857e618d89..86714471823 100644
--- a/app/assets/javascripts/lib/utils/text_utility.js
+++ b/app/assets/javascripts/lib/utils/text_utility.js
@@ -232,3 +232,11 @@ export const truncateNamespace = (string = '') => {
 
   return namespace;
 };
+
+/**
+ * Tests that the input is a String and has at least
+ * one non-whitespace character
+ * @param {String} obj The object to test
+ * @returns {Boolean}
+ */
+export const hasContent = obj => isString(obj) && obj.trim() !== '';
diff --git a/app/assets/javascripts/releases/components/app_edit.vue b/app/assets/javascripts/releases/components/app_edit.vue
index 06e388002e4..df356c18417 100644
--- a/app/assets/javascripts/releases/components/app_edit.vue
+++ b/app/assets/javascripts/releases/components/app_edit.vue
@@ -1,6 +1,6 @@
 <script>
-import { mapState, mapActions } from 'vuex';
-import { GlDeprecatedButton, GlLink, GlFormInput, GlFormGroup } from '@gitlab/ui';
+import { mapState, mapActions, mapGetters } from 'vuex';
+import { GlNewButton, GlFormInput, GlFormGroup } from '@gitlab/ui';
 import { escape as esc } from 'lodash';
 import { __, sprintf } from '~/locale';
 import MarkdownField from '~/vue_shared/components/markdown/field.vue';
@@ -15,8 +15,7 @@ export default {
   components: {
     GlFormInput,
     GlFormGroup,
-    GlDeprecatedButton,
-    GlLink,
+    GlNewButton,
     MarkdownField,
     AssetLinksForm,
   },
@@ -27,12 +26,14 @@ export default {
   computed: {
     ...mapState('detail', [
       'isFetchingRelease',
+      'isUpdatingRelease',
       'fetchError',
       'markdownDocsPath',
       'markdownPreviewPath',
       'releasesPagePath',
       'updateReleaseApiDocsPath',
     ]),
+    ...mapGetters('detail', ['isValid']),
     showForm() {
       return !this.isFetchingRelease && !this.fetchError;
     },
@@ -87,6 +88,9 @@ export default {
     showAssetLinksForm() {
       return this.glFeatures.releaseAssetLinkEditing;
     },
+    isSaveChangesDisabled() {
+      return this.isUpdatingRelease || !this.isValid;
+    },
   },
   created() {
     this.fetchRelease();
@@ -163,17 +167,19 @@ export default {
       <asset-links-form v-if="showAssetLinksForm" />
 
       <div class="d-flex pt-3">
-        <gl-deprecated-button
-          class="mr-auto js-submit-button"
+        <gl-new-button
+          class="mr-auto js-no-auto-disable"
+          category="primary"
           variant="success"
           type="submit"
           :aria-label="__('Save changes')"
+          :disabled="isSaveChangesDisabled"
         >
           {{ __('Save changes') }}
-        </gl-deprecated-button>
-        <gl-link :href="cancelPath" class="js-cancel-button btn btn-default">
+        </gl-new-button>
+        <gl-new-button :href="cancelPath" class="js-cancel-button">
           {{ __('Cancel') }}
-        </gl-link>
+        </gl-new-button>
       </div>
     </form>
   </div>
diff --git a/app/assets/javascripts/releases/components/asset_links_form.vue b/app/assets/javascripts/releases/components/asset_links_form.vue
index 7299fd24ec5..6ca700c2b30 100644
--- a/app/assets/javascripts/releases/components/asset_links_form.vue
+++ b/app/assets/javascripts/releases/components/asset_links_form.vue
@@ -1,10 +1,10 @@
 <script>
-import { mapState, mapActions } from 'vuex';
+import { mapState, mapActions, mapGetters } from 'vuex';
 import {
   GlSprintf,
   GlLink,
   GlFormGroup,
-  GlDeprecatedButton,
+  GlNewButton,
   GlIcon,
   GlTooltipDirective,
   GlFormInput,
@@ -12,13 +12,14 @@ import {
 
 export default {
   name: 'AssetLinksForm',
-  components: { GlSprintf, GlLink, GlFormGroup, GlDeprecatedButton, GlIcon, GlFormInput },
+  components: { GlSprintf, GlLink, GlFormGroup, GlNewButton, GlIcon, GlFormInput },
   directives: { GlTooltip: GlTooltipDirective },
   computed: {
     ...mapState('detail', ['release', 'releaseAssetsDocsPath']),
+    ...mapGetters('detail', ['validationErrors']),
   },
   created() {
-    this.addEmptyAssetLink();
+    this.ensureAtLeastOneLink();
   },
   methods: {
     ...mapActions('detail', [
@@ -32,6 +33,7 @@ export default {
     },
     onRemoveClicked(linkId) {
       this.removeAssetLink(linkId);
+      this.ensureAtLeastOneLink();
     },
     onUrlInput(linkIdToUpdate, newUrl) {
       this.updateAssetLinkUrl({ linkIdToUpdate, newUrl });
@@ -39,6 +41,37 @@ export default {
     onLinkTitleInput(linkIdToUpdate, newName) {
       this.updateAssetLinkName({ linkIdToUpdate, newName });
     },
+    hasDuplicateUrl(link) {
+      return Boolean(this.getLinkErrors(link).isDuplicate);
+    },
+    hasBadFormat(link) {
+      return Boolean(this.getLinkErrors(link).isBadFormat);
+    },
+    hasEmptyUrl(link) {
+      return Boolean(this.getLinkErrors(link).isUrlEmpty);
+    },
+    hasEmptyName(link) {
+      return Boolean(this.getLinkErrors(link).isNameEmpty);
+    },
+    getLinkErrors(link) {
+      return this.validationErrors.assets.links[link.id] || {};
+    },
+    isUrlValid(link) {
+      return !this.hasDuplicateUrl(link) && !this.hasBadFormat(link) && !this.hasEmptyUrl(link);
+    },
+    isNameValid(link) {
+      return !this.hasEmptyName(link);
+    },
+
+    /**
+     * Make sure the form is never completely empty by adding an
+     * empty row if the form contains 0 links
+     */
+    ensureAtLeastOneLink() {
+      if (this.release.assets.links.length === 0) {
+        this.addEmptyAssetLink();
+      }
+    },
   },
 };
 </script>
@@ -69,60 +102,93 @@ export default {
     <p>
       {{
         __(
-          'Point to any links you like: documentation, built binaries, or other related materials. These can be internal or external links from your GitLab instance.',
+          'Point to any links you like: documentation, built binaries, or other related materials. These can be internal or external links from your GitLab instance. Duplicate URLs are not allowed.',
         )
       }}
     </p>
     <div
       v-for="(link, index) in release.assets.links"
       :key="link.id"
-      class="d-flex flex-column flex-sm-row align-items-stretch align-items-sm-end"
+      class="row flex-column flex-sm-row align-items-stretch align-items-sm-start"
     >
       <gl-form-group
-        class="url-field form-group flex-grow-1 mr-sm-4"
+        class="url-field form-group col"
         :label="__('URL')"
         :label-for="`asset-url-${index}`"
       >
         <gl-form-input
           :id="`asset-url-${index}`"
+          ref="urlInput"
           :value="link.url"
           type="text"
           class="form-control"
+          :state="isUrlValid(link)"
           @change="onUrlInput(link.id, $event)"
         />
+        <template #invalid-feedback>
+          <span v-if="hasEmptyUrl(link)" class="invalid-feedback d-inline">
+            {{ __('URL is required') }}
+          </span>
+          <span v-else-if="hasBadFormat(link)" class="invalid-feedback d-inline">
+            <gl-sprintf
+              :message="
+                __(
+                  'URL must start with %{codeStart}http://%{codeEnd}, %{codeStart}https://%{codeEnd}, or %{codeStart}ftp://%{codeEnd}',
+                )
+              "
+            >
+              <template #code="{ content }">
+                <code>{{ content }}</code>
+              </template>
+            </gl-sprintf>
+          </span>
+          <span v-else-if="hasDuplicateUrl(link)" class="invalid-feedback d-inline">
+            {{ __('This URL is already used for another link; duplicate URLs are not allowed') }}
+          </span>
+        </template>
       </gl-form-group>
 
       <gl-form-group
-        class="link-title-field flex-grow-1 mr-sm-4"
+        class="link-title-field col"
         :label="__('Link title')"
         :label-for="`asset-link-name-${index}`"
       >
         <gl-form-input
           :id="`asset-link-name-${index}`"
+          ref="nameInput"
           :value="link.name"
           type="text"
           class="form-control"
+          :state="isNameValid(link)"
           @change="onLinkTitleInput(link.id, $event)"
         />
+        <template v-slot:invalid-feedback>
+          <span v-if="hasEmptyName(link)" class="invalid-feedback d-inline">
+            {{ __('Link title is required') }}
+          </span>
+        </template>
       </gl-form-group>
 
-      <gl-deprecated-button
-        v-gl-tooltip
-        class="mb-5 mb-sm-3 flex-grow-0 flex-shrink-0 remove-button"
-        :aria-label="__('Remove asset link')"
-        :title="__('Remove asset link')"
-        @click="onRemoveClicked(link.id)"
-      >
-        <gl-icon class="m-0" name="remove" />
-        <span class="d-inline d-sm-none">{{ __('Remove asset link') }}</span>
-      </gl-deprecated-button>
+      <div class="mb-5 mb-sm-3 mt-sm-4 col col-sm-auto">
+        <gl-new-button
+          v-gl-tooltip
+          class="remove-button w-100"
+          :aria-label="__('Remove asset link')"
+          :title="__('Remove asset link')"
+          @click="onRemoveClicked(link.id)"
+        >
+          <gl-icon class="mr-1 mr-sm-0 mb-1" :size="16" name="remove" />
+          <span class="d-inline d-sm-none">{{ __('Remove asset link') }}</span>
+        </gl-new-button>
+      </div>
     </div>
-    <gl-deprecated-button
+    <gl-new-button
+      ref="addAnotherLinkButton"
       variant="link"
       class="align-self-end mb-5 mb-sm-0"
       @click="onAddAnotherClicked"
     >
       {{ __('Add another link') }}
-    </gl-deprecated-button>
+    </gl-new-button>
   </div>
 </template>
diff --git a/app/assets/javascripts/releases/stores/modules/detail/getters.js b/app/assets/javascripts/releases/stores/modules/detail/getters.js
index 562284dc48d..84dc2fca4be 100644
--- a/app/assets/javascripts/releases/stores/modules/detail/getters.js
+++ b/app/assets/javascripts/releases/stores/modules/detail/getters.js
@@ -1,9 +1,13 @@
+import { isEmpty } from 'lodash';
+import { hasContent } from '~/lib/utils/text_utility';
+
 /**
+ * @param {Object} link The link to test
  * @returns {Boolean} `true` if the release link is empty, i.e. it has
  * empty (or whitespace-only) values for both `url` and `name`.
  * Otherwise, `false`.
  */
-const isEmptyReleaseLink = l => !/\S/.test(l.url) && !/\S/.test(l.name);
+const isEmptyReleaseLink = link => !hasContent(link.url) && !hasContent(link.name);
 
 /** Returns all release links that aren't empty */
 export const releaseLinksToCreate = state => {
@@ -22,3 +26,67 @@ export const releaseLinksToDelete = state => {
 
   return state.originalRelease.assets.links;
 };
+
+/** Returns all validation errors on the release object */
+export const validationErrors = state => {
+  const errors = {
+    assets: {
+      links: {},
+    },
+  };
+
+  if (!state.release) {
+    return errors;
+  }
+
+  // Each key of this object is a URL, and the value is an
+  // array of Release link objects that share this URL.
+  // This is used for detecting duplicate URLs.
+  const urlToLinksMap = new Map();
+
+  state.release.assets.links.forEach(link => {
+    errors.assets.links[link.id] = {};
+
+    // Only validate non-empty URLs
+    if (isEmptyReleaseLink(link)) {
+      return;
+    }
+
+    if (!hasContent(link.url)) {
+      errors.assets.links[link.id].isUrlEmpty = true;
+    }
+
+    if (!hasContent(link.name)) {
+      errors.assets.links[link.id].isNameEmpty = true;
+    }
+
+    const normalizedUrl = link.url.trim().toLowerCase();
+
+    // Compare each URL to every other URL and flag any duplicates
+    if (urlToLinksMap.has(normalizedUrl)) {
+      // a duplicate URL was found!
+
+      // add a validation error for each link that shares this URL
+      const duplicates = urlToLinksMap.get(normalizedUrl);
+      duplicates.push(link);
+      duplicates.forEach(duplicateLink => {
+        errors.assets.links[duplicateLink.id].isDuplicate = true;
+      });
+    } else {
+      // no duplicate URL was found
+
+      urlToLinksMap.set(normalizedUrl, [link]);
+    }
+
+    if (!/^(http|https|ftp):\/\//.test(normalizedUrl)) {
+      errors.assets.links[link.id].isBadFormat = true;
+    }
+  });
+
+  return errors;
+};
+
+/** Returns whether or not the release object is valid */
+export const isValid = (_state, getters) => {
+  return Object.values(getters.validationErrors.assets.links).every(isEmpty);
+};