Merge branch 'jswain_whats_new_remove_tabbed_interface' into 'master'

Remove self-managed specific What's New interface

See merge request gitlab-org/gitlab!58521

app/assets/javascripts/whats_new/components/app.vue

 <script>
-import {
-  GlDrawer,
-  GlInfiniteScroll,
-  GlResizeObserverDirective,
-  GlTabs,
-  GlTab,
-  GlBadge,
-  GlLoadingIcon,
-} from '@gitlab/ui';
+import { GlDrawer, GlInfiniteScroll, GlResizeObserverDirective } from '@gitlab/ui';
 import { mapState, mapActions } from 'vuex';
 import Tracking from '~/tracking';
 import { getDrawerBodyHeight } from '../utils/get_drawer_body_height';
@@ -20,37 +12,24 @@ export default {
   components: {
     GlDrawer,
     GlInfiniteScroll,
-    GlTabs,
-    GlTab,
     SkeletonLoader,
     Feature,
-    GlBadge,
-    GlLoadingIcon,
   },
   directives: {
     GlResizeObserver: GlResizeObserverDirective,
   },
   mixins: [trackingMixin],
   props: {
-    storageKey: {
+    versionDigest: {
       type: String,
       required: true,
     },
-    versions: {
-      type: Array,
-      required: true,
-    },
-    gitlabDotCom: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
   },
   computed: {
     ...mapState(['open', 'features', 'pageInfo', 'drawerBodyHeight', 'fetching']),
   },
   mounted() {
-    this.openDrawer(this.storageKey);
+    this.openDrawer(this.versionDigest);
     this.fetchItems();
 
     const body = document.querySelector('body');
@@ -70,16 +49,6 @@ export default {
       const height = getDrawerBodyHeight(this.$refs.drawer.$el);
       this.setDrawerBodyHeight(height);
     },
-    featuresForVersion(version) {
-      return this.features.filter((feature) => {
-        return feature.release === parseFloat(version);
-      });
-    },
-    fetchVersion(version) {
-      if (this.featuresForVersion(version).length === 0) {
-        this.fetchItems({ version });
-      }
-    },
   },
 };
 </script>
@@ -99,7 +68,6 @@ export default {
       </template>
       <template v-if="features.length">
         <gl-infinite-scroll
-          v-if="gitlabDotCom"
           :fetched-items="features.length"
           :max-list-height="drawerBodyHeight"
           class="gl-p-0"
@@ -109,26 +77,6 @@ export default {
             <feature v-for="feature in features" :key="feature.title" :feature="feature" />
           </template>
         </gl-infinite-scroll>
-        <gl-tabs v-else :style="{ height: `${drawerBodyHeight}px` }" class="gl-p-0">
-          <gl-tab
-            v-for="(version, index) in versions"
-            :key="version"
-            @click="fetchVersion(version)"
-          >
-            <template #title>
-              <span>{{ version }}</span>
-              <gl-badge v-if="index === 0">{{ __('Your Version') }}</gl-badge>
-            </template>
-            <gl-loading-icon v-if="fetching" size="lg" class="text-center" />
-            <template v-else>
-              <feature
-                v-for="feature in featuresForVersion(version)"
-                :key="feature.title"
-                :feature="feature"
-              />
-            </template>
-          </gl-tab>
-        </gl-tabs>
       </template>
       <div v-else class="gl-mt-5">
         <skeleton-loader />

app/assets/javascripts/whats_new/index.js

@@ -2,7 +2,7 @@ import Vue from 'vue';
 import { mapState } from 'vuex';
 import App from './components/app.vue';
 import store from './store';
-import { getStorageKey, setNotification } from './utils/notification';
+import { getVersionDigest, setNotification } from './utils/notification';
 
 let whatsNewApp;
 
@@ -27,9 +27,7 @@ export default (el) => {
       render(createElement) {
         return createElement('app', {
           props: {
-            storageKey: getStorageKey(el),
-            versions: JSON.parse(el.getAttribute('data-versions')),
-            gitlabDotCom: el.getAttribute('data-gitlab-dot-com'),
+            versionDigest: getVersionDigest(el),
           },
         });
       },

app/assets/javascripts/whats_new/store/actions.js

 import axios from '~/lib/utils/axios_utils';
 import { parseIntPagination, normalizeHeaders } from '~/lib/utils/common_utils';
+import { STORAGE_KEY } from '../utils/notification';
 import * as types from './mutation_types';
 
 export default {
   closeDrawer({ commit }) {
     commit(types.CLOSE_DRAWER);
   },
-  openDrawer({ commit }, storageKey) {
+  openDrawer({ commit }, versionDigest) {
     commit(types.OPEN_DRAWER);
 
-    if (storageKey) {
-      localStorage.setItem(storageKey, JSON.stringify(false));
+    if (versionDigest) {
+      localStorage.setItem(STORAGE_KEY, versionDigest);
     }
   },
-  fetchItems({ commit, state }, { page, version } = { page: null, version: null }) {
+  fetchItems({ commit, state }, { page } = { page: null }) {
     if (state.fetching) {
       return false;
     }
@@ -24,7 +25,6 @@ export default {
       .get('/-/whats_new', {
         params: {
           page,
-          version,
         },
       })
       .then(({ data, headers }) => {

app/assets/javascripts/whats_new/utils/notification.js

-export const getStorageKey = (appEl) => appEl.getAttribute('data-storage-key');
+export const STORAGE_KEY = 'display-whats-new-notification';
+
+export const getVersionDigest = (appEl) => appEl.getAttribute('data-version-digest');
 
 export const setNotification = (appEl) => {
-  const storageKey = getStorageKey(appEl);
+  const versionDigest = getVersionDigest(appEl);
   const notificationEl = document.querySelector('.header-help');
   let notificationCountEl = notificationEl.querySelector('.js-whats-new-notification-count');
 
-  if (JSON.parse(localStorage.getItem(storageKey)) === false) {
+  const legacyStorageKey = 'display-whats-new-notification-13.10';
+  const localStoragePairs = [
+    [legacyStorageKey, false],
+    [STORAGE_KEY, versionDigest],
+  ];
+  if (localStoragePairs.some((pair) => localStorage.getItem(pair[0]) === pair[1].toString())) {
     notificationEl.classList.remove('with-notifications');
     if (notificationCountEl) {
       notificationCountEl.parentElement.removeChild(notificationCountEl);

app/controllers/whats_new_controller.rb

@@ -5,7 +5,7 @@ class WhatsNewController < ApplicationController
 
   skip_before_action :authenticate_user!
 
-  before_action :check_valid_page_param, :set_pagination_headers, unless: -> { has_version_param? }
+  before_action :check_valid_page_param, :set_pagination_headers
 
   feature_category :navigation
 
@@ -29,19 +29,11 @@ class WhatsNewController < ApplicationController
 
   def highlights
     strong_memoize(:highlights) do
-      if has_version_param?
-        ReleaseHighlight.for_version(version: params[:version])
-      else
-        ReleaseHighlight.paginated(page: current_page)
-      end
+      ReleaseHighlight.paginated(page: current_page)
     end
   end
 
   def set_pagination_headers
     response.set_header('X-Next-Page', highlights.next_page)
   end
-
-  def has_version_param?
-    params[:version].present?
-  end
 end

app/helpers/whats_new_helper.rb

@@ -5,15 +5,7 @@ module WhatsNewHelper
     ReleaseHighlight.most_recent_item_count
   end
 
-  def whats_new_storage_key
-    most_recent_version = ReleaseHighlight.versions&.first
-
-    return unless most_recent_version
-
-    ['display-whats-new-notification', most_recent_version].join('-')
-  end
-
-  def whats_new_versions
-    ReleaseHighlight.versions
+  def whats_new_version_digest
+    ReleaseHighlight.most_recent_version_digest
   end
 end

app/models/release_highlight.rb

@@ -3,17 +3,6 @@
 class ReleaseHighlight
   CACHE_DURATION = 1.hour
   FILES_PATH = Rails.root.join('data', 'whats_new', '*.yml')
-  RELEASE_VERSIONS_IN_A_YEAR = 12
-
-  def self.for_version(version:)
-    index = self.versions.index(version)
-
-    return if index.nil?
-
-    page = index + 1
-
-    self.paginated(page: page)
-  end
 
   def self.paginated(page: 1)
     key = self.cache_key("items:page-#{page}")
@@ -82,15 +71,15 @@ class ReleaseHighlight
     end
   end
 
-  def self.versions
-    key = self.cache_key('versions')
+  def self.most_recent_version_digest
+    key = self.cache_key('most_recent_version_digest')
 
     Gitlab::ProcessMemoryCache.cache_backend.fetch(key, expires_in: CACHE_DURATION) do
-      versions = self.file_paths.first(RELEASE_VERSIONS_IN_A_YEAR).map do |path|
-        /\d*\_(\d*\_\d*)\.yml$/.match(path).captures[0].gsub(/0(?=\d)/, "").tr("_", ".")
-      end
+      version = self.paginated&.items&.first&.[]('release')&.to_s
+
+      next if version.nil?
 
-      versions.uniq
+      Digest::SHA256.hexdigest(version)
     end
   end
 

app/views/layouts/header/_default.html.haml

@@ -120,7 +120,7 @@
         = sprite_icon('ellipsis_h', size: 12, css_class: 'more-icon js-navbar-toggle-right')
         = sprite_icon('close', size: 12, css_class: 'close-icon js-navbar-toggle-left')
 
-#whats-new-app{ data: { storage_key: whats_new_storage_key, versions: whats_new_versions, gitlab_dot_com: Gitlab.dev_env_org_or_com? } }
+#whats-new-app{ data: { version_digest: whats_new_version_digest } }
 
 - if can?(current_user, :update_user_status, current_user)
   .js-set-status-modal-wrapper{ data: user_status_data }

app/views/layouts/header/_whats_new_dropdown_item.html.haml

 %li
-  %button.gl-justify-content-space-between.gl-align-items-center.js-whats-new-trigger{ type: 'button', data: { storage_key: whats_new_storage_key }, class: 'gl-display-flex!' }
+  %button.gl-justify-content-space-between.gl-align-items-center.js-whats-new-trigger{ type: 'button', class: 'gl-display-flex!' }
     = _("What's new")
     %span.js-whats-new-notification-count.whats-new-notification-count
       = whats_new_most_recent_release_items_count

locale/gitlab.pot

@@ -35947,9 +35947,6 @@ msgstr ""
 msgid "Your U2F device was registered!"
 msgstr ""
 
-msgid "Your Version"
-msgstr ""
-
 msgid "Your WebAuthn device did not send a valid JSON response."
 msgstr ""
 

spec/frontend/fixtures/static/whats_new_notification.html

 <div class='whats-new-notification-fixture-root'>
-  <div class='app' data-storage-key='storage-key'></div>
+  <div class='app' data-version-digest='version-digest'></div>
   <div class='header-help'>
     <div class='js-whats-new-notification-count'></div>
   </div>

spec/frontend/whats_new/components/app_spec.js

-import { GlDrawer, GlInfiniteScroll, GlTabs } from '@gitlab/ui';
+import { GlDrawer, GlInfiniteScroll } from '@gitlab/ui';
 import { createLocalVue, mount } from '@vue/test-utils';
 import Vuex from 'vuex';
 import { mockTracking, unmockTracking, triggerEvent } from 'helpers/tracking_helper';
@@ -21,12 +21,9 @@ describe('App', () => {
   let actions;
   let state;
   let trackingSpy;
-  let gitlabDotCom = true;
 
   const buildProps = () => ({
-    storageKey: 'storage-key',
-    versions: ['3.11', '3.10'],
-    gitlabDotCom,
+    versionDigest: 'version-digest',
   });
 
   const buildWrapper = () => {
@@ -91,7 +88,7 @@ describe('App', () => {
     });
 
     it('dispatches openDrawer and tracking calls when mounted', () => {
-      expect(actions.openDrawer).toHaveBeenCalledWith(expect.any(Object), 'storage-key');
+      expect(actions.openDrawer).toHaveBeenCalledWith(expect.any(Object), 'version-digest');
       expect(trackingSpy).toHaveBeenCalledWith(undefined, 'click_whats_new_drawer', {
         label: 'namespace_id',
         value: 'namespace-840',
@@ -176,54 +173,4 @@ describe('App', () => {
       );
     });
   });
-
-  describe('self managed', () => {
-    const findTabs = () => wrapper.find(GlTabs);
-
-    const clickSecondTab = async () => {
-      const secondTab = wrapper.findAll('.nav-link').at(1);
-      await secondTab.trigger('click');
-      await new Promise((resolve) => requestAnimationFrame(resolve));
-    };
-
-    beforeEach(() => {
-      gitlabDotCom = false;
-      setup();
-    });
-
-    it('renders tabs with drawer body height and content', () => {
-      const scroll = findInfiniteScroll();
-      const tabs = findTabs();
-
-      expect(scroll.exists()).toBe(false);
-      expect(tabs.attributes().style).toBe(`height: ${MOCK_DRAWER_BODY_HEIGHT}px;`);
-      expect(wrapper.find('h5').text()).toBe('Whats New Drawer');
-    });
-
-    describe('fetchVersion', () => {
-      beforeEach(() => {
-        actions.fetchItems.mockClear();
-      });
-
-      it('when version isnt fetched, clicking a tab calls fetchItems', async () => {
-        const fetchVersionSpy = jest.spyOn(wrapper.vm, 'fetchVersion');
-        await clickSecondTab();
-
-        expect(fetchVersionSpy).toHaveBeenCalledWith('3.10');
-        expect(actions.fetchItems).toHaveBeenCalledWith(expect.anything(), { version: '3.10' });
-      });
-
-      it('when version has been fetched, clicking a tab calls fetchItems', async () => {
-        wrapper.vm.$store.state.features.push({ title: 'GitLab Stories', release: 3.1 });
-        await wrapper.vm.$nextTick();
-
-        const fetchVersionSpy = jest.spyOn(wrapper.vm, 'fetchVersion');
-        await clickSecondTab();
-
-        expect(fetchVersionSpy).toHaveBeenCalledWith('3.10');
-        expect(actions.fetchItems).not.toHaveBeenCalled();
-        expect(wrapper.find('.tab-pane.active h5').text()).toBe('GitLab Stories');
-      });
-    });
-  });
 });

spec/frontend/whats_new/store/actions_spec.js

@@ -11,9 +11,12 @@ describe('whats new actions', () => {
     useLocalStorageSpy();
 
     it('should commit openDrawer', () => {
-      testAction(actions.openDrawer, 'storage-key', {}, [{ type: types.OPEN_DRAWER }]);
+      testAction(actions.openDrawer, 'digest-hash', {}, [{ type: types.OPEN_DRAWER }]);
 
-      expect(window.localStorage.setItem).toHaveBeenCalledWith('storage-key', 'false');
+      expect(window.localStorage.setItem).toHaveBeenCalledWith(
+        'display-whats-new-notification',
+        'digest-hash',
+      );
     });
   });
 
@@ -45,12 +48,12 @@ describe('whats new actions', () => {
       axiosMock.reset();
 
       axiosMock
-        .onGet('/-/whats_new', { params: { page: 8, version: 40 } })
+        .onGet('/-/whats_new', { params: { page: 8 } })
         .replyOnce(200, [{ title: 'GitLab Stories' }]);
 
       testAction(
         actions.fetchItems,
-        { page: 8, version: 40 },
+        { page: 8 },
         {},
         expect.arrayContaining([
           { type: types.ADD_FEATURES, payload: [{ title: 'GitLab Stories' }] },

spec/frontend/whats_new/utils/notification_spec.js

 import { useLocalStorageSpy } from 'helpers/local_storage_helper';
-import { setNotification, getStorageKey } from '~/whats_new/utils/notification';
+import { setNotification, getVersionDigest } from '~/whats_new/utils/notification';
 
 describe('~/whats_new/utils/notification', () => {
   useLocalStorageSpy();
@@ -33,10 +33,23 @@ describe('~/whats_new/utils/notification', () => {
       expect(notificationEl.classList).toContain('with-notifications');
     });
 
-    it('removes class and count element when storage key is true', () => {
+    it('removes class and count element when legacy storage key is false', () => {
       const notificationEl = findNotificationEl();
       notificationEl.classList.add('with-notifications');
-      localStorage.setItem('storage-key', 'false');
+      localStorage.setItem('display-whats-new-notification-13.10', 'false');
+
+      expect(findNotificationCountEl()).toExist();
+
+      subject();
+
+      expect(findNotificationCountEl()).not.toExist();
+      expect(notificationEl.classList).not.toContain('with-notifications');
+    });
+
+    it('removes class and count element when storage key has current digest', () => {
+      const notificationEl = findNotificationEl();
+      notificationEl.classList.add('with-notifications');
+      localStorage.setItem('display-whats-new-notification', 'version-digest');
 
       expect(findNotificationCountEl()).toExist();
 
@@ -47,9 +60,9 @@ describe('~/whats_new/utils/notification', () => {
     });
   });
 
-  describe('getStorageKey', () => {
+  describe('getVersionDigest', () => {
     it('retrieves the storage key data attribute from the el', () => {
-      expect(getStorageKey(getAppEl())).toBe('storage-key');
+      expect(getVersionDigest(getAppEl())).toBe('version-digest');
     });
   });
 });

spec/helpers/whats_new_helper_spec.rb

@@ -3,25 +3,13 @@
 require 'spec_helper'
 
 RSpec.describe WhatsNewHelper do
-  describe '#whats_new_storage_key' do
-    subject { helper.whats_new_storage_key }
+  describe '#whats_new_version_digest' do
+    let(:digest) { 'digest' }
 
-    context 'when version exist' do
-      let(:release_item) { double(:item) }
+    it 'calls ReleaseHighlight.most_recent_version_digest' do
+      expect(ReleaseHighlight).to receive(:most_recent_version_digest).and_return(digest)
 
-      before do
-        allow(ReleaseHighlight).to receive(:versions).and_return([84.0])
-      end
-
-      it { is_expected.to eq('display-whats-new-notification-84.0') }
-    end
-
-    context 'when most recent release highlights do NOT exist' do
-      before do
-        allow(ReleaseHighlight).to receive(:versions).and_return(nil)
-      end
-
-      it { is_expected.to be_nil }
+      expect(helper.whats_new_version_digest).to eq(digest)
     end
   end
 
@@ -44,14 +32,4 @@ RSpec.describe WhatsNewHelper do
       end
     end
   end
-
-  describe '#whats_new_versions' do
-    let(:versions) { [84.0] }
-
-    it 'returns ReleaseHighlight.versions' do
-      expect(ReleaseHighlight).to receive(:versions).and_return(versions)
-
-      expect(helper.whats_new_versions).to eq(versions)
-    end
-  end
 end

spec/models/release_highlight_spec.rb

@@ -13,26 +13,6 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache do
     ReleaseHighlight.instance_variable_set(:@file_paths, nil)
   end
 
-  describe '.for_version' do
-    subject { ReleaseHighlight.for_version(version: version) }
-
-    let(:version) { '1.1' }
-
-    context 'with version param that exists' do
-      it 'returns items from that version' do
-        expect(subject.items.first['title']).to eq("It's gonna be a bright")
-      end
-    end
-
-    context 'with version param that does NOT exist' do
-      let(:version) { '84.0' }
-
-      it 'returns nil' do
-        expect(subject).to be_nil
-      end
-    end
-  end
-
   describe '.paginated' do
     let(:dot_com) { false }
 
@@ -143,28 +123,27 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache do
     end
   end
 
-  describe '.versions' do
-    subject { described_class.versions }
+  describe '.most_recent_version_digest' do
+    subject { ReleaseHighlight.most_recent_version_digest }
 
     it 'uses process memory cache' do
-      expect(Gitlab::ProcessMemoryCache.cache_backend).to receive(:fetch).with("release_highlight:versions:#{Gitlab.revision}", { expires_in: described_class::CACHE_DURATION })
+      expect(Gitlab::ProcessMemoryCache.cache_backend).to receive(:fetch).with("release_highlight:most_recent_version_digest:#{Gitlab.revision}", expires_in: described_class::CACHE_DURATION)
 
       subject
     end
 
-    it 'returns versions from the file paths' do
-      expect(subject).to eq(['1.5', '1.2', '1.1'])
+    context 'when recent release items exist' do
+      it 'returns a digest from the release of the first item of the most recent file' do
+        # this value is coming from fixture data
+        expect(subject).to eq(Digest::SHA256.hexdigest('01.05'))
+      end
     end
 
-    context 'when there are more than 12 versions' do
-      let(:file_paths) do
-        i = 0
-        Array.new(20) { "20201225_01_#{i += 1}.yml" }
-      end
+    context 'when recent release items do NOT exist' do
+      it 'returns nil' do
+        allow(ReleaseHighlight).to receive(:paginated).and_return(nil)
 
-      it 'limits to 12 versions' do
-        allow(ReleaseHighlight).to receive(:file_paths).and_return(file_paths)
-        expect(subject.count).to eq(12)
+        expect(subject).to be_nil
       end
     end
   end

spec/requests/whats_new_controller_spec.rb

@@ -35,16 +35,5 @@ RSpec.describe WhatsNewController, :clean_gitlab_redis_cache do
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
-
-    context 'with version param' do
-      it 'returns items without pagination headers' do
-        allow(ReleaseHighlight).to receive(:for_version).with(version: '42').and_return(highlights)
-
-        get whats_new_path(version: 42), xhr: true
-
-        expect(response.body).to eq(highlights.items.to_json)
-        expect(response.headers['X-Next-Page']).to be_nil
-      end
-    end
   end
 end