Merge branch 'deprecation-warning-for-dynamic-milestones' into 'master'

Deprecation warning for dynamic milestones

Closes #42336

See merge request gitlab-org/gitlab-ce!17505

5 files changed, 218 insertions, 168 deletions

diff --git a/spec/features/milestone_spec.rb b/spec/features/milestone_spec.rb
index 19152bf1f0f..6c51e4bbe26 100644
--- a/spec/features/milestone_spec.rb
+++ b/spec/features/milestone_spec.rb
@@ -108,4 +108,18 @@ feature 'Milestone' do
       expect(page).to have_selector('.js-delete-milestone-button', count: 0)
     end
   end
+
+  feature 'deprecation popover', :js do
+    it 'opens deprecation popover' do
+      milestone = create(:milestone, project: project)
+
+      visit group_milestone_path(group, milestone, title: milestone.title)
+
+      expect(page).to have_selector('.milestone-deprecation-message')
+
+      find('.milestone-deprecation-message .js-popover-link').click
+
+      expect(page).to have_selector('.milestone-deprecation-message .popover')
+    end
+  end
 end
diff --git a/spec/javascripts/feature_highlight/feature_highlight_helper_spec.js b/spec/javascripts/feature_highlight/feature_highlight_helper_spec.js
index 480c138b9db..2ab6a0077b5 100644
--- a/spec/javascripts/feature_highlight/feature_highlight_helper_spec.js
+++ b/spec/javascripts/feature_highlight/feature_highlight_helper_spec.js
@@ -3,12 +3,11 @@ import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
 import {
   getSelector,
-  togglePopover,
   dismiss,
-  mouseleave,
-  mouseenter,
   inserted,
 } from '~/feature_highlight/feature_highlight_helper';
+import { togglePopover } from '~/shared/popover';
+
 import getSetTimeoutPromise from 'spec/helpers/set_timeout_promise_helper';
 
 describe('feature highlight helper', () => {
@@ -19,110 +18,6 @@ describe('feature highlight helper', () => {
     });
   });
 
-  describe('togglePopover', () => {
-    describe('togglePopover(true)', () => {
-      it('returns true when popover is shown', () => {
-        const context = {
-          hasClass: () => false,
-          popover: () => {},
-          toggleClass: () => {},
-        };
-
-        expect(togglePopover.call(context, true)).toEqual(true);
-      });
-
-      it('returns false when popover is already shown', () => {
-        const context = {
-          hasClass: () => true,
-        };
-
-        expect(togglePopover.call(context, true)).toEqual(false);
-      });
-
-      it('shows popover', (done) => {
-        const context = {
-          hasClass: () => false,
-          popover: () => {},
-          toggleClass: () => {},
-        };
-
-        spyOn(context, 'popover').and.callFake((method) => {
-          expect(method).toEqual('show');
-          done();
-        });
-
-        togglePopover.call(context, true);
-      });
-
-      it('adds disable-animation and js-popover-show class', (done) => {
-        const context = {
-          hasClass: () => false,
-          popover: () => {},
-          toggleClass: () => {},
-        };
-
-        spyOn(context, 'toggleClass').and.callFake((classNames, show) => {
-          expect(classNames).toEqual('disable-animation js-popover-show');
-          expect(show).toEqual(true);
-          done();
-        });
-
-        togglePopover.call(context, true);
-      });
-    });
-
-    describe('togglePopover(false)', () => {
-      it('returns true when popover is hidden', () => {
-        const context = {
-          hasClass: () => true,
-          popover: () => {},
-          toggleClass: () => {},
-        };
-
-        expect(togglePopover.call(context, false)).toEqual(true);
-      });
-
-      it('returns false when popover is already hidden', () => {
-        const context = {
-          hasClass: () => false,
-        };
-
-        expect(togglePopover.call(context, false)).toEqual(false);
-      });
-
-      it('hides popover', (done) => {
-        const context = {
-          hasClass: () => true,
-          popover: () => {},
-          toggleClass: () => {},
-        };
-
-        spyOn(context, 'popover').and.callFake((method) => {
-          expect(method).toEqual('hide');
-          done();
-        });
-
-        togglePopover.call(context, false);
-      });
-
-      it('removes disable-animation and js-popover-show class', (done) => {
-        const context = {
-          hasClass: () => true,
-          popover: () => {},
-          toggleClass: () => {},
-        };
-
-        spyOn(context, 'toggleClass').and.callFake((classNames, show) => {
-          expect(classNames).toEqual('disable-animation js-popover-show');
-          expect(show).toEqual(false);
-          done();
-        });
-
-        togglePopover.call(context, false);
-      });
-    });
-  });
-
   describe('dismiss', () => {
     let mock;
     const context = {
@@ -163,56 +58,6 @@ describe('feature highlight helper', () => {
     });
   });
 
-  describe('mouseleave', () => {
-    it('calls hide popover if .popover:hover is false', () => {
-      const fakeJquery = {
-        length: 0,
-      };
-
-      spyOn($.fn, 'init').and.callFake(selector => (selector === '.popover:hover' ? fakeJquery : $.fn));
-      spyOn(togglePopover, 'call');
-      mouseleave();
-      expect(togglePopover.call).toHaveBeenCalledWith(jasmine.any(Object), false);
-    });
-
-    it('does not call hide popover if .popover:hover is true', () => {
-      const fakeJquery = {
-        length: 1,
-      };
-
-      spyOn($.fn, 'init').and.callFake(selector => (selector === '.popover:hover' ? fakeJquery : $.fn));
-      spyOn(togglePopover, 'call');
-      mouseleave();
-      expect(togglePopover.call).not.toHaveBeenCalledWith(false);
-    });
-  });
-
-  describe('mouseenter', () => {
-    const context = {};
-
-    it('shows popover', () => {
-      spyOn(togglePopover, 'call').and.returnValue(false);
-      mouseenter.call(context);
-      expect(togglePopover.call).toHaveBeenCalledWith(jasmine.any(Object), true);
-    });
-
-    it('registers mouseleave event if popover is showed', (done) => {
-      spyOn(togglePopover, 'call').and.returnValue(true);
-      spyOn($.fn, 'on').and.callFake((eventName) => {
-        expect(eventName).toEqual('mouseleave');
-        done();
-      });
-      mouseenter.call(context);
-    });
-
-    it('does not register mouseleave event if popover is not showed', () => {
-      spyOn(togglePopover, 'call').and.returnValue(false);
-      const spy = spyOn($.fn, 'on').and.callFake(() => {});
-      mouseenter.call(context);
-      expect(spy).not.toHaveBeenCalled();
-    });
-  });
-
   describe('inserted', () => {
     it('registers click event callback', (done) => {
       const context = {
diff --git a/spec/javascripts/feature_highlight/feature_highlight_spec.js b/spec/javascripts/feature_highlight/feature_highlight_spec.js
index d2dd39d49d1..ec46d4f905a 100644
--- a/spec/javascripts/feature_highlight/feature_highlight_spec.js
+++ b/spec/javascripts/feature_highlight/feature_highlight_spec.js
@@ -1,6 +1,6 @@
 import $ from 'jquery';
-import * as featureHighlightHelper from '~/feature_highlight/feature_highlight_helper';
 import * as featureHighlight from '~/feature_highlight/feature_highlight';
+import * as popover from '~/shared/popover';
 import axios from '~/lib/utils/axios_utils';
 import MockAdapter from 'axios-mock-adapter';
 
@@ -29,7 +29,6 @@ describe('feature highlight', () => {
       mock = new MockAdapter(axios);
       mock.onGet('/test').reply(200);
       spyOn(window, 'addEventListener');
-      spyOn(window, 'removeEventListener');
       featureHighlight.setupFeatureHighlightPopover('test', 0);
     });
 
@@ -45,14 +44,14 @@ describe('feature highlight', () => {
     });
 
     it('setup mouseenter', () => {
-      const toggleSpy = spyOn(featureHighlightHelper.togglePopover, 'call');
+      const toggleSpy = spyOn(popover.togglePopover, 'call');
       $(selector).trigger('mouseenter');
 
       expect(toggleSpy).toHaveBeenCalledWith(jasmine.any(Object), true);
     });
 
     it('setup debounced mouseleave', (done) => {
-      const toggleSpy = spyOn(featureHighlightHelper.togglePopover, 'call');
+      const toggleSpy = spyOn(popover.togglePopover, 'call');
       $(selector).trigger('mouseleave');
 
       // Even though we've set the debounce to 0ms, setTimeout is needed for the debounce
@@ -64,12 +63,7 @@ describe('feature highlight', () => {
 
     it('setup show.bs.popover', () => {
       $(selector).trigger('show.bs.popover');
-      expect(window.addEventListener).toHaveBeenCalledWith('scroll', jasmine.any(Function));
-    });
-
-    it('setup hide.bs.popover', () => {
-      $(selector).trigger('hide.bs.popover');
-      expect(window.removeEventListener).toHaveBeenCalledWith('scroll', jasmine.any(Function));
+      expect(window.addEventListener).toHaveBeenCalledWith('scroll', jasmine.any(Function), { once: true });
     });
 
     it('removes disabled attribute', () => {
@@ -85,7 +79,7 @@ describe('feature highlight', () => {
     it('toggles when clicked', () => {
       $(selector).trigger('mouseenter');
       const popoverId = $(selector).attr('aria-describedby');
-      const toggleSpy = spyOn(featureHighlightHelper.togglePopover, 'call');
+      const toggleSpy = spyOn(popover.togglePopover, 'call');
 
       $(`#${popoverId} .dismiss-feature-highlight`).click();
 
diff --git a/spec/javascripts/shared/popover_spec.js b/spec/javascripts/shared/popover_spec.js
new file mode 100644
index 00000000000..1d574c9424b
--- /dev/null
+++ b/spec/javascripts/shared/popover_spec.js
@@ -0,0 +1,162 @@
+import $ from 'jquery';
+import {
+  togglePopover,
+  mouseleave,
+  mouseenter,
+} from '~/shared/popover';
+
+describe('popover', () => {
+  describe('togglePopover', () => {
+    describe('togglePopover(true)', () => {
+      it('returns true when popover is shown', () => {
+        const context = {
+          hasClass: () => false,
+          popover: () => {},
+          toggleClass: () => {},
+        };
+
+        expect(togglePopover.call(context, true)).toEqual(true);
+      });
+
+      it('returns false when popover is already shown', () => {
+        const context = {
+          hasClass: () => true,
+        };
+
+        expect(togglePopover.call(context, true)).toEqual(false);
+      });
+
+      it('shows popover', (done) => {
+        const context = {
+          hasClass: () => false,
+          popover: () => {},
+          toggleClass: () => {},
+        };
+
+        spyOn(context, 'popover').and.callFake((method) => {
+          expect(method).toEqual('show');
+          done();
+        });
+
+        togglePopover.call(context, true);
+      });
+
+      it('adds disable-animation and js-popover-show class', (done) => {
+        const context = {
+          hasClass: () => false,
+          popover: () => {},
+          toggleClass: () => {},
+        };
+
+        spyOn(context, 'toggleClass').and.callFake((classNames, show) => {
+          expect(classNames).toEqual('disable-animation js-popover-show');
+          expect(show).toEqual(true);
+          done();
+        });
+
+        togglePopover.call(context, true);
+      });
+    });
+
+    describe('togglePopover(false)', () => {
+      it('returns true when popover is hidden', () => {
+        const context = {
+          hasClass: () => true,
+          popover: () => {},
+          toggleClass: () => {},
+        };
+
+        expect(togglePopover.call(context, false)).toEqual(true);
+      });
+
+      it('returns false when popover is already hidden', () => {
+        const context = {
+          hasClass: () => false,
+        };
+
+        expect(togglePopover.call(context, false)).toEqual(false);
+      });
+
+      it('hides popover', (done) => {
+        const context = {
+          hasClass: () => true,
+          popover: () => {},
+          toggleClass: () => {},
+        };
+
+        spyOn(context, 'popover').and.callFake((method) => {
+          expect(method).toEqual('hide');
+          done();
+        });
+
+        togglePopover.call(context, false);
+      });
+
+      it('removes disable-animation and js-popover-show class', (done) => {
+        const context = {
+          hasClass: () => true,
+          popover: () => {},
+          toggleClass: () => {},
+        };
+
+        spyOn(context, 'toggleClass').and.callFake((classNames, show) => {
+          expect(classNames).toEqual('disable-animation js-popover-show');
+          expect(show).toEqual(false);
+          done();
+        });
+
+        togglePopover.call(context, false);
+      });
+    });
+  });
+
+  describe('mouseleave', () => {
+    it('calls hide popover if .popover:hover is false', () => {
+      const fakeJquery = {
+        length: 0,
+      };
+
+      spyOn($.fn, 'init').and.callFake(selector => (selector === '.popover:hover' ? fakeJquery : $.fn));
+      spyOn(togglePopover, 'call');
+      mouseleave();
+      expect(togglePopover.call).toHaveBeenCalledWith(jasmine.any(Object), false);
+    });
+
+    it('does not call hide popover if .popover:hover is true', () => {
+      const fakeJquery = {
+        length: 1,
+      };
+
+      spyOn($.fn, 'init').and.callFake(selector => (selector === '.popover:hover' ? fakeJquery : $.fn));
+      spyOn(togglePopover, 'call');
+      mouseleave();
+      expect(togglePopover.call).not.toHaveBeenCalledWith(false);
+    });
+  });
+
+  describe('mouseenter', () => {
+    const context = {};
+
+    it('shows popover', () => {
+      spyOn(togglePopover, 'call').and.returnValue(false);
+      mouseenter.call(context);
+      expect(togglePopover.call).toHaveBeenCalledWith(jasmine.any(Object), true);
+    });
+
+    it('registers mouseleave event if popover is showed', (done) => {
+      spyOn(togglePopover, 'call').and.returnValue(true);
+      spyOn($.fn, 'on').and.callFake((eventName) => {
+        expect(eventName).toEqual('mouseleave');
+        done();
+      });
+      mouseenter.call(context);
+    });
+
+    it('does not register mouseleave event if popover is not showed', () => {
+      spyOn(togglePopover, 'call').and.returnValue(false);
+      const spy = spyOn($.fn, 'on').and.callFake(() => {});
+      mouseenter.call(context);
+      expect(spy).not.toHaveBeenCalled();
+    });
+  });
+});
diff --git a/spec/views/shared/milestones/_top.html.haml.rb b/spec/views/shared/milestones/_top.html.haml.rb
new file mode 100644
index 00000000000..516d81c87ac
--- /dev/null
+++ b/spec/views/shared/milestones/_top.html.haml.rb
@@ -0,0 +1,35 @@
+require 'spec_helper'
+
+describe 'shared/milestones/_top.html.haml' do
+  set(:group) { create(:group) }
+  let(:project) { create(:project, group: group) }
+  let(:milestone) { create(:milestone, project: project) }
+
+  before do
+    allow(milestone).to receive(:milestones) { [] }
+  end
+
+  it 'renders a deprecation message for a legacy milestone' do
+    allow(milestone).to receive(:legacy_group_milestone?) { true }
+
+    render 'shared/milestones/top', milestone: milestone
+
+    expect(rendered).to have_css('.milestone-deprecation-message')
+  end
+
+  it 'renders a deprecation message for a dashboard milestone' do
+    allow(milestone).to receive(:dashboard_milestone?) { true }
+
+    render 'shared/milestones/top', milestone: milestone
+
+    expect(rendered).to have_css('.milestone-deprecation-message')
+  end
+
+  it 'does not render a deprecation message for a non-legacy and non-dashboard milestone' do
+    assign :group, group
+
+    render 'shared/milestones/top', milestone: milestone
+
+    expect(rendered).not_to have_css('.milestone-deprecation-message')
+  end
+end