From 53a679f76fc4d398438bf6ca431eabecaabebb9e Mon Sep 17 00:00:00 2001
From: Nathan Friend <nathan@gitlab.com>
Date: Tue, 31 Mar 2020 11:28:32 -0400
Subject: [PATCH] Allow Guest users to view Releases page

This commit fixes a bug that was preventing Guest users from viewing the
Releases page of private projects.
---
 .../releases/components/app_index.vue         |   2 +-
 .../releases/components/release_block.vue     |  10 +-
 .../nfriend-fix-guest-user-releases.yml       |   5 +
 .../releases/user_views_releases_spec.rb      | 114 +++++++++++-------
 .../releases/components/release_block_spec.js |   8 ++
 5 files changed, 91 insertions(+), 48 deletions(-)
 create mode 100644 changelogs/unreleased/nfriend-fix-guest-user-releases.yml

diff --git a/app/assets/javascripts/releases/components/app_index.vue b/app/assets/javascripts/releases/components/app_index.vue
index 511b3cda9c8d50..215a376fc7670c 100644
--- a/app/assets/javascripts/releases/components/app_index.vue
+++ b/app/assets/javascripts/releases/components/app_index.vue
@@ -103,7 +103,7 @@ export default {
     <div v-else-if="shouldRenderSuccessState" class="js-success-state">
       <release-block
         v-for="(release, index) in releases"
-        :key="release.tagName"
+        :key="index"
         :release="release"
         :class="{ 'linked-card': releases.length > 1 && index !== releases.length - 1 }"
       />
diff --git a/app/assets/javascripts/releases/components/release_block.vue b/app/assets/javascripts/releases/components/release_block.vue
index 515aa6294764b6..58045b57d80f1f 100644
--- a/app/assets/javascripts/releases/components/release_block.vue
+++ b/app/assets/javascripts/releases/components/release_block.vue
@@ -37,7 +37,11 @@ export default {
     };
   },
   computed: {
-    id() {
+    htmlId() {
+      if (!this.release.tagName) {
+        return null;
+      }
+
       return slugify(this.release.tagName);
     },
     assets() {
@@ -72,7 +76,7 @@ export default {
     this.renderGFM();
 
     const hash = getLocationHash();
-    if (hash && slugify(hash) === this.id) {
+    if (hash && slugify(hash) === this.htmlId) {
       this.isHighlighted = true;
       setTimeout(() => {
         this.isHighlighted = false;
@@ -89,7 +93,7 @@ export default {
 };
 </script>
 <template>
-  <div :id="id" :class="{ 'bg-line-target-blue': isHighlighted }" class="card release-block">
+  <div :id="htmlId" :class="{ 'bg-line-target-blue': isHighlighted }" class="card release-block">
     <release-block-header :release="release" />
     <div class="card-body">
       <div v-if="shouldRenderMilestoneInfo">
diff --git a/changelogs/unreleased/nfriend-fix-guest-user-releases.yml b/changelogs/unreleased/nfriend-fix-guest-user-releases.yml
new file mode 100644
index 00000000000000..784fd421fc08dd
--- /dev/null
+++ b/changelogs/unreleased/nfriend-fix-guest-user-releases.yml
@@ -0,0 +1,5 @@
+---
+title: Fix Releases page for Guest users of private projects
+merge_request: 28447
+author:
+type: fixed
diff --git a/spec/features/projects/releases/user_views_releases_spec.rb b/spec/features/projects/releases/user_views_releases_spec.rb
index a4ba81ffeb96ac..e21d8ec16e1076 100644
--- a/spec/features/projects/releases/user_views_releases_spec.rb
+++ b/spec/features/projects/releases/user_views_releases_spec.rb
@@ -3,41 +3,33 @@
 require 'spec_helper'
 
 describe 'User views releases', :js do
-  let!(:project) { create(:project, :repository) }
-  let!(:release) { create(:release, project: project ) }
-  let!(:user) { create(:user) }
+  let_it_be(:project) { create(:project, :repository, :private) }
+  let_it_be(:release) { create(:release, project: project, name: 'The first release' ) }
+  let_it_be(:maintainer) { create(:user) }
+  let_it_be(:guest) { create(:user) }
 
   before do
-    project.add_maintainer(user)
-
-    gitlab_sign_in(user)
+    project.add_maintainer(maintainer)
+    project.add_guest(guest)
   end
 
-  it 'sees the release' do
-    visit project_releases_path(project)
-
-    expect(page).to have_content(release.name)
-    expect(page).to have_content(release.tag)
-    expect(page).not_to have_content('Upcoming Release')
-  end
-
-  context 'when there is a link as an asset' do
-    let!(:release_link) { create(:release_link, release: release, url: url ) }
-    let(:url) { "#{project.web_url}/-/jobs/1/artifacts/download" }
-    let(:direct_asset_link) { Gitlab::Routing.url_helpers.project_release_url(project, release) << release_link.filepath }
+  context('when the user is a maintainer') do
+    before do
+      gitlab_sign_in(maintainer)
+    end
 
-    it 'sees the link' do
+    it 'sees the release' do
       visit project_releases_path(project)
 
-      page.within('.js-assets-list') do
-        expect(page).to have_link release_link.name, href: direct_asset_link
-        expect(page).not_to have_content('(external source)')
-      end
+      expect(page).to have_content(release.name)
+      expect(page).to have_content(release.tag)
+      expect(page).not_to have_content('Upcoming Release')
     end
 
-    context 'when there is a link redirect' do
-      let!(:release_link) { create(:release_link, release: release, name: 'linux-amd64 binaries', filepath: '/binaries/linux-amd64', url: url) }
+    context 'when there is a link as an asset' do
+      let!(:release_link) { create(:release_link, release: release, url: url ) }
       let(:url) { "#{project.web_url}/-/jobs/1/artifacts/download" }
+      let(:direct_asset_link) { Gitlab::Routing.url_helpers.project_release_url(project, release) << release_link.filepath }
 
       it 'sees the link' do
         visit project_releases_path(project)
@@ -47,39 +39,73 @@
           expect(page).not_to have_content('(external source)')
         end
       end
-    end
 
-    context 'when url points to external resource' do
-      let(:url) { 'http://google.com/download' }
+      context 'when there is a link redirect' do
+        let!(:release_link) { create(:release_link, release: release, name: 'linux-amd64 binaries', filepath: '/binaries/linux-amd64', url: url) }
+        let(:url) { "#{project.web_url}/-/jobs/1/artifacts/download" }
 
-      it 'sees that the link is external resource' do
-        visit project_releases_path(project)
+        it 'sees the link' do
+          visit project_releases_path(project)
 
-        page.within('.js-assets-list') do
-          expect(page).to have_content('(external source)')
+          page.within('.js-assets-list') do
+            expect(page).to have_link release_link.name, href: direct_asset_link
+            expect(page).not_to have_content('(external source)')
+          end
+        end
+      end
+
+      context 'when url points to external resource' do
+        let(:url) { 'http://google.com/download' }
+
+        it 'sees that the link is external resource' do
+          visit project_releases_path(project)
+
+          page.within('.js-assets-list') do
+            expect(page).to have_content('(external source)')
+          end
         end
       end
     end
-  end
 
-  context 'with an upcoming release' do
-    let(:tomorrow) { Time.zone.now + 1.day }
-    let!(:release) { create(:release, project: project, released_at: tomorrow ) }
+    context 'with an upcoming release' do
+      let(:tomorrow) { Time.zone.now + 1.day }
+      let!(:release) { create(:release, project: project, released_at: tomorrow ) }
 
-    it 'sees the upcoming tag' do
-      visit project_releases_path(project)
+      it 'sees the upcoming tag' do
+        visit project_releases_path(project)
+
+        expect(page).to have_content('Upcoming Release')
+      end
+    end
+
+    context 'with a tag containing a slash' do
+      it 'sees the release' do
+        release = create :release, :with_evidence, project: project, tag: 'debian/2.4.0-1'
+        visit project_releases_path(project)
 
-      expect(page).to have_content('Upcoming Release')
+        expect(page).to have_content(release.name)
+        expect(page).to have_content(release.tag)
+      end
     end
   end
 
-  context 'with a tag containing a slash' do
-    it 'sees the release' do
-      release = create :release, :with_evidence, project: project, tag: 'debian/2.4.0-1'
+  context('when the user is a guest') do
+    before do
+      gitlab_sign_in(guest)
+    end
+
+    it 'renders release info except for Git-related data' do
       visit project_releases_path(project)
 
-      expect(page).to have_content(release.name)
-      expect(page).to have_content(release.tag)
+      within('.release-block') do
+        expect(page).to have_content(release.description)
+
+        # The following properties (sometimes) include Git info,
+        # so they are not rendered for Guest users
+        expect(page).not_to have_content(release.name)
+        expect(page).not_to have_content(release.tag)
+        expect(page).not_to have_content(release.commit.short_id)
+      end
     end
   end
 end
diff --git a/spec/frontend/releases/components/release_block_spec.js b/spec/frontend/releases/components/release_block_spec.js
index 7ea2379ea359ed..9846fcb65ebaba 100644
--- a/spec/frontend/releases/components/release_block_spec.js
+++ b/spec/frontend/releases/components/release_block_spec.js
@@ -165,6 +165,14 @@ describe('Release block', () => {
     });
   });
 
+  it('does not set the ID if tagName is missing', () => {
+    release.tagName = undefined;
+
+    return factory(release).then(() => {
+      expect(wrapper.attributes().id).toBeUndefined();
+    });
+  });
+
   describe('evidence block', () => {
     it('renders the evidence block when the evidence is available and the feature flag is true', () =>
       factory(release, { releaseEvidenceCollection: true }).then(() =>
-- 
GitLab