Merge branch 'fix-ci-variables-ux' into 'master'

Improve UX project/group ci variables

See merge request gitlab-org/gitlab!26098

app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue

@@ -7,6 +7,7 @@ import {
   GlFormSelect,
   GlFormGroup,
   GlFormInput,
+  GlFormTextarea,
   GlFormCheckbox,
   GlLink,
   GlIcon,
@@ -19,6 +20,7 @@ export default {
     GlFormSelect,
     GlFormGroup,
     GlFormInput,
+    GlFormTextarea,
     GlFormCheckbox,
     GlLink,
     GlIcon,
@@ -34,17 +36,29 @@ export default {
       'maskableRegex',
     ]),
     canSubmit() {
+      if (this.variableData.masked && this.maskedState === false) {
+        return false;
+      }
       return this.variableData.key !== '' && this.variableData.secret_value !== '';
     },
     canMask() {
       const regex = RegExp(this.maskableRegex);
       return regex.test(this.variableData.secret_value);
     },
+    displayMaskedError() {
+      return !this.canMask && this.variableData.masked && this.variableData.secret_value !== '';
+    },
+    maskedState() {
+      if (this.displayMaskedError) {
+        return false;
+      }
+      return null;
+    },
     variableData() {
       return this.variableBeingEdited || this.variable;
     },
     modalActionText() {
-      return this.variableBeingEdited ? __('Update Variable') : __('Add variable');
+      return this.variableBeingEdited ? __('Update variable') : __('Add variable');
     },
     primaryAction() {
       return {
@@ -52,11 +66,23 @@ export default {
         attributes: { variant: 'success', disabled: !this.canSubmit },
       };
     },
+    deleteAction() {
+      if (this.variableBeingEdited) {
+        return {
+          text: __('Delete variable'),
+          attributes: { variant: 'danger', category: 'secondary' },
+        };
+      }
+      return null;
+    },
     cancelAction() {
       return {
         text: __('Cancel'),
       };
     },
+    maskedFeedback() {
+      return __('This variable can not be masked');
+    },
   },
   methods: {
     ...mapActions([
@@ -65,6 +91,7 @@ export default {
       'resetEditing',
       'displayInputValue',
       'clearModal',
+      'deleteVariable',
     ]),
     updateOrAddVariable() {
       if (this.variableBeingEdited) {
@@ -89,74 +116,93 @@ export default {
     :modal-id="$options.modalId"
     :title="modalActionText"
     :action-primary="primaryAction"
+    :action-secondary="deleteAction"
     :action-cancel="cancelAction"
     @ok="updateOrAddVariable"
     @hidden="resetModalHandler"
+    @secondary="deleteVariable(variableBeingEdited)"
   >
     <form>
-      <gl-form-group label="Type" label-for="ci-variable-type">
-        <gl-form-select
-          id="ci-variable-type"
-          v-model="variableData.variable_type"
-          :options="typeOptions"
-        />
-      </gl-form-group>
-
-      <div class="d-flex">
-        <gl-form-group label="Key" label-for="ci-variable-key" class="w-50 append-right-15">
+      <gl-form-group :label="__('Key')" label-for="ci-variable-key">
         <gl-form-input
           id="ci-variable-key"
           v-model="variableData.key"
-            type="text"
           data-qa-selector="variable_key"
         />
       </gl-form-group>
 
-        <gl-form-group label="Value" label-for="ci-variable-value" class="w-50">
-          <gl-form-input
+      <gl-form-group
+        :label="__('Value')"
+        label-for="ci-variable-value"
+        :state="maskedState"
+        :invalid-feedback="maskedFeedback"
+      >
+        <gl-form-textarea
           id="ci-variable-value"
           v-model="variableData.secret_value"
-            type="text"
+          rows="3"
+          max-rows="6"
           data-qa-selector="variable_value"
         />
       </gl-form-group>
-      </div>
 
-      <gl-form-group v-if="!isGroup" label="Environment scope" label-for="ci-variable-env">
+      <div class="d-flex">
+        <gl-form-group
+          :label="__('Type')"
+          label-for="ci-variable-type"
+          class="w-50 append-right-15"
+          :class="{ 'w-100': isGroup }"
+        >
+          <gl-form-select
+            id="ci-variable-type"
+            v-model="variableData.variable_type"
+            :options="typeOptions"
+          />
+        </gl-form-group>
+
+        <gl-form-group
+          v-if="!isGroup"
+          :label="__('Environment scope')"
+          label-for="ci-variable-env"
+          class="w-50"
+        >
           <gl-form-select
             id="ci-variable-env"
             v-model="variableData.environment_scope"
             :options="environments"
           />
         </gl-form-group>
+      </div>
 
-      <gl-form-group label="Flags" label-for="ci-variable-flags">
+      <gl-form-group :label="__('Flags')" label-for="ci-variable-flags">
         <gl-form-checkbox v-model="variableData.protected" class="mb-0">
           {{ __('Protect variable') }}
           <gl-link href="/help/ci/variables/README#protected-environment-variables">
             <gl-icon name="question" :size="12" />
           </gl-link>
-          <p class="prepend-top-4 clgray">
-            {{ __('Allow variables to run on protected branches and tags.') }}
+          <p class="prepend-top-4 text-secondary">
+            {{ __('Export variable to pipelines running on protected branches and tags only.') }}
           </p>
         </gl-form-checkbox>
 
         <gl-form-checkbox
           ref="masked-ci-variable"
           v-model="variableData.masked"
-          :disabled="!canMask"
           data-qa-selector="variable_masked"
         >
           {{ __('Mask variable') }}
           <gl-link href="/help/ci/variables/README#masked-variables">
             <gl-icon name="question" :size="12" />
           </gl-link>
-          <p class="prepend-top-4 append-bottom-0 clgray">
-            {{
-              __(
-                'Variables will be masked in job logs. Requires values to meet regular expression requirements.',
-              )
-            }}
+          <p class="prepend-top-4 append-bottom-0 text-secondary">
+            {{ __('Variable will be masked in job logs.') }}
+            <span
+              :class="{
+                'bold text-plain': displayMaskedError,
+              }"
+            >
+              {{ __('Requires values to meet regular expression requirements.') }}</span
+            >
             <gl-link href="/help/ci/variables/README#masked-variables">{{
               __('More information')
             }}</gl-link>

app/assets/javascripts/ci_variable_list/components/ci_variable_popover.vue

+<script>
+import { GlPopover, GlIcon, GlButton, GlTooltipDirective } from '@gitlab/ui';
+
+export default {
+  maxTextLength: 95,
+  components: {
+    GlPopover,
+    GlIcon,
+    GlButton,
+  },
+  directives: {
+    GlTooltip: GlTooltipDirective,
+  },
+  props: {
+    target: {
+      type: String,
+      required: true,
+    },
+    value: {
+      type: String,
+      required: true,
+    },
+    tooltipText: {
+      type: String,
+      required: true,
+    },
+  },
+  computed: {
+    displayValue() {
+      if (this.value.length > this.$options.maxTextLength) {
+        return `${this.value.substring(0, this.$options.maxTextLength)}...`;
+      }
+      return this.value;
+    },
+  },
+};
+</script>
+
+<template>
+  <div id="popover-container">
+    <gl-popover :target="target" triggers="hover" placement="top" container="popover-container">
+      <div class="d-flex justify-content-between position-relative">
+        <div class="pr-5 w-100 ci-popover-value">{{ displayValue }}</div>
+        <gl-button
+          v-gl-tooltip
+          class="btn-transparent btn-clipboard position-absolute position-top-0 position-right-0"
+          :title="tooltipText"
+          :data-clipboard-text="value"
+        >
+          <gl-icon name="copy-to-clipboard" />
+        </gl-button>
+      </div>
+    </gl-popover>
+  </div>
+</template>

app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue

@@ -3,44 +3,58 @@ import { GlTable, GlButton, GlModalDirective, GlIcon } from '@gitlab/ui';
 import { s__, __ } from '~/locale';
 import { mapState, mapActions } from 'vuex';
 import { ADD_CI_VARIABLE_MODAL_ID } from '../constants';
+import CiVariablePopover from './ci_variable_popover.vue';
 
 export default {
   modalId: ADD_CI_VARIABLE_MODAL_ID,
+  trueIcon: 'mobile-issue-close',
+  falseIcon: 'close',
+  iconSize: 16,
   fields: [
     {
       key: 'variable_type',
       label: s__('CiVariables|Type'),
+      customStyle: { width: '70px' },
     },
     {
       key: 'key',
       label: s__('CiVariables|Key'),
+      tdClass: 'text-plain',
+      sortable: true,
+      customStyle: { width: '40%' },
     },
     {
       key: 'value',
       label: s__('CiVariables|Value'),
       tdClass: 'qa-ci-variable-input-value',
+      customStyle: { width: '40%' },
     },
     {
       key: 'protected',
       label: s__('CiVariables|Protected'),
+      customStyle: { width: '100px' },
     },
     {
       key: 'masked',
       label: s__('CiVariables|Masked'),
+      customStyle: { width: '100px' },
     },
     {
       key: 'environment_scope',
-      label: s__('CiVariables|Environment Scope'),
+      label: s__('CiVariables|Environments'),
+      customStyle: { width: '20%' },
     },
     {
       key: 'actions',
       label: '',
+      customStyle: { width: '35px' },
     },
   ],
   components: {
     GlTable,
     GlButton,
     GlIcon,
+    CiVariablePopover,
   },
   directives: {
     GlModalDirective,
@@ -64,7 +78,7 @@ export default {
     this.fetchVariables();
   },
   methods: {
-    ...mapActions(['fetchVariables', 'deleteVariable', 'toggleValues', 'editVariable']),
+    ...mapActions(['fetchVariables', 'toggleValues', 'editVariable']),
   },
 };
 </script>
@@ -74,42 +88,82 @@ export default {
     <gl-table
       :fields="fields"
       :items="variables"
-      responsive
-      show-empty
       tbody-tr-class="js-ci-variable-row"
+      sort-by="key"
+      sort-direction="asc"
+      stacked="lg"
+      fixed
+      show-empty
+      sort-icon-left
+      no-sort-reset
     >
-      <template #cell(value)="data">
-        <span v-if="valuesHidden">*****************</span>
-        <span v-else>{{ data.value }}</span>
+      <template #table-colgroup="scope">
+        <col v-for="field in scope.fields" :key="field.key" :style="field.customStyle" />
+      </template>
+      <template #cell(key)="{ item }">
+        <div class="d-flex truncated-container">
+          <span :id="`ci-variable-key-${item.id}`" class="d-inline-block mw-100 text-truncate">{{
+            item.key
+          }}</span>
+          <ci-variable-popover
+            :target="`ci-variable-key-${item.id}`"
+            :value="item.key"
+            :tooltip-text="__('Copy key')"
+          />
+        </div>
+      </template>
+      <template #cell(value)="{ item }">
+        <span v-if="valuesHidden">*********************</span>
+        <div v-else class="d-flex truncated-container">
+          <span :id="`ci-variable-value-${item.id}`" class="d-inline-block mw-100 text-truncate">{{
+            item.value
+          }}</span>
+          <ci-variable-popover
+            :target="`ci-variable-value-${item.id}`"
+            :value="item.value"
+            :tooltip-text="__('Copy value')"
+          />
+        </div>
+      </template>
+      <template #cell(protected)="{ item }">
+        <gl-icon v-if="item.protected" :size="$options.iconSize" :name="$options.trueIcon" />
+        <gl-icon v-else :size="$options.iconSize" :name="$options.falseIcon" />
       </template>
-      <template #cell(actions)="data">
+      <template #cell(masked)="{ item }">
+        <gl-icon v-if="item.masked" :size="$options.iconSize" :name="$options.trueIcon" />
+        <gl-icon v-else :size="$options.iconSize" :name="$options.falseIcon" />
+      </template>
+      <template #cell(environment_scope)="{ item }">
+        <div class="d-flex truncated-container">
+          <span :id="`ci-variable-env-${item.id}`" class="d-inline-block mw-100 text-truncate">{{
+            item.environment_scope
+          }}</span>
+          <ci-variable-popover
+            :target="`ci-variable-env-${item.id}`"
+            :value="item.environment_scope"
+            :tooltip-text="__('Copy environment')"
+          />
+        </div>
+      </template>
+      <template #cell(actions)="{ item }">
         <gl-button
           ref="edit-ci-variable"
           v-gl-modal-directive="$options.modalId"
-          @click="editVariable(data.item)"
+          @click="editVariable(item)"
         >
-          <gl-icon name="pencil" />
-        </gl-button>
-        <gl-button
-          ref="delete-ci-variable"
-          category="secondary"
-          variant="danger"
-          @click="deleteVariable(data.item)"
-        >
-          <gl-icon name="remove" />
+          <gl-icon :size="$options.iconSize" name="pencil" />
         </gl-button>
       </template>
       <template #empty>
-        <p ref="empty-variables" class="settings-message text-center empty-variables">
-          {{
-            __(
-              'There are currently no variables, add a variable with the Add Variable button below.',
-            )
-          }}
+        <p ref="empty-variables" class="text-center empty-variables text-plain">
+          {{ __('There are no variables yet.') }}
         </p>
       </template>
     </gl-table>
-    <div class="ci-variable-actions d-flex justify-content-end">
+    <div
+      class="ci-variable-actions d-flex justify-content-end"
+      :class="{ 'justify-content-center': !tableIsNotEmpty }"
+    >
       <gl-button
         v-if="tableIsNotEmpty"
         ref="secret-value-reveal-button"

app/assets/javascripts/ci_variable_list/constants.js

-// eslint-disable-next-line import/prefer-default-export
+import { __ } from '~/locale';
+
+// eslint-disable import/prefer-default-export
 export const ADD_CI_VARIABLE_MODAL_ID = 'add-ci-variable';
+
+export const displayText = {
+  variableText: __('Var'),
+  fileText: __('File'),
+  allEnvironmentsText: __('All'),
+};
+
+export const types = {
+  variableType: 'env_var',
+  fileType: 'file',
+  allEnvironmentsType: '*',
+};

app/assets/javascripts/ci_variable_list/store/mutations.js

 import * as types from './mutation_types';
-import { __ } from '~/locale';
+import { displayText } from '../constants';
 
 export default {
   [types.REQUEST_VARIABLES](state) {
@@ -61,7 +61,7 @@ export default {
   [types.RECEIVE_ENVIRONMENTS_SUCCESS](state, environments) {
     state.isLoading = false;
     state.environments = environments;
-    state.environments.unshift(__('All environments'));
+    state.environments.unshift(displayText.allEnvironmentsText);
   },
 
   [types.VARIABLE_BEING_EDITED](state, variable) {
@@ -70,12 +70,12 @@ export default {
 
   [types.CLEAR_MODAL](state) {
     state.variable = {
-      variable_type: __('Variable'),
+      variable_type: displayText.variableText,
       key: '',
       secret_value: '',
       protected: false,
       masked: false,
-      environment_scope: __('All environments'),
+      environment_scope: displayText.allEnvironmentsText,
     };
   },
 

app/assets/javascripts/ci_variable_list/store/state.js

-import { __ } from '~/locale';
+import { displayText } from '../constants';
 
 export default () => ({
   endpoint: null,
@@ -8,17 +8,17 @@ export default () => ({
   isLoading: false,
   isDeleting: false,
   variable: {
-    variable_type: __('Variable'),
+    variable_type: displayText.variableText,
     key: '',
     secret_value: '',
     protected: false,
     masked: false,
-    environment_scope: __('All environments'),
+    environment_scope: displayText.allEnvironmentsText,
   },
   variables: null,
   valuesHidden: true,
   error: null,
   environments: [],
-  typeOptions: [__('Variable'), __('File')],
+  typeOptions: [displayText.variableText, displayText.fileText],
   variableBeingEdited: null,
 });

app/assets/javascripts/ci_variable_list/store/utils.js

-import { __ } from '~/locale';
 import { cloneDeep } from 'lodash';
+import { displayText, types } from '../constants';
 
-const variableType = 'env_var';
-const fileType = 'file';
-
-const variableTypeHandler = type => (type === 'Variable' ? variableType : fileType);
+const variableTypeHandler = type =>
+  type === displayText.variableText ? types.variableType : types.fileType;
 
 export const prepareDataForDisplay = variables => {
   const variablesToDisplay = [];
   variables.forEach(variable => {
     const variableCopy = variable;
-    if (variableCopy.variable_type === variableType) {
-      variableCopy.variable_type = __('Variable');
+    if (variableCopy.variable_type === types.variableType) {
+      variableCopy.variable_type = displayText.variableText;
     } else {
-      variableCopy.variable_type = __('File');
+      variableCopy.variable_type = displayText.fileText;
     }
+    variableCopy.secret_value = variableCopy.value;
 
-    if (variableCopy.environment_scope === '*') {
-      variableCopy.environment_scope = __('All environments');
+    if (variableCopy.environment_scope === types.allEnvironmentsType) {
+      variableCopy.environment_scope = displayText.allEnvironmentsText;
     }
     variablesToDisplay.push(variableCopy);
   });
@@ -29,9 +28,8 @@ export const prepareDataForApi = (variable, destroy = false) => {
   variableCopy.protected = variableCopy.protected.toString();
   variableCopy.masked = variableCopy.masked.toString();
   variableCopy.variable_type = variableTypeHandler(variableCopy.variable_type);
-
-  if (variableCopy.environment_scope === __('All environments')) {
-    variableCopy.environment_scope = __('*');
+  if (variableCopy.environment_scope === displayText.allEnvironmentsText) {
+    variableCopy.environment_scope = types.allEnvironmentsType;
   }
 
   if (destroy) {

app/assets/stylesheets/pages/settings.scss

@@ -376,8 +376,29 @@
 }
 
 .ci-variable-table {
-  table tr th {
+  table {
+    thead {
+      border-bottom: 1px solid $white-normal;
+    }
+
+    tr {
+      td,
+      th {
+        padding-left: 0;
+      }
+
+      th {
         background-color: transparent;
+        font-weight: $gl-font-weight-bold;
         border: 0;
       }
+    }
+  }
+
+  @media(max-width: map-get($grid-breakpoints, lg)-1) {
+    .truncated-container {
+      justify-content: flex-end;
+    }
+  }
 }
+

app/serializers/group_variable_entity.rb

@@ -4,6 +4,7 @@ class GroupVariableEntity < Grape::Entity
   expose :id
   expose :key
   expose :value
+  expose :variable_type
 
   expose :protected?, as: :protected
   expose :masked?, as: :masked

doc/ci/variables/README.md

@@ -185,14 +185,19 @@ For a deeper look into them, see [`.gitlab-ci.yml` defined variables](#gitlab-ci
 
 #### Via the UI
 
-From the UI, navigate to your project's **Settings > CI/CD** and
-expand **Variables**. Create a new variable by choosing its **type**, naming
-it in the field **Input variable key**, and defining its value in the
-**Input variable value** field:
+From within the UI, you can add or update custom environment variables:
 
-![CI/CD settings - new variable](img/new_custom_variables_example.png)
+1. Go to your project's **Settings > CI/CD** and expand the **Variables** section.
+1. Click the **Add variable** button. In the **Add variable** modal, fill in the details:
 
-You'll also see the option to mask and/or protect your variables.
+    - **Key**: Must be one line, with no spaces, using only letters, numbers, `-` or `_`.
+    - **Value**: No limitations.
+    - **Type**: `File` or `Variable`.
+    - **Environment scope**: `All`, or specific environments.
+    - **Protect variable** (Optional): If selected, the variable will only be available in pipelines that run on protected branches or tags.
+    - **Mask variable** (Optional): If selected, the variable's **Value** will be masked in job logs. The variable will fail to save if the value does not meet the [masking requirements](#masked-variables).
+
+After a variable is created, you can update any of the details by clicking on the **{pencil}** **Edit** button.
 
 Once you've set the variables, call them from the `.gitlab-ci.yml` file:
 

locale/gitlab.pot

@@ -1684,9 +1684,6 @@ msgstr ""
 msgid "Allow users to request access (if visibility is public or internal)"
 msgstr ""
 
-msgid "Allow variables to run on protected branches and tags."
-msgstr ""
-
 msgid "Allowed email domain restriction only permitted for top-level groups"
 msgstr ""
 
@@ -3796,7 +3793,7 @@ msgstr ""
 msgid "CiVariables|Cannot use Masked Variable with current value"
 msgstr ""
 
-msgid "CiVariables|Environment Scope"
+msgid "CiVariables|Environments"
 msgstr ""
 
 msgid "CiVariables|Input variable key"
@@ -5564,6 +5561,9 @@ msgstr ""
 msgid "Copy commit SHA"
 msgstr ""
 
+msgid "Copy environment"
+msgstr ""
+
 msgid "Copy evidence SHA"
 msgstr ""
 
@@ -5576,6 +5576,9 @@ msgstr ""
 msgid "Copy impersonation token"
 msgstr ""
 
+msgid "Copy key"
+msgstr ""
+
 msgid "Copy labels and milestone from %{source_issuable_reference}."
 msgstr ""
 
@@ -5600,6 +5603,9 @@ msgstr ""
 msgid "Copy trigger token"
 msgstr ""
 
+msgid "Copy value"
+msgstr ""
+
 msgid "Could not add admins as members"
 msgstr ""
 
@@ -6370,6 +6376,9 @@ msgstr ""
 msgid "Delete this attachment"
 msgstr ""
 
+msgid "Delete variable"
+msgstr ""
+
 msgid "DeleteProject|Failed to remove project repository. Please try again or contact administrator."
 msgstr ""
 
@@ -8263,6 +8272,9 @@ msgstr ""
 msgid "Export this project with all its related data in order to move your project to a new GitLab instance. Once the export is finished, you can import the file from the \"New Project\" page."
 msgstr ""
 
+msgid "Export variable to pipelines running on protected branches and tags only."
+msgstr ""
+
 msgid "External Classification Policy Authorization"
 msgstr ""
 
@@ -8833,6 +8845,9 @@ msgstr ""
 msgid "Fixed:"
 msgstr ""
 
+msgid "Flags"
+msgstr ""
+
 msgid "FlowdockService|Flowdock Git source token"
 msgstr ""
 
@@ -16728,6 +16743,9 @@ msgid_plural "Requires %d more approvals."
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "Requires values to meet regular expression requirements."
+msgstr ""
+
 msgid "Resend confirmation email"
 msgstr ""
 
@@ -19860,9 +19878,6 @@ msgstr ""
 msgid "The value lying at the midpoint of a series of observed values. E.g., between 3, 5, 9, the median is 5. Between 3, 5, 7, 8, the median is (5+7)/2 = 6."
 msgstr ""
 
-msgid "There are currently no variables, add a variable with the Add Variable button below."
-msgstr ""
-
 msgid "There are no GPG keys associated with this account."
 msgstr ""
 
@@ -19917,6 +19932,9 @@ msgstr ""
 msgid "There are no projects shared with this group yet"
 msgstr ""
 
+msgid "There are no variables yet."
+msgstr ""
+
 msgid "There is a limit of %{ci_project_subscriptions_limit} subscriptions from or to a project."
 msgstr ""
 
@@ -20436,6 +20454,9 @@ msgstr ""
 msgid "This user will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches. Upon creation or when reassigning you can only assign yourself to be the mirror user."
 msgstr ""
 
+msgid "This variable can not be masked"
+msgstr ""
+
 msgid "This will help us personalize your onboarding experience."
 msgstr ""
 
@@ -21373,9 +21394,6 @@ msgstr ""
 msgid "Update"
 msgstr ""
 
-msgid "Update Variable"
-msgstr ""
-
 msgid "Update all"
 msgstr ""
 
@@ -21394,6 +21412,9 @@ msgstr ""
 msgid "Update now"
 msgstr ""
 
+msgid "Update variable"
+msgstr ""
+
 msgid "Update your bookmarked URLs as filtered/sorted branches URL has been changed."
 msgstr ""
 
@@ -22003,13 +22024,13 @@ msgstr ""
 msgid "Value Stream Analytics gives an overview of how much time it takes to go from idea to production in your project."
 msgstr ""
 
-msgid "Variable"
+msgid "Var"
 msgstr ""
 
-msgid "Variables"
+msgid "Variable will be masked in job logs."
 msgstr ""
 
-msgid "Variables will be masked in job logs. Requires values to meet regular expression requirements."
+msgid "Variables"
 msgstr ""
 
 msgid "Various container registry settings."

spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js

@@ -35,10 +35,6 @@ describe('Ci variable modal', () => {
     expect(findModal().props('actionPrimary').attributes.disabled).toBeTruthy();
   });
 
-  it('masked checkbox is disabled when value does not meet regex requirements', () => {
-    expect(wrapper.find({ ref: 'masked-ci-variable' }).attributes('disabled')).toBeTruthy();
-  });
-
   describe('Adding a new variable', () => {
     beforeEach(() => {
       const [variable] = mockData.mockVariables;
@@ -49,13 +45,6 @@ describe('Ci variable modal', () => {
       expect(findModal().props('actionPrimary').attributes.disabled).toBeFalsy();
     });
 
-    it('masked checkbox is enabled when value meets regex requirements', () => {
-      store.state.maskableRegex = '^[a-zA-Z0-9_+=/@:-]{8,}$';
-      return wrapper.vm.$nextTick(() => {
-        expect(wrapper.find({ ref: 'masked-ci-variable' }).attributes('disabled')).toBeFalsy();
-      });
-    });
-
     it('Add variable button dispatches addVariable action', () => {
       findModal().vm.$emit('ok');
       expect(store.dispatch).toHaveBeenCalledWith('addVariable');
@@ -74,7 +63,7 @@ describe('Ci variable modal', () => {
     });
 
     it('button text is Update variable when updating', () => {
-      expect(wrapper.vm.modalActionText).toBe('Update Variable');
+      expect(wrapper.vm.modalActionText).toBe('Update variable');
     });
 
     it('Update variable button dispatches updateVariable with correct variable', () => {
@@ -89,5 +78,10 @@ describe('Ci variable modal', () => {
       findModal().vm.$emit('hidden');
       expect(store.dispatch).toHaveBeenCalledWith('resetEditing');
     });
+
+    it('dispatches deleteVariable with correct variable to delete', () => {
+      findModal().vm.$emit('secondary');
+      expect(store.dispatch).toHaveBeenCalledWith('deleteVariable', mockData.mockVariables[0]);
+    });
   });
 });

spec/frontend/ci_variable_list/components/ci_variable_popover_spec.js

+import { shallowMount } from '@vue/test-utils';
+import { GlButton } from '@gitlab/ui';
+import CiVariablePopover from '~/ci_variable_list/components/ci_variable_popover.vue';
+import mockData from '../services/mock_data';
+
+describe('Ci Variable Popover', () => {
+  let wrapper;
+
+  const defaultProps = {
+    target: 'ci-variable-value-22',
+    value: mockData.mockPemCert,
+    tooltipText: 'Copy value',
+  };
+
+  const createComponent = (props = defaultProps) => {
+    wrapper = shallowMount(CiVariablePopover, {
+      propsData: { ...props },
+    });
+  };
+
+  const findButton = () => wrapper.find(GlButton);
+
+  beforeEach(() => {
+    createComponent();
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+
+  it('displays max count plus ... when character count is over 95', () => {
+    expect(wrapper.text()).toHaveLength(98);
+  });
+
+  it('copies full value to clipboard', () => {
+    expect(findButton().attributes('data-clipboard-text')).toEqual(mockData.mockPemCert);
+  });
+
+  it('displays full value when count is less than max count', () => {
+    createComponent({
+      target: 'ci-variable-value-22',
+      value: 'test_variable_value',
+      tooltipText: 'Copy value',
+    });
+    expect(wrapper.text()).toEqual('test_variable_value');
+  });
+});

spec/frontend/ci_variable_list/components/ci_variable_table_spec.js

@@ -17,12 +17,12 @@ describe('Ci variable table', () => {
     store.state.isGroup = true;
     jest.spyOn(store, 'dispatch').mockImplementation();
     wrapper = mount(CiVariableTable, {
+      attachToDocument: true,
       localVue,
       store,
     });
   };
 
-  const findDeleteButton = () => wrapper.find({ ref: 'delete-ci-variable' });
   const findRevealButton = () => wrapper.find({ ref: 'secret-value-reveal-button' });
   const findEditButton = () => wrapper.find({ ref: 'edit-ci-variable' });
   const findEmptyVariablesPlaceholder = () => wrapper.find({ ref: 'empty-variables' });
@@ -71,11 +71,6 @@ describe('Ci variable table', () => {
       store.state.variables = mockData.mockVariables;
     });
 
-    it('dispatches deleteVariable with correct variable to delete', () => {
-      findDeleteButton().trigger('click');
-      expect(store.dispatch).toHaveBeenCalledWith('deleteVariable', mockData.mockVariables[0]);
-    });
-
     it('reveals secret values when button is clicked', () => {
       findRevealButton().trigger('click');
       expect(store.dispatch).toHaveBeenCalledWith('toggleValues', false);

spec/frontend/ci_variable_list/services/mock_data.js

@@ -6,8 +6,9 @@ export default {
       key: 'test_var',
       masked: false,
       protected: false,
+      secret_value: 'test_val',
       value: 'test_val',
-      variable_type: 'Variable',
+      variable_type: 'Var',
     },
   ],
 
@@ -18,6 +19,7 @@ export default {
       key: 'test_var',
       masked: false,
       protected: false,
+      secret_value: 'test_val',
       value: 'test_val',
       variable_type: 'env_var',
     },
@@ -27,6 +29,7 @@ export default {
       key: 'test_var_2',
       masked: false,
       protected: false,
+      secret_value: 'test_val_2',
       value: 'test_val_2',
       variable_type: 'file',
     },
@@ -34,20 +37,22 @@ export default {
 
   mockVariablesDisplay: [
     {
-      environment_scope: 'All environments',
+      environment_scope: 'All',
       id: 113,
       key: 'test_var',
       masked: false,
       protected: false,
+      secret_value: 'test_val',
       value: 'test_val',
-      variable_type: 'Variable',
+      variable_type: 'Var',
     },
     {
-      environment_scope: 'All environments',
+      environment_scope: 'All',
       id: 114,
       key: 'test_var_2',
       masked: false,
       protected: false,
+      secret_value: 'test_val_2',
       value: 'test_val_2',
       variable_type: 'File',
     },
@@ -69,4 +74,18 @@ export default {
       state: 'available',
     },
   ],
+
+  mockPemCert: `-----BEGIN CERTIFICATE REQUEST-----
+  MIIB9TCCAWACAQAwgbgxGTAXBgNVBAoMEFF1b1ZhZGlzIExpbWl0ZWQxHDAaBgNV
+  BAsME0RvY3VtZW50IERlcGFydG1lbnQxOTA3BgNVBAMMMFdoeSBhcmUgeW91IGRl
+  Y29kaW5nIG1lPyAgVGhpcyBpcyBvbmx5IGEgdGVzdCEhITERMA8GA1UEBwwISGFt
+  aWx0b24xETAPBgNVBAgMCFBlbWJyb2tlMQswCQYDVQQGEwJCTTEPMA0GCSqGSIb3
+  DQEJARYAMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCJ9WRanG/fUvcfKiGl
+  EL4aRLjGt537mZ28UU9/3eiJeJznNSOuNLnF+hmabAu7H0LT4K7EdqfF+XUZW/2j
+  RKRYcvOUDGF9A7OjW7UfKk1In3+6QDCi7X34RE161jqoaJjrm/T18TOKcgkkhRzE
+  apQnIDm0Ea/HVzX/PiSOGuertwIDAQABMAsGCSqGSIb3DQEBBQOBgQBzMJdAV4QP
+  Awel8LzGx5uMOshezF/KfP67wJ93UW+N7zXY6AwPgoLj4Kjw+WtU684JL8Dtr9FX
+  ozakE+8p06BpxegR4BR3FMHf6p+0jQxUEAkAyb/mVgm66TyghDGC6/YkiKoZptXQ
+  98TwDIK/39WEB/V607As+KoYazQG8drorw==
+  -----END CERTIFICATE REQUEST-----`,
 };

spec/frontend/ci_variable_list/store/mutations_spec.js

@@ -48,12 +48,12 @@ describe('CI variable list mutations', () => {
   describe('CLEAR_MODAL', () => {
     it('should clear modal state ', () => {
       const modalState = {
-        variable_type: 'Variable',
+        variable_type: 'Var',
         key: '',
         secret_value: '',
         protected: false,
         masked: false,
-        environment_scope: 'All environments',
+        environment_scope: 'All',
       };
 
       mutations[types.CLEAR_MODAL](stateCopy);

spec/frontend/ci_variable_list/store/utils_spec.js

@@ -19,6 +19,7 @@ describe('CI variables store utils', () => {
       key: 'test_var',
       masked: 'false',
       protected: 'false',
+      secret_value: 'test_val',
       value: 'test_val',
       variable_type: 'env_var',
     });
@@ -29,6 +30,7 @@ describe('CI variables store utils', () => {
       key: 'test_var_2',
       masked: 'false',
       protected: 'false',
+      secret_value: 'test_val_2',
       value: 'test_val_2',
       variable_type: 'file',
     });

spec/serializers/group_variable_entity_spec.rb

@@ -10,7 +10,7 @@ describe GroupVariableEntity do
     subject { entity.as_json }
 
     it 'contains required fields' do
-      expect(subject).to include(:id, :key, :value, :protected)
+      expect(subject).to include(:id, :key, :value, :protected, :variable_type)
     end
   end
 end