Fixes #3330 - Scrolling to Article by passing ID to URL is broken.

app/assets/javascripts/app/controllers/ticket_zoom.coffee

@@ -282,24 +282,23 @@ class App.TicketZoom extends App.Controller
       article_id = @pagePositionData
       @pagePositionData = undefined
 
-    # trigger shown to article
-    if !@shown
-      @shown = true
-      App.Event.trigger('ui::ticket::shown', { ticket_id: @ticket_id })
-      @scrollToPosition('bottom', 50, article_id)
-      return
-
     # scroll to article if given
     if article_id && article_id isnt @last_article_id
-      @last_article_id = article_id
       @scrollToPosition('article', 300, article_id)
-      return
 
     # scroll to end if new article has been added
-    if !@last_ticket_article_ids || !_.isEqual(_.sortBy(@last_ticket_article_ids), _.sortBy(@ticket_article_ids))
-      @last_ticket_article_ids = @ticket_article_ids
+    else if !@last_ticket_article_ids || !_.isEqual(_.sortBy(@last_ticket_article_ids), _.sortBy(@ticket_article_ids))
       @scrollToPosition('bottom', 100, article_id)
-      return
+
+    # trigger shown to article
+    else if !@shown
+      App.Event.trigger('ui::ticket::shown', { ticket_id: @ticket_id })
+      @scrollToPosition('bottom', 50, article_id)
+
+    # save page position state
+    @shown                   = true
+    @last_ticket_article_ids = @ticket_article_ids
+    @last_article_id         = article_id
 
   setPosition: (position) =>
     @$('.main').scrollTop(position)

spec/support/capybara/custom_extensions.rb

@@ -16,11 +16,11 @@ class Capybara::Node::Element
   #   still moving after max number of checks was passed
   #
   # @return [Capybara::Node::Element] the element/node
-  def in_fixed_position(checks: 100)
+  def in_fixed_position(checks: 100, wait: 0.2)
     previous = native.location
 
     (checks + 1).times do
-      sleep 0.2
+      sleep wait
 
       current = native.location
 

spec/system/ticket/zoom_spec.rb

@@ -1266,6 +1266,61 @@ RSpec.describe 'Ticket zoom', type: :system do
     end
   end
 
+  describe 'Article ID in URL' do
+    let(:ticket) { create(:ticket, group: Group.first) }
+    let(:article_count) { 20 }
+    let(:article_at_the_top) { ticket.articles.first }
+    let(:article_in_the_middle) { ticket.articles[ article_count / 2 ] }
+    let(:article_at_the_bottom) { ticket.articles.last }
+
+    before do
+      article_count.times do
+        create(:'ticket/article', ticket: ticket, body: SecureRandom.uuid)
+      end
+    end
+
+    it 'scrolls to given Article ID' do
+      ensure_websocket do
+        visit "ticket/zoom/#{ticket.id}/#{article_in_the_middle.id}"
+        await_empty_ajax_queue
+        # workaround because browser scrolls in test initially to the bottom
+        # maybe because the articles are not present?!
+        refresh
+
+        # scroll to article in the middle of the page
+        within :active_content do
+          find("div#article-content-#{article_in_the_middle.id}").in_fixed_position(wait: 0.5)
+
+          expect(find("div#article-content-#{article_at_the_top.id}")).to be_obscured
+          expect(find("div#article-content-#{article_in_the_middle.id}")).not_to be_obscured
+          expect(find("div#article-content-#{article_at_the_bottom.id}")).to be_obscured
+        end
+
+        # scroll to article at the top of the page
+        visit "ticket/zoom/#{ticket.id}/#{article_at_the_top.id}"
+        await_empty_ajax_queue
+        within :active_content do
+          find("div#article-content-#{article_in_the_middle.id}").in_fixed_position(wait: 0.5)
+
+          expect(find("div#article-content-#{article_at_the_top.id}")).not_to be_obscured
+          expect(find("div#article-content-#{article_in_the_middle.id}")).to be_obscured
+          expect(find("div#article-content-#{article_at_the_bottom.id}")).to be_obscured
+        end
+
+        # scroll to article at the bottom of the page
+        visit "ticket/zoom/#{ticket.id}/#{article_at_the_bottom.id}"
+        await_empty_ajax_queue
+        within :active_content do
+          find("div#article-content-#{article_in_the_middle.id}").in_fixed_position(wait: 0.5)
+
+          expect(find("div#article-content-#{article_at_the_top.id}")).to be_obscured
+          expect(find("div#article-content-#{article_in_the_middle.id}")).to be_obscured
+          expect(find("div#article-content-#{article_at_the_bottom.id}")).not_to be_obscured
+        end
+      end
+    end
+  end
+
   describe 'Macros', authenticated_as: :authenticate do
     let(:macro) { create :macro, perform: { 'article.note'=>{ 'body' => 'macro <b>body</b>', 'internal' => 'true', 'subject' => 'macro note' } } }
     let!(:ticket) { create(:ticket, group: Group.find_by(name: 'Users')) }