Commit 7568f8de authored by James Lopez's avatar James Lopez Committed by Rémy Coutable

Fix missing project attributes used by the Import/Export feature

parent 32bf2df9
...@@ -1484,6 +1484,9 @@ class Project < ActiveRecord::Base ...@@ -1484,6 +1484,9 @@ class Project < ActiveRecord::Base
else else
update_attribute(name, value) update_attribute(name, value)
end end
rescue ActiveRecord::RecordNotSaved => e
handle_update_attribute_error(e, value)
end end
def change_repository_storage(new_repository_storage_key) def change_repository_storage(new_repository_storage_key)
...@@ -1637,4 +1640,16 @@ class Project < ActiveRecord::Base ...@@ -1637,4 +1640,16 @@ class Project < ActiveRecord::Base
ContainerRepository.build_root_repository(self).has_tags? ContainerRepository.build_root_repository(self).has_tags?
end end
def handle_update_attribute_error(ex, value)
if ex.message.start_with?('Failed to replace')
if value.respond_to?(:each)
invalid = value.detect(&:invalid?)
raise ex, ([ex.message] + invalid.errors.full_messages).join(' ') if invalid
end
end
raise ex
end
end end
---
title: Add missing project attributes to Import/Export
merge_request:
author:
...@@ -41,7 +41,6 @@ project_tree: ...@@ -41,7 +41,6 @@ project_tree:
- :statuses - :statuses
- triggers: - triggers:
- :trigger_schedule - :trigger_schedule
- :deploy_keys
- :services - :services
- :hooks - :hooks
- protected_branches: - protected_branches:
...@@ -53,10 +52,6 @@ project_tree: ...@@ -53,10 +52,6 @@ project_tree:
# Only include the following attributes for the models specified. # Only include the following attributes for the models specified.
included_attributes: included_attributes:
project:
- :description
- :visibility_level
- :archived
user: user:
- :id - :id
- :email - :email
...@@ -66,6 +61,34 @@ included_attributes: ...@@ -66,6 +61,34 @@ included_attributes:
# Do not include the following attributes for the models specified. # Do not include the following attributes for the models specified.
excluded_attributes: excluded_attributes:
project:
- :name
- :path
- :namespace_id
- :creator_id
- :import_url
- :import_status
- :avatar
- :import_type
- :import_source
- :import_error
- :mirror
- :runners_token
- :repository_storage
- :repository_read_only
- :lfs_enabled
- :import_jid
- :created_at
- :updated_at
- :import_jid
- :import_jid
- :id
- :star_count
- :last_activity_at
- :mirror_last_update_at
- :mirror_last_successful_update_at
- :mirror_user_id
- :mirror_trigger_builds
snippets: snippets:
- :expired_at - :expired_at
merge_request_diff: merge_request_diff:
...@@ -94,3 +117,5 @@ methods: ...@@ -94,3 +117,5 @@ methods:
- :utf8_st_diffs - :utf8_st_diffs
merge_requests: merge_requests:
- :diff_head_sha - :diff_head_sha
project:
- :description_html
\ No newline at end of file
...@@ -71,14 +71,14 @@ module Gitlab ...@@ -71,14 +71,14 @@ module Gitlab
def restore_project def restore_project
return @project unless @tree_hash return @project unless @tree_hash
@project.update(project_params) @project.update_columns(project_params)
@project @project
end end
def project_params def project_params
@tree_hash.reject do |key, value| @tree_hash.reject do |key, value|
# return params that are not 1 to many or 1 to 1 relations # return params that are not 1 to many or 1 to 1 relations
value.is_a?(Array) || key == key.singularize value.respond_to?(:each) && !Project.column_names.include?(key)
end end
end end
......
...@@ -15,7 +15,10 @@ module Gitlab ...@@ -15,7 +15,10 @@ module Gitlab
# Outputs a hash in the format described here: http://api.rubyonrails.org/classes/ActiveModel/Serializers/JSON.html # Outputs a hash in the format described here: http://api.rubyonrails.org/classes/ActiveModel/Serializers/JSON.html
# for outputting a project in JSON format, including its relations and sub relations. # for outputting a project in JSON format, including its relations and sub relations.
def project_tree def project_tree
@attributes_finder.find_included(:project).merge(include: build_hash(@tree)) attributes = @attributes_finder.find(:project)
project_attributes = attributes.is_a?(Hash) ? attributes[:project] : {}
project_attributes.merge(include: build_hash(@tree))
rescue => e rescue => e
@shared.error(e) @shared.error(e)
false false
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
"description": "Nisi et repellendus ut enim quo accusamus vel magnam.", "description": "Nisi et repellendus ut enim quo accusamus vel magnam.",
"visibility_level": 10, "visibility_level": 10,
"archived": false, "archived": false,
"description_html": "description",
"labels": [ "labels": [
{ {
"id": 2, "id": 2,
......
...@@ -30,6 +30,10 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do ...@@ -30,6 +30,10 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do
expect(project.project_feature.merge_requests_access_level).to eq(ProjectFeature::ENABLED) expect(project.project_feature.merge_requests_access_level).to eq(ProjectFeature::ENABLED)
end end
it 'has the project html description' do
expect(Project.find_by_path('project').description_html).to eq('description')
end
it 'has the same label associated to two issues' do it 'has the same label associated to two issues' do
expect(ProjectLabel.find_by_title('test2').issues.count).to eq(2) expect(ProjectLabel.find_by_title('test2').issues.count).to eq(2)
end end
......
...@@ -6,7 +6,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do ...@@ -6,7 +6,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do
let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared) } let(:project_tree_saver) { described_class.new(project: project, current_user: user, shared: shared) }
let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { setup_project } let!(:project) { setup_project }
before do before do
project.team << [user, :master] project.team << [user, :master]
...@@ -189,6 +189,16 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do ...@@ -189,6 +189,16 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do
end end
end end
end end
context 'project attributes' do
it 'contains the html description' do
expect(saved_project_json).to include("description_html" => 'description')
end
it 'does not contain the runners token' do
expect(saved_project_json).not_to include("runners_token" => 'token')
end
end
end end
end end
...@@ -209,6 +219,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do ...@@ -209,6 +219,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do
releases: [release], releases: [release],
group: group group: group
) )
project.update_column(:description_html, 'description')
project_label = create(:label, project: project) project_label = create(:label, project: project)
group_label = create(:group_label, group: group) group_label = create(:group_label, group: group)
create(:label_link, label: project_label, target: issue) create(:label_link, label: project_label, target: issue)
......
...@@ -5,7 +5,7 @@ describe Gitlab::ImportExport::Reader, lib: true do ...@@ -5,7 +5,7 @@ describe Gitlab::ImportExport::Reader, lib: true do
let(:test_config) { 'spec/support/import_export/import_export.yml' } let(:test_config) { 'spec/support/import_export/import_export.yml' }
let(:project_tree_hash) do let(:project_tree_hash) do
{ {
only: [:name, :path], except: [:id, :created_at],
include: [:issues, :labels, include: [:issues, :labels,
{ merge_requests: { { merge_requests: {
only: [:id], only: [:id],
......
...@@ -333,6 +333,37 @@ Project: ...@@ -333,6 +333,37 @@ Project:
- snippets_enabled - snippets_enabled
- visibility_level - visibility_level
- archived - archived
- created_at
- updated_at
- last_activity_at
- star_count
- ci_id
- shared_runners_enabled
- build_coverage_regex
- build_allow_git_fetchs
- build_timeout
- pending_delete
- public_builds
- last_repository_check_failed
- last_repository_check_at
- container_registry_enabled
- only_allow_merge_if_pipeline_succeeds
- has_external_issue_tracker
- request_access_enabled
- has_external_wiki
- only_allow_merge_if_all_discussions_are_resolved
- auto_cancel_pending_pipelines
- printing_merge_request_link_enabled
- build_allow_git_fetch
- merge_requests_template
- merge_requests_rebase_enabled
- approvals_before_merge
- reset_approvals_on_push
- merge_requests_ff_only_enabled
- issues_template
- repository_size_limit
- sync_time
- service_desk_enabled
Author: Author:
- name - name
ProjectFeature: ProjectFeature:
......
...@@ -2329,4 +2329,23 @@ describe Project, models: true do ...@@ -2329,4 +2329,23 @@ describe Project, models: true do
expect(project.pipeline_status).to be_loaded expect(project.pipeline_status).to be_loaded
end end
end end
describe '#append_or_update_attribute' do
let(:project) { create(:project) }
it 'shows full error updating an invalid MR' do
error_message = 'Failed to replace merge_requests because one or more of the new records could not be saved.'\
' Validate fork Source project is not a fork of the target project'
expect { project.append_or_update_attribute(:merge_requests, [create(:merge_request)]) }.
to raise_error(ActiveRecord::RecordNotSaved, error_message)
end
it 'updates the project succesfully' do
merge_request = create(:merge_request, target_project: project, source_project: project)
expect { project.append_or_update_attribute(:merge_requests, [merge_request]) }.
not_to raise_error
end
end
end end
...@@ -11,9 +11,6 @@ project_tree: ...@@ -11,9 +11,6 @@ project_tree:
- :user - :user
included_attributes: included_attributes:
project:
- :name
- :path
merge_requests: merge_requests:
- :id - :id
user: user:
...@@ -21,4 +18,7 @@ included_attributes: ...@@ -21,4 +18,7 @@ included_attributes:
excluded_attributes: excluded_attributes:
merge_requests: merge_requests:
- :iid - :iid
\ No newline at end of file project:
- :id
- :created_at
\ No newline at end of file
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment