summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2015-12-03 23:33:52 -0800
committerStan Hu <stanhu@gmail.com>2015-12-05 00:04:44 -0800
commitd800a949d2d5497e8aff3ae28ec8520e5b99cdb8 (patch)
tree0d8945e63d13b6738299f21bad7eb9ff3359dc77
parentd2f9a9012d55a7dc9c8e9e61b946c52836a1d8f1 (diff)
downloadgitlab-ce-d800a949d2d5497e8aff3ae28ec8520e5b99cdb8.tar.gz
Fix Error 500 when creating global milestones with Unicode characters
Two issues: 1. The constraints in the resources were incorrect. Here's what it was before: ``` group_milestone GET /groups/:group_id/milestones/:id(.:format) groups/milestones#show {:id=>/[a-zA-Z.0-9_\-]+(?<!\.atom)/, :group_id=>/[a-zA-Z.0-9_\-]+(?<!\.atom)/} ``` In this case, id is actually the title of the milestone, which can be anything at the moment. After: ``` group_milestone GET /groups/:group_id/milestones/:id(.:format) groups/milestones#show {:id=>/[^\/]+/, :group_id=>/[a-zA-Z.0-9_\-]+(?<!\.atom)/} ``` 2. `parameterize` would strip all Unicode characters, leaving a blank string. Rails would report something like: ActionView::Template::Error (No route matches {:action=>"show", :controller=>"groups/milestones", :group_id=>#<Group id: 48, name: "ops-dev", path: "ops-dev", owner_id: nil, created_at: "2015-11-15 08:55:30", updated_at: "2015-12-02 06:23:26", type: "Group", description: "", avatar: "sha1.c71e73d51af1865c1bbbf6208e10044d46c9bb93.png", public: false>, :id=>"", :title=>"肯定不是中文的问题"} missing required keys: [:id]): This change uses the babosa library to create a better slug, which surprisingly isn't actually used by the global milestone controllers. Instead, they use the title passed as a query string for some reason. Closes https://github.com/gitlabhq/gitlabhq/issues/9881 Fix constraints
-rw-r--r--CHANGELOG1
-rw-r--r--Gemfile1
-rw-r--r--Gemfile.lock2
-rw-r--r--app/controllers/groups/milestones_controller.rb2
-rw-r--r--app/models/global_milestone.rb2
-rw-r--r--config/routes.rb2
-rw-r--r--spec/controllers/groups/milestones_controller_spec.rb27
7 files changed, 34 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 7b2f1528656..228848c13f2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,7 @@ v 8.3.0 (unreleased)
v 8.2.3
- Fix application settings cache not expiring after changes (Stan Hu)
+ - Fix Error 500s when creating global milestones with Unicode characters (Stan Hu)
v 8.2.2
- Fix 404 in redirection after removing a project (Stan Hu)
diff --git a/Gemfile b/Gemfile
index 67640bb9ae0..860e6bee47d 100644
--- a/Gemfile
+++ b/Gemfile
@@ -171,6 +171,7 @@ gem "underscore-rails", "~> 1.4.4"
# Sanitize user input
gem "sanitize", '~> 2.0'
+gem 'babosa', '~> 1.0.2'
# Protect against bruteforcing
gem "rack-attack", '~> 4.3.0'
diff --git a/Gemfile.lock b/Gemfile.lock
index cd1855758b2..be3e39d8e84 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -73,6 +73,7 @@ GEM
descendants_tracker (~> 0.0.4)
ice_nine (~> 0.11.0)
thread_safe (~> 0.3, >= 0.3.1)
+ babosa (1.0.2)
bcrypt (3.1.10)
benchmark-ips (2.3.0)
better_errors (1.0.1)
@@ -823,6 +824,7 @@ DEPENDENCIES
asciidoctor (~> 1.5.2)
attr_encrypted (~> 1.3.4)
awesome_print (~> 1.2.0)
+ babosa (~> 1.0.2)
benchmark-ips
better_errors (~> 1.0.1)
binding_of_caller (~> 0.7.2)
diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb
index 10233222ee1..0c2a350bc39 100644
--- a/app/controllers/groups/milestones_controller.rb
+++ b/app/controllers/groups/milestones_controller.rb
@@ -46,7 +46,7 @@ class Groups::MilestonesController < Groups::ApplicationController
end
def milestone_path(title)
- group_milestone_path(@group, title.parameterize, title: title)
+ group_milestone_path(@group, title.to_slug.to_s, title: title)
end
def projects
diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index 1321ccd963f..85aa71662fe 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -16,7 +16,7 @@ class GlobalMilestone
end
def safe_title
- @title.parameterize
+ @title.to_slug.to_s
end
def projects
diff --git a/config/routes.rb b/config/routes.rb
index 5c114452a3f..fdd387fd184 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -368,7 +368,7 @@ Rails.application.routes.draw do
end
resource :avatar, only: [:destroy]
- resources :milestones, only: [:index, :show, :update, :new, :create]
+ resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
end
end
diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb
new file mode 100644
index 00000000000..eb0c6ac6d80
--- /dev/null
+++ b/spec/controllers/groups/milestones_controller_spec.rb
@@ -0,0 +1,27 @@
+require 'spec_helper'
+
+describe Groups::MilestonesController do
+ let(:group) { create(:group) }
+ let(:project) { create(:project, group: group) }
+ let(:project2) { create(:empty_project, group: group) }
+ let(:user) { create(:user) }
+ let(:title) { '肯定不是中文的问题' }
+
+ before do
+ sign_in(user)
+ group.add_owner(user)
+ project.team << [user, :master]
+ controller.instance_variable_set(:@group, group)
+ end
+
+ describe "#create" do
+ it "should create group milestone with Chinese title" do
+ post :create,
+ group_id: group.id,
+ milestone: { project_ids: [project.id, project2.id], title: title }
+
+ expect(response).to redirect_to(group_milestone_path(group, title.to_slug.to_s, title: title))
+ expect(Milestone.where(title: title).count).to eq(2)
+ end
+ end
+end