Merge branch 'jswain_whats_new_async' into 'master'

Make "Whats New" async

See merge request gitlab-org/gitlab!43202

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

 <script>
 import { mapState, mapActions } from 'vuex';
 import { GlDrawer, GlBadge, GlIcon, GlLink } from '@gitlab/ui';
+import SkeletonLoader from './skeleton_loader.vue';
 import Tracking from '~/tracking';
 
 const trackingMixin = Tracking.mixin();
@@ -11,14 +12,10 @@ export default {
     GlBadge,
     GlIcon,
     GlLink,
+    SkeletonLoader,
   },
   mixins: [trackingMixin],
   props: {
-    features: {
-      type: String,
-      required: false,
-      default: null,
-    },
     storageKey: {
       type: String,
       required: true,
@@ -26,21 +23,11 @@ export default {
     },
   },
   computed: {
-    ...mapState(['open']),
-    parsedFeatures() {
-      let features;
-
-      try {
-        features = JSON.parse(this.$props.features) || [];
-      } catch (err) {
-        features = [];
-      }
-
-      return features;
-    },
+    ...mapState(['open', 'features']),
   },
   mounted() {
     this.openDrawer(this.storageKey);
+    this.fetchItems();
 
     const body = document.querySelector('body');
     const namespaceId = body.getAttribute('data-namespace-id');
@@ -48,7 +35,7 @@ export default {
     this.track('click_whats_new_drawer', { label: 'namespace_id', value: namespaceId });
   },
   methods: {
-    ...mapActions(['openDrawer', 'closeDrawer']),
+    ...mapActions(['openDrawer', 'closeDrawer', 'fetchItems']),
   },
 };
 </script>
@@ -60,46 +47,52 @@ export default {
         <h4 class="page-title my-2">{{ __("What's new at GitLab") }}</h4>
       </template>
       <div class="pb-6">
-        <div v-for="feature in parsedFeatures" :key="feature.title" class="mb-6">
-          <gl-link
-            :href="feature.url"
-            target="_blank"
-            data-testid="whats-new-title-link"
-            data-track-event="click_whats_new_item"
-            :data-track-label="feature.title"
-            :data-track-property="feature.url"
-          >
-            <h5 class="gl-font-base">{{ feature.title }}</h5>
-          </gl-link>
-          <div class="mb-2">
-            <template v-for="package_name in feature.packages">
-              <gl-badge :key="package_name" size="sm" class="whats-new-item-badge mr-1">
-                <gl-icon name="license" />{{ package_name }}
-              </gl-badge>
-            </template>
+        <template v-if="features">
+          <div v-for="feature in features" :key="feature.title" class="mb-6">
+            <gl-link
+              :href="feature.url"
+              target="_blank"
+              data-testid="whats-new-title-link"
+              data-track-event="click_whats_new_item"
+              :data-track-label="feature.title"
+              :data-track-property="feature.url"
+            >
+              <h5 class="gl-font-base">{{ feature.title }}</h5>
+            </gl-link>
+            <div v-if="feature.packages" class="gl-mb-3">
+              <template v-for="package_name in feature.packages">
+                <gl-badge :key="package_name" size="sm" class="whats-new-item-badge gl-mr-2">
+                  <gl-icon name="license" />{{ package_name }}
+                </gl-badge>
+              </template>
+            </div>
+            <gl-link
+              :href="feature.url"
+              target="_blank"
+              data-track-event="click_whats_new_item"
+              :data-track-label="feature.title"
+              :data-track-property="feature.url"
+            >
+              <img
+                :alt="feature.title"
+                :src="feature.image_url"
+                class="img-thumbnail px-6 gl-py-3 whats-new-item-image"
+              />
+            </gl-link>
+            <p class="gl-pt-3">{{ feature.body }}</p>
+            <gl-link
+              :href="feature.url"
+              target="_blank"
+              data-track-event="click_whats_new_item"
+              :data-track-label="feature.title"
+              :data-track-property="feature.url"
+              >{{ __('Learn more') }}</gl-link
+            >
           </div>
-          <gl-link
-            :href="feature.url"
-            target="_blank"
-            data-track-event="click_whats_new_item"
-            :data-track-label="feature.title"
-            :data-track-property="feature.url"
-          >
-            <img
-              :alt="feature.title"
-              :src="feature.image_url"
-              class="img-thumbnail px-6 py-2 whats-new-item-image"
-            />
-          </gl-link>
-          <p class="pt-2">{{ feature.body }}</p>
-          <gl-link
-            :href="feature.url"
-            target="_blank"
-            data-track-event="click_whats_new_item"
-            :data-track-label="feature.title"
-            :data-track-property="feature.url"
-            >{{ __('Learn more') }}</gl-link
-          >
+        </template>
+        <div v-else class="gl-mt-5">
+          <skeleton-loader />
+          <skeleton-loader />
         </div>
       </div>
     </gl-drawer>

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

+<script>
+import { GlSkeletonLoader } from '@gitlab/ui';
+
+export default {
+  components: {
+    GlSkeletonLoader,
+  },
+};
+</script>
+
+<template>
+  <gl-skeleton-loader :width="350" :height="420">
+    <rect width="350" height="16" />
+    <rect y="25" width="110" height="16" rx="8" />
+    <rect x="115" y="25" width="110" height="16" rx="8" />
+    <rect x="230" y="25" width="110" height="16" rx="8" />
+    <rect y="50" width="350" height="165" rx="12" />
+    <rect y="230" width="480" height="8" />
+    <rect y="254" width="560" height="8" />
+    <rect y="278" width="320" height="8" />
+    <rect y="302" width="480" height="8" />
+    <rect y="326" width="560" height="8" />
+    <rect y="365" width="80" height="8" />
+  </gl-skeleton-loader>
+</template>

app/assets/javascripts/whats_new/index.js

@@ -19,7 +19,6 @@ export default () => {
       render(createElement) {
         return createElement('app', {
           props: {
-            features: whatsNewElm.getAttribute('data-features'),
             storageKey: whatsNewElm.getAttribute('data-storage-key'),
           },
         });

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

 import * as types from './mutation_types';
+import axios from '~/lib/utils/axios_utils';
 
 export default {
   closeDrawer({ commit }) {
@@ -11,4 +12,9 @@ export default {
       localStorage.setItem(storageKey, JSON.stringify(false));
     }
   },
+  fetchItems({ commit }) {
+    return axios.get('/-/whats_new').then(({ data }) => {
+      commit(types.SET_FEATURES, data);
+    });
+  },
 };

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

 export const CLOSE_DRAWER = 'CLOSE_DRAWER';
 export const OPEN_DRAWER = 'OPEN_DRAWER';
+export const SET_FEATURES = 'SET_FEATURES';

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

@@ -7,4 +7,7 @@ export default {
   [types.OPEN_DRAWER](state) {
     state.open = true;
   },
+  [types.SET_FEATURES](state, data) {
+    state.features = data;
+  },
 };

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

 export default {
   open: false,
+  features: null,
 };

app/controllers/whats_new_controller.rb

+# frozen_string_literal: true
+
+class WhatsNewController < ApplicationController
+  include Gitlab::WhatsNew
+
+  skip_before_action :authenticate_user!
+
+  before_action :check_feature_flag
+
+  feature_category :navigation
+
+  def index
+    respond_to do |format|
+      format.js do
+        render json: whats_new_most_recent_release_items
+      end
+    end
+  end
+
+  private
+
+  def check_feature_flag
+    render_404 unless Feature.enabled?(:whats_new_drawer, current_user)
+  end
+end

app/helpers/whats_new_helper.rb

 # frozen_string_literal: true
 
 module WhatsNewHelper
-  EMPTY_JSON = ''.to_json
+  include Gitlab::WhatsNew
 
   def whats_new_most_recent_release_items_count
-    items = parsed_most_recent_release_items
-
-    return unless items.is_a?(Array)
-
-    items.count
+    Gitlab::ProcessMemoryCache.cache_backend.fetch('whats_new:release_items_count', expires_in: CACHE_DURATION) do
+      whats_new_most_recent_release_items&.count
+    end
   end
 
   def whats_new_storage_key
-    items = parsed_most_recent_release_items
-
-    return unless items.is_a?(Array)
-
-    release = items.first.try(:[], 'release')
-
-    ['display-whats-new-notification', release].compact.join('-')
-  end
+    return unless whats_new_most_recent_version
 
-  def whats_new_most_recent_release_items
-    YAML.load_file(most_recent_release_file_path).to_json
-
-  rescue => e
-    Gitlab::ErrorTracking.track_exception(e, yaml_file_path: most_recent_release_file_path)
-
-    EMPTY_JSON
+    ['display-whats-new-notification', whats_new_most_recent_version].join('-')
   end
 
   private
 
-  def parsed_most_recent_release_items
-    Gitlab::Json.parse(whats_new_most_recent_release_items)
-  end
-
-  def most_recent_release_file_path
-    Dir.glob(files_path).max
-  end
-
-  def files_path
-    Rails.root.join('data', 'whats_new', '*.yml')
+  def whats_new_most_recent_version
+    Gitlab::ProcessMemoryCache.cache_backend.fetch('whats_new:release_version', expires_in: CACHE_DURATION) do
+      if whats_new_most_recent_release_items
+        whats_new_most_recent_release_items.first.try(:[], 'release')
+      end
+    end
   end
 end

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

@@ -99,8 +99,8 @@
         = 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')
 
-- if ::Feature.enabled?(:whats_new_drawer)
-  #whats-new-app{ data: { features: whats_new_most_recent_release_items, storage_key: whats_new_storage_key } }
+- if ::Feature.enabled?(:whats_new_drawer, current_user)
+  #whats-new-app{ data: { storage_key: whats_new_storage_key } }
 
 - if can?(current_user, :update_user_status, current_user)
   .js-set-status-modal-wrapper{ data: { current_emoji: current_user.status.present? ? current_user.status.emoji : '', current_message: current_user.status.present? ? current_user.status.message : '' } }

config/routes.rb

@@ -83,6 +83,8 @@ Rails.application.routes.draw do
       get '/autocomplete/namespace_routes' => 'autocomplete#namespace_routes'
     end
 
+    get '/whats_new' => 'whats_new#index'
+
     # '/-/health' implemented by BasicHealthCheck middleware
     get 'liveness' => 'health#liveness'
     get 'readiness' => 'health#readiness'

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

-- if ::Feature.enabled?(:whats_new_dropdown)
-  - if ::Feature.enabled?(:whats_new_drawer)
+- if ::Feature.enabled?(:whats_new_dropdown, current_user)
+  - if ::Feature.enabled?(:whats_new_drawer, current_user)
     %li
       %button.js-whats-new-trigger{ type: 'button', data: { storage_key: whats_new_storage_key } }
         = _("See what's new at GitLab")

lib/gitlab/whats_new.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module WhatsNew
+    CACHE_DURATION = 1.day
+    WHATS_NEW_FILES_PATH = Rails.root.join('data', 'whats_new', '*.yml')
+
+    private
+
+    def whats_new_most_recent_release_items
+      Rails.cache.fetch('whats_new:release_items', expires_in: CACHE_DURATION) do
+        file = File.read(most_recent_release_file_path)
+
+        items = YAML.safe_load(file, permitted_classes: [Date])
+
+        items if items.is_a?(Array)
+      end
+    rescue => e
+      Gitlab::ErrorTracking.track_exception(e, yaml_file_path: most_recent_release_file_path)
+
+      nil
+    end
+
+    def most_recent_release_file_path
+      @most_recent_release_file_path ||= Dir.glob(WHATS_NEW_FILES_PATH).max
+    end
+  end
+end

spec/frontend/whats_new/components/app_spec.js

@@ -8,21 +8,23 @@ const localVue = createLocalVue();
 localVue.use(Vuex);
 
 describe('App', () => {
+  const propsData = { storageKey: 'storage-key' };
   let wrapper;
   let store;
   let actions;
   let state;
-  let propsData = { features: '[ {"title":"Whats New Drawer"} ]', storageKey: 'storage-key' };
   let trackingSpy;
 
   const buildWrapper = () => {
     actions = {
       openDrawer: jest.fn(),
       closeDrawer: jest.fn(),
+      fetchItems: jest.fn(),
     };
 
     state = {
       open: true,
+      features: null,
     };
 
     store = new Vuex.Store({
@@ -37,12 +39,15 @@ describe('App', () => {
     });
   };
 
-  beforeEach(() => {
+  beforeEach(async () => {
     document.body.dataset.page = 'test-page';
     document.body.dataset.namespaceId = 'namespace-840';
 
     trackingSpy = mockTracking('_category_', null, jest.spyOn);
     buildWrapper();
+
+    wrapper.vm.$store.state.features = [{ title: 'Whats New Drawer', url: 'www.url.com' }];
+    await wrapper.vm.$nextTick();
   });
 
   afterEach(() => {
@@ -77,29 +82,18 @@ describe('App', () => {
     expect(getDrawer().props('open')).toBe(openState);
   });
 
-  it('renders features when provided as props', () => {
+  it('renders features when provided via ajax', () => {
+    expect(actions.fetchItems).toHaveBeenCalled();
     expect(wrapper.find('h5').text()).toBe('Whats New Drawer');
   });
 
-  it('handles bad json argument gracefully', () => {
-    propsData = { features: 'this is not json', storageKey: 'storage-key' };
-    buildWrapper();
-
-    expect(getDrawer().exists()).toBe(true);
-  });
-
   it('send an event when feature item is clicked', () => {
-    propsData = {
-      features: '[ {"title":"Whats New Drawer", "url": "www.url.com"} ]',
-      storageKey: 'storage-key',
-    };
-    buildWrapper();
     trackingSpy = mockTracking('_category_', wrapper.element, jest.spyOn);
 
     const link = wrapper.find('[data-testid="whats-new-title-link"]');
     triggerEvent(link.element);
 
-    expect(trackingSpy.mock.calls[2]).toMatchObject([
+    expect(trackingSpy.mock.calls[1]).toMatchObject([
       '_category_',
       'click_whats_new_item',
       {

spec/frontend/whats_new/store/actions_spec.js

 import testAction from 'helpers/vuex_action_helper';
 import { useLocalStorageSpy } from 'helpers/local_storage_helper';
+import MockAdapter from 'axios-mock-adapter';
+import waitForPromises from 'helpers/wait_for_promises';
 import actions from '~/whats_new/store/actions';
 import * as types from '~/whats_new/store/mutation_types';
+import axios from '~/lib/utils/axios_utils';
 
 describe('whats new actions', () => {
   describe('openDrawer', () => {
@@ -19,4 +22,27 @@ describe('whats new actions', () => {
       testAction(actions.closeDrawer, {}, {}, [{ type: types.CLOSE_DRAWER }]);
     });
   });
+
+  describe('fetchItems', () => {
+    let axiosMock;
+
+    beforeEach(async () => {
+      axiosMock = new MockAdapter(axios);
+      axiosMock
+        .onGet('/-/whats_new')
+        .replyOnce(200, [{ title: 'Whats New Drawer', url: 'www.url.com' }]);
+
+      await waitForPromises();
+    });
+
+    afterEach(() => {
+      axiosMock.restore();
+    });
+
+    it('should commit setFeatures', () => {
+      testAction(actions.fetchItems, {}, {}, [
+        { type: types.SET_FEATURES, payload: [{ title: 'Whats New Drawer', url: 'www.url.com' }] },
+      ]);
+    });
+  });
 });

spec/frontend/whats_new/store/mutations_spec.js

@@ -22,4 +22,11 @@ describe('whats new mutations', () => {
       expect(state.open).toBe(false);
     });
   });
+
+  describe('setFeatures', () => {
+    it('sets features to data', () => {
+      mutations[types.SET_FEATURES](state, 'bells and whistles');
+      expect(state.features).toBe('bells and whistles');
+    });
+  });
 });

spec/helpers/whats_new_helper_spec.rb

@@ -7,23 +7,17 @@ RSpec.describe WhatsNewHelper do
     subject { helper.whats_new_storage_key }
 
     before do
-      allow(helper).to receive(:whats_new_most_recent_release_items).and_return(json)
+      allow(helper).to receive(:whats_new_most_recent_version).and_return(version)
     end
 
-    context 'when recent release items exist' do
-      let(:json) { [{ release: 84.0 }].to_json }
+    context 'when version exist' do
+      let(:version) { '84.0' }
 
       it { is_expected.to eq('display-whats-new-notification-84.0') }
-
-      context 'when the release items are missing the release key' do
-        let(:json) { [{ title: 'bells!' }].to_json }
-
-        it { is_expected.to eq('display-whats-new-notification') }
-      end
     end
 
     context 'when recent release items do NOT exist' do
-      let(:json) { WhatsNewHelper::EMPTY_JSON }
+      let(:version) { nil }
 
       it { is_expected.to be_nil }
     end
@@ -32,37 +26,26 @@ RSpec.describe WhatsNewHelper do
   describe '#whats_new_most_recent_release_items_count' do
     subject { helper.whats_new_most_recent_release_items_count }
 
-    before do
-      allow(helper).to receive(:whats_new_most_recent_release_items).and_return(json)
-    end
-
     context 'when recent release items exist' do
-      let(:json) { [:bells, :and, :whistles].to_json }
+      let(:fixture_dir_glob) { Dir.glob(File.join('spec', 'fixtures', 'whats_new', '*.yml')) }
 
-      it { is_expected.to eq(3) }
-    end
-
-    context 'when recent release items do NOT exist' do
-      let(:json) { WhatsNewHelper::EMPTY_JSON }
+      it 'returns the count from the most recent file' do
+        expect(Dir).to receive(:glob).with(Rails.root.join('data', 'whats_new', '*.yml')).and_return(fixture_dir_glob)
 
-      it { is_expected.to be_nil }
+        expect(subject).to eq(1)
+      end
     end
-  end
-
-  describe '#whats_new_most_recent_release_items' do
-    let(:fixture_dir_glob) { Dir.glob(File.join('spec', 'fixtures', 'whats_new', '*.yml')) }
-
-    it 'returns json from the most recent file' do
-      allow(Dir).to receive(:glob).with(Rails.root.join('data', 'whats_new', '*.yml')).and_return(fixture_dir_glob)
 
-      expect(helper.whats_new_most_recent_release_items).to include({ title: "bright and sunshinin' day" }.to_json)
-    end
+    context 'when recent release items do NOT exist' do
+      before do
+        allow(YAML).to receive(:safe_load).and_raise
 
-    it 'fails gracefully and logs an error' do
-      allow(YAML).to receive(:load_file).and_raise
+        expect(Gitlab::ErrorTracking).to receive(:track_exception)
+      end
 
-      expect(Gitlab::ErrorTracking).to receive(:track_exception)
-      expect(helper.whats_new_most_recent_release_items).to eq(''.to_json)
+      it 'fails gracefully and logs an error' do
+        expect(subject).to be_nil
+      end
     end
   end
 end

spec/requests/whats_new_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe WhatsNewController do
+  describe 'whats_new_path' do
+    before do
+      allow_any_instance_of(WhatsNewController).to receive(:whats_new_most_recent_release_items).and_return('items')
+    end
+
+    context 'with whats_new_drawer feature enabled' do
+      before do
+        stub_feature_flags(whats_new_drawer: true)
+      end
+
+      it 'is successful' do
+        get whats_new_path, xhr: true
+
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+    end
+
+    context 'with whats_new_drawer feature disabled' do
+      before do
+        stub_feature_flags(whats_new_drawer: false)
+      end
+
+      it 'returns a 404' do
+        get whats_new_path, xhr: true
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
+end