Merge branch '344671-replace-not-connected-to-never-contacted-for-runner-status' into 'master'

Use "never contacted" wording in runner's frontend

See merge request gitlab-org/gitlab!77375

app/assets/javascripts/runner/admin_runners/index.js

@@ -2,6 +2,8 @@ import { GlToast } from '@gitlab/ui';
 import Vue from 'vue';
 import VueApollo from 'vue-apollo';
 import createDefaultClient from '~/lib/graphql';
+import { visitUrl } from '~/lib/utils/url_utility';
+import { updateOutdatedUrl } from '~/runner/runner_search_utils';
 import AdminRunnersApp from './admin_runners_app.vue';
 
 Vue.use(GlToast);
@@ -14,6 +16,15 @@ export const initAdminRunners = (selector = '#js-admin-runners') => {
     return null;
   }
 
+  // Redirect outdated URLs
+  const updatedUrlQuery = updateOutdatedUrl();
+  if (updatedUrlQuery) {
+    visitUrl(updatedUrlQuery);
+
+    // Prevent mounting the rest of the app, redirecting now.
+    return null;
+  }
+
   // TODO `activeRunnersCount` should be implemented using a GraphQL API
   // https://gitlab.com/gitlab-org/gitlab/-/issues/333806
   const {

app/assets/javascripts/runner/components/cells/runner_status_cell.vue

@@ -28,7 +28,15 @@ export default {
 
 <template>
   <div>
-    <runner-status-badge :runner="runner" size="sm" />
-    <runner-paused-badge v-if="paused" size="sm" />
+    <runner-status-badge
+      :runner="runner"
+      size="sm"
+      class="gl-display-inline-block gl-max-w-full gl-text-truncate"
+    />
+    <runner-paused-badge
+      v-if="paused"
+      size="sm"
+      class="gl-display-inline-block gl-max-w-full gl-text-truncate"
+    />
   </div>
 </template>

app/assets/javascripts/runner/components/runner_status_badge.vue

@@ -4,11 +4,10 @@ import { __, s__, sprintf } from '~/locale';
 import { getTimeago } from '~/lib/utils/datetime_utility';
 import {
   I18N_ONLINE_RUNNER_TIMEAGO_DESCRIPTION,
-  I18N_NOT_CONNECTED_RUNNER_DESCRIPTION,
+  I18N_NEVER_CONTACTED_RUNNER_DESCRIPTION,
   I18N_OFFLINE_RUNNER_TIMEAGO_DESCRIPTION,
   I18N_STALE_RUNNER_DESCRIPTION,
   STATUS_ONLINE,
-  STATUS_NOT_CONNECTED,
   STATUS_NEVER_CONTACTED,
   STATUS_OFFLINE,
   STATUS_STALE,
@@ -45,12 +44,11 @@ export default {
               timeAgo: this.contactedAtTimeAgo,
             }),
           };
-        case STATUS_NOT_CONNECTED:
         case STATUS_NEVER_CONTACTED:
           return {
             variant: 'muted',
-            label: s__('Runners|not connected'),
-            tooltip: I18N_NOT_CONNECTED_RUNNER_DESCRIPTION,
+            label: s__('Runners|never contacted'),
+            tooltip: I18N_NEVER_CONTACTED_RUNNER_DESCRIPTION,
           };
         case STATUS_OFFLINE:
           return {

app/assets/javascripts/runner/components/search_tokens/status_token_config.js

@@ -6,7 +6,7 @@ import {
   STATUS_PAUSED,
   STATUS_ONLINE,
   STATUS_OFFLINE,
-  STATUS_NOT_CONNECTED,
+  STATUS_NEVER_CONTACTED,
   STATUS_STALE,
   PARAM_KEY_STATUS,
 } from '../../constants';
@@ -16,7 +16,7 @@ const options = [
   { value: STATUS_PAUSED, title: s__('Runners|Paused') },
   { value: STATUS_ONLINE, title: s__('Runners|Online') },
   { value: STATUS_OFFLINE, title: s__('Runners|Offline') },
-  { value: STATUS_NOT_CONNECTED, title: s__('Runners|Not connected') },
+  { value: STATUS_NEVER_CONTACTED, title: s__('Runners|Never contacted') },
   { value: STATUS_STALE, title: s__('Runners|Stale') },
 ];
 

app/assets/javascripts/runner/constants.js

@@ -18,8 +18,8 @@ export const I18N_PROJECT_RUNNER_DESCRIPTION = s__('Runners|Associated with one
 export const I18N_ONLINE_RUNNER_TIMEAGO_DESCRIPTION = s__(
   'Runners|Runner is online; last contact was %{timeAgo}',
 );
-export const I18N_NOT_CONNECTED_RUNNER_DESCRIPTION = s__(
-  'Runners|This runner has never connected to this instance',
+export const I18N_NEVER_CONTACTED_RUNNER_DESCRIPTION = s__(
+  'Runners|This runner has never contacted this instance',
 );
 export const I18N_OFFLINE_RUNNER_TIMEAGO_DESCRIPTION = s__(
   'Runners|No recent contact from this runner; last contact was %{timeAgo}',
@@ -60,7 +60,6 @@ export const STATUS_ACTIVE = 'ACTIVE';
 export const STATUS_PAUSED = 'PAUSED';
 
 export const STATUS_ONLINE = 'ONLINE';
-export const STATUS_NOT_CONNECTED = 'NOT_CONNECTED';
 export const STATUS_NEVER_CONTACTED = 'NEVER_CONTACTED';
 export const STATUS_OFFLINE = 'OFFLINE';
 export const STATUS_STALE = 'STALE';

app/assets/javascripts/runner/runner_search_utils.js

@@ -16,6 +16,7 @@ import {
   PARAM_KEY_BEFORE,
   DEFAULT_SORT,
   RUNNER_PAGE_SIZE,
+  STATUS_NEVER_CONTACTED,
 } from './constants';
 
 /**
@@ -79,6 +80,33 @@ const getPaginationFromParams = (params) => {
   };
 };
 
+// Outdated URL parameters
+const STATUS_NOT_CONNECTED = 'NOT_CONNECTED';
+
+/**
+ * Returns an updated URL for old (or deprecated) admin runner URLs.
+ *
+ * Use for redirecting users to currently used URLs.
+ *
+ * @param {String?} URL
+ * @returns Updated URL if outdated, `null` otherwise
+ */
+export const updateOutdatedUrl = (url = window.location.href) => {
+  const urlObj = new URL(url);
+  const query = urlObj.search;
+
+  const params = queryToObject(query, { gatherArrays: true });
+
+  const runnerType = params[PARAM_KEY_STATUS]?.[0] || null;
+  if (runnerType === STATUS_NOT_CONNECTED) {
+    const updatedParams = {
+      [PARAM_KEY_STATUS]: [STATUS_NEVER_CONTACTED],
+    };
+    return setUrlParams(updatedParams, url, false, true, true);
+  }
+  return null;
+};
+
 /**
  * Takes a URL query and transforms it into a "search" object
  * @param {String?} query

app/helpers/ci/runners_helper.rb

@@ -24,7 +24,7 @@ module Ci
           span_class = 'gl-text-gray-600'
         end
       when :not_connected, :never_contacted
-        title = s_("Runners|New runner, has not connected yet")
+        title = s_("Runners|New runner, has not contacted yet")
         icon = 'warning-solid'
       when :offline
         title = s_("Runners|Runner is offline, last contact was %{runner_contact} ago") % { runner_contact: time_ago_in_words(runner.contacted_at) }

locale/gitlab.pot

@@ -30540,10 +30540,13 @@ msgstr ""
 msgid "Runners|Name"
 msgstr ""
 
+msgid "Runners|Never contacted"
+msgstr ""
+
 msgid "Runners|New registration token generated!"
 msgstr ""
 
-msgid "Runners|New runner, has not connected yet"
+msgid "Runners|New runner, has not contacted yet"
 msgstr ""
 
 msgid "Runners|No contact from this runner in over 3 months"
@@ -30555,9 +30558,6 @@ msgstr ""
 msgid "Runners|Not available to run jobs"
 msgstr ""
 
-msgid "Runners|Not connected"
-msgstr ""
-
 msgid "Runners|Offline"
 msgstr ""
 
@@ -30672,7 +30672,7 @@ msgstr ""
 msgid "Runners|The runner will be permanently deleted and no longer available for projects or groups in the instance. Are you sure you want to continue?"
 msgstr ""
 
-msgid "Runners|This runner has never connected to this instance"
+msgid "Runners|This runner has never contacted this instance"
 msgstr ""
 
 msgid "Runners|This runner is associated with one or more projects."
@@ -30741,7 +30741,7 @@ msgstr ""
 msgid "Runners|locked"
 msgstr ""
 
-msgid "Runners|not connected"
+msgid "Runners|never contacted"
 msgstr ""
 
 msgid "Runners|offline"

spec/features/admin/admin_runners_spec.rb

@@ -192,17 +192,21 @@ RSpec.describe "Admin Runners" do
           expect(page).not_to have_content 'runner-a-2'
         end
 
-        it 'shows correct runner when type is selected and search term is entered' do
-          create(:ci_runner, :instance, description: 'runner-connected', contacted_at: Time.now)
-          create(:ci_runner, :instance, description: 'runner-not-connected', contacted_at: nil)
+        it 'shows correct runner when status filter is entered' do
+          never_connected = create(:ci_runner, :instance, description: 'runner-never-contacted', contacted_at: nil)
+          create(:ci_runner, :instance, description: 'runner-contacted', contacted_at: Time.now)
 
           visit admin_runners_path
 
-          # use the string "Not" to avoid using space and trigger an early selection
-          input_filtered_search_filter_is_only('Status', 'Not')
+          # use the string "Never" to avoid using space and trigger an early selection
+          input_filtered_search_filter_is_only('Status', 'Never')
+
+          expect(page).to have_content 'runner-never-contacted'
+          expect(page).not_to have_content 'runner-contacted'
 
-          expect(page).not_to have_content 'runner-connected'
-          expect(page).to have_content 'runner-not-connected'
+          within "[data-testid='runner-row-#{never_connected.id}']" do
+            expect(page).to have_selector '.badge', text: 'never contacted'
+          end
         end
       end
 
@@ -378,6 +382,14 @@ RSpec.describe "Admin Runners" do
       end
     end
 
+    context "when visiting outdated URLs" do
+      it 'updates NOT_CONNECTED runner status to NEVER_CONNECTED' do
+        visit admin_runners_path('status[]': 'NOT_CONNECTED')
+
+        expect(page).to have_current_path(admin_runners_path('status[]': 'NEVER_CONTACTED') )
+      end
+    end
+
     describe 'runners registration' do
       let!(:token) { Gitlab::CurrentSettings.runners_registration_token }
 

spec/frontend/runner/components/runner_list_spec.js

@@ -69,7 +69,9 @@ describe('RunnerList', () => {
     const { id, description, version, ipAddress, shortSha } = mockRunners[0];
 
     // Badges
-    expect(findCell({ fieldKey: 'status' }).text()).toMatchInterpolatedText('not connected paused');
+    expect(findCell({ fieldKey: 'status' }).text()).toMatchInterpolatedText(
+      'never contacted paused',
+    );
 
     // Runner summary
     expect(findCell({ fieldKey: 'summary' }).text()).toContain(

spec/frontend/runner/components/runner_status_badge_spec.js

@@ -6,7 +6,6 @@ import {
   STATUS_ONLINE,
   STATUS_OFFLINE,
   STATUS_STALE,
-  STATUS_NOT_CONNECTED,
   STATUS_NEVER_CONTACTED,
 } from '~/runner/constants';
 
@@ -50,20 +49,7 @@ describe('RunnerTypeBadge', () => {
     expect(getTooltip().value).toBe('Runner is online; last contact was 1 minute ago');
   });
 
-  it('renders not connected state', () => {
-    createComponent({
-      runner: {
-        contactedAt: null,
-        status: STATUS_NOT_CONNECTED,
-      },
-    });
-
-    expect(wrapper.text()).toBe('not connected');
-    expect(findBadge().props('variant')).toBe('muted');
-    expect(getTooltip().value).toMatch('This runner has never connected');
-  });
-
-  it('renders never contacted state as not connected, for backwards compatibility', () => {
+  it('renders never contacted state', () => {
     createComponent({
       runner: {
         contactedAt: null,
@@ -71,9 +57,9 @@ describe('RunnerTypeBadge', () => {
       },
     });
 
-    expect(wrapper.text()).toBe('not connected');
+    expect(wrapper.text()).toBe('never contacted');
     expect(findBadge().props('variant')).toBe('muted');
-    expect(getTooltip().value).toMatch('This runner has never connected');
+    expect(getTooltip().value).toMatch('This runner has never contacted');
   });
 
   it('renders offline state', () => {

spec/frontend/runner/runner_search_utils_spec.js

 import { RUNNER_PAGE_SIZE } from '~/runner/constants';
 import {
   searchValidator,
+  updateOutdatedUrl,
   fromUrlQueryToSearch,
   fromSearchToUrl,
   fromSearchToVariables,
@@ -190,6 +191,23 @@ describe('search_params.js', () => {
     });
   });
 
+  describe('updateOutdatedUrl', () => {
+    it('returns null for urls that do not need updating', () => {
+      expect(updateOutdatedUrl('http://test.host/')).toBe(null);
+      expect(updateOutdatedUrl('http://test.host/?a=b')).toBe(null);
+    });
+
+    it('returns updated url for updating NOT_CONNECTED to NEVER_CONTACTED', () => {
+      expect(updateOutdatedUrl('http://test.host/admin/runners?status[]=NOT_CONNECTED')).toBe(
+        'http://test.host/admin/runners?status[]=NEVER_CONTACTED',
+      );
+
+      expect(updateOutdatedUrl('http://test.host/admin/runners?status[]=NOT_CONNECTED&a=b')).toBe(
+        'http://test.host/admin/runners?status[]=NEVER_CONTACTED&a=b',
+      );
+    });
+  });
+
   describe('fromUrlQueryToSearch', () => {
     examples.forEach(({ name, urlQuery, search }) => {
       it(`Converts ${name} to a search object`, () => {

spec/helpers/ci/runners_helper_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe Ci::RunnersHelper do
   describe '#runner_status_icon', :clean_gitlab_redis_cache do
     it "returns - not contacted yet" do
       runner = create(:ci_runner)
-      expect(helper.runner_status_icon(runner)).to include("not connected yet")
+      expect(helper.runner_status_icon(runner)).to include("not contacted yet")
     end
 
     it "returns offline text" do