From c50ec72b52e9ed7270f7c81c2c71fd8e5a28eeb0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 23 Nov 2012 21:11:09 +0300 Subject: [PATCH] Deprecate code for Project. Use title and path --- app/controllers/admin/projects_controller.rb | 2 +- app/controllers/application_controller.rb | 2 +- app/models/namespace.rb | 14 ++++---- app/models/project.rb | 33 ++++++++++--------- app/models/user.rb | 5 +-- app/observers/project_observer.rb | 12 +++---- app/roles/push_observer.rb | 2 +- app/roles/repository.rb | 8 ++--- app/views/admin/dashboard/index.html.haml | 2 +- app/views/admin/groups/_form.html.haml | 4 +-- app/views/admin/projects/_form.html.haml | 7 ---- app/views/projects/_form.html.haml | 7 ---- db/fixtures/development/001_admin.rb | 9 ++--- db/fixtures/development/003_users.rb | 16 ++++----- db/fixtures/production/001_admin.rb | 9 ++--- .../20121123164910_rename_code_to_path.rb | 11 +++++++ db/schema.rb | 5 ++- lib/api/helpers.rb | 2 +- lib/gitlab/auth.rb | 1 + spec/factories.rb | 1 + spec/observers/user_observer_spec.rb | 2 +- spec/routing/admin_routing_spec.rb | 8 ----- 22 files changed, 79 insertions(+), 83 deletions(-) create mode 100644 db/migrate/20121123164910_rename_code_to_path.rb diff --git a/app/controllers/admin/projects_controller.rb b/app/controllers/admin/projects_controller.rb index 418ed4a7..c10fa721 100644 --- a/app/controllers/admin/projects_controller.rb +++ b/app/controllers/admin/projects_controller.rb @@ -5,7 +5,7 @@ class Admin::ProjectsController < AdminController @projects = Project.scoped @projects = @projects.where(namespace_id: params[:namespace_id]) if params[:namespace_id].present? @projects = @projects.search(params[:name]) if params[:name].present? - @projects = @projects.includes(:namespace).order("namespaces.code, projects.name ASC").page(params[:page]).per(20) + @projects = @projects.includes(:namespace).order("namespaces.path, projects.name ASC").page(params[:page]).per(20) end def show diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3062b594..847523d6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -66,7 +66,7 @@ class ApplicationController < ActionController::Base id = params[:project_id] || params[:id] id = id.split("/") if id.include?("/") - @project ||= current_user.projects.find_by_code(id) + @project ||= current_user.projects.find_by_path(id) @project || render_404 end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 24711535..4f536555 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -1,11 +1,13 @@ class Namespace < ActiveRecord::Base - attr_accessible :code, :name, :owner_id + attr_accessible :name, :path - has_many :projects + has_many :projects, dependent: :destroy belongs_to :owner, class_name: "User" validates :name, presence: true, uniqueness: true - validates :code, presence: true, uniqueness: true + validates :path, uniqueness: true, presence: true, length: { within: 1..255 }, + format: { with: /\A[a-zA-Z][a-zA-Z0-9_\-\.]*\z/, + message: "only letters, digits & '_' '-' '.' allowed. Letter should be first" } validates :owner, presence: true delegate :name, to: :owner, allow_nil: true, prefix: true @@ -15,11 +17,11 @@ class Namespace < ActiveRecord::Base scope :root, where('type IS NULL') def self.search query - where("name LIKE :query OR code LIKE :query", query: "%#{query}%") + where("name LIKE :query OR path LIKE :query", query: "%#{query}%") end def to_param - code + path end def human_name @@ -27,7 +29,7 @@ class Namespace < ActiveRecord::Base end def ensure_dir_exist - namespace_dir_path = File.join(Gitlab.config.git_base_path, code) + namespace_dir_path = File.join(Gitlab.config.git_base_path, path) Dir.mkdir(namespace_dir_path) unless File.exists?(namespace_dir_path) end end diff --git a/app/models/project.rb b/app/models/project.rb index 0ffa76cb..2de5b2f3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -27,7 +27,7 @@ class Project < ActiveRecord::Base include Authority include Team - attr_accessible :name, :path, :description, :code, :default_branch, :issues_enabled, + attr_accessible :name, :path, :description, :default_branch, :issues_enabled, :wall_enabled, :merge_requests_enabled, :wiki_enabled, as: [:default, :admin] attr_accessible :namespace_id, as: :admin @@ -58,16 +58,16 @@ class Project < ActiveRecord::Base # Validations validates :owner, presence: true validates :description, length: { within: 0..2000 } - validates :name, uniqueness: true, presence: true, length: { within: 0..255 } - validates :path, uniqueness: true, presence: true, length: { within: 0..255 }, - format: { with: /\A[a-zA-Z][a-zA-Z0-9_\-\.]*\z/, - message: "only letters, digits & '_' '-' '.' allowed. Letter should be first" } - validates :code, presence: true, uniqueness: true, length: { within: 1..255 }, + validates :name, presence: true, length: { within: 0..255 } + validates :path, presence: true, length: { within: 0..255 }, format: { with: /\A[a-zA-Z][a-zA-Z0-9_\-\.]*\z/, message: "only letters, digits & '_' '-' '.' allowed. Letter should be first" } validates :issues_enabled, :wall_enabled, :merge_requests_enabled, :wiki_enabled, inclusion: { in: [true, false] } + validates_uniqueness_of :name, scope: :namespace_id + validates_uniqueness_of :path, scope: :namespace_id + validate :check_limit, :repo_name # Scopes @@ -81,20 +81,23 @@ class Project < ActiveRecord::Base end def search query - where("projects.name LIKE :query OR projects.code LIKE :query OR projects.path LIKE :query", query: "%#{query}%") + where("projects.name LIKE :query OR projects.path LIKE :query", query: "%#{query}%") end def create_by_user(params, user) - namespace_id = params.delete(:namespace_id) || namespace.try(:id) + namespace_id = params.delete(:namespace_id) + namespace_id ||= current_user.namespace_id project = Project.new params Project.transaction do - # Build gitlab-hq code from GitLab HQ name + # Parametrize path for project # - slug = project.name.dup.parameterize - project.code = project.path = slug + # Ex. + # 'GitLab HQ'.parameterize => "gitlab-hq" + # + project.path = project.name.dup.parameterize project.owner = user project.namespace_id = namespace_id @@ -149,14 +152,14 @@ class Project < ActiveRecord::Base def to_param if namespace - namespace.code + "/" + code + namespace.path + "/" + path else - code + path end end def web_url - [Gitlab.config.url, code].join("/") + [Gitlab.config.url, path].join("/") end def common_notes @@ -213,7 +216,7 @@ class Project < ActiveRecord::Base def path_with_namespace if namespace - namespace.code + '/' + path + namespace.path + '/' + path else path end diff --git a/app/models/user.rb b/app/models/user.rb index b50fe3bd..20a5c479 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -69,7 +69,8 @@ class User < ActiveRecord::Base before_save :ensure_authentication_token alias_attribute :private_token, :authentication_token - delegate :code, to: :namespace, allow_nil: true, prefix: true + delegate :path, to: :namespace, allow_nil: true, prefix: true + delegate :id, to: :namespace, allow_nil: true, prefix: true # Scopes scope :not_in_project, ->(project) { where("id not in (:ids)", ids: project.users.map(&:id) ) } @@ -121,7 +122,7 @@ class User < ActiveRecord::Base def namespaces namespaces = [] - namespaces << self.namespace + namespaces << self.namespace if self.namespace namespaces = namespaces + Group.all if admin namespaces end diff --git a/app/observers/project_observer.rb b/app/observers/project_observer.rb index 16457e0c..6d92562a 100644 --- a/app/observers/project_observer.rb +++ b/app/observers/project_observer.rb @@ -1,15 +1,13 @@ class ProjectObserver < ActiveRecord::Observer - def before_save(project) + def after_save(project) + project.update_repository + # Move repository if namespace changed if project.namespace_id_changed? and not project.new_record? move_project(project) end end - def after_save(project) - project.update_repository - end - def after_destroy(project) log_info("Project \"#{project.name}\" was removed") @@ -27,8 +25,8 @@ class ProjectObserver < ActiveRecord::Observer end def move_project(project) - old_dir = Namespace.find_by_id(project.namespace_id_was).try(:code) || '' - new_dir = Namespace.find_by_id(project.namespace_id).try(:code) || '' + old_dir = Namespace.find_by_id(project.namespace_id_was).try(:path) || '' + new_dir = Namespace.find_by_id(project.namespace_id).try(:path) || '' # Create new dir if missing new_dir_path = File.join(Gitlab.config.git_base_path, new_dir) diff --git a/app/roles/push_observer.rb b/app/roles/push_observer.rb index 2ee60646..c5c5203d 100644 --- a/app/roles/push_observer.rb +++ b/app/roles/push_observer.rb @@ -114,7 +114,7 @@ module PushObserver id: commit.id, message: commit.safe_message, timestamp: commit.date.xmlschema, - url: "#{Gitlab.config.url}/#{code}/commits/#{commit.id}", + url: "#{Gitlab.config.url}/#{path}/commits/#{commit.id}", author: { name: commit.author_name, email: commit.author_email diff --git a/app/roles/repository.rb b/app/roles/repository.rb index e49761b6..ba61aa4c 100644 --- a/app/roles/repository.rb +++ b/app/roles/repository.rb @@ -87,7 +87,7 @@ module Repository end def namespace_dir - namespace.try(:code) || '' + namespace.try(:path) || '' end def update_repository @@ -164,12 +164,12 @@ module Repository return nil unless commit # Build file path - file_name = self.code + "-" + commit.id.to_s + ".tar.gz" - storage_path = Rails.root.join("tmp", "repositories", self.code) + file_name = self.path + "-" + commit.id.to_s + ".tar.gz" + storage_path = Rails.root.join("tmp", "repositories", self.path) file_path = File.join(storage_path, file_name) # Put files into a directory before archiving - prefix = self.code + "/" + prefix = self.path + "/" # Create file if not exists unless File.exists?(file_path) diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index b0b59a46..ad8d9f00 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -27,7 +27,7 @@ = link_to admin_projects_path do %h1= Project.count %hr - = link_to 'New Project', new_admin_project_path, class: "btn small" + = link_to 'New Project', new_project_path, class: "btn small" .span4 .ui-box %h5 Users diff --git a/app/views/admin/groups/_form.html.haml b/app/views/admin/groups/_form.html.haml index e85cce66..40d361b0 100644 --- a/app/views/admin/groups/_form.html.haml +++ b/app/views/admin/groups/_form.html.haml @@ -8,12 +8,12 @@ .input = f.text_field :name, placeholder: "Example Group", class: "xxlarge" .clearfix - = f.label :code do + = f.label :path do URL .input .input-prepend %span.add-on= web_app_url + 'groups/' - = f.text_field :code, placeholder: "example" + = f.text_field :path, placeholder: "example" .form-actions = f.submit 'Save group', class: "btn save-btn" diff --git a/app/views/admin/projects/_form.html.haml b/app/views/admin/projects/_form.html.haml index a238d938..eb12c61f 100644 --- a/app/views/admin/projects/_form.html.haml +++ b/app/views/admin/projects/_form.html.haml @@ -18,13 +18,6 @@ Path .input = text_field_tag :ppath, @project.path_to_repo, class: "xlarge", disabled: true - .clearfix - = f.label :code do - URL - .input - .input-prepend - %span.add-on= web_app_url - = f.text_field :code, placeholder: "example" - unless project.new_record? .clearfix diff --git a/app/views/projects/_form.html.haml b/app/views/projects/_form.html.haml index 9ee65942..68b9e789 100644 --- a/app/views/projects/_form.html.haml +++ b/app/views/projects/_form.html.haml @@ -19,13 +19,6 @@ .input-prepend %strong = text_field_tag :ppath, @project.path_to_repo, class: "xlarge", disabled: true - .clearfix - = f.label :code do - URL - .input - .input-prepend - %span.add-on= web_app_url - = f.text_field :code, placeholder: "example" - unless @project.new_record? || @project.heads.empty? .clearfix diff --git a/db/fixtures/development/001_admin.rb b/db/fixtures/development/001_admin.rb index c857f6bc..51939e8e 100644 --- a/db/fixtures/development/001_admin.rb +++ b/db/fixtures/development/001_admin.rb @@ -1,9 +1,10 @@ unless User.count > 0 admin = User.create( - :email => "admin@local.host", - :name => "Administrator", - :password => "5iveL!fe", - :password_confirmation => "5iveL!fe" + email: "admin@local.host", + name: "Administrator", + username: 'root', + password: "5iveL!fe", + password_confirmation: "5iveL!fe" ) admin.projects_limit = 10000 diff --git a/db/fixtures/development/003_users.rb b/db/fixtures/development/003_users.rb index 309eb90b..25705f1b 100644 --- a/db/fixtures/development/003_users.rb +++ b/db/fixtures/development/003_users.rb @@ -1,11 +1,11 @@ User.seed(:id, [ - { :id => 2, :name => Faker::Internet.user_name, :email => Faker::Internet.email}, - { :id => 3, :name => Faker::Internet.user_name, :email => Faker::Internet.email}, - { :id => 4, :name => Faker::Internet.user_name, :email => Faker::Internet.email}, - { :id => 5, :name => Faker::Internet.user_name, :email => Faker::Internet.email}, - { :id => 6, :name => Faker::Internet.user_name, :email => Faker::Internet.email}, - { :id => 7, :name => Faker::Internet.user_name, :email => Faker::Internet.email}, - { :id => 8, :name => Faker::Internet.user_name, :email => Faker::Internet.email}, - { :id => 9, :name => Faker::Internet.user_name, :email => Faker::Internet.email} + { id: 2, username: Faker::Internet.user_name, name: Faker::Name.name, email: Faker::Internet.email}, + { id: 3, username: Faker::Internet.user_name, name: Faker::Name.name, email: Faker::Internet.email}, + { id: 4, username: Faker::Internet.user_name, name: Faker::Name.name, email: Faker::Internet.email}, + { id: 5, username: Faker::Internet.user_name, name: Faker::Name.name, email: Faker::Internet.email}, + { id: 6, username: Faker::Internet.user_name, name: Faker::Name.name, email: Faker::Internet.email}, + { id: 7, username: Faker::Internet.user_name, name: Faker::Name.name, email: Faker::Internet.email}, + { id: 8, username: Faker::Internet.user_name, name: Faker::Name.name, email: Faker::Internet.email}, + { id: 9, username: Faker::Internet.user_name, name: Faker::Name.name, email: Faker::Internet.email} ]) diff --git a/db/fixtures/production/001_admin.rb b/db/fixtures/production/001_admin.rb index cfff6bf8..f119694d 100644 --- a/db/fixtures/production/001_admin.rb +++ b/db/fixtures/production/001_admin.rb @@ -1,8 +1,9 @@ admin = User.create( - :email => "admin@local.host", - :name => "Administrator", - :password => "5iveL!fe", - :password_confirmation => "5iveL!fe" + email: "admin@local.host", + name: "Administrator", + username: 'root', + password: "5iveL!fe", + password_confirmation: "5iveL!fe" ) admin.projects_limit = 10000 diff --git a/db/migrate/20121123164910_rename_code_to_path.rb b/db/migrate/20121123164910_rename_code_to_path.rb new file mode 100644 index 00000000..fb10baf5 --- /dev/null +++ b/db/migrate/20121123164910_rename_code_to_path.rb @@ -0,0 +1,11 @@ +class RenameCodeToPath < ActiveRecord::Migration + def up + remove_column :projects, :code + rename_column :namespaces, :code, :path + end + + def down + add_column :projects, :code, :string + rename_column :namespaces, :path, :code + end +end diff --git a/db/schema.rb b/db/schema.rb index 8ce3df0d..32dafed2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20121123104937) do +ActiveRecord::Schema.define(:version => 20121123164910) do create_table "events", :force => true do |t| t.string "target_type" @@ -82,7 +82,7 @@ ActiveRecord::Schema.define(:version => 20121123104937) do create_table "namespaces", :force => true do |t| t.string "name", :null => false - t.string "code", :null => false + t.string "path", :null => false t.integer "owner_id", :null => false t.datetime "created_at", :null => false t.datetime "updated_at", :null => false @@ -111,7 +111,6 @@ ActiveRecord::Schema.define(:version => 20121123104937) do t.datetime "created_at", :null => false t.datetime "updated_at", :null => false t.boolean "private_flag", :default => true, :null => false - t.string "code" t.integer "owner_id" t.string "default_branch" t.boolean "issues_enabled", :default => true, :null => false diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index a339ec4a..e9305b40 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -6,7 +6,7 @@ module Gitlab def user_project if @project ||= current_user.projects.find_by_id(params[:id]) || - current_user.projects.find_by_code(params[:id]) + current_user.projects.find_by_path(params[:id]) else not_found! end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 5a24c5d0..056fb034 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -34,6 +34,7 @@ module Gitlab extern_uid: uid, provider: provider, name: name, + username: email.match(/^[^@]*/)[0], email: email, password: password, password_confirmation: password, diff --git a/spec/factories.rb b/spec/factories.rb index a49cd69e..51b4c5c9 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -12,6 +12,7 @@ FactoryGirl.define do factory :user, aliases: [:author, :assignee, :owner] do email { Faker::Internet.email } name + username 'john' password "123456" password_confirmation { password } diff --git a/spec/observers/user_observer_spec.rb b/spec/observers/user_observer_spec.rb index 08254f44..ea5cf797 100644 --- a/spec/observers/user_observer_spec.rb +++ b/spec/observers/user_observer_spec.rb @@ -13,7 +13,7 @@ describe UserObserver do end context 'when a new user is created' do - let(:user) { double(:user, id: 42, password: 'P@ssword!', name: 'John', email: 'u@mail.local') } + let(:user) { double(:user, id: 42, password: 'P@ssword!', name: 'John', email: 'u@mail.local', username: 'root') } let(:notification) { double :notification } it 'sends an email' do diff --git a/spec/routing/admin_routing_spec.rb b/spec/routing/admin_routing_spec.rb index 60261c7a..fb26bf98 100644 --- a/spec/routing/admin_routing_spec.rb +++ b/spec/routing/admin_routing_spec.rb @@ -78,14 +78,6 @@ describe Admin::ProjectsController, "routing" do get("/admin/projects").should route_to('admin/projects#index') end - it "to #create" do - post("/admin/projects").should route_to('admin/projects#create') - end - - it "to #new" do - get("/admin/projects/new").should route_to('admin/projects#new') - end - it "to #edit" do get("/admin/projects/gitlab/edit").should route_to('admin/projects#edit', id: 'gitlab') end