app/controllers/application_controller.rb | 7 +++- app/controllers/repositories_controller.rb | 9 ++++- app/models/project.rb | 33 ++++++++++++----- app/models/project_identifier.rb | 25 +++++++++++++ app/models/repository.rb | 33 ++++++++++++----- app/models/repository_identifier.rb | 26 ++++++++++++++ app/views/projects/_form.html.erb | 12 +++---- app/views/repositories/_form.html.erb | 4 +-- config/locales/en.yml | 7 ++-- db/migrate/20210402143502_create_identifiers.rb | 17 +++++++++ db/migrate/20210402153405_migrate_identifiers.rb | 11 ++++++ lib/redmine/core_ext/active_record.rb | 44 +++++++++++++++++++++++ test/functional/issues_controller_test.rb | 16 +++++++++ test/functional/repositories_controller_test.rb | 33 +++++++++++++++++ test/unit/project_test.rb | 19 +++++----- test/unit/repository_git_test.rb | 2 +- test/unit/repository_test.rb | 46 +++++++++--------------- 17 files changed, 273 insertions(+), 71 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b5644e89d..a75a26116 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -325,7 +325,12 @@ class ApplicationController < ActionController::Base def find_project(project_id=params[:id]) @project = Project.find(project_id) rescue ActiveRecord::RecordNotFound - render_404 + project_alias = Project.find_alias(project_id) + if @project = project_alias&.project + flash[:warning] = l(:warning_identifier_renamed, alias: project_alias.identifier, identifier: @project.identifier) + else + render_404 + end end # Find project of id params[:project_id] diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index dfa96cc0e..4204f941d 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -333,9 +333,16 @@ class RepositoriesController < ApplicationController REV_PARAM_RE = %r{\A[a-f0-9]*\Z}i def find_project_repository - @project = Project.find(params[:id]) + return unless find_project if params[:repository_id].present? @repository = @project.repositories.find_by_identifier_param(params[:repository_id]) + unless @repository + repository_alias = @project.repositories.find_alias(params[:repository_id]) + @repository = repository_alias&.repository + if @repository + flash[:warning] = l(:warning_identifier_renamed, alias: repository_alias.identifier, identifier: @repository.identifier) + end + end else @repository = @project.repository end diff --git a/app/models/project.rb b/app/models/project.rb index ab864436a..28ab125c8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -52,6 +52,8 @@ class Project < ActiveRecord::Base has_many :repositories, :dependent => :destroy has_many :changesets, :through => :repository has_one :wiki, :dependent => :destroy + has_one :project_identifier, :primary_key => 'identifier', :foreign_key => 'identifier' + has_many :project_identifiers, :dependent => :destroy # Custom field for the project issues has_and_belongs_to_many :issue_custom_fields, lambda {order(:position)}, @@ -72,17 +74,21 @@ class Project < ActiveRecord::Base :author => nil validates_presence_of :name, :identifier - validates_uniqueness_of :identifier, :if => proc {|p| p.identifier_changed?}, :case_sensitive => true + validates :identifier, :if => proc {|p| p.identifier_changed? && p.identifier.present?}, :project_identifier_uniqueness => true validates_length_of :name, :maximum => 255 validates_length_of :homepage, :maximum => 255 validates_length_of :identifier, :maximum => IDENTIFIER_MAX_LENGTH # downcase letters, digits, dashes but not digits only validates_format_of :identifier, :with => /\A(?!\d+$)[a-z0-9\-_]*\z/, :if => proc {|p| p.identifier_changed?} + after_validation :reset_invalid_identifier + # reserved words validates_exclusion_of :identifier, :in => %w(new) validate :validate_parent + after_save :update_identifier, + :if => proc {|project| project.saved_change_to_identifier? && project.identifier.present? && !project.new_record?} after_save :update_inherited_members, :if => proc {|project| project.saved_change_to_inherit_members?} after_save :remove_inherited_member_roles, :add_inherited_member_roles, @@ -137,13 +143,6 @@ class Project < ActiveRecord::Base end end - def identifier=(identifier) - super unless identifier_frozen? - end - - def identifier_frozen? - errors[:identifier].blank? && !(new_record? || identifier.blank?) - end # returns latest created projects # non public projects will be returned only if user is a member of those @@ -350,6 +349,14 @@ class Project < ActiveRecord::Base end end + def self.find_alias(*args) + if args.first && args.first.is_a?(String) && !/^\d*$/.match?(args.first) + scope = ProjectIdentifier + scope = scope.where(project_id: current_scope) if scope_attributes? + scope.find_by_identifier(*args) + end + end + def self.find_by_param(*args) self.find(*args) end @@ -993,6 +1000,10 @@ class Project < ActiveRecord::Base end end + def update_identifier + self.project_identifiers << project_identifiers.build(:identifier => identifier, :project_id => self.id) + end + def remove_inherited_member_roles member_roles = MemberRole.where(:member_id => membership_ids).to_a member_role_ids = member_roles.map(&:id) @@ -1032,6 +1043,12 @@ class Project < ActiveRecord::Base end end + def reset_invalid_identifier + if errors[:identifier].present? + self.identifier = identifier_was + end + end + # Copies wiki from +project+ def copy_wiki(project) # Check that the source project has a wiki first diff --git a/app/models/project_identifier.rb b/app/models/project_identifier.rb new file mode 100644 index 000000000..7f63d3bd6 --- /dev/null +++ b/app/models/project_identifier.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +# Redmine - project management software +# Copyright (C) 2006-2021 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +class ProjectIdentifier < ActiveRecord::Base + belongs_to :project + + validates :identifier, :uniqueness => { :case_sensitive => true }, :presence => true + validates :project_id, :presence => true +end diff --git a/app/models/repository.rb b/app/models/repository.rb index e65c800e6..543700994 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -29,11 +29,15 @@ class Repository < ActiveRecord::Base belongs_to :project has_many :changesets, lambda{order("#{Changeset.table_name}.committed_on DESC, #{Changeset.table_name}.id DESC")} has_many :filechanges, :class_name => 'Change', :through => :changesets + has_one :repository_identifier, :primary_key => 'identifier', :foreign_key => 'identifier' + has_many :repository_identifiers, :dependent => :destroy serialize :extra_info before_validation :normalize_identifier before_save :check_default + after_save :update_identifier, + :if => proc {|repository| repository.saved_change_to_identifier? && !repository.new_record? && repository.project} # Raw SQL to delete changesets and changes in the database # has_many :changesets, :dependent => :destroy is too slow for big repositories @@ -43,13 +47,14 @@ class Repository < ActiveRecord::Base validates_length_of :password, :maximum => 255, :allow_nil => true validates_length_of :root_url, :url, maximum: 255 validates_length_of :identifier, :maximum => IDENTIFIER_MAX_LENGTH, :allow_blank => true - validates_uniqueness_of :identifier, :scope => :project_id, :case_sensitive => true + validates :identifier, :if => proc { |r| r.identifier_changed? }, :repository_identifier_uniqueness => true validates_exclusion_of :identifier, :in => %w(browse show entry raw changes annotate diff statistics graph revisions revision) # donwcase letters, digits, dashes, underscores but not digits only validates_format_of :identifier, :with => /\A(?!\d+$)[a-z0-9\-_]*\z/, :allow_blank => true # Checks if the SCM is enabled when creating a repository validate :repo_create_validation, :on => :create validate :validate_repository_path + after_validation :reset_invalid_identifier safe_attributes( 'identifier', @@ -124,14 +129,6 @@ class Repository < ActiveRecord::Base end end - def identifier=(identifier) - super unless identifier_frozen? - end - - def identifier_frozen? - errors[:identifier].blank? && !(new_record? || identifier.blank?) - end - def identifier_param if identifier.present? identifier @@ -150,6 +147,12 @@ class Repository < ActiveRecord::Base end end + def reset_invalid_identifier + if errors[:identifier].present? + self.identifier = identifier_was + end + end + def self.find_by_identifier_param(param) if /^\d+$/.match?(param.to_s) find_by_id(param) @@ -158,6 +161,14 @@ class Repository < ActiveRecord::Base end end + def self.find_alias(param) + if !/^\d+$/.match?(param.to_s) + scope = RepositoryIdentifier + scope = scope.where(repository_id: current_scope) if scope_attributes? + scope.find_by_identifier(param) + end + end + # TODO: should return an empty hash instead of nil to avoid many ||{} def extra_info h = read_attribute(:extra_info) @@ -481,6 +492,10 @@ class Repository < ActiveRecord::Base self.identifier = identifier.to_s.strip end + def update_identifier + self.repository_identifiers << repository_identifiers.build(:project_id => project.id, :identifier => identifier, :repository_id => self.id) + end + def check_default if !is_default? && set_as_default? self.is_default = true diff --git a/app/models/repository_identifier.rb b/app/models/repository_identifier.rb new file mode 100644 index 000000000..74d412409 --- /dev/null +++ b/app/models/repository_identifier.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +# Redmine - project management software +# Copyright (C) 2006-2021 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +class RepositoryIdentifier < ActiveRecord::Base + belongs_to :repository + belongs_to :project + + validates :identifier, :uniqueness => { :scope => :project_id, :case_sensitive => true } + validates :repository_id, :presence => true +end diff --git a/app/views/projects/_form.html.erb b/app/views/projects/_form.html.erb index 7c988fb0e..be840eb07 100644 --- a/app/views/projects/_form.html.erb +++ b/app/views/projects/_form.html.erb @@ -5,10 +5,10 @@
<%= f.text_field :name, :required => true, :size => 60 %>
<%= f.text_area :description, :rows => 8, :class => 'wiki-edit' %>
-<%= f.text_field :identifier, :required => true, :size => 60, :disabled => @project.identifier_frozen?, :maxlength => Project::IDENTIFIER_MAX_LENGTH %> -<% unless @project.identifier_frozen? %> +
+ <%= f.text_field :identifier, :required => true, :size => 60, :maxlength => Project::IDENTIFIER_MAX_LENGTH %> <%= l(:text_length_between, :min => 1, :max => Project::IDENTIFIER_MAX_LENGTH) %> <%= l(:text_project_identifier_info).html_safe %> -<% end %>
+<%= f.text_field :homepage, :size => 60 %>
<%= f.check_box :is_public %> @@ -44,10 +44,8 @@ <% end %> -<% unless @project.identifier_frozen? %> - <% content_for :header_tags do %> - <%= javascript_include_tag 'project_identifier' %> - <% end %> +<% content_for :header_tags do %> + <%= javascript_include_tag 'project_identifier' %> <% end %> <% if !User.current.admin? && @project.inherit_members? && @project.parent && User.current.member_of?(@project.parent) %> diff --git a/app/views/repositories/_form.html.erb b/app/views/repositories/_form.html.erb index b0f145ac0..349788b67 100644 --- a/app/views/repositories/_form.html.erb +++ b/app/views/repositories/_form.html.erb @@ -10,12 +10,10 @@
<%= f.check_box :is_default, :label => :field_repository_is_default %>
-<%= f.text_field :identifier, :disabled => @repository.identifier_frozen? %> -<% unless @repository.identifier_frozen? %> + <%= f.text_field :identifier %> <%= l(:text_length_between, :min => 1, :max => Repository::IDENTIFIER_MAX_LENGTH) %> <%= l(:text_repository_identifier_info).html_safe %> -<% end %>
<% button_disabled = true %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 9d779a2fe..82693abb7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -116,6 +116,7 @@ en: too_short: "is too short (minimum is %{count} characters)" wrong_length: "is the wrong length (should be %{count} characters)" taken: "has already been taken" + used: "has already been used once" not_a_number: "is not a number" not_a_date: "is not a valid date" greater_than: "must be greater than %{count}" @@ -1199,7 +1200,7 @@ en: text_tip_issue_begin_day: issue beginning this day text_tip_issue_end_day: issue ending this day text_tip_issue_begin_end_day: issue beginning and ending this day - text_project_identifier_info: 'Only lower case letters (a-z), numbers, dashes and underscores are allowed.