Add unique constraint to release and url

Fix coding style

Improve coding style

Decouple UPDATE and DELETE operations of asset links

Rename links_attributes to assets:links

Rename exposed param and updated spec

8 files changed, 28 insertions, 110 deletions

diff --git a/app/models/releases/source.rb b/app/models/releases/source.rb
index 254abf86e77..4d3d54457af 100644
--- a/app/models/releases/source.rb
+++ b/app/models/releases/source.rb
@@ -3,6 +3,7 @@
 module Releases
   class Source
     include ActiveModel::Model
+
     attr_accessor :project, :tag_name, :format
 
     FORMATS = %w(zip tar.gz tar.bz2 tar).freeze
diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb
index 106fed3811d..c6e143d440d 100644
--- a/app/services/releases/create_service.rb
+++ b/app/services/releases/create_service.rb
@@ -44,7 +44,7 @@ module Releases
         author: current_user,
         tag: tag.name,
         sha: tag.dereferenced_target.sha,
-        links_attributes: params[:links_attributes] || []
+        links_attributes: params.dig(:assets, 'links') || []
       )
 
       success(tag: tag, release: release)
diff --git a/db/migrate/20181228175414_create_releases_link_table.rb b/db/migrate/20181228175414_create_releases_link_table.rb
index bf1e50dd36e..a8eb9db8461 100644
--- a/db/migrate/20181228175414_create_releases_link_table.rb
+++ b/db/migrate/20181228175414_create_releases_link_table.rb
@@ -12,6 +12,7 @@ class CreateReleasesLinkTable < ActiveRecord::Migration[5.0]
       t.string :name, null: false
       t.timestamps_with_timezone null: false
 
+      t.index [:release_id, :url], unique: true
       t.index [:release_id, :name], unique: true
     end
   end
diff --git a/db/schema.rb b/db/schema.rb
index 55b9a6aaf99..c9605e69b33 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1805,6 +1805,7 @@ ActiveRecord::Schema.define(version: 20190103140724) do
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
     t.index ["release_id", "name"], name: "index_release_links_on_release_id_and_name", unique: true, using: :btree
+    t.index ["release_id", "url"], name: "index_release_links_on_release_id_and_url", unique: true, using: :btree
   end
 
   create_table "releases", force: :cascade do |t|
diff --git a/lib/api/releases.rb b/lib/api/releases.rb
index 63c755f3620..c3d4101528c 100644
--- a/lib/api/releases.rb
+++ b/lib/api/releases.rb
@@ -49,9 +49,11 @@ module API
         requires :name,        type: String, desc: 'The name of the release'
         requires :description, type: String, desc: 'The release notes'
         optional :ref,         type: String, desc: 'The commit sha or branch name'
-        optional :links_attributes, type: Array do
-          requires :name, type: String
-          requires :url, type: String
+        optional :assets, type: Hash do
+          optional :links, type: Array do
+            requires :name, type: String
+            requires :url, type: String
+          end
         end
       end
       post ':id/releases' do
@@ -76,13 +78,6 @@ module API
         requires :tag_name,    type: String, desc: 'The name of the tag', as: :tag
         optional :name,        type: String, desc: 'The name of the release'
         optional :description, type: String, desc: 'Release notes with markdown support'
-        optional :links_attributes, type: Array do
-          optional :id, type: Integer
-          optional :name, type: String
-          optional :url, type: String
-          optional :_destroy, type: Integer
-          at_least_one_of :name, :url, :_destroy
-        end
       end
       put ':id/releases/:tag_name', requirements: RELEASE_ENDPOINT_REQUIREMETS do
         authorize_update_release!
diff --git a/spec/models/releases/link_spec.rb b/spec/models/releases/link_spec.rb
index 0f9ac7ec76a..116833d5ed8 100644
--- a/spec/models/releases/link_spec.rb
+++ b/spec/models/releases/link_spec.rb
@@ -1,6 +1,6 @@
-require 'rails_helper'
+require 'spec_helper'
 
-RSpec.describe Releases::Link do
+describe Releases::Link do
   let(:release) { create(:release, project: project) }
   let(:project) { create(:project) }
 
diff --git a/spec/models/releases/source_spec.rb b/spec/models/releases/source_spec.rb
index 526ee8eafc9..2c34225d45a 100644
--- a/spec/models/releases/source_spec.rb
+++ b/spec/models/releases/source_spec.rb
@@ -1,6 +1,6 @@
-require 'rails_helper'
+require 'spec_helper'
 
-RSpec.describe Releases::Source do
+describe Releases::Source do
   set(:project) { create(:project, :repository, name: 'finance-cal') }
   let(:tag_name) { 'v1.0' }
 
diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb
index 8433497863e..978fa0142c2 100644
--- a/spec/requests/api/releases_spec.rb
+++ b/spec/requests/api/releases_spec.rb
@@ -315,9 +315,9 @@ describe API::Releases do
         context 'when create one asset' do
           let(:params) do
             base_params.merge({
-              links_attributes: [
-                { name: 'beta', url: 'https://dosuken.example.com/inspection.exe' }
-              ]
+              assets: {
+                links: [{ name: 'beta', url: 'https://dosuken.example.com/inspection.exe' }]
+              }
             })
           end
 
@@ -346,10 +346,12 @@ describe API::Releases do
         context 'when create two assets' do
           let(:params) do
             base_params.merge({
-              links_attributes: [
-                { name: 'alpha', url: 'https://dosuken.example.com/alpha.exe' },
-                { name: 'beta', url: 'https://dosuken.example.com/beta.exe' }
-              ]
+              assets: {
+                links: [
+                  { name: 'alpha', url: 'https://dosuken.example.com/alpha.exe' },
+                  { name: 'beta', url: 'https://dosuken.example.com/beta.exe' }
+                ]
+              }
             })
           end
 
@@ -367,10 +369,12 @@ describe API::Releases do
           context 'when link names are duplicates' do
             let(:params) do
               base_params.merge({
-                links_attributes: [
-                  { name: 'alpha', url: 'https://dosuken.example.com/alpha.exe' },
-                  { name: 'alpha', url: 'https://dosuken.example.com/beta.exe' }
-                ]
+                assets: {
+                  links: [
+                    { name: 'alpha', url: 'https://dosuken.example.com/alpha.exe' },
+                    { name: 'alpha', url: 'https://dosuken.example.com/beta.exe' }
+                  ]
+                }
               })
             end
 
@@ -562,90 +566,6 @@ describe API::Releases do
       end
     end
 
-    context 'when links_attributes param is specified' do
-      context 'when the release does not have any link assets' do
-        let(:params) do
-          { links_attributes: [{ name: 'Beta release',
-                                 url: 'http://dosuken.com/win.exe' }] }
-        end
-
-        it 'creates an asset' do
-          put api("/projects/#{project.id}/releases/v0.1", maintainer),
-              params: params
-
-          expect(json_response['assets']['links'].count).to eq(1)
-          expect(json_response['assets']['links'].first['name'])
-            .to eq('Beta release')
-          expect(json_response['assets']['links'].first['url'])
-            .to eq('http://dosuken.com/win.exe')
-        end
-
-        context 'when url is invalid' do
-          let(:params) do
-            { links_attributes: [{ name: 'Beta release',
-                                   url: 'SELECT 1 from ci_builds;' }] }
-          end
-
-          it 'returns an error' do
-            put api("/projects/#{project.id}/releases/v0.1", maintainer),
-                params: params
-
-            expect(json_response['message']['links.url'].first)
-              .to include('Only allowed protocols are http, https')
-          end
-        end
-      end
-
-      context 'when the release has asset links' do
-        let!(:release_link_1) do
-          create(:release_link,
-                 name: 'gcc',
-                 url: 'http://dosuken.com/executable-gcc',
-                 release: release,
-                 created_at: 1.day.ago)
-        end
-
-        let!(:release_link_2) do
-          create(:release_link,
-                 name: 'llvm',
-                 url: 'http://dosuken.com/executable-llvm',
-                 release: release,
-                 created_at: 2.days.ago)
-        end
-
-        context 'when updates link names' do
-          let(:params) do
-            { links_attributes: [{ id: release_link_1.id, name: 'bin-gcc' },
-                                 { id: release_link_2.id, name: 'bin-llvm' }] }
-          end
-
-          it 'updates the asset' do
-            put api("/projects/#{project.id}/releases/v0.1", maintainer),
-                params: params
-
-            expect(json_response['assets']['links'].first['name'])
-              .to eq('bin-gcc')
-            expect(json_response['assets']['links'].second['name'])
-              .to eq('bin-llvm')
-          end
-        end
-
-        context 'when destroys an asset' do
-          let(:params) do
-            { links_attributes: [{ id: release_link_1.id, _destroy: '1' }] }
-          end
-
-          it 'updates the asset' do
-            put api("/projects/#{project.id}/releases/v0.1", maintainer),
-                params: params
-
-            expect(json_response['assets']['links'].count).to eq(1)
-            expect(json_response['assets']['links'].first['name']).to eq('llvm')
-          end
-        end
-      end
-    end
-
     context 'when feature flag is disabled' do
       before do
         stub_feature_flags(releases_page: false)