Merge branch '22947-fix_issues_atom_feed_url' into 'master'

Issues atom feed url reflect filters on dashboard

Makes the issue atom feed url reflect the selected filters on the issue dashboard page and project's issues page.

## Are there points in the code the reviewer needs to double check?

The solution isn't based on javascript and filters functionality but on the current url parameters. It didn't seem right especially when writing tests.

## Why was this MR needed?

After filtering the issues, users may press the feed button and see a different list of issues to what is shown on the HTML page

Closes #22947

See merge request !7259

app/views/dashboard/issues.atom.builder

@@ -1,7 +1,7 @@
 xml.instruct!
 xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://search.yahoo.com/mrss/" do
   xml.title   "#{current_user.name} issues"
-  xml.link    href: issues_dashboard_url(format: :atom, private_token: current_user.try(:private_token)), rel: "self", type: "application/atom+xml"
+  xml.link    href: url_for(params), rel: "self", type: "application/atom+xml"
   xml.link    href: issues_dashboard_url, rel: "alternate", type: "text/html"
   xml.id      issues_dashboard_url
   xml.updated @issues.first.created_at.xmlschema if @issues.reorder(nil).any?

app/views/dashboard/issues.html.haml

@@ -2,13 +2,13 @@
 - header_title  "Issues", issues_dashboard_path(assignee_id: current_user.id)
 = content_for :meta_tags do
   - if current_user
-    = auto_discovery_link_tag(:atom, issues_dashboard_url(format: :atom, private_token: current_user.private_token), title: "#{current_user.name} issues")
+    = auto_discovery_link_tag(:atom, url_for(params.merge(format: :atom, private_token: current_user.private_token)), title: "#{current_user.name} issues")
 
 .top-area
   = render 'shared/issuable/nav', type: :issues
   .nav-controls
     - if current_user
-      = link_to issues_dashboard_url(format: :atom, private_token: current_user.private_token), class: 'btn' do
+      = link_to url_for(params.merge(format: :atom, private_token: current_user.private_token)), class: 'btn' do
         = icon('rss')
         %span.icon-label
           Subscribe

app/views/groups/issues.atom.builder

@@ -1,7 +1,7 @@
 xml.instruct!
 xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://search.yahoo.com/mrss/" do
   xml.title   "#{@group.name} issues"
-  xml.link    href: issues_group_url(format: :atom, private_token: current_user.try(:private_token)), rel: "self", type: "application/atom+xml"
+  xml.link    href: url_for(params), rel: "self", type: "application/atom+xml"
   xml.link    href: issues_group_url, rel: "alternate", type: "text/html"
   xml.id      issues_group_url
   xml.updated @issues.first.created_at.xmlschema if @issues.reorder(nil).any?

app/views/groups/issues.html.haml

@@ -1,13 +1,13 @@
 - page_title "Issues"
 = content_for :meta_tags do
   - if current_user
-    = auto_discovery_link_tag(:atom, issues_group_url(@group, format: :atom, private_token: current_user.private_token), title: "#{@group.name} issues")
+    = auto_discovery_link_tag(:atom, url_for(params.merge(format: :atom, private_token: current_user.private_token)), title: "#{@group.name} issues")
 
 .top-area
   = render 'shared/issuable/nav', type: :issues
   .nav-controls
     - if current_user
-      = link_to issues_group_url(@group, format: :atom, private_token: current_user.private_token), class: 'btn' do
+      = link_to url_for(params.merge(format: :atom, private_token: current_user.private_token)), class: 'btn' do
         = icon('rss')
         %span.icon-label
           Subscribe

app/views/projects/issues/index.atom.builder

@@ -1,7 +1,7 @@
 xml.instruct!
 xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://search.yahoo.com/mrss/" do
   xml.title   "#{@project.name} issues"
-  xml.link    href: namespace_project_issues_url(@project.namespace, @project, format: :atom, private_token: current_user.try(:private_token)), rel: "self", type: "application/atom+xml"
+  xml.link    href: url_for(params), rel: "self", type: "application/atom+xml"
   xml.link    href: namespace_project_issues_url(@project.namespace, @project), rel: "alternate", type: "text/html"
   xml.id      namespace_project_issues_url(@project.namespace, @project)
   xml.updated @issues.first.created_at.xmlschema if @issues.reorder(nil).any?

app/views/projects/issues/index.html.haml

@@ -8,7 +8,7 @@
 
 = content_for :meta_tags do
   - if current_user
-    = auto_discovery_link_tag(:atom, namespace_project_issues_url(@project.namespace, @project, :atom, private_token: current_user.private_token), title: "#{@project.name} issues")
+    = auto_discovery_link_tag(:atom, url_for(params.merge(format: :atom, private_token: current_user.private_token)), title: "#{@project.name} issues")
 
 %div{ class: (container_class) }
   - if @project.issues.any?
@@ -16,7 +16,7 @@
       = render 'shared/issuable/nav', type: :issues
       .nav-controls
         - if current_user
-          = link_to namespace_project_issues_path(@project.namespace, @project, :atom, { private_token: current_user.private_token }), class: 'btn append-right-10' do
+          = link_to url_for(params.merge(format: :atom, private_token: current_user.private_token)), class: 'btn append-right-10' do
             = icon('rss')
             %span.icon-label
               Subscribe

changelogs/unreleased/22947-fix_issues_atom_feed_url.yml

@@ -0,0 +1,4 @@
+---
+title: Issues atom feed url reflect filters on dashboard
+merge_request: 7114
+author: Lucas Deschamps

spec/features/atom/dashboard_issues_spec.rb

@@ -19,6 +19,17 @@ describe "Dashboard Issues Feed", feature: true  do
         expect(body).to have_selector('title', text: "#{user.name} issues")
       end
 
+      it "renders atom feed with url parameters" do
+        visit issues_dashboard_path(:atom, private_token: user.private_token, state: 'opened', assignee_id: user.id)
+
+        link = find('link[type="application/atom+xml"]')
+        params = CGI::parse(URI.parse(link[:href]).query)
+
+        expect(params).to include('private_token' => [user.private_token])
+        expect(params).to include('state' => ['opened'])
+        expect(params).to include('assignee_id' => [user.id.to_s])
+      end
+
       context "issue with basic fields" do
         let!(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'test desc') }
 

spec/features/atom/issues_spec.rb

@@ -3,10 +3,14 @@ require 'spec_helper'
 describe 'Issues Feed', feature: true  do
   describe 'GET /issues' do
     let!(:user)     { create(:user) }
+    let!(:group)    { create(:group) }
     let!(:project)  { create(:project) }
     let!(:issue)    { create(:issue, author: user, project: project) }
 
-    before { project.team << [user, :developer] }
+    before do
+      project.team << [user, :developer]
+      group.add_developer(user)
+    end
 
     context 'when authenticated' do
       it 'renders atom feed' do
@@ -33,5 +37,28 @@ describe 'Issues Feed', feature: true  do
         expect(body).to have_selector('entry summary', text: issue.title)
       end
     end
+
+    it "renders atom feed with url parameters for project issues" do
+      visit namespace_project_issues_path(project.namespace, project,
+                                          :atom, private_token: user.private_token, state: 'opened', assignee_id: user.id)
+
+      link = find('link[type="application/atom+xml"]')
+      params = CGI::parse(URI.parse(link[:href]).query)
+
+      expect(params).to include('private_token' => [user.private_token])
+      expect(params).to include('state' => ['opened'])
+      expect(params).to include('assignee_id' => [user.id.to_s])
+    end
+
+    it "renders atom feed with url parameters for group issues" do
+      visit issues_group_path(group, :atom, private_token: user.private_token, state: 'opened', assignee_id: user.id)
+
+      link = find('link[type="application/atom+xml"]')
+      params = CGI::parse(URI.parse(link[:href]).query)
+
+      expect(params).to include('private_token' => [user.private_token])
+      expect(params).to include('state' => ['opened'])
+      expect(params).to include('assignee_id' => [user.id.to_s])
+    end
   end
 end

spec/features/dashboard_issues_spec.rb

@@ -44,6 +44,22 @@ describe "Dashboard Issues filtering", feature: true, js: true do
       expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
       expect(page).to have_selector('.issue', count: 1)
     end
+
+    it 'updates atom feed link' do
+      visit_issues(milestone_title: '', assignee_id: user.id)
+
+      link = find('.nav-controls a', text: 'Subscribe')
+      params = CGI::parse(URI.parse(link[:href]).query)
+      auto_discovery_link = find('link[type="application/atom+xml"]', visible: false)
+      auto_discovery_params = CGI::parse(URI.parse(auto_discovery_link[:href]).query)
+
+      expect(params).to include('private_token' => [user.private_token])
+      expect(params).to include('milestone_title' => [''])
+      expect(params).to include('assignee_id' => [user.id.to_s])
+      expect(auto_discovery_params).to include('private_token' => [user.private_token])
+      expect(auto_discovery_params).to include('milestone_title' => [''])
+      expect(auto_discovery_params).to include('assignee_id' => [user.id.to_s])
+    end
   end
 
   def show_milestone_dropdown
@@ -51,7 +67,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do
     expect(page).to have_selector('.dropdown-content', visible: true)
   end
 
-  def visit_issues
-    visit issues_dashboard_path
+  def visit_issues(*args)
+    visit issues_dashboard_path(*args)
   end
 end

spec/features/issues/filter_issues_spec.rb

@@ -4,6 +4,7 @@ describe 'Filter issues', feature: true do
   include WaitForAjax
 
   let!(:project)   { create(:project) }
+  let!(:group)     { create(:group) }
   let!(:user)      { create(:user)}
   let!(:milestone) { create(:milestone, project: project) }
   let!(:label)     { create(:label, project: project) }
@@ -11,6 +12,7 @@ describe 'Filter issues', feature: true do
 
   before do
     project.team << [user, :master]
+    group.add_developer(user)
     login_as(user)
     create(:issue, project: project)
   end
@@ -347,4 +349,36 @@ describe 'Filter issues', feature: true do
       end
     end
   end
+
+  it 'updates atom feed link for project issues' do
+    visit namespace_project_issues_path(project.namespace, project, milestone_title: '', assignee_id: user.id)
+
+    link = find('.nav-controls a', text: 'Subscribe')
+    params = CGI::parse(URI.parse(link[:href]).query)
+    auto_discovery_link = find('link[type="application/atom+xml"]', visible: false)
+    auto_discovery_params = CGI::parse(URI.parse(auto_discovery_link[:href]).query)
+
+    expect(params).to include('private_token' => [user.private_token])
+    expect(params).to include('milestone_title' => [''])
+    expect(params).to include('assignee_id' => [user.id.to_s])
+    expect(auto_discovery_params).to include('private_token' => [user.private_token])
+    expect(auto_discovery_params).to include('milestone_title' => [''])
+    expect(auto_discovery_params).to include('assignee_id' => [user.id.to_s])
+  end
+
+  it 'updates atom feed link for group issues' do
+    visit issues_group_path(group, milestone_title: '', assignee_id: user.id)
+
+    link = find('.nav-controls a', text: 'Subscribe')
+    params = CGI::parse(URI.parse(link[:href]).query)
+    auto_discovery_link = find('link[type="application/atom+xml"]', visible: false)
+    auto_discovery_params = CGI::parse(URI.parse(auto_discovery_link[:href]).query)
+
+    expect(params).to include('private_token' => [user.private_token])
+    expect(params).to include('milestone_title' => [''])
+    expect(params).to include('assignee_id' => [user.id.to_s])
+    expect(auto_discovery_params).to include('private_token' => [user.private_token])
+    expect(auto_discovery_params).to include('milestone_title' => [''])
+    expect(auto_discovery_params).to include('assignee_id' => [user.id.to_s])
+  end
 end