From 2c5a95cbeb3ac4a19ad177d03d2703235e1f1c3c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 12 Oct 2016 18:49:41 +0300 Subject: Rename users routing from /u/:username to /users/:username for consistency with other routes Signed-off-by: Dmitriy Zaporozhets --- config/routes/user.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/config/routes/user.rb b/config/routes/user.rb index 54bbcb18f6a..ae15b9d02a3 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -1,8 +1,5 @@ require 'constraints/user_url_constrainer' -get '/u/:username', to: redirect('/%{username}'), - constraints: { username: /[a-zA-Z.0-9_\-]+(? Date: Thu, 13 Oct 2016 14:11:26 +0300 Subject: Update users routing spec Signed-off-by: Dmitriy Zaporozhets --- spec/features/users_spec.rb | 12 ++++++++++++ spec/routing/routing_spec.rb | 12 ++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index 6498b7317b4..c78c84dc5d0 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -49,6 +49,18 @@ feature 'Users', feature: true do expect(current_path).to eq user_path(user) expect(page).to have_text(user.name) end + + scenario '/u/user1/groups redirects to user groups page' do + visit '/u/user1/groups' + + expect(current_path).to eq user_groups_path(user) + end + + scenario '/u/user1/projects redirects to user projects page' do + visit '/u/user1/projects' + + expect(current_path).to eq user_projects_path(user) + end end def errors_on_page(page) diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 0ee1c811dfb..f848d96c729 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -15,27 +15,27 @@ describe UsersController, "routing" do end it "to #groups" do - expect(get("/u/User/groups")).to route_to('users#groups', username: 'User') + expect(get("/users/User/groups")).to route_to('users#groups', username: 'User') end it "to #projects" do - expect(get("/u/User/projects")).to route_to('users#projects', username: 'User') + expect(get("/users/User/projects")).to route_to('users#projects', username: 'User') end it "to #contributed" do - expect(get("/u/User/contributed")).to route_to('users#contributed', username: 'User') + expect(get("/users/User/contributed")).to route_to('users#contributed', username: 'User') end it "to #snippets" do - expect(get("/u/User/snippets")).to route_to('users#snippets', username: 'User') + expect(get("/users/User/snippets")).to route_to('users#snippets', username: 'User') end it "to #calendar" do - expect(get("/u/User/calendar")).to route_to('users#calendar', username: 'User') + expect(get("/users/User/calendar")).to route_to('users#calendar', username: 'User') end it "to #calendar_activities" do - expect(get("/u/User/calendar_activities")).to route_to('users#calendar_activities', username: 'User') + expect(get("/users/User/calendar_activities")).to route_to('users#calendar_activities', username: 'User') end end -- cgit v1.2.1 From cb7872c3a0c789f9e906492098bb7d643f135e52 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 13 Oct 2016 14:24:09 +0300 Subject: Remove /u/ prefix from user pages in documentation Signed-off-by: Dmitriy Zaporozhets --- doc/api/award_emoji.md | 18 +++++++++--------- doc/api/builds.md | 8 ++++---- doc/api/commits.md | 8 ++++---- doc/api/deployments.md | 12 ++++++------ doc/api/issues.md | 38 +++++++++++++++++++------------------- doc/api/keys.md | 2 +- doc/api/merge_requests.md | 18 +++++++++--------- doc/api/notes.md | 6 +++--- doc/api/pipelines.md | 10 +++++----- doc/api/projects.md | 10 +++++----- doc/api/todos.md | 18 +++++++++--------- doc/api/users.md | 24 ++++++++++++------------ doc/development/performance.md | 2 +- 13 files changed, 87 insertions(+), 87 deletions(-) diff --git a/doc/api/award_emoji.md b/doc/api/award_emoji.md index c464e3f3f71..06111f4ab67 100644 --- a/doc/api/award_emoji.md +++ b/doc/api/award_emoji.md @@ -43,7 +43,7 @@ Example Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://gitlab.example.com/u/root" + "web_url": "http://gitlab.example.com/root" }, "created_at": "2016-06-15T10:09:34.206Z", "updated_at": "2016-06-15T10:09:34.206Z", @@ -59,7 +59,7 @@ Example Response: "id": 26, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/7e65550957227bd38fe2d7fbc6fd2f7b?s=80&d=identicon", - "web_url": "http://gitlab.example.com/u/user4" + "web_url": "http://gitlab.example.com/user4" }, "created_at": "2016-06-15T10:09:34.177Z", "updated_at": "2016-06-15T10:09:34.177Z", @@ -103,7 +103,7 @@ Example Response: "id": 26, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/7e65550957227bd38fe2d7fbc6fd2f7b?s=80&d=identicon", - "web_url": "http://gitlab.example.com/u/user4" + "web_url": "http://gitlab.example.com/user4" }, "created_at": "2016-06-15T10:09:34.177Z", "updated_at": "2016-06-15T10:09:34.177Z", @@ -146,7 +146,7 @@ Example Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://gitlab.example.com/u/root" + "web_url": "http://gitlab.example.com/root" }, "created_at": "2016-06-17T17:47:29.266Z", "updated_at": "2016-06-17T17:47:29.266Z", @@ -190,7 +190,7 @@ Example Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://gitlab.example.com/u/root" + "web_url": "http://gitlab.example.com/root" }, "created_at": "2016-06-17T17:47:29.266Z", "updated_at": "2016-06-17T17:47:29.266Z", @@ -238,7 +238,7 @@ Example Response: "id": 26, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/7e65550957227bd38fe2d7fbc6fd2f7b?s=80&d=identicon", - "web_url": "http://gitlab.example.com/u/user4" + "web_url": "http://gitlab.example.com/user4" }, "created_at": "2016-06-15T10:09:34.197Z", "updated_at": "2016-06-15T10:09:34.197Z", @@ -279,7 +279,7 @@ Example Response: "id": 26, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/7e65550957227bd38fe2d7fbc6fd2f7b?s=80&d=identicon", - "web_url": "http://gitlab.example.com/u/user4" + "web_url": "http://gitlab.example.com/user4" }, "created_at": "2016-06-15T10:09:34.197Z", "updated_at": "2016-06-15T10:09:34.197Z", @@ -319,7 +319,7 @@ Example Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://gitlab.example.com/u/root" + "web_url": "http://gitlab.example.com/root" }, "created_at": "2016-06-17T19:59:55.888Z", "updated_at": "2016-06-17T19:59:55.888Z", @@ -362,7 +362,7 @@ Example Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://gitlab.example.com/u/root" + "web_url": "http://gitlab.example.com/root" }, "created_at": "2016-06-17T19:59:55.888Z", "updated_at": "2016-06-17T19:59:55.888Z", diff --git a/doc/api/builds.md b/doc/api/builds.md index e8a9e4743d3..e40f198696d 100644 --- a/doc/api/builds.md +++ b/doc/api/builds.md @@ -64,7 +64,7 @@ Example of response "state": "active", "twitter": "", "username": "root", - "web_url": "http://gitlab.dev/u/root", + "web_url": "http://gitlab.dev/root", "website_url": "" } }, @@ -108,7 +108,7 @@ Example of response "state": "active", "twitter": "", "username": "root", - "web_url": "http://gitlab.dev/u/root", + "web_url": "http://gitlab.dev/root", "website_url": "" } } @@ -212,7 +212,7 @@ Example of response "state": "active", "twitter": "", "username": "root", - "web_url": "http://gitlab.dev/u/root", + "web_url": "http://gitlab.dev/root", "website_url": "" } } @@ -279,7 +279,7 @@ Example of response "state": "active", "twitter": "", "username": "root", - "web_url": "http://gitlab.dev/u/root", + "web_url": "http://gitlab.dev/root", "website_url": "" } } diff --git a/doc/api/commits.md b/doc/api/commits.md index 3e20beefb8a..6e0882a94de 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -288,7 +288,7 @@ Example response: ```json { "author" : { - "web_url" : "https://gitlab.example.com/u/thedude", + "web_url" : "https://gitlab.example.com/thedude", "avatar_url" : "https://gitlab.example.com/uploads/user/avatar/28/The-Big-Lebowski-400-400.png", "username" : "thedude", "state" : "active", @@ -343,7 +343,7 @@ Example response: "author" : { "username" : "thedude", "state" : "active", - "web_url" : "https://gitlab.example.com/u/thedude", + "web_url" : "https://gitlab.example.com/thedude", "avatar_url" : "https://gitlab.example.com/uploads/user/avatar/28/The-Big-Lebowski-400-400.png", "id" : 28, "name" : "Jeff Lebowski" @@ -370,7 +370,7 @@ Example response: "id" : 28, "name" : "Jeff Lebowski", "username" : "thedude", - "web_url" : "https://gitlab.example.com/u/thedude", + "web_url" : "https://gitlab.example.com/thedude", "state" : "active", "avatar_url" : "https://gitlab.example.com/uploads/user/avatar/28/The-Big-Lebowski-400-400.png" }, @@ -408,7 +408,7 @@ Example response: ```json { "author" : { - "web_url" : "https://gitlab.example.com/u/thedude", + "web_url" : "https://gitlab.example.com/thedude", "name" : "Jeff Lebowski", "avatar_url" : "https://gitlab.example.com/uploads/user/avatar/28/The-Big-Lebowski-400-400.png", "username" : "thedude", diff --git a/doc/api/deployments.md b/doc/api/deployments.md index 417962de82d..3d95c4cde60 100644 --- a/doc/api/deployments.md +++ b/doc/api/deployments.md @@ -56,7 +56,7 @@ Example of response "state": "active", "twitter": "", "username": "root", - "web_url": "http://localhost:3000/u/root", + "web_url": "http://localhost:3000/root", "website_url": "" } }, @@ -75,7 +75,7 @@ Example of response "name": "Administrator", "state": "active", "username": "root", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" } }, { @@ -114,7 +114,7 @@ Example of response "state": "active", "twitter": "", "username": "root", - "web_url": "http://localhost:3000/u/root", + "web_url": "http://localhost:3000/root", "website_url": "" } }, @@ -133,7 +133,7 @@ Example of response "name": "Administrator", "state": "active", "username": "root", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" } } ] @@ -169,7 +169,7 @@ Example of response "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "environment": { "id": 9, @@ -193,7 +193,7 @@ Example of response "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://localhost:3000/u/root", + "web_url": "http://localhost:3000/root", "created_at": "2016-08-11T07:09:20.351Z", "is_admin": true, "bio": null, diff --git a/doc/api/issues.md b/doc/api/issues.md index eed0d2fce51..134263d27b4 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -46,7 +46,7 @@ Example response: "author" : { "state" : "active", "id" : 18, - "web_url" : "https://gitlab.example.com/u/eileen.lowe", + "web_url" : "https://gitlab.example.com/eileen.lowe", "name" : "Alexandra Bashirian", "avatar_url" : null, "username" : "eileen.lowe" @@ -67,7 +67,7 @@ Example response: "state" : "active", "id" : 1, "name" : "Administrator", - "web_url" : "https://gitlab.example.com/u/root", + "web_url" : "https://gitlab.example.com/root", "avatar_url" : null, "username" : "root" }, @@ -134,7 +134,7 @@ Example response: }, "author" : { "state" : "active", - "web_url" : "https://gitlab.example.com/u/root", + "web_url" : "https://gitlab.example.com/root", "avatar_url" : null, "username" : "root", "id" : 1, @@ -145,7 +145,7 @@ Example response: "iid" : 1, "assignee" : { "avatar_url" : null, - "web_url" : "https://gitlab.example.com/u/lennie", + "web_url" : "https://gitlab.example.com/lennie", "state" : "active", "username" : "lennie", "id" : 9, @@ -215,7 +215,7 @@ Example response: }, "author" : { "state" : "active", - "web_url" : "https://gitlab.example.com/u/root", + "web_url" : "https://gitlab.example.com/root", "avatar_url" : null, "username" : "root", "id" : 1, @@ -226,7 +226,7 @@ Example response: "iid" : 1, "assignee" : { "avatar_url" : null, - "web_url" : "https://gitlab.example.com/u/lennie", + "web_url" : "https://gitlab.example.com/lennie", "state" : "active", "username" : "lennie", "id" : 9, @@ -281,7 +281,7 @@ Example response: }, "author" : { "state" : "active", - "web_url" : "https://gitlab.example.com/u/root", + "web_url" : "https://gitlab.example.com/root", "avatar_url" : null, "username" : "root", "id" : 1, @@ -292,7 +292,7 @@ Example response: "iid" : 1, "assignee" : { "avatar_url" : null, - "web_url" : "https://gitlab.example.com/u/lennie", + "web_url" : "https://gitlab.example.com/lennie", "state" : "active", "username" : "lennie", "id" : 9, @@ -357,7 +357,7 @@ Example response: "name" : "Alexandra Bashirian", "avatar_url" : null, "state" : "active", - "web_url" : "https://gitlab.example.com/u/eileen.lowe", + "web_url" : "https://gitlab.example.com/eileen.lowe", "id" : 18, "username" : "eileen.lowe" }, @@ -414,7 +414,7 @@ Example response: "username" : "eileen.lowe", "id" : 18, "state" : "active", - "web_url" : "https://gitlab.example.com/u/eileen.lowe" + "web_url" : "https://gitlab.example.com/eileen.lowe" }, "state" : "closed", "title" : "Issues with auth", @@ -500,7 +500,7 @@ Example response: "id": 12, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/46f6f7dc858ada7be1853f7fb96e81da?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/axel.block" + "web_url": "https://gitlab.example.com/axel.block" }, "author": { "name": "Kris Steuber", @@ -508,7 +508,7 @@ Example response: "id": 10, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/7a190fecbaa68212a4b68aeb6e3acd10?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/solon.cremin" + "web_url": "https://gitlab.example.com/solon.cremin" }, "due_date": null, "web_url": "http://example.com/example/example/issues/11", @@ -557,7 +557,7 @@ Example response: "id": 12, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/46f6f7dc858ada7be1853f7fb96e81da?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/axel.block" + "web_url": "https://gitlab.example.com/axel.block" }, "author": { "name": "Kris Steuber", @@ -565,7 +565,7 @@ Example response: "id": 10, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/7a190fecbaa68212a4b68aeb6e3acd10?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/solon.cremin" + "web_url": "https://gitlab.example.com/solon.cremin" }, "due_date": null, "web_url": "http://example.com/example/example/issues/11", @@ -614,7 +614,7 @@ Example response: "id": 21, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/3e6f06a86cf27fa8b56f3f74f7615987?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/keyon" + "web_url": "https://gitlab.example.com/keyon" }, "author": { "name": "Vivian Hermann", @@ -622,7 +622,7 @@ Example response: "id": 11, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/5224fd70153710e92fb8bcf79ac29d67?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/orville" + "web_url": "https://gitlab.example.com/orville" }, "subscribed": false, "due_date": null, @@ -669,7 +669,7 @@ Example response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/root" + "web_url": "https://gitlab.example.com/root" }, "action_name": "marked", "target_type": "Issue", @@ -700,7 +700,7 @@ Example response: "id": 14, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/a7fa515d53450023c83d62986d0658a8?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/francisca" + "web_url": "https://gitlab.example.com/francisca" }, "author": { "name": "Maxie Medhurst", @@ -708,7 +708,7 @@ Example response: "id": 12, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/a0d477b3ea21970ce6ffcbb817b0b435?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/craig_rutherford" + "web_url": "https://gitlab.example.com/craig_rutherford" }, "subscribed": true, "user_notes_count": 7, diff --git a/doc/api/keys.md b/doc/api/keys.md index faa6f212b43..b68f08a007d 100644 --- a/doc/api/keys.md +++ b/doc/api/keys.md @@ -24,7 +24,7 @@ Parameters: "id": 25, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/cfa35b8cd2ec278026357769582fa563?s=40\u0026d=identicon", - "web_url": "http://localhost:3000/u/john_smith", + "web_url": "http://localhost:3000/john_smith", "created_at": "2015-09-03T07:24:01.670Z", "is_admin": false, "bio": null, diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 494040a1ce8..f4167403c2c 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -621,7 +621,7 @@ Example response when the GitLab issue tracker is used: "author" : { "state" : "active", "id" : 18, - "web_url" : "https://gitlab.example.com/u/eileen.lowe", + "web_url" : "https://gitlab.example.com/eileen.lowe", "name" : "Alexandra Bashirian", "avatar_url" : null, "username" : "eileen.lowe" @@ -642,7 +642,7 @@ Example response when the GitLab issue tracker is used: "state" : "active", "id" : 1, "name" : "Administrator", - "web_url" : "https://gitlab.example.com/u/root", + "web_url" : "https://gitlab.example.com/root", "avatar_url" : null, "username" : "root" }, @@ -711,7 +711,7 @@ Example response: "id": 19, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/39ce4a2822cc896933ffbd68c1470e55?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/leila" + "web_url": "https://gitlab.example.com/leila" }, "assignee": { "name": "Celine Wehner", @@ -719,7 +719,7 @@ Example response: "id": 16, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/f4cd5605b769dd2ce405a27c6e6f2684?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/carli" + "web_url": "https://gitlab.example.com/carli" }, "source_project_id": 5, "target_project_id": 5, @@ -787,7 +787,7 @@ Example response: "id": 19, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/39ce4a2822cc896933ffbd68c1470e55?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/leila" + "web_url": "https://gitlab.example.com/leila" }, "assignee": { "name": "Celine Wehner", @@ -795,7 +795,7 @@ Example response: "id": 16, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/f4cd5605b769dd2ce405a27c6e6f2684?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/carli" + "web_url": "https://gitlab.example.com/carli" }, "source_project_id": 5, "target_project_id": 5, @@ -858,7 +858,7 @@ Example response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/root" + "web_url": "https://gitlab.example.com/root" }, "action_name": "marked", "target_type": "MergeRequest", @@ -881,7 +881,7 @@ Example response: "id": 14, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/a7fa515d53450023c83d62986d0658a8?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/francisca" + "web_url": "https://gitlab.example.com/francisca" }, "assignee": { "name": "Dr. Gabrielle Strosin", @@ -889,7 +889,7 @@ Example response: "id": 4, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/733005fcd7e6df12d2d8580171ccb966?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/barrett.krajcik" + "web_url": "https://gitlab.example.com/barrett.krajcik" }, "source_project_id": 3, "target_project_id": 3, diff --git a/doc/api/notes.md b/doc/api/notes.md index 572844b8b3f..58d40eecf3e 100644 --- a/doc/api/notes.md +++ b/doc/api/notes.md @@ -143,7 +143,7 @@ Example Response: "state": "active", "created_at": "2013-09-30T13:46:01Z", "avatar_url": "http://www.gravatar.com/avatar/5224fd70153710e92fb8bcf79ac29d67?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/pipin" + "web_url": "https://gitlab.example.com/pipin" }, "created_at": "2016-04-05T22:10:44.164Z", "system": false, @@ -268,7 +268,7 @@ Example Response: "state": "active", "created_at": "2013-09-30T13:46:01Z", "avatar_url": "http://www.gravatar.com/avatar/5224fd70153710e92fb8bcf79ac29d67?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/pipin" + "web_url": "https://gitlab.example.com/pipin" }, "created_at": "2016-04-06T16:51:53.239Z", "system": false, @@ -398,7 +398,7 @@ Example Response: "state": "active", "created_at": "2013-09-30T13:46:01Z", "avatar_url": "http://www.gravatar.com/avatar/5224fd70153710e92fb8bcf79ac29d67?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/pipin" + "web_url": "https://gitlab.example.com/pipin" }, "created_at": "2016-04-05T22:11:59.923Z", "system": false, diff --git a/doc/api/pipelines.md b/doc/api/pipelines.md index 847408a7f61..a29b3eb6f44 100644 --- a/doc/api/pipelines.md +++ b/doc/api/pipelines.md @@ -34,7 +34,7 @@ Example of response "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "created_at": "2016-08-16T10:23:19.007Z", "updated_at": "2016-08-16T10:23:19.216Z", @@ -57,7 +57,7 @@ Example of response "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "created_at": "2016-08-16T10:23:21.184Z", "updated_at": "2016-08-16T10:23:21.314Z", @@ -103,7 +103,7 @@ Example of response "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "created_at": "2016-08-11T11:28:34.085Z", "updated_at": "2016-08-11T11:32:35.169Z", @@ -148,7 +148,7 @@ Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "created_at": "2016-08-11T11:28:34.085Z", "updated_at": "2016-08-11T11:32:35.169Z", @@ -193,7 +193,7 @@ Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "created_at": "2016-08-11T11:28:34.085Z", "updated_at": "2016-08-11T11:32:35.169Z", diff --git a/doc/api/projects.md b/doc/api/projects.md index f96bf7f6d63..b7791b4748a 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -465,7 +465,7 @@ Parameters: "id": 1, "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "author_username": "root" }, @@ -482,7 +482,7 @@ Parameters: "id": 1, "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "author_username": "john", "data": { @@ -528,7 +528,7 @@ Parameters: "id": 1, "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "author_username": "root" }, @@ -552,7 +552,7 @@ Parameters: "id": 1, "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "created_at": "2015-12-04T10:33:56.698Z", "system": false, @@ -567,7 +567,7 @@ Parameters: "id": 1, "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "author_username": "root" } diff --git a/doc/api/todos.md b/doc/api/todos.md index 0cd644dfd2f..a5e81801024 100644 --- a/doc/api/todos.md +++ b/doc/api/todos.md @@ -44,7 +44,7 @@ Example Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/root" + "web_url": "https://gitlab.example.com/root" }, "action_name": "marked", "target_type": "MergeRequest", @@ -67,7 +67,7 @@ Example Response: "id": 12, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/a0d477b3ea21970ce6ffcbb817b0b435?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/craig_rutherford" + "web_url": "https://gitlab.example.com/craig_rutherford" }, "assignee": { "name": "Administrator", @@ -75,7 +75,7 @@ Example Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/root" + "web_url": "https://gitlab.example.com/root" }, "source_project_id": 2, "target_project_id": 2, @@ -117,7 +117,7 @@ Example Response: "id": 12, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/a0d477b3ea21970ce6ffcbb817b0b435?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/craig_rutherford" + "web_url": "https://gitlab.example.com/craig_rutherford" }, "action_name": "assigned", "target_type": "MergeRequest", @@ -140,7 +140,7 @@ Example Response: "id": 12, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/a0d477b3ea21970ce6ffcbb817b0b435?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/craig_rutherford" + "web_url": "https://gitlab.example.com/craig_rutherford" }, "assignee": { "name": "Administrator", @@ -148,7 +148,7 @@ Example Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/root" + "web_url": "https://gitlab.example.com/root" }, "source_project_id": 2, "target_project_id": 2, @@ -215,7 +215,7 @@ Example Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/root" + "web_url": "https://gitlab.example.com/root" }, "action_name": "marked", "target_type": "MergeRequest", @@ -238,7 +238,7 @@ Example Response: "id": 12, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/a0d477b3ea21970ce6ffcbb817b0b435?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/craig_rutherford" + "web_url": "https://gitlab.example.com/craig_rutherford" }, "assignee": { "name": "Administrator", @@ -246,7 +246,7 @@ Example Response: "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "https://gitlab.example.com/u/root" + "web_url": "https://gitlab.example.com/root" }, "source_project_id": 2, "target_project_id": 2, diff --git a/doc/api/users.md b/doc/api/users.md index a52b2d51d78..2b12770d5a5 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -20,7 +20,7 @@ GET /users "name": "John Smith", "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/cd8.jpeg", - "web_url": "http://localhost:3000/u/john_smith" + "web_url": "http://localhost:3000/john_smith" }, { "id": 2, @@ -28,7 +28,7 @@ GET /users "name": "Jack Smith", "state": "blocked", "avatar_url": "http://gravatar.com/../e32131cd8.jpeg", - "web_url": "http://localhost:3000/u/jack_smith" + "web_url": "http://localhost:3000/jack_smith" } ] ``` @@ -48,7 +48,7 @@ GET /users "name": "John Smith", "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/index.jpg", - "web_url": "http://localhost:3000/u/john_smith", + "web_url": "http://localhost:3000/john_smith", "created_at": "2012-05-23T08:00:58Z", "is_admin": false, "bio": null, @@ -81,7 +81,7 @@ GET /users "name": "Jack Smith", "state": "blocked", "avatar_url": "http://localhost:3000/uploads/user/avatar/2/index.jpg", - "web_url": "http://localhost:3000/u/jack_smith", + "web_url": "http://localhost:3000/jack_smith", "created_at": "2012-05-23T08:01:01Z", "is_admin": false, "bio": null, @@ -141,7 +141,7 @@ Parameters: "name": "John Smith", "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/cd8.jpeg", - "web_url": "http://localhost:3000/u/john_smith", + "web_url": "http://localhost:3000/john_smith", "created_at": "2012-05-23T08:00:58Z", "is_admin": false, "bio": null, @@ -172,7 +172,7 @@ Parameters: "name": "John Smith", "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/index.jpg", - "web_url": "http://localhost:3000/u/john_smith", + "web_url": "http://localhost:3000/john_smith", "created_at": "2012-05-23T08:00:58Z", "is_admin": false, "bio": null, @@ -293,7 +293,7 @@ GET /user "name": "John Smith", "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/index.jpg", - "web_url": "http://localhost:3000/u/john_smith", + "web_url": "http://localhost:3000/john_smith", "created_at": "2012-05-23T08:00:58Z", "is_admin": false, "bio": null, @@ -665,7 +665,7 @@ Example response: "id": 1, "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "author_username": "root" }, @@ -682,7 +682,7 @@ Example response: "id": 1, "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "author_username": "john", "data": { @@ -728,7 +728,7 @@ Example response: "id": 1, "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "author_username": "root" }, @@ -752,7 +752,7 @@ Example response: "id": 1, "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "created_at": "2015-12-04T10:33:56.698Z", "system": false, @@ -767,7 +767,7 @@ Example response: "id": 1, "state": "active", "avatar_url": "http://localhost:3000/uploads/user/avatar/1/fox_avatar.png", - "web_url": "http://localhost:3000/u/root" + "web_url": "http://localhost:3000/root" }, "author_username": "root" } diff --git a/doc/development/performance.md b/doc/development/performance.md index 7ff603e2c4a..a00c0fe07b5 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -253,5 +253,5 @@ impact on runtime performance, and as such, using a constant instead of referencing an object directly may even slow code down. [#15607]: https://gitlab.com/gitlab-org/gitlab-ce/issues/15607 -[yorickpeterse]: https://gitlab.com/u/yorickpeterse +[yorickpeterse]: https://gitlab.com/yorickpeterse [anti-pattern]: https://en.wikipedia.org/wiki/Anti-pattern -- cgit v1.2.1 From 41de407837dec94925af40c9c556ec9303f4de4e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 13 Oct 2016 14:25:53 +0300 Subject: Add user routing rename to CHANGELOG Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index e478e8c3365..2cbf5bc3277 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -102,6 +102,7 @@ v 8.13.0 (unreleased) - Fix a typo in doc/api/labels.md - API: all unknown routing will be handled with 404 Not Found - Make guests unable to view MRs on private projects + - Change user pages routing from /u/:username/PATH to /users/:username/PATH v 8.12.6 - Update mailroom to 0.8.1 in Gemfile.lock !6814 -- cgit v1.2.1 From 5c5259335f8bcc4de117c1e36648a269911281fb Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 1 Sep 2016 11:39:37 +0100 Subject: Add instrumentation to conflict classes --- config/initializers/metrics.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb index be22085b0df..3b8771543e4 100644 --- a/config/initializers/metrics.rb +++ b/config/initializers/metrics.rb @@ -67,6 +67,7 @@ if Gitlab::Metrics.enabled? ['app', 'finders'] => ['app', 'finders'], ['app', 'mailers', 'emails'] => ['app', 'mailers'], ['app', 'services', '**'] => ['app', 'services'], + ['lib', 'gitlab', 'conflicts'] => ['lib'], ['lib', 'gitlab', 'diff'] => ['lib'], ['lib', 'gitlab', 'email', 'message'] => ['lib'], ['lib', 'gitlab', 'checks'] => ['lib'] -- cgit v1.2.1 From 3f71c43e88c56bb5310c8814cd9f95cafb4f53ef Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 1 Sep 2016 13:59:10 +0100 Subject: Allow setting content for resolutions When reading conflicts: 1. Add a `type` field. `text` works as before, and has `sections`; `text-editor` is a file with ambiguous conflict markers that can only be resolved in an editor. 2. Add a `content_path` field pointing to a JSON representation of the file's content for a single file. 3. Hitting `content_path` returns a similar datastructure to the `file`, but without the `content_path` and `sections` fields, and with a `content` field containing the full contents of the file (with conflict markers). When writing conflicts: 1. Instead of `sections` being at the top level, they are now in a `files` array. This matches the read format better. 2. The `files` array contains file hashes, each of which must contain: a. `new_path` b. `old_path` c. EITHER `sections` (which works as before) or `content` (with the full content of the resolved file). --- app/controllers/application_controller.rb | 7 +- .../projects/merge_requests_controller.rb | 20 ++- app/models/merge_request.rb | 2 +- app/services/merge_requests/resolve_service.rb | 24 ++- config/routes/project.rb | 1 + lib/gitlab/conflict/file.rb | 51 ++++++- lib/gitlab/conflict/file_collection.rb | 4 + lib/gitlab/conflict/parser.rb | 15 +- lib/gitlab/conflict/resolution_error.rb | 6 + .../projects/merge_requests_controller_spec.rb | 168 +++++++++++++++++++-- .../merge_requests/resolve_service_spec.rb | 139 +++++++++++++++-- 11 files changed, 395 insertions(+), 42 deletions(-) create mode 100644 lib/gitlab/conflict/resolution_error.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b3455e04c29..bf37421771f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -114,7 +114,12 @@ class ApplicationController < ActionController::Base end def render_404 - render file: Rails.root.join("public", "404"), layout: false, status: "404" + respond_to do |format| + format.json { head :not_found } + format.any do + render file: Rails.root.join("public", "404"), layout: false, status: "404" + end + end end def no_cache_headers diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 869d96b86f4..2c7a0062f2b 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -9,15 +9,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController before_action :module_enabled before_action :merge_request, only: [ - :edit, :update, :show, :diffs, :commits, :conflicts, :builds, :pipelines, :merge, :merge_check, + :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines, :merge, :merge_check, :ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip, :resolve_conflicts, :assign_related_issues ] before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds, :pipelines] - before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :builds, :pipelines] + before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines] before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds, :merge_check] before_action :define_commit_vars, only: [:diffs] before_action :define_diff_comment_vars, only: [:diffs] - before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :pipelines] + before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :conflict_for_path, :pipelines] before_action :close_merge_request_without_source_project, only: [:show, :diffs, :commits, :builds, :pipelines] before_action :apply_diff_view_cookie!, only: [:new_diffs] before_action :build_merge_request, only: [:new, :new_diffs] @@ -33,7 +33,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController before_action :authenticate_user!, only: [:assign_related_issues] - before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :resolve_conflicts] + before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :conflict_for_path, :resolve_conflicts] def index @merge_requests = merge_requests_collection @@ -170,6 +170,16 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end + def conflict_for_path + return render_404 unless @merge_request.conflicts_can_be_resolved_in_ui? + + file = @merge_request.conflicts.file_for_path(params[:old_path], params[:new_path]) + + return render_404 unless file + + render json: file, full_content: true + end + def resolve_conflicts return render_404 unless @merge_request.conflicts_can_be_resolved_in_ui? @@ -184,7 +194,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController flash[:notice] = 'All merge conflicts were resolved. The merge request can now be merged.' render json: { redirect_to: namespace_project_merge_request_url(@project.namespace, @project, @merge_request, resolved_conflicts: true) } - rescue Gitlab::Conflict::File::MissingResolution => e + rescue Gitlab::Conflict::ResolutionError => e render status: :bad_request, json: { message: e.message } end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a743bf313ae..1fb0371fe06 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -868,7 +868,7 @@ class MergeRequest < ActiveRecord::Base # files. conflicts.files.each(&:lines) @conflicts_can_be_resolved_in_ui = conflicts.files.length > 0 - rescue Rugged::OdbError, Gitlab::Conflict::Parser::ParserError, Gitlab::Conflict::FileCollection::ConflictSideMissing + rescue Rugged::OdbError, Gitlab::Conflict::Parser::UnresolvableError, Gitlab::Conflict::FileCollection::ConflictSideMissing @conflicts_can_be_resolved_in_ui = false end end diff --git a/app/services/merge_requests/resolve_service.rb b/app/services/merge_requests/resolve_service.rb index 19caa038c44..d22a1d3e0ad 100644 --- a/app/services/merge_requests/resolve_service.rb +++ b/app/services/merge_requests/resolve_service.rb @@ -1,5 +1,8 @@ module MergeRequests class ResolveService < MergeRequests::BaseService + class MissingFiles < Gitlab::Conflict::ResolutionError + end + attr_accessor :conflicts, :rugged, :merge_index, :merge_request def execute(merge_request) @@ -10,8 +13,16 @@ module MergeRequests fetch_their_commit! - conflicts.files.each do |file| - write_resolved_file_to_index(file, params[:sections]) + params[:files].each do |file_params| + conflict_file = merge_request.conflicts.file_for_path(file_params[:old_path], file_params[:new_path]) + + write_resolved_file_to_index(conflict_file, file_params) + end + + unless merge_index.conflicts.empty? + missing_files = merge_index.conflicts.map { |file| file[:ours][:path] } + + raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}" end commit_params = { @@ -23,8 +34,13 @@ module MergeRequests project.repository.resolve_conflicts(current_user, merge_request.source_branch, commit_params) end - def write_resolved_file_to_index(file, resolutions) - new_file = file.resolve_lines(resolutions).map(&:text).join("\n") + def write_resolved_file_to_index(file, params) + new_file = if params[:sections] + file.resolve_lines(params[:sections]).map(&:text).join("\n") + elsif params[:content] + file.resolve_content(params[:content]) + end + our_path = file.our_path merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode) diff --git a/config/routes/project.rb b/config/routes/project.rb index f9d58f5d5b2..cbce9cd47a0 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -267,6 +267,7 @@ resources :namespaces, path: '/', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only: get :commits get :diffs get :conflicts + get :conflict_for_path get :builds get :pipelines get :merge_check diff --git a/lib/gitlab/conflict/file.rb b/lib/gitlab/conflict/file.rb index dff9e29c6a5..26a9f170298 100644 --- a/lib/gitlab/conflict/file.rb +++ b/lib/gitlab/conflict/file.rb @@ -4,12 +4,12 @@ module Gitlab include Gitlab::Routing.url_helpers include IconsHelper - class MissingResolution < StandardError + class MissingResolution < ResolutionError end CONTEXT_LINES = 3 - attr_reader :merge_file_result, :their_path, :our_path, :our_mode, :merge_request, :repository + attr_reader :merge_file_result, :their_path, :our_path, :our_mode, :merge_request, :repository, :type def initialize(merge_file_result, conflict, merge_request:) @merge_file_result = merge_file_result @@ -21,12 +21,24 @@ module Gitlab @match_line_headers = {} end + def content + merge_file_result[:data] + end + # Array of Gitlab::Diff::Line objects def lines - @lines ||= Gitlab::Conflict::Parser.new.parse(merge_file_result[:data], + return @lines if defined?(@lines) + + begin + @type = 'text' + @lines = Gitlab::Conflict::Parser.new.parse(content, our_path: our_path, their_path: their_path, parent_file: self) + rescue Gitlab::Conflict::Parser::ParserError + @type = 'text-editor' + @lines = nil + end end def resolve_lines(resolution) @@ -53,6 +65,14 @@ module Gitlab end.compact end + def resolve_content(resolution) + if resolution == content + raise MissingResolution, "Resolved content has no changes for file #{our_path}" + end + + resolution + end + def highlight_lines! their_file = lines.reject { |line| line.type == 'new' }.map(&:text).join("\n") our_file = lines.reject { |line| line.type == 'old' }.map(&:text).join("\n") @@ -170,21 +190,36 @@ module Gitlab match_line.text = "@@ -#{match_line.old_pos},#{line.old_pos} +#{match_line.new_pos},#{line.new_pos} @@#{header}" end - def as_json(opts = nil) - { + def as_json(opts = {}) + json_hash = { old_path: their_path, new_path: our_path, blob_icon: file_type_icon_class('file', our_mode, our_path), blob_path: namespace_project_blob_path(merge_request.project.namespace, merge_request.project, - ::File.join(merge_request.diff_refs.head_sha, our_path)), - sections: sections + ::File.join(merge_request.diff_refs.head_sha, our_path)) } + + if opts[:full_content] + json_hash.merge(content: content) + else + json_hash.merge!(sections: sections) if type == 'text' + + json_hash.merge(type: type, content_path: content_path) + end + end + + def content_path + conflict_for_path_namespace_project_merge_request_path(merge_request.project.namespace, + merge_request.project, + merge_request, + old_path: their_path, + new_path: our_path) end # Don't try to print merge_request or repository. def inspect - instance_variables = [:merge_file_result, :their_path, :our_path, :our_mode].map do |instance_variable| + instance_variables = [:merge_file_result, :their_path, :our_path, :our_mode, :type].map do |instance_variable| value = instance_variable_get("@#{instance_variable}") "#{instance_variable}=\"#{value}\"" diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index bbd0427a2c8..fa5bd4649d4 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -30,6 +30,10 @@ module Gitlab end end + def file_for_path(old_path, new_path) + files.find { |file| file.their_path == old_path && file.our_path == new_path } + end + def as_json(opts = nil) { target_branch: merge_request.target_branch, diff --git a/lib/gitlab/conflict/parser.rb b/lib/gitlab/conflict/parser.rb index 98e842cded3..ddd657903fb 100644 --- a/lib/gitlab/conflict/parser.rb +++ b/lib/gitlab/conflict/parser.rb @@ -1,19 +1,24 @@ module Gitlab module Conflict class Parser - class ParserError < StandardError + class UnresolvableError < StandardError end - class UnexpectedDelimiter < ParserError + class UnmergeableFile < UnresolvableError end - class MissingEndDelimiter < ParserError + class UnsupportedEncoding < UnresolvableError + end + + # Recoverable errors - the conflict can be resolved in an editor, but not with + # sections. + class ParserError < StandardError end - class UnmergeableFile < ParserError + class UnexpectedDelimiter < ParserError end - class UnsupportedEncoding < ParserError + class MissingEndDelimiter < ParserError end def parse(text, our_path:, their_path:, parent_file: nil) diff --git a/lib/gitlab/conflict/resolution_error.rb b/lib/gitlab/conflict/resolution_error.rb new file mode 100644 index 00000000000..a0f2006bc24 --- /dev/null +++ b/lib/gitlab/conflict/resolution_error.rb @@ -0,0 +1,6 @@ +module Gitlab + module Conflict + class ResolutionError < StandardError + end + end +end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 84298f8bef4..1311b4aa264 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -570,7 +570,7 @@ describe Projects::MergeRequestsController do context 'when the conflicts cannot be resolved in the UI' do before do allow_any_instance_of(Gitlab::Conflict::Parser). - to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnexpectedDelimiter) + to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnmergeableFile) get :conflicts, namespace_id: merge_request_with_conflicts.project.namespace.to_param, @@ -655,6 +655,61 @@ describe Projects::MergeRequestsController do id: merge_request.iid expect(merge_request.reload.title).to eq(merge_request.wipless_title) + end + + describe 'GET conflict_for_path' do + let(:json_response) { JSON.parse(response.body) } + + def conflict_for_path(path) + get :conflict_for_path, + namespace_id: merge_request_with_conflicts.project.namespace.to_param, + project_id: merge_request_with_conflicts.project.to_param, + id: merge_request_with_conflicts.iid, + old_path: path, + new_path: path, + format: 'json' + end + + context 'when the conflicts cannot be resolved in the UI' do + before do + allow_any_instance_of(Gitlab::Conflict::Parser). + to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnmergeableFile) + + conflict_for_path('files/ruby/regex.rb') + end + + + it 'returns a 404 status code' do + expect(response).to have_http_status(:not_found) + end + end + + context 'when the file does not exist cannot be resolved in the UI' do + before { conflict_for_path('files/ruby/regexp.rb') } + + it 'returns a 404 status code' do + expect(response).to have_http_status(:not_found) + end + end + + context 'with an existing file' do + let(:path) { 'files/ruby/regex.rb' } + + before { conflict_for_path(path) } + + it 'returns a 200 status code' do + expect(response).to have_http_status(:ok) + end + + it 'returns the file in JSON format' do + content = merge_request_with_conflicts.conflicts.file_for_path(path, path).content + + expect(json_response).to include('old_path' => path, + 'new_path' => path, + 'blob_icon' => 'file-text-o', + 'blob_path' => a_string_ending_with(path), + 'content' => content) + end end end @@ -662,22 +717,37 @@ describe Projects::MergeRequestsController do let(:json_response) { JSON.parse(response.body) } let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha } - def resolve_conflicts(sections) + def resolve_conflicts(files) post :resolve_conflicts, namespace_id: merge_request_with_conflicts.project.namespace.to_param, project_id: merge_request_with_conflicts.project.to_param, id: merge_request_with_conflicts.iid, format: 'json', - sections: sections, + files: files, commit_message: 'Commit message' end context 'with valid params' do before do - resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin') + resolved_files = [ + { + 'new_path' => 'files/ruby/popen.rb', + 'old_path' => 'files/ruby/popen.rb', + 'sections' => { + '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' + } + }, { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ] + + resolve_conflicts(resolved_files) end it 'creates a new commit on the branch' do @@ -692,7 +762,23 @@ describe Projects::MergeRequestsController do context 'when sections are missing' do before do - resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head') + resolved_files = [ + { + 'new_path' => 'files/ruby/popen.rb', + 'old_path' => 'files/ruby/popen.rb', + 'sections' => { + '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' + } + }, { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head' + } + } + ] + + resolve_conflicts(resolved_files) end it 'returns a 400 error' do @@ -700,7 +786,71 @@ describe Projects::MergeRequestsController do end it 'has a message with the name of the first missing section' do - expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9') + expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21') + end + + it 'does not create a new commit' do + expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha) + end + end + + context 'when files are missing' do + before do + resolved_files = [ + { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ] + + resolve_conflicts(resolved_files) + end + + it 'returns a 400 error' do + expect(response).to have_http_status(:bad_request) + end + + it 'has a message with the name of the missing file' do + expect(json_response['message']).to include('files/ruby/popen.rb') + end + + it 'does not create a new commit' do + expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha) + end + end + + context 'when a file has identical content to the conflict' do + before do + resolved_files = [ + { + 'new_path' => 'files/ruby/popen.rb', + 'old_path' => 'files/ruby/popen.rb', + 'content' => merge_request_with_conflicts.conflicts.file_for_path('files/ruby/popen.rb', 'files/ruby/popen.rb').content + }, { + 'new_path' => 'files/ruby/regex.rb', + 'old_path' => 'files/ruby/regex.rb', + 'sections' => { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ] + + resolve_conflicts(resolved_files) + end + + it 'returns a 400 error' do + expect(response).to have_http_status(:bad_request) + end + + it 'has a message with the path of the problem file' do + expect(json_response['message']).to include('files/ruby/popen.rb') end it 'does not create a new commit' do diff --git a/spec/services/merge_requests/resolve_service_spec.rb b/spec/services/merge_requests/resolve_service_spec.rb index d71932458fa..e667e93bea4 100644 --- a/spec/services/merge_requests/resolve_service_spec.rb +++ b/spec/services/merge_requests/resolve_service_spec.rb @@ -24,15 +24,26 @@ describe MergeRequests::ResolveService do end describe '#execute' do - context 'with valid params' do + context 'with section params' do let(:params) do { - sections: { - '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', - '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' - }, + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + sections: { + '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' + } + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + sections: { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ], commit_message: 'This is a commit message!' } end @@ -74,8 +85,96 @@ describe MergeRequests::ResolveService do end end - context 'when a resolution is missing' do - let(:invalid_params) { { sections: { '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' } } } + context 'with content and sections params' do + let(:popen_content) { "class Popen\nend" } + + let(:params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: popen_content + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + sections: { + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin', + '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin' + } + } + ], + commit_message: 'This is a commit message!' + } + end + + before do + MergeRequests::ResolveService.new(project, user, params).execute(merge_request) + end + + it 'creates a commit with the message' do + expect(merge_request.source_branch_head.message).to eq(params[:commit_message]) + end + + it 'creates a commit with the correct parents' do + expect(merge_request.source_branch_head.parents.map(&:id)). + to eq(['1450cd639e0bc6721eb02800169e464f212cde06', + '75284c70dd26c87f2a3fb65fd5a1f0b0138d3a6b']) + end + + it 'sets the content to the content given' do + blob = merge_request.source_project.repository.blob_at(merge_request.source_branch_head.sha, + 'files/ruby/popen.rb') + + expect(blob.data).to eq(popen_content) + end + end + + context 'when a resolution section is missing' do + let(:invalid_params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: '' + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + sections: { '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head' } + } + ], + commit_message: 'This is a commit message!' + } + end + + let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } + + it 'raises a MissingResolution error' do + expect { service.execute(merge_request) }. + to raise_error(Gitlab::Conflict::File::MissingResolution) + end + end + + context 'when the content of a file is unchanged' do + let(:invalid_params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: '' + }, { + old_path: 'files/ruby/regex.rb', + new_path: 'files/ruby/regex.rb', + content: merge_request.conflicts.file_for_path('files/ruby/regex.rb', 'files/ruby/regex.rb').content + } + ], + commit_message: 'This is a commit message!' + } + end + let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } it 'raises a MissingResolution error' do @@ -83,5 +182,27 @@ describe MergeRequests::ResolveService do to raise_error(Gitlab::Conflict::File::MissingResolution) end end + + context 'when a file is missing' do + let(:invalid_params) do + { + files: [ + { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen.rb', + content: '' + } + ], + commit_message: 'This is a commit message!' + } + end + + let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } + + it 'raises a MissingFiles error' do + expect { service.execute(merge_request) }. + to raise_error(MergeRequests::ResolveService::MissingFiles) + end + end end end -- cgit v1.2.1 From 9727366b5a0a39a125925e2a7a78ed47e29b02f7 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 1 Sep 2016 16:19:16 +0100 Subject: Make RuboCop happy --- spec/controllers/projects/merge_requests_controller_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 1311b4aa264..b4f637c93e2 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -678,7 +678,6 @@ describe Projects::MergeRequestsController do conflict_for_path('files/ruby/regex.rb') end - it 'returns a 404 status code' do expect(response).to have_http_status(:not_found) end -- cgit v1.2.1 From 241cca011f9a35c03e8f5fae1381a4af8a8f26bb Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 1 Sep 2016 16:20:29 +0100 Subject: Fix specs --- app/controllers/application_controller.rb | 4 ++-- spec/support/test_env.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bf37421771f..81c0fa26d18 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -115,10 +115,10 @@ class ApplicationController < ActionController::Base def render_404 respond_to do |format| - format.json { head :not_found } - format.any do + format.html do render file: Rails.root.join("public", "404"), layout: false, status: "404" end + format.any { head :not_found } end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index d56274d0979..243d671c521 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -26,10 +26,10 @@ module TestEnv 'expand-collapse-lines' => '238e82d', 'video' => '8879059', 'crlf-diff' => '5938907', - 'conflict-start' => '75284c7', + 'conflict-start' => '824be60', 'conflict-resolvable' => '1450cd6', 'conflict-binary-file' => '259a6fb', - 'conflict-contains-conflict-markers' => '5e0964c', + 'conflict-contains-conflict-markers' => '78a3086', 'conflict-missing-side' => 'eb227b3', 'conflict-non-utf8' => 'd0a293c', 'conflict-too-large' => '39fa04f', -- cgit v1.2.1 From 082be0917cf15d410b1db3ca49b32049a56c117e Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 1 Sep 2016 16:50:53 +0100 Subject: Fix MR model spec --- spec/models/merge_request_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 5884b4cff8c..91a423b670c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1155,12 +1155,6 @@ describe MergeRequest, models: true do expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_falsey end - it 'returns a falsey value when the conflicts contain a file with ambiguous conflict markers' do - merge_request = create_merge_request('conflict-contains-conflict-markers') - - expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_falsey - end - it 'returns a falsey value when the conflicts contain a file edited in one branch and deleted in another' do merge_request = create_merge_request('conflict-missing-side') @@ -1172,6 +1166,12 @@ describe MergeRequest, models: true do expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_truthy end + + it 'returns a truthy value when the conflicts have to be resolved in an editor' do + merge_request = create_merge_request('conflict-contains-conflict-markers') + + expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_truthy + end end describe "#forked_source_project_missing?" do -- cgit v1.2.1 From 98dd958df3b4dd1b58b3a3387ea9d52dc78af46b Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 1 Sep 2016 17:44:35 +0100 Subject: Fix resolve service specs --- spec/services/merge_requests/resolve_service_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/services/merge_requests/resolve_service_spec.rb b/spec/services/merge_requests/resolve_service_spec.rb index e667e93bea4..388abb6a0df 100644 --- a/spec/services/merge_requests/resolve_service_spec.rb +++ b/spec/services/merge_requests/resolve_service_spec.rb @@ -60,7 +60,7 @@ describe MergeRequests::ResolveService do it 'creates a commit with the correct parents' do expect(merge_request.source_branch_head.parents.map(&:id)). to eq(['1450cd639e0bc6721eb02800169e464f212cde06', - '75284c70dd26c87f2a3fb65fd5a1f0b0138d3a6b']) + '824be604a34828eb682305f0d963056cfac87b2d']) end end @@ -120,7 +120,7 @@ describe MergeRequests::ResolveService do it 'creates a commit with the correct parents' do expect(merge_request.source_branch_head.parents.map(&:id)). to eq(['1450cd639e0bc6721eb02800169e464f212cde06', - '75284c70dd26c87f2a3fb65fd5a1f0b0138d3a6b']) + '824be604a34828eb682305f0d963056cfac87b2d']) end it 'sets the content to the content given' do -- cgit v1.2.1 From 7529bbae944823041e8690c011c4cfb39534074b Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Fri, 2 Sep 2016 16:16:54 +0100 Subject: Add JSON Schema --- .../projects/merge_requests_controller_spec.rb | 4 + spec/fixtures/api/schemas/conflicts.json | 137 +++++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 spec/fixtures/api/schemas/conflicts.json diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index b4f637c93e2..06b37aa4997 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -597,6 +597,10 @@ describe Projects::MergeRequestsController do format: 'json' end + it 'matches the schema' do + expect(response).to match_response_schema('conflicts') + end + it 'includes meta info about the MR' do expect(json_response['commit_message']).to include('Merge branch') expect(json_response['commit_sha']).to match(/\h{40}/) diff --git a/spec/fixtures/api/schemas/conflicts.json b/spec/fixtures/api/schemas/conflicts.json new file mode 100644 index 00000000000..a947783d505 --- /dev/null +++ b/spec/fixtures/api/schemas/conflicts.json @@ -0,0 +1,137 @@ +{ + "type": "object", + "required": [ + "commit_message", + "commit_sha", + "source_branch", + "target_branch", + "files" + ], + "properties": { + "commit_message": {"type": "string"}, + "commit_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"}, + "source_branch": {"type": "string"}, + "target_branch": {"type": "string"}, + "files": { + "type": "array", + "items": { + "oneOf": [ + { "$ref": "#/definitions/conflict-text-with-sections" }, + { "$ref": "#/definitions/conflict-text-for-editor" } + ] + } + } + }, + "definitions": { + "conflict-base": { + "type": "object", + "required": [ + "old_path", + "new_path", + "blob_icon", + "blob_path" + ], + "properties": { + "old_path": {"type": "string"}, + "new_path": {"type": "string"}, + "blob_icon": {"type": "string"}, + "blob_path": {"type": "string"} + } + }, + "conflict-text-for-editor": { + "allOf": [ + {"$ref": "#/definitions/conflict-base"}, + { + "type": "object", + "required": [ + "type", + "content_path" + ], + "properties": { + "type": {"type": {"enum": ["text-editor"]}}, + "content_path": {"type": "string"} + } + } + ] + }, + "conflict-text-with-sections": { + "allOf": [ + {"$ref": "#/definitions/conflict-base"}, + { + "type": "object", + "required": [ + "type", + "content_path", + "sections" + ], + "properties": { + "type": {"type": {"enum": ["text"]}}, + "content_path": {"type": "string"}, + "sections": { + "type": "array", + "items": { + "oneOf": [ + { "$ref": "#/definitions/section-context" }, + { "$ref": "#/definitions/section-conflict" } + ] + } + } + } + } + ] + }, + "section-base": { + "type": "object", + "required": [ + "conflict", + "lines" + ], + "properties": { + "conflict": {"type": "boolean"}, + "lines": { + "type": "array", + "items": { + "type": "object", + "required": [ + "old_line", + "new_line", + "text", + "rich_text" + ], + "properties": { + "type": {"type": "string"}, + "old_line": {"type": "string"}, + "new_line": {"type": "string"}, + "text": {"type": "string"}, + "rich_text": {"type": "string"} + } + } + } + } + }, + "section-context": { + "allOf": [ + {"$ref": "#/definitions/section-base"}, + { + "type": "object", + "properties": { + "conflict": {"enum": [false]} + } + } + ] + }, + "section-conflict": { + "allOf": [ + {"$ref": "#/definitions/section-base"}, + { + "type": "object", + "required": ["id"], + "properties": { + "conflict": {"enum": [true]}, + "id": {"type": "string"} + } + } + ] + } + } +} -- cgit v1.2.1 From 4743d19463e7aef965665e43238af73820d18d7f Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Wed, 7 Sep 2016 12:28:13 +0100 Subject: Simplify conflict file JSON creation --- lib/gitlab/conflict/file.rb | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/conflict/file.rb b/lib/gitlab/conflict/file.rb index 26a9f170298..661e43d3fa9 100644 --- a/lib/gitlab/conflict/file.rb +++ b/lib/gitlab/conflict/file.rb @@ -9,7 +9,7 @@ module Gitlab CONTEXT_LINES = 3 - attr_reader :merge_file_result, :their_path, :our_path, :our_mode, :merge_request, :repository, :type + attr_reader :merge_file_result, :their_path, :our_path, :our_mode, :merge_request, :repository def initialize(merge_file_result, conflict, merge_request:) @merge_file_result = merge_file_result @@ -25,6 +25,12 @@ module Gitlab merge_file_result[:data] end + def type + lines unless @type + + @type.inquiry + end + # Array of Gitlab::Diff::Line objects def lines return @lines if defined?(@lines) @@ -200,12 +206,14 @@ module Gitlab ::File.join(merge_request.diff_refs.head_sha, our_path)) } - if opts[:full_content] - json_hash.merge(content: content) - else - json_hash.merge!(sections: sections) if type == 'text' - - json_hash.merge(type: type, content_path: content_path) + json_hash.tap do |json_hash| + if opts[:full_content] + json_hash[:content] = content + else + json_hash[:sections] = sections if type.text? + json_hash[:type] = type + json_hash[:content_path] = content_path + end end end -- cgit v1.2.1 From 6af52d7d23cf9dbfcd58a2d3031ed19887f7a558 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 20 Sep 2016 09:16:35 +0300 Subject: We now support resolving conflicts with ambiguous markers --- spec/features/merge_requests/conflicts_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb index 759edf8ec80..1e8aeacea05 100644 --- a/spec/features/merge_requests/conflicts_spec.rb +++ b/spec/features/merge_requests/conflicts_spec.rb @@ -42,7 +42,6 @@ feature 'Merge request conflict resolution', js: true, feature: true do UNRESOLVABLE_CONFLICTS = { 'conflict-too-large' => 'when the conflicts contain a large file', 'conflict-binary-file' => 'when the conflicts contain a binary file', - 'conflict-contains-conflict-markers' => 'when the conflicts contain a file with ambiguous conflict markers', 'conflict-missing-side' => 'when the conflicts contain a file edited in one branch and deleted in another', 'conflict-non-utf8' => 'when the conflicts contain a non-UTF-8 file', } -- cgit v1.2.1 From 26f658decd3943bbc66c567ea91e7b859b32e0e6 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 8 Sep 2016 11:44:04 -0500 Subject: Implement editor to manually resolve merge conflicts --- .../merge_conflict_data_provider.js.es6 | 165 ++++++++++++++++----- .../javascripts/merge_conflict_resolver.js.es6 | 60 ++++++-- .../components/diff_file_editor.js.es6 | 63 ++++++++ app/assets/stylesheets/pages/merge_conflicts.scss | 26 ++++ .../projects/merge_requests/conflicts.html.haml | 3 + .../conflicts/_diff_file_editor.html.haml | 9 ++ .../conflicts/_file_actions.html.haml | 12 ++ .../conflicts/_inline_view.html.haml | 10 +- .../conflicts/_parallel_view.html.haml | 10 +- .../conflicts/_submit_form.html.haml | 9 +- .../components/_diff_file_editor.html.haml | 6 + 11 files changed, 300 insertions(+), 73 deletions(-) create mode 100644 app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 create mode 100644 app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml create mode 100644 app/views/projects/merge_requests/conflicts/_file_actions.html.haml create mode 100644 app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml diff --git a/app/assets/javascripts/merge_conflict_data_provider.js.es6 b/app/assets/javascripts/merge_conflict_data_provider.js.es6 index 13ee794ba38..bfed78ff99c 100644 --- a/app/assets/javascripts/merge_conflict_data_provider.js.es6 +++ b/app/assets/javascripts/merge_conflict_data_provider.js.es6 @@ -2,7 +2,9 @@ const HEAD_HEADER_TEXT = 'HEAD//our changes'; const ORIGIN_HEADER_TEXT = 'origin//their changes'; const HEAD_BUTTON_TITLE = 'Use ours'; const ORIGIN_BUTTON_TITLE = 'Use theirs'; - +const INTERACTIVE_RESOLVE_MODE = 'interactive'; +const EDIT_RESOLVE_MODE = 'edit'; +const DEFAULT_RESOLVE_MODE = INTERACTIVE_RESOLVE_MODE; class MergeConflictDataProvider { @@ -18,8 +20,7 @@ class MergeConflictDataProvider { diffViewType : diffViewType, fixedLayout : fixedLayout, isSubmitting : false, - conflictsData : {}, - resolutionData : {} + conflictsData : {} } } @@ -35,9 +36,9 @@ class MergeConflictDataProvider { data.shortCommitSha = data.commit_sha.slice(0, 7); data.commitMessage = data.commit_message; + this.decorateFiles(data); this.setParallelLines(data); this.setInlineLines(data); - this.updateResolutionsData(data); } vueInstance.conflictsData = data; @@ -47,16 +48,12 @@ class MergeConflictDataProvider { vueInstance.conflictsData.conflictsText = conflictsText; } - - updateResolutionsData(data) { - const vi = this.vueInstance; - - data.files.forEach( (file) => { - file.sections.forEach( (section) => { - if (section.conflict) { - vi.$set(`resolutionData['${section.id}']`, false); - } - }); + decorateFiles(data) { + data.files.forEach((file) => { + file.content = ''; + file.resolutionData = {}; + file.promptDiscardConfirmation = false; + file.resolveMode = DEFAULT_RESOLVE_MODE; }); } @@ -165,11 +162,14 @@ class MergeConflictDataProvider { } - handleSelected(sectionId, selection) { + handleSelected(file, sectionId, selection) { const vi = this.vueInstance; + let files = vi.conflictsData.files; + + vi.$set(`conflictsData.files[${files.indexOf(file)}].resolutionData['${sectionId}']`, selection); + - vi.resolutionData[sectionId] = selection; - vi.conflictsData.files.forEach( (file) => { + files.forEach( (file) => { file.inlineLines.forEach( (line) => { if (line.id === sectionId && (line.hasConflict || line.isHeader)) { this.markLine(line, selection); @@ -208,6 +208,48 @@ class MergeConflictDataProvider { .toggleClass('container-limited', !vi.isParallel && vi.fixedLayout); } + setFileResolveMode(file, mode) { + const vi = this.vueInstance; + + // Restore Interactive mode when switching to Edit mode + if (mode === EDIT_RESOLVE_MODE) { + file.resolutionData = {}; + + this.restoreFileLinesState(file); + } + + file.resolveMode = mode; + } + + + restoreFileLinesState(file) { + file.inlineLines.forEach((line) => { + if (line.hasConflict || line.isHeader) { + line.isSelected = false; + line.isUnselected = false; + } + }); + + file.parallelLines.forEach((lines) => { + const left = lines[0]; + const right = lines[1]; + const isLeftMatch = left.hasConflict || left.isHeader; + const isRightMatch = right.hasConflict || right.isHeader; + + if (isLeftMatch || isRightMatch) { + left.isSelected = false; + left.isUnselected = false; + right.isSelected = false; + right.isUnselected = false; + } + }); + } + + + setPromptConfirmationState(file, state) { + file.promptDiscardConfirmation = state; + } + markLine(line, selection) { if (selection === 'head' && line.isHead) { @@ -226,31 +268,54 @@ class MergeConflictDataProvider { getConflictsCount() { - return Object.keys(this.vueInstance.resolutionData).length; - } - - - getResolvedCount() { - let count = 0; - const data = this.vueInstance.resolutionData; + const files = this.vueInstance.conflictsData.files; + let count = 0; - for (const id in data) { - const resolution = data[id]; - if (resolution) { - count++; - } - } + files.forEach((file) => { + file.sections.forEach((section) => { + if (section.conflict) { + count++; + } + }); + }); return count; } isReadyToCommit() { - const { conflictsData, isSubmitting } = this.vueInstance - const allResolved = this.getConflictsCount() === this.getResolvedCount(); - const hasCommitMessage = $.trim(conflictsData.commitMessage).length; + const vi = this.vueInstance; + const files = this.vueInstance.conflictsData.files; + const hasCommitMessage = $.trim(this.vueInstance.conflictsData.commitMessage).length; + let unresolved = 0; + + for (let i = 0, l = files.length; i < l; i++) { + let file = files[i]; + + if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { + let numberConflicts = 0; + let resolvedConflicts = Object.keys(file.resolutionData).length + + for (let j = 0, k = file.sections.length; j < k; j++) { + if (file.sections[j].conflict) { + numberConflicts++; + } + } + + if (resolvedConflicts !== numberConflicts) { + unresolved++; + } + } else if (file.resolveMode === EDIT_RESOLVE_MODE) { + // Unlikely to happen since switching to Edit mode saves content automatically. + // Checking anyway in case the save strategy changes in the future + if (!file.content) { + unresolved++; + continue; + } + } + } - return !isSubmitting && hasCommitMessage && allResolved; + return !vi.isSubmitting && hasCommitMessage && !unresolved; } @@ -332,10 +397,33 @@ class MergeConflictDataProvider { getCommitData() { - return { - commit_message: this.vueInstance.conflictsData.commitMessage, - sections: this.vueInstance.resolutionData - } + let conflictsData = this.vueInstance.conflictsData; + let commitData = {}; + + commitData = { + commitMessage: conflictsData.commitMessage, + files: [] + }; + + conflictsData.files.forEach((file) => { + let addFile; + + addFile = { + old_path: file.old_path, + new_path: file.new_path + }; + + // Submit only one data for type of editing + if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { + addFile.sections = file.resolutionData; + } else if (file.resolveMode === EDIT_RESOLVE_MODE) { + addFile.content = file.content; + } + + commitData.files.push(addFile); + }); + + return commitData; } @@ -343,5 +431,4 @@ class MergeConflictDataProvider { const { old_path, new_path } = file; return old_path === new_path ? new_path : `${old_path} → ${new_path}`; } - } diff --git a/app/assets/javascripts/merge_conflict_resolver.js.es6 b/app/assets/javascripts/merge_conflict_resolver.js.es6 index 7e756433bf5..c317a6521b5 100644 --- a/app/assets/javascripts/merge_conflict_resolver.js.es6 +++ b/app/assets/javascripts/merge_conflict_resolver.js.es6 @@ -1,13 +1,16 @@ //= require vue +//= require ./merge_conflicts/components/diff_file_editor + +const INTERACTIVE_RESOLVE_MODE = 'interactive'; +const EDIT_RESOLVE_MODE = 'edit'; class MergeConflictResolver { constructor() { - this.dataProvider = new MergeConflictDataProvider() - this.initVue() + this.dataProvider = new MergeConflictDataProvider(); + this.initVue(); } - initVue() { const that = this; this.vue = new Vue({ @@ -17,15 +20,28 @@ class MergeConflictResolver { created : this.fetchData(), computed : this.setComputedProperties(), methods : { - handleSelected(sectionId, selection) { - that.dataProvider.handleSelected(sectionId, selection); + handleSelected(file, sectionId, selection) { + that.dataProvider.handleSelected(file, sectionId, selection); }, handleViewTypeChange(newType) { that.dataProvider.updateViewType(newType); }, commit() { that.commit(); - } + }, + onClickResolveModeButton(file, mode) { + that.toggleResolveMode(file, mode); + }, + acceptDiscardConfirmation(file) { + that.dataProvider.setPromptConfirmationState(file, false); + that.dataProvider.setFileResolveMode(file, INTERACTIVE_RESOLVE_MODE); + }, + cancelDiscardConfirmation(file) { + that.dataProvider.setPromptConfirmationState(file, false); + }, + }, + components: { + 'diff-file-editor': window.gl.diffFileEditor } }) } @@ -36,7 +52,6 @@ class MergeConflictResolver { return { conflictsCount() { return dp.getConflictsCount() }, - resolvedCount() { return dp.getResolvedCount() }, readyToCommit() { return dp.isReadyToCommit() }, commitButtonText() { return dp.getCommitButtonText() } } @@ -69,14 +84,29 @@ class MergeConflictResolver { commit() { this.vue.isSubmitting = true; - $.post($('#conflicts').data('resolveConflictsPath'), this.dataProvider.getCommitData()) - .done((data) => { - window.location.href = data.redirect_to; - }) - .error(() => { - this.vue.isSubmitting = false; - new Flash('Something went wrong!'); - }); + $.ajax({ + url: $('#conflicts').data('resolveConflictsPath'), + data: JSON.stringify(this.dataProvider.getCommitData()), + contentType: "application/json", + dataType: 'json', + method: 'POST' + }) + .done((data) => { + window.location.href = data.redirect_to; + }) + .error(() => { + this.vue.isSubmitting = false; + new Flash('Something went wrong!'); + }); } + + toggleResolveMode(file, mode) { + if (mode === INTERACTIVE_RESOLVE_MODE && file.resolveEditChanged) { + this.dataProvider.setPromptConfirmationState(file, true); + return; + } + + this.dataProvider.setFileResolveMode(file, mode); + } } diff --git a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 new file mode 100644 index 00000000000..570d9ff877c --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 @@ -0,0 +1,63 @@ +((global) => { + global.diffFileEditor = Vue.extend({ + props: ['file', 'loadFile'], + template: '#diff-file-editor', + data() { + return { + originalState: '', + saved: false, + loading: false, + fileLoaded: false + } + }, + computed: { + classObject() { + return { + 'load-file': this.loadFile, + 'saved': this.saved, + 'is-loading': this.loading + }; + } + }, + watch: { + loadFile(val) { + const self = this; + + if (!val || this.fileLoaded || this.loading) { + return + } + + this.loading = true; + + $.get(this.file.content_path) + .done((file) => { + $(self.$el).find('textarea').val(file.content); + + self.originalState = file.content; + self.fileLoaded = true; + self.saveDiffResolution(); + }) + .fail(() => { + console.log('error'); + }) + .always(() => { + self.loading = false; + }); + } + }, + methods: { + saveDiffResolution() { + this.saved = true; + + // This probably be better placed in the data provider + this.file.content = this.$el.querySelector('textarea').value; + this.file.resolveEditChanged = this.file.content !== this.originalState; + this.file.promptDiscardConfirmation = false; + }, + onInput() { + this.saveDiffResolution(); + } + } + }); + +})(window.gl || (window.gl = {})); diff --git a/app/assets/stylesheets/pages/merge_conflicts.scss b/app/assets/stylesheets/pages/merge_conflicts.scss index 5ec660799e3..577a97e8c0e 100644 --- a/app/assets/stylesheets/pages/merge_conflicts.scss +++ b/app/assets/stylesheets/pages/merge_conflicts.scss @@ -235,4 +235,30 @@ $colors: ( .btn-success .fa-spinner { color: #fff; } + + .editor-wrap { + &.is-loading { + .editor { + display: none; + } + + .loading-text { + display: block; + } + } + + &.saved { + .editor { + border-top: solid 1px green; + } + } + + .editor { + border-top: solid 1px yellow; + } + + .loading-text { + display: none; + } + } } diff --git a/app/views/projects/merge_requests/conflicts.html.haml b/app/views/projects/merge_requests/conflicts.html.haml index a524936f73c..e3add1f799f 100644 --- a/app/views/projects/merge_requests/conflicts.html.haml +++ b/app/views/projects/merge_requests/conflicts.html.haml @@ -27,3 +27,6 @@ = render partial: "projects/merge_requests/conflicts/parallel_view", locals: { class_bindings: class_bindings } = render partial: "projects/merge_requests/conflicts/inline_view", locals: { class_bindings: class_bindings } = render partial: "projects/merge_requests/conflicts/submit_form" + +-# Components += render partial: 'projects/merge_requests/conflicts/components/diff_file_editor' diff --git a/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml new file mode 100644 index 00000000000..d0c518e5249 --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml @@ -0,0 +1,9 @@ +- if_condition = local_assigns.fetch(:if_condition, '') + +.diff-editor-wrap{ "v-show" => if_condition } + .discard-changes-alert-wrap{ "v-if" => "file.promptDiscardConfirmation" } + %p + Are you sure to discard your changes? + %button.btn.btn-sm.btn-close{ "@click" => "acceptDiscardConfirmation(file)" } Discard changes + %button.btn.btn-sm{ "@click" => "cancelDiscardConfirmation(file)" } Cancel + %diff-file-editor{":file" => "file", ":load-file" => if_condition } diff --git a/app/views/projects/merge_requests/conflicts/_file_actions.html.haml b/app/views/projects/merge_requests/conflicts/_file_actions.html.haml new file mode 100644 index 00000000000..124dfde7b22 --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/_file_actions.html.haml @@ -0,0 +1,12 @@ +.file-actions + .btn-group + %button.btn{ ":class" => "{ 'active': file.resolveMode == 'interactive' }", + '@click' => "onClickResolveModeButton(file, 'interactive')", + type: 'button' } + Interactive mode + %button.btn{ ':class' => "{ 'active': file.resolveMode == 'edit' }", + '@click' => "onClickResolveModeButton(file, 'edit')", + type: 'button' } + Edit inline + %a.btn.view-file.btn-file-option{":href" => "file.blobPath"} + View file @{{conflictsData.shortCommitSha}} diff --git a/app/views/projects/merge_requests/conflicts/_inline_view.html.haml b/app/views/projects/merge_requests/conflicts/_inline_view.html.haml index 19c7da4b5e3..7120b6ff48d 100644 --- a/app/views/projects/merge_requests/conflicts/_inline_view.html.haml +++ b/app/views/projects/merge_requests/conflicts/_inline_view.html.haml @@ -3,12 +3,9 @@ .file-title %i.fa.fa-fw{":class" => "file.iconClass"} %strong {{file.filePath}} - .file-actions - %a.btn.view-file.btn-file-option{":href" => "file.blobPath"} - View file @{{conflictsData.shortCommitSha}} - + = render partial: 'projects/merge_requests/conflicts/file_actions' .diff-content.diff-wrap-lines - .diff-wrap-lines.code.file-content.js-syntax-highlight + .diff-wrap-lines.code.file-content.js-syntax-highlight{ 'v-show' => "file.resolveMode === 'interactive'" } %table %tr.line_holder.diff-inline{"v-for" => "line in file.inlineLines"} %template{"v-if" => "!line.isHeader"} @@ -24,5 +21,6 @@ %td.diff-line-num.header{":class" => class_bindings} %td.line_content.header{":class" => class_bindings} %strong {{{line.richText}}} - %button.btn{"@click" => "handleSelected(line.id, line.section)"} + %button.btn{ "@click" => "handleSelected(file, line.id, line.section)" } {{line.buttonTitle}} + = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.resolveMode === 'edit' && !isParallel" } diff --git a/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml b/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml index 2e6f67c2eaf..18c830ddb17 100644 --- a/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml +++ b/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml @@ -3,12 +3,9 @@ .file-title %i.fa.fa-fw{":class" => "file.iconClass"} %strong {{file.filePath}} - .file-actions - %a.btn.view-file.btn-file-option{":href" => "file.blobPath"} - View file @{{conflictsData.shortCommitSha}} - + = render partial: 'projects/merge_requests/conflicts/file_actions' .diff-content.diff-wrap-lines - .diff-wrap-lines.code.file-content.js-syntax-highlight + .diff-wrap-lines.code.file-content.js-syntax-highlight{ 'v-show' => "file.resolveMode === 'interactive'" } %table %tr.line_holder.parallel{"v-for" => "section in file.parallelLines"} %template{"v-for" => "line in section"} @@ -17,7 +14,7 @@ %td.diff-line-num.header{":class" => class_bindings} %td.line_content.header{":class" => class_bindings} %strong {{line.richText}} - %button.btn{"@click" => "handleSelected(line.id, line.section)"} + %button.btn{"@click" => "handleSelected(file, line.id, line.section)"} {{line.buttonTitle}} %template{"v-if" => "!line.isHeader"} @@ -25,3 +22,4 @@ {{line.lineNumber}} %td.line_content.parallel{":class" => class_bindings} {{{line.richText}}} + = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.resolveMode === 'edit' && isParallel" } diff --git a/app/views/projects/merge_requests/conflicts/_submit_form.html.haml b/app/views/projects/merge_requests/conflicts/_submit_form.html.haml index 78bd4133ea2..380f8722186 100644 --- a/app/views/projects/merge_requests/conflicts/_submit_form.html.haml +++ b/app/views/projects/merge_requests/conflicts/_submit_form.html.haml @@ -1,15 +1,10 @@ -.content-block.oneline-block.files-changed - %strong.resolved-count {{resolvedCount}} - of - %strong.total-count {{conflictsCount}} - conflicts have been resolved - +.content-block .commit-message-container.form-group .max-width-marker %textarea.form-control.js-commit-message{"v-model" => "conflictsData.commitMessage"} {{{conflictsData.commitMessage}}} - %button{type: "button", class: "btn btn-success js-submit-button", ":disabled" => "!readyToCommit", "@click" => "commit()"} + %button{type: "button", class: "btn btn-success js-submit-button", "@click" => "commit()", ":disabled" => "!readyToCommit" } %span {{commitButtonText}} = link_to "Cancel", namespace_project_merge_request_path(@merge_request.project.namespace, @merge_request.project, @merge_request), class: "btn btn-cancel" diff --git a/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml new file mode 100644 index 00000000000..0556341fd64 --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml @@ -0,0 +1,6 @@ +%template{ id: "diff-file-editor" } + %div + .editor-wrap{ ":class" => "classObject" } + %p.loading-text Loading... + .editor + %textarea{ "@input" => "onInput", cols: '80', rows: '20' } -- cgit v1.2.1 From a3eb39a1068df93d732585272c5eaff380801430 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Fri, 16 Sep 2016 19:41:15 -0500 Subject: Replace textarea with Ace editor --- .../components/diff_file_editor.js.es6 | 27 ++++++++++++++++------ app/assets/stylesheets/pages/merge_conflicts.scss | 7 ++++++ .../projects/merge_requests/conflicts.html.haml | 2 ++ .../components/_diff_file_editor.html.haml | 2 +- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 index 570d9ff877c..abdf73febb4 100644 --- a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 +++ b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 @@ -4,7 +4,7 @@ template: '#diff-file-editor', data() { return { - originalState: '', + originalContent: '', saved: false, loading: false, fileLoaded: false @@ -23,6 +23,8 @@ loadFile(val) { const self = this; + this.resetEditorContent(); + if (!val || this.fileLoaded || this.loading) { return } @@ -31,10 +33,19 @@ $.get(this.file.content_path) .done((file) => { - $(self.$el).find('textarea').val(file.content); - self.originalState = file.content; + let content = self.$el.querySelector('pre'); + let fileContent = document.createTextNode(file.content); + + content.textContent = fileContent.textContent; + + self.originalContent = file.content; self.fileLoaded = true; + self.editor = ace.edit(content); + self.editor.$blockScrolling = Infinity; // Turn off annoying warning + self.editor.on('change', () => { + self.saveDiffResolution(); + }); self.saveDiffResolution(); }) .fail(() => { @@ -50,12 +61,14 @@ this.saved = true; // This probably be better placed in the data provider - this.file.content = this.$el.querySelector('textarea').value; - this.file.resolveEditChanged = this.file.content !== this.originalState; + this.file.content = this.editor.getValue(); + this.file.resolveEditChanged = this.file.content !== this.originalContent; this.file.promptDiscardConfirmation = false; }, - onInput() { - this.saveDiffResolution(); + resetEditorContent() { + if (this.fileLoaded) { + this.editor.setValue(this.originalContent, -1); + } } } }); diff --git a/app/assets/stylesheets/pages/merge_conflicts.scss b/app/assets/stylesheets/pages/merge_conflicts.scss index 577a97e8c0e..2a8c693b2a8 100644 --- a/app/assets/stylesheets/pages/merge_conflicts.scss +++ b/app/assets/stylesheets/pages/merge_conflicts.scss @@ -255,6 +255,13 @@ $colors: ( .editor { border-top: solid 1px yellow; + + pre { + height: 350px; + border: none; + border-radius: 0; + margin-bottom: 0; + } } .loading-text { diff --git a/app/views/projects/merge_requests/conflicts.html.haml b/app/views/projects/merge_requests/conflicts.html.haml index e3add1f799f..ff641b90b86 100644 --- a/app/views/projects/merge_requests/conflicts.html.haml +++ b/app/views/projects/merge_requests/conflicts.html.haml @@ -6,6 +6,8 @@ 'unselected': line.isUnselected }" - page_title "Merge Conflicts", "#{@merge_request.title} (#{@merge_request.to_reference}", "Merge Requests" +- content_for :page_specific_javascripts do + = page_specific_javascript_tag('lib/ace.js') = render "projects/merge_requests/show/mr_title" .merge-request-details.issuable-details diff --git a/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml index 0556341fd64..940558e02e0 100644 --- a/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml +++ b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml @@ -3,4 +3,4 @@ .editor-wrap{ ":class" => "classObject" } %p.loading-text Loading... .editor - %textarea{ "@input" => "onInput", cols: '80', rows: '20' } + %pre{ "style" => "height: 350px" } -- cgit v1.2.1 From 17830296a6143c4d76155449ff4da4377e263657 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 19 Sep 2016 10:10:17 -0500 Subject: Styles for discard alert --- app/assets/stylesheets/framework/variables.scss | 1 + app/assets/stylesheets/pages/merge_conflicts.scss | 16 +++++++++++++--- .../merge_requests/conflicts/_diff_file_editor.html.haml | 7 ++++--- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 4c34ed3ebf7..7690d65de8e 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -56,6 +56,7 @@ $border-gray-light: #dcdcdc; $border-gray-normal: #d7d7d7; $border-gray-dark: #c6cacf; +$border-green-extra-light: #9adb84; $border-green-light: #2faa60; $border-green-normal: #2ca05b; $border-green-dark: #279654; diff --git a/app/assets/stylesheets/pages/merge_conflicts.scss b/app/assets/stylesheets/pages/merge_conflicts.scss index 2a8c693b2a8..ea848b2c2c0 100644 --- a/app/assets/stylesheets/pages/merge_conflicts.scss +++ b/app/assets/stylesheets/pages/merge_conflicts.scss @@ -249,13 +249,11 @@ $colors: ( &.saved { .editor { - border-top: solid 1px green; + border-top: solid 2px $border-green-extra-light; } } .editor { - border-top: solid 1px yellow; - pre { height: 350px; border: none; @@ -268,4 +266,16 @@ $colors: ( display: none; } } + + .discard-changes-alert { + background-color: $background-color; + text-align: right; + padding: $gl-padding-top $gl-padding; + color: $gl-text-color; + + .discard-actions { + display: inline-block; + margin-left: 10px; + } + } } diff --git a/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml index d0c518e5249..a335470de75 100644 --- a/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml +++ b/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml @@ -2,8 +2,9 @@ .diff-editor-wrap{ "v-show" => if_condition } .discard-changes-alert-wrap{ "v-if" => "file.promptDiscardConfirmation" } - %p + .discard-changes-alert Are you sure to discard your changes? - %button.btn.btn-sm.btn-close{ "@click" => "acceptDiscardConfirmation(file)" } Discard changes - %button.btn.btn-sm{ "@click" => "cancelDiscardConfirmation(file)" } Cancel + .discard-actions + %button.btn.btn-sm.btn-close{ "@click" => "acceptDiscardConfirmation(file)" } Discard changes + %button.btn.btn-sm{ "@click" => "cancelDiscardConfirmation(file)" } Cancel %diff-file-editor{":file" => "file", ":load-file" => if_condition } -- cgit v1.2.1 From 02bfb0ff1bf53abf44c50d8310ad0856d26860cf Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 19 Sep 2016 10:51:09 -0500 Subject: Style for resolve conflicts form --- app/assets/stylesheets/pages/merge_conflicts.scss | 4 ++++ .../conflicts/_submit_form.html.haml | 27 ++++++++++++++-------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/pages/merge_conflicts.scss b/app/assets/stylesheets/pages/merge_conflicts.scss index ea848b2c2c0..c851bd52b1a 100644 --- a/app/assets/stylesheets/pages/merge_conflicts.scss +++ b/app/assets/stylesheets/pages/merge_conflicts.scss @@ -278,4 +278,8 @@ $colors: ( margin-left: 10px; } } + + .resolve-conflicts-form { + padding-top: $gl-padding; + } } diff --git a/app/views/projects/merge_requests/conflicts/_submit_form.html.haml b/app/views/projects/merge_requests/conflicts/_submit_form.html.haml index 380f8722186..fbb15c307c4 100644 --- a/app/views/projects/merge_requests/conflicts/_submit_form.html.haml +++ b/app/views/projects/merge_requests/conflicts/_submit_form.html.haml @@ -1,10 +1,17 @@ -.content-block - .commit-message-container.form-group - .max-width-marker - %textarea.form-control.js-commit-message{"v-model" => "conflictsData.commitMessage"} - {{{conflictsData.commitMessage}}} - - %button{type: "button", class: "btn btn-success js-submit-button", "@click" => "commit()", ":disabled" => "!readyToCommit" } - %span {{commitButtonText}} - - = link_to "Cancel", namespace_project_merge_request_path(@merge_request.project.namespace, @merge_request.project, @merge_request), class: "btn btn-cancel" +.form-horizontal.resolve-conflicts-form + .form-group + %label.col-sm-2.control-label{ "for" => "commit-message" } + Commit message + .col-sm-10 + .commit-message-container + .max-width-marker + %textarea.form-control.js-commit-message#commit-message{ "v-model" => "conflictsData.commitMessage", "rows" => "5" } + {{{conflictsData.commitMessage}}} + .form-group + .col-sm-offset-2.col-sm-10 + .row + .col-xs-6 + %button{ type: "button", class: "btn btn-success js-submit-button", "@click" => "commit()", ":disabled" => "!readyToCommit" } + %span {{commitButtonText}} + .col-xs-6.text-right + = link_to "Cancel", namespace_project_merge_request_path(@merge_request.project.namespace, @merge_request.project, @merge_request), class: "btn btn-cancel" -- cgit v1.2.1 From 0402a18367a7a08d3f40f8b63b961a0e1abb345a Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 19 Sep 2016 11:04:52 -0500 Subject: Replace loading text with spinner --- app/assets/stylesheets/pages/merge_conflicts.scss | 4 ++-- .../merge_requests/conflicts/components/_diff_file_editor.html.haml | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/pages/merge_conflicts.scss b/app/assets/stylesheets/pages/merge_conflicts.scss index c851bd52b1a..d447ca53c16 100644 --- a/app/assets/stylesheets/pages/merge_conflicts.scss +++ b/app/assets/stylesheets/pages/merge_conflicts.scss @@ -242,7 +242,7 @@ $colors: ( display: none; } - .loading-text { + .loading { display: block; } } @@ -262,7 +262,7 @@ $colors: ( } } - .loading-text { + .loading { display: none; } } diff --git a/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml index 940558e02e0..9965bdf9028 100644 --- a/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml +++ b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml @@ -1,6 +1,7 @@ %template{ id: "diff-file-editor" } %div .editor-wrap{ ":class" => "classObject" } - %p.loading-text Loading... + .loading + %i.fa.fa-spinner.fa-spin .editor %pre{ "style" => "height: 350px" } -- cgit v1.2.1 From 12ca16080b86b2b0cee8037800952347c2dcd8c4 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Tue, 20 Sep 2016 12:37:53 -0500 Subject: check if files is set before counting --- app/assets/javascripts/merge_conflict_data_provider.js.es6 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/javascripts/merge_conflict_data_provider.js.es6 b/app/assets/javascripts/merge_conflict_data_provider.js.es6 index bfed78ff99c..6d1d3f36b33 100644 --- a/app/assets/javascripts/merge_conflict_data_provider.js.es6 +++ b/app/assets/javascripts/merge_conflict_data_provider.js.es6 @@ -268,6 +268,10 @@ class MergeConflictDataProvider { getConflictsCount() { + if (!this.vueInstance.conflictsData.files) { + return 0; + } + const files = this.vueInstance.conflictsData.files; let count = 0; -- cgit v1.2.1 From 4178ddee18918c5186ba75aaf9b303138fb97b30 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Sun, 25 Sep 2016 15:45:58 -0500 Subject: Add tests to check if files are resolved with Edit Inline mode --- spec/features/merge_requests/conflicts_spec.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb index 1e8aeacea05..d2057aa2fe7 100644 --- a/spec/features/merge_requests/conflicts_spec.rb +++ b/spec/features/merge_requests/conflicts_spec.rb @@ -36,6 +36,32 @@ feature 'Merge request conflict resolution', js: true, feature: true do retry end end + + context 'when in inline mode' do + it 'resolves files manually' do + within find('.files-wrapper .diff-file.inline-view', text: 'files/ruby/popen.rb') do + click_button 'Edit inline' + wait_for_ajax + execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[0]).setValue("One morning");'); + end + + within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do + click_button 'Edit inline' + wait_for_ajax + execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[1]).setValue("Gregor Samsa woke from troubled dreams");'); + end + + click_button 'Commit conflict resolution' + wait_for_ajax + expect(page).to have_content('All merge conflicts were resolved') + + click_on 'Changes' + wait_for_ajax + + expect(page).to have_content('One morning') + expect(page).to have_content('Gregor Samsa woke from troubled dreams') + end + end end end -- cgit v1.2.1 From e84f959ae47e35eaebdc6c0adaf1e089326601ce Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 26 Sep 2016 15:49:04 +0100 Subject: Fix editor spec --- spec/features/merge_requests/conflicts_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb index d2057aa2fe7..721360b207e 100644 --- a/spec/features/merge_requests/conflicts_spec.rb +++ b/spec/features/merge_requests/conflicts_spec.rb @@ -54,6 +54,7 @@ feature 'Merge request conflict resolution', js: true, feature: true do click_button 'Commit conflict resolution' wait_for_ajax expect(page).to have_content('All merge conflicts were resolved') + merge_request.reload_diff click_on 'Changes' wait_for_ajax -- cgit v1.2.1 From a8ac9089afb664e569b34c61dc6782d20d1019d1 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 28 Sep 2016 05:12:13 -0500 Subject: Refactor JS code - Use a store base object to manage application state. - Add a service to handle ajax requests. - Load code only when needed --- app/assets/javascripts/dispatcher.js | 3 - .../components/diff_file_editor.js.es6 | 5 +- .../merge_conflicts/merge_conflict_service.js.es6 | 30 ++ .../merge_conflicts/merge_conflict_store.js.es6 | 419 +++++++++++++++++++++ .../merge_conflicts/merge_conflicts_bundle.js.es6 | 84 +++++ .../projects/merge_requests/conflicts.html.haml | 3 +- .../conflicts/_commit_stats.html.haml | 6 +- .../conflicts/_submit_form.html.haml | 1 - config/application.rb | 1 + spec/features/merge_requests/conflicts_spec.rb | 4 +- 10 files changed, 545 insertions(+), 11 deletions(-) create mode 100644 app/assets/javascripts/merge_conflicts/merge_conflict_service.js.es6 create mode 100644 app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 create mode 100644 app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index f3ef13ce20e..2c277766d2d 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -97,9 +97,6 @@ new ZenMode(); new MergedButtons(); break; - case "projects:merge_requests:conflicts": - window.mcui = new MergeConflictResolver() - break; case 'projects:merge_requests:index': shortcut_handler = new ShortcutsNavigation(); Issuable.init(); diff --git a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 index abdf73febb4..49d30f6e9e8 100644 --- a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 +++ b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 @@ -1,5 +1,8 @@ ((global) => { - global.diffFileEditor = Vue.extend({ + + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.diffFileEditor = Vue.extend({ props: ['file', 'loadFile'], template: '#diff-file-editor', data() { diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_service.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflict_service.js.es6 new file mode 100644 index 00000000000..da2fb8b1323 --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_service.js.es6 @@ -0,0 +1,30 @@ +((global) => { + global.mergeConflicts = global.mergeConflicts || {}; + + class mergeConflictsService { + constructor(options) { + this.conflictsPath = options.conflictsPath; + this.resolveConflictsPath = options.resolveConflictsPath; + } + + fetchConflictsData() { + return $.ajax({ + dataType: 'json', + url: this.conflictsPath + }); + } + + submitResolveConflicts(data) { + return $.ajax({ + url: this.resolveConflictsPath, + data: JSON.stringify(data), + contentType: 'application/json', + dataType: 'json', + method: 'POST' + }); + } + }; + + global.mergeConflicts.mergeConflictsService = mergeConflictsService; + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 new file mode 100644 index 00000000000..d35e6d8aed6 --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 @@ -0,0 +1,419 @@ +((global) => { + global.mergeConflicts = global.mergeConflicts || {}; + + const diffViewType = $.cookie('diff_view'); + const HEAD_HEADER_TEXT = 'HEAD//our changes'; + const ORIGIN_HEADER_TEXT = 'origin//their changes'; + const HEAD_BUTTON_TITLE = 'Use ours'; + const ORIGIN_BUTTON_TITLE = 'Use theirs'; + const INTERACTIVE_RESOLVE_MODE = 'interactive'; + const EDIT_RESOLVE_MODE = 'edit'; + const DEFAULT_RESOLVE_MODE = INTERACTIVE_RESOLVE_MODE; + const VIEW_TYPES = { + INLINE: 'inline', + PARALLEL: 'parallel' + }; + + global.mergeConflicts.mergeConflictsStore = { + state: { + isLoading: true, + hasError: false, + isSubmitting: false, + isParallel: diffViewType === VIEW_TYPES.PARALLEL, + diffViewType: diffViewType, + conflictsData: {} + }, + + setConflictsData(data) { + this.decorateFiles(data.files); + this.setInlineLines(data.files); + this.setParallelLines(data.files); + + this.state.conflictsData = { + files: data.files, + commitMessage: data.commit_message, + sourceBranch: data.source_branch, + targetBranch: data.target_branch, + commitMessage: data.commit_message, + shortCommitSha: data.commit_sha.slice(0, 7), + }; + }, + + decorateFiles(files) { + files.forEach((file) => { + file.content = ''; + file.resolutionData = {}; + file.promptDiscardConfirmation = false; + file.resolveMode = DEFAULT_RESOLVE_MODE; + }); + }, + + setInlineLines(files) { + files.forEach((file) => { + file.iconClass = `fa-${file.blob_icon}`; + file.blobPath = file.blob_path; + file.filePath = this.getFilePath(file); + file.inlineLines = []; + + file.sections.forEach((section) => { + let currentLineType = 'new'; + const { conflict, lines, id } = section; + + if (conflict) { + file.inlineLines.push(this.getHeadHeaderLine(id)); + } + + lines.forEach((line) => { + const { type } = line; + + if ((type === 'new' || type === 'old') && currentLineType !== type) { + currentLineType = type; + file.inlineLines.push({ lineType: 'emptyLine', richText: '' }); + } + + this.decorateLineForInlineView(line, id, conflict); + file.inlineLines.push(line); + }) + + if (conflict) { + file.inlineLines.push(this.getOriginHeaderLine(id)); + } + }); + }); + }, + + setParallelLines(files) { + files.forEach((file) => { + file.filePath = this.getFilePath(file); + file.iconClass = `fa-${file.blob_icon}`; + file.blobPath = file.blob_path; + file.parallelLines = []; + const linesObj = { left: [], right: [] }; + + file.sections.forEach((section) => { + const { conflict, lines, id } = section; + + if (conflict) { + linesObj.left.push(this.getOriginHeaderLine(id)); + linesObj.right.push(this.getHeadHeaderLine(id)); + } + + lines.forEach((line) => { + const { type } = line; + + if (conflict) { + if (type === 'old') { + linesObj.left.push(this.getLineForParallelView(line, id, 'conflict')); + } + else if (type === 'new') { + linesObj.right.push(this.getLineForParallelView(line, id, 'conflict', true)); + } + } + else { + const lineType = type || 'context'; + + linesObj.left.push (this.getLineForParallelView(line, id, lineType)); + linesObj.right.push(this.getLineForParallelView(line, id, lineType, true)); + } + }); + + this.checkLineLengths(linesObj); + }); + + for (let i = 0, len = linesObj.left.length; i < len; i++) { + file.parallelLines.push([ + linesObj.right[i], + linesObj.left[i] + ]); + } + + return file; + }); + }, + + setLoadingState(state) { + this.state.isLoading = state; + }, + + setErrorState(state) { + this.state.hasError = state; + }, + + setFailedRequest(message) { + console.log('setFailedRequest'); + this.state.hasError = true; + this.state.conflictsData.errorMessage = message; + }, + + getConflictsCount() { + if (!this.state.conflictsData.files) { + return 0; + } + + const files = this.state.conflictsData.files; + let count = 0; + + files.forEach((file) => { + file.sections.forEach((section) => { + if (section.conflict) { + count++; + } + }); + }); + + return count; + }, + + getConflictsCountText() { + const count = this.getConflictsCount(); + const text = count ? 'conflicts' : 'conflict'; + + return `${count} ${text}`; + }, + + setViewType(viewType) { + this.state.diffView = viewType; + this.state.isParallel = viewType === VIEW_TYPES.PARALLEL; + + $.cookie('diff_view', viewType, { + path: gon.relative_url_root || '/' + }); + }, + + getHeadHeaderLine(id) { + return { + id: id, + richText: HEAD_HEADER_TEXT, + buttonTitle: HEAD_BUTTON_TITLE, + type: 'new', + section: 'head', + isHeader: true, + isHead: true, + isSelected: false, + isUnselected: false + } + }, + + decorateLineForInlineView(line, id, conflict) { + const { type } = line; + line.id = id; + line.hasConflict = conflict; + line.isHead = type === 'new'; + line.isOrigin = type === 'old'; + line.hasMatch = type === 'match'; + line.richText = line.rich_text; + line.isSelected = false; + line.isUnselected = false; + }, + + getLineForParallelView(line, id, lineType, isHead) { + const { old_line, new_line, rich_text } = line; + const hasConflict = lineType === 'conflict'; + + return { + id, + lineType, + hasConflict, + isHead: hasConflict && isHead, + isOrigin: hasConflict && !isHead, + hasMatch: lineType === 'match', + lineNumber: isHead ? new_line : old_line, + section: isHead ? 'head' : 'origin', + richText: rich_text, + isSelected: false, + isUnselected: false + } + }, + + getOriginHeaderLine(id) { + return { + id: id, + richText: ORIGIN_HEADER_TEXT, + buttonTitle: ORIGIN_BUTTON_TITLE, + type: 'old', + section: 'origin', + isHeader: true, + isOrigin: true, + isSelected: false, + isUnselected: false + } + }, + + getFilePath(file) { + const { old_path, new_path } = file; + return old_path === new_path ? new_path : `${old_path} → ${new_path}`; + }, + + checkLineLengths(linesObj) { + let { left, right } = linesObj; + + if (left.length !== right.length) { + if (left.length > right.length) { + const diff = left.length - right.length; + for (let i = 0; i < diff; i++) { + right.push({ lineType: 'emptyLine', richText: '' }); + } + } + else { + const diff = right.length - left.length; + for (let i = 0; i < diff; i++) { + left.push({ lineType: 'emptyLine', richText: '' }); + } + } + } + }, + + setPromptConfirmationState(file, state) { + file.promptDiscardConfirmation = state; + }, + + setFileResolveMode(file, mode) { + // Restore Interactive mode when switching to Edit mode + if (mode === EDIT_RESOLVE_MODE) { + file.resolutionData = {}; + + this.restoreFileLinesState(file); + } + + file.resolveMode = mode; + }, + + restoreFileLinesState(file) { + file.inlineLines.forEach((line) => { + if (line.hasConflict || line.isHeader) { + line.isSelected = false; + line.isUnselected = false; + } + }); + + file.parallelLines.forEach((lines) => { + const left = lines[0]; + const right = lines[1]; + const isLeftMatch = left.hasConflict || left.isHeader; + const isRightMatch = right.hasConflict || right.isHeader; + + if (isLeftMatch || isRightMatch) { + left.isSelected = false; + left.isUnselected = false; + right.isSelected = false; + right.isUnselected = false; + } + }); + }, + + isReadyToCommit() { + const files = this.state.conflictsData.files; + const hasCommitMessage = $.trim(this.state.conflictsData.commitMessage).length; + let unresolved = 0; + + for (let i = 0, l = files.length; i < l; i++) { + let file = files[i]; + + if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { + let numberConflicts = 0; + let resolvedConflicts = Object.keys(file.resolutionData).length + + for (let j = 0, k = file.sections.length; j < k; j++) { + if (file.sections[j].conflict) { + numberConflicts++; + } + } + + if (resolvedConflicts !== numberConflicts) { + unresolved++; + } + } else if (file.resolveMode === EDIT_RESOLVE_MODE) { + // Unlikely to happen since switching to Edit mode saves content automatically. + // Checking anyway in case the save strategy changes in the future + if (!file.content) { + unresolved++; + continue; + } + } + } + + return !this.state.isSubmitting && hasCommitMessage && !unresolved; + }, + + getCommitButtonText() { + const initial = 'Commit conflict resolution'; + const inProgress = 'Committing...'; + + return this.state ? this.state.isSubmitting ? inProgress : initial : initial; + }, + + getCommitData() { + let commitData = {}; + + commitData = { + commit_message: this.state.conflictsData.commitMessage, + files: [] + }; + + this.state.conflictsData.files.forEach((file) => { + let addFile; + + addFile = { + old_path: file.old_path, + new_path: file.new_path + }; + + // Submit only one data for type of editing + if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { + addFile.sections = file.resolutionData; + } else if (file.resolveMode === EDIT_RESOLVE_MODE) { + addFile.content = file.content; + } + + commitData.files.push(addFile); + }); + + return commitData; + }, + + handleSelected(file, sectionId, selection) { + Vue.set(file.resolutionData, sectionId, selection); + + this.state.conflictsData.files.forEach((file) => { + file.inlineLines.forEach((line) => { + if (line.id === sectionId && (line.hasConflict || line.isHeader)) { + this.markLine(line, selection); + } + }); + + file.parallelLines.forEach((lines) => { + const left = lines[0]; + const right = lines[1]; + const hasSameId = right.id === sectionId || left.id === sectionId; + const isLeftMatch = left.hasConflict || left.isHeader; + const isRightMatch = right.hasConflict || right.isHeader; + + if (hasSameId && (isLeftMatch || isRightMatch)) { + this.markLine(left, selection); + this.markLine(right, selection); + } + }) + }); + }, + + markLine(line, selection) { + if (selection === 'head' && line.isHead) { + line.isSelected = true; + line.isUnselected = false; + } + else if (selection === 'origin' && line.isOrigin) { + line.isSelected = true; + line.isUnselected = false; + } + else { + line.isSelected = false; + line.isUnselected = true; + } + }, + + setSubmitState(state) { + this.state.isSubmitting = state; + } + }; + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 new file mode 100644 index 00000000000..b5123e22f7a --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 @@ -0,0 +1,84 @@ +//= require vue +//= require ./merge_conflict_store +//= require ./merge_conflict_service +//= require ./components/diff_file_editor + +$(() => { + const INTERACTIVE_RESOLVE_MODE = 'interactive'; + const $conflicts = $(document.getElementById('conflicts')); + const mergeConflictsStore = gl.mergeConflicts.mergeConflictsStore; + const mergeConflictsService = new gl.mergeConflicts.mergeConflictsService({ + conflictsPath: $conflicts.data('conflictsPath'), + resolveConflictsPath: $conflicts.data('resolveConflictsPath') + }); + + gl.MergeConflictsResolverApp = new Vue({ + el: '#conflicts', + data: mergeConflictsStore.state, + components: { + 'diff-file-editor': gl.mergeConflicts.diffFileEditor + }, + computed: { + conflictsCountText() { return mergeConflictsStore.getConflictsCountText() }, + readyToCommit() { return mergeConflictsStore.isReadyToCommit() }, + commitButtonText() { return mergeConflictsStore.getCommitButtonText() } + }, + created() { + mergeConflictsService + .fetchConflictsData() + .done((data) => { + if (data.type === 'error') { + mergeConflictsStore.setFailedRequest(data.message); + } else { + mergeConflictsStore.setConflictsData(data); + } + }) + .error(() => { + mergeConflictsStore.setFailedRequest(); + }) + .always(() => { + mergeConflictsStore.setLoadingState(false); + + this.$nextTick(() => { + $conflicts.find('.js-syntax-highlight').syntaxHighlight(); + }); + }); + }, + methods: { + handleSelected(file, sectionId, selection) { + mergeConflictsStore.handleSelected(file, sectionId, selection); + }, + handleViewTypeChange(viewType) { + mergeConflictsStore.setViewType(viewType); + }, + onClickResolveModeButton(file, mode) { + if (mode === INTERACTIVE_RESOLVE_MODE && file.resolveEditChanged) { + mergeConflictsStore.setPromptConfirmationState(file, true); + return; + } + + mergeConflictsStore.setFileResolveMode(file, mode); + }, + acceptDiscardConfirmation(file) { + mergeConflictsStore.setPromptConfirmationState(file, false); + mergeConflictsStore.setFileResolveMode(file, INTERACTIVE_RESOLVE_MODE); + }, + cancelDiscardConfirmation(file) { + mergeConflictsStore.setPromptConfirmationState(file, false); + }, + commit() { + mergeConflictsStore.setSubmitState(true); + + mergeConflictsService + .submitResolveConflicts(mergeConflictsStore.getCommitData()) + .done((data) => { + window.location.href = data.redirect_to; + }) + .error(() => { + mergeConflictsStore.setSubmitState(false); + new Flash('Failed to save merge conflicts resolutions. Please try again!'); + }); + } + } + }) +}); diff --git a/app/views/projects/merge_requests/conflicts.html.haml b/app/views/projects/merge_requests/conflicts.html.haml index ff641b90b86..997f40c0588 100644 --- a/app/views/projects/merge_requests/conflicts.html.haml +++ b/app/views/projects/merge_requests/conflicts.html.haml @@ -7,6 +7,7 @@ - page_title "Merge Conflicts", "#{@merge_request.title} (#{@merge_request.to_reference}", "Merge Requests" - content_for :page_specific_javascripts do + = page_specific_javascript_tag('merge_conflicts/merge_conflicts_bundle.js') = page_specific_javascript_tag('lib/ace.js') = render "projects/merge_requests/show/mr_title" @@ -26,8 +27,8 @@ = render partial: "projects/merge_requests/conflicts/commit_stats" .files-wrapper{"v-if" => "!isLoading && !hasError"} - = render partial: "projects/merge_requests/conflicts/parallel_view", locals: { class_bindings: class_bindings } = render partial: "projects/merge_requests/conflicts/inline_view", locals: { class_bindings: class_bindings } + = render partial: "projects/merge_requests/conflicts/parallel_view", locals: { class_bindings: class_bindings } = render partial: "projects/merge_requests/conflicts/submit_form" -# Components diff --git a/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml b/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml index 457c467fba9..a3831d5a34e 100644 --- a/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml +++ b/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml @@ -13,8 +13,8 @@ .js-toggle-container .commit-stat-summary Showing - %strong.cred {{conflictsCount}} {{conflictsData.conflictsText}} + %strong.cred {{conflictsCountText}} between - %strong {{conflictsData.source_branch}} + %strong {{conflictsData.sourceBranch}} and - %strong {{conflictsData.target_branch}} + %strong {{conflictsData.targetBranch}} diff --git a/app/views/projects/merge_requests/conflicts/_submit_form.html.haml b/app/views/projects/merge_requests/conflicts/_submit_form.html.haml index fbb15c307c4..6ffaa9ad4d2 100644 --- a/app/views/projects/merge_requests/conflicts/_submit_form.html.haml +++ b/app/views/projects/merge_requests/conflicts/_submit_form.html.haml @@ -6,7 +6,6 @@ .commit-message-container .max-width-marker %textarea.form-control.js-commit-message#commit-message{ "v-model" => "conflictsData.commitMessage", "rows" => "5" } - {{{conflictsData.commitMessage}}} .form-group .col-sm-offset-2.col-sm-10 .row diff --git a/config/application.rb b/config/application.rb index 962ffe0708d..8a9c539cb43 100644 --- a/config/application.rb +++ b/config/application.rb @@ -89,6 +89,7 @@ module Gitlab config.assets.precompile << "profile/profile_bundle.js" config.assets.precompile << "diff_notes/diff_notes_bundle.js" config.assets.precompile << "boards/boards_bundle.js" + config.assets.precompile << "merge_conflicts/merge_conflicts_bundle.js" config.assets.precompile << "boards/test_utils/simulate_drag.js" config.assets.precompile << "blob_edit/blob_edit_bundle.js" config.assets.precompile << "snippet/snippet_bundle.js" diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb index 721360b207e..f2ff000486b 100644 --- a/spec/features/merge_requests/conflicts_spec.rb +++ b/spec/features/merge_requests/conflicts_spec.rb @@ -42,13 +42,13 @@ feature 'Merge request conflict resolution', js: true, feature: true do within find('.files-wrapper .diff-file.inline-view', text: 'files/ruby/popen.rb') do click_button 'Edit inline' wait_for_ajax - execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[0]).setValue("One morning");'); + execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[0]).setValue("One morning");') end within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do click_button 'Edit inline' wait_for_ajax - execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[1]).setValue("Gregor Samsa woke from troubled dreams");'); + execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[1]).setValue("Gregor Samsa woke from troubled dreams");') end click_button 'Commit conflict resolution' -- cgit v1.2.1 From 197ac5ebbfd8ca7fbcb79776fd25ca0e213376c4 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 29 Sep 2016 20:26:01 -0500 Subject: Ability to resolve conflicts for files with `text-editor` as conflict type --- .../components/diff_file_editor.js.es6 | 43 ++-- .../merge_conflicts/merge_conflict_store.js.es6 | 240 +++++++++++---------- .../conflicts/_diff_file_editor.html.haml | 6 +- .../conflicts/_file_actions.html.haml | 2 +- .../conflicts/_inline_view.html.haml | 41 ++-- .../conflicts/_parallel_view.html.haml | 2 +- spec/features/merge_requests/conflicts_spec.rb | 38 +++- 7 files changed, 213 insertions(+), 159 deletions(-) diff --git a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 index 49d30f6e9e8..46aad9fe33c 100644 --- a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 +++ b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 @@ -7,10 +7,10 @@ template: '#diff-file-editor', data() { return { - originalContent: '', saved: false, loading: false, - fileLoaded: false + fileLoaded: false, + originalContent: '', } }, computed: { @@ -23,43 +23,48 @@ } }, watch: { - loadFile(val) { - const self = this; - + ['file.showEditor'](val) { this.resetEditorContent(); if (!val || this.fileLoaded || this.loading) { - return + return; } + this.loadEditor(); + } + }, + ready() { + if (this.file.loadEditor) { + this.loadEditor(); + } + }, + methods: { + loadEditor() { this.loading = true; $.get(this.file.content_path) .done((file) => { - - let content = self.$el.querySelector('pre'); + let content = this.$el.querySelector('pre'); let fileContent = document.createTextNode(file.content); content.textContent = fileContent.textContent; - self.originalContent = file.content; - self.fileLoaded = true; - self.editor = ace.edit(content); - self.editor.$blockScrolling = Infinity; // Turn off annoying warning - self.editor.on('change', () => { - self.saveDiffResolution(); + this.originalContent = file.content; + this.fileLoaded = true; + this.editor = ace.edit(content); + this.editor.$blockScrolling = Infinity; // Turn off annoying warning + this.editor.on('change', () => { + this.saveDiffResolution(); }); - self.saveDiffResolution(); + this.saveDiffResolution(); }) .fail(() => { console.log('error'); }) .always(() => { - self.loading = false; + this.loading = false; }); - } - }, - methods: { + }, saveDiffResolution() { this.saved = true; diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 index d35e6d8aed6..e1f862797f5 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 @@ -13,6 +13,10 @@ INLINE: 'inline', PARALLEL: 'parallel' }; + const CONFLICT_TYPES = { + TEXT: 'text', + TEXT_EDITOR: 'text-editor' + }; global.mergeConflicts.mergeConflictsStore = { state: { @@ -26,8 +30,6 @@ setConflictsData(data) { this.decorateFiles(data.files); - this.setInlineLines(data.files); - this.setParallelLines(data.files); this.state.conflictsData = { files: data.files, @@ -45,90 +47,90 @@ file.resolutionData = {}; file.promptDiscardConfirmation = false; file.resolveMode = DEFAULT_RESOLVE_MODE; - }); - }, - - setInlineLines(files) { - files.forEach((file) => { + file.filePath = this.getFilePath(file); file.iconClass = `fa-${file.blob_icon}`; file.blobPath = file.blob_path; - file.filePath = this.getFilePath(file); - file.inlineLines = []; - file.sections.forEach((section) => { - let currentLineType = 'new'; - const { conflict, lines, id } = section; + if (file.type === CONFLICT_TYPES.TEXT) { + file.showEditor = false; + file.loadEditor = false; - if (conflict) { - file.inlineLines.push(this.getHeadHeaderLine(id)); - } + this.setInlineLine(file); + this.setParallelLine(file); + } else if (file.type === CONFLICT_TYPES.TEXT_EDITOR) { + file.showEditor = true; + file.loadEditor = true; + } + }); + }, - lines.forEach((line) => { - const { type } = line; + setInlineLine(file) { + file.inlineLines = []; - if ((type === 'new' || type === 'old') && currentLineType !== type) { - currentLineType = type; - file.inlineLines.push({ lineType: 'emptyLine', richText: '' }); - } + file.sections.forEach((section) => { + let currentLineType = 'new'; + const { conflict, lines, id } = section; + + if (conflict) { + file.inlineLines.push(this.getHeadHeaderLine(id)); + } - this.decorateLineForInlineView(line, id, conflict); - file.inlineLines.push(line); - }) + lines.forEach((line) => { + const { type } = line; - if (conflict) { - file.inlineLines.push(this.getOriginHeaderLine(id)); + if ((type === 'new' || type === 'old') && currentLineType !== type) { + currentLineType = type; + file.inlineLines.push({ lineType: 'emptyLine', richText: '' }); } - }); + + this.decorateLineForInlineView(line, id, conflict); + file.inlineLines.push(line); + }) + + if (conflict) { + file.inlineLines.push(this.getOriginHeaderLine(id)); + } }); }, - setParallelLines(files) { - files.forEach((file) => { - file.filePath = this.getFilePath(file); - file.iconClass = `fa-${file.blob_icon}`; - file.blobPath = file.blob_path; - file.parallelLines = []; - const linesObj = { left: [], right: [] }; - - file.sections.forEach((section) => { - const { conflict, lines, id } = section; + setParallelLine(file) { + file.parallelLines = []; + const linesObj = { left: [], right: [] }; - if (conflict) { - linesObj.left.push(this.getOriginHeaderLine(id)); - linesObj.right.push(this.getHeadHeaderLine(id)); - } + file.sections.forEach((section) => { + const { conflict, lines, id } = section; - lines.forEach((line) => { - const { type } = line; + if (conflict) { + linesObj.left.push(this.getOriginHeaderLine(id)); + linesObj.right.push(this.getHeadHeaderLine(id)); + } - if (conflict) { - if (type === 'old') { - linesObj.left.push(this.getLineForParallelView(line, id, 'conflict')); - } - else if (type === 'new') { - linesObj.right.push(this.getLineForParallelView(line, id, 'conflict', true)); - } - } - else { - const lineType = type || 'context'; + lines.forEach((line) => { + const { type } = line; - linesObj.left.push (this.getLineForParallelView(line, id, lineType)); - linesObj.right.push(this.getLineForParallelView(line, id, lineType, true)); + if (conflict) { + if (type === 'old') { + linesObj.left.push(this.getLineForParallelView(line, id, 'conflict')); + } else if (type === 'new') { + linesObj.right.push(this.getLineForParallelView(line, id, 'conflict', true)); } - }); + } else { + const lineType = type || 'context'; - this.checkLineLengths(linesObj); + linesObj.left.push (this.getLineForParallelView(line, id, lineType)); + linesObj.right.push(this.getLineForParallelView(line, id, lineType, true)); + } }); - for (let i = 0, len = linesObj.left.length; i < len; i++) { - file.parallelLines.push([ - linesObj.right[i], - linesObj.left[i] - ]); - } - - return file; + this.checkLineLengths(linesObj); }); + + for (let i = 0, len = linesObj.left.length; i < len; i++) { + file.parallelLines.push([ + linesObj.right[i], + linesObj.left[i] + ]); + } }, setLoadingState(state) { @@ -140,13 +142,12 @@ }, setFailedRequest(message) { - console.log('setFailedRequest'); this.state.hasError = true; this.state.conflictsData.errorMessage = message; }, getConflictsCount() { - if (!this.state.conflictsData.files) { + if (!this.state.conflictsData.files.length) { return 0; } @@ -154,11 +155,15 @@ let count = 0; files.forEach((file) => { - file.sections.forEach((section) => { - if (section.conflict) { - count++; - } - }); + if (file.type === CONFLICT_TYPES.TEXT) { + file.sections.forEach((section) => { + if (section.conflict) { + count++; + } + }); + } else { + count++; + } }); return count; @@ -172,7 +177,7 @@ }, setViewType(viewType) { - this.state.diffView = viewType; + this.state.diffView = viewType; this.state.isParallel = viewType === VIEW_TYPES.PARALLEL; $.cookie('diff_view', viewType, { @@ -253,8 +258,7 @@ for (let i = 0; i < diff; i++) { right.push({ lineType: 'emptyLine', richText: '' }); } - } - else { + } else { const diff = right.length - left.length; for (let i = 0; i < diff; i++) { left.push({ lineType: 'emptyLine', richText: '' }); @@ -268,8 +272,12 @@ }, setFileResolveMode(file, mode) { - // Restore Interactive mode when switching to Edit mode - if (mode === EDIT_RESOLVE_MODE) { + if (mode === INTERACTIVE_RESOLVE_MODE) { + file.showEditor = false; + } else if (mode === EDIT_RESOLVE_MODE) { + // Restore Interactive mode when switching to Edit mode + file.showEditor = true; + file.loadEditor = true; file.resolutionData = {}; this.restoreFileLinesState(file); @@ -287,9 +295,9 @@ }); file.parallelLines.forEach((lines) => { - const left = lines[0]; - const right = lines[1]; - const isLeftMatch = left.hasConflict || left.isHeader; + const left = lines[0]; + const right = lines[1]; + const isLeftMatch = left.hasConflict || left.isHeader; const isRightMatch = right.hasConflict || right.isHeader; if (isLeftMatch || isRightMatch) { @@ -313,14 +321,17 @@ let numberConflicts = 0; let resolvedConflicts = Object.keys(file.resolutionData).length - for (let j = 0, k = file.sections.length; j < k; j++) { - if (file.sections[j].conflict) { - numberConflicts++; + // We only check if + if (file.type === CONFLICT_TYPES.TEXT) { + for (let j = 0, k = file.sections.length; j < k; j++) { + if (file.sections[j].conflict) { + numberConflicts++; + } } - } - if (resolvedConflicts !== numberConflicts) { - unresolved++; + if (resolvedConflicts !== numberConflicts) { + unresolved++; + } } } else if (file.resolveMode === EDIT_RESOLVE_MODE) { // Unlikely to happen since switching to Edit mode saves content automatically. @@ -358,10 +369,15 @@ new_path: file.new_path }; - // Submit only one data for type of editing - if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { - addFile.sections = file.resolutionData; - } else if (file.resolveMode === EDIT_RESOLVE_MODE) { + if (file.type === CONFLICT_TYPES.TEXT) { + + // Submit only one data for type of editing + if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { + addFile.sections = file.resolutionData; + } else if (file.resolveMode === EDIT_RESOLVE_MODE) { + addFile.content = file.content; + } + } else if (file.type === CONFLICT_TYPES.TEXT_EDITOR) { addFile.content = file.content; } @@ -374,39 +390,35 @@ handleSelected(file, sectionId, selection) { Vue.set(file.resolutionData, sectionId, selection); - this.state.conflictsData.files.forEach((file) => { - file.inlineLines.forEach((line) => { - if (line.id === sectionId && (line.hasConflict || line.isHeader)) { - this.markLine(line, selection); - } - }); + file.inlineLines.forEach((line) => { + if (line.id === sectionId && (line.hasConflict || line.isHeader)) { + this.markLine(line, selection); + } + }); - file.parallelLines.forEach((lines) => { - const left = lines[0]; - const right = lines[1]; - const hasSameId = right.id === sectionId || left.id === sectionId; - const isLeftMatch = left.hasConflict || left.isHeader; - const isRightMatch = right.hasConflict || right.isHeader; + file.parallelLines.forEach((lines) => { + const left = lines[0]; + const right = lines[1]; + const hasSameId = right.id === sectionId || left.id === sectionId; + const isLeftMatch = left.hasConflict || left.isHeader; + const isRightMatch = right.hasConflict || right.isHeader; - if (hasSameId && (isLeftMatch || isRightMatch)) { - this.markLine(left, selection); - this.markLine(right, selection); - } - }) + if (hasSameId && (isLeftMatch || isRightMatch)) { + this.markLine(left, selection); + this.markLine(right, selection); + } }); }, markLine(line, selection) { if (selection === 'head' && line.isHead) { - line.isSelected = true; + line.isSelected = true; line.isUnselected = false; - } - else if (selection === 'origin' && line.isOrigin) { - line.isSelected = true; + } else if (selection === 'origin' && line.isOrigin) { + line.isSelected = true; line.isUnselected = false; - } - else { - line.isSelected = false; + } else { + line.isSelected = false; line.isUnselected = true; } }, diff --git a/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml index a335470de75..497db7e3f9b 100644 --- a/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml +++ b/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml @@ -1,10 +1,8 @@ -- if_condition = local_assigns.fetch(:if_condition, '') - -.diff-editor-wrap{ "v-show" => if_condition } +.diff-editor-wrap{ "v-show" => "file.showEditor" } .discard-changes-alert-wrap{ "v-if" => "file.promptDiscardConfirmation" } .discard-changes-alert Are you sure to discard your changes? .discard-actions %button.btn.btn-sm.btn-close{ "@click" => "acceptDiscardConfirmation(file)" } Discard changes %button.btn.btn-sm{ "@click" => "cancelDiscardConfirmation(file)" } Cancel - %diff-file-editor{":file" => "file", ":load-file" => if_condition } + %diff-file-editor{":file" => "file"} diff --git a/app/views/projects/merge_requests/conflicts/_file_actions.html.haml b/app/views/projects/merge_requests/conflicts/_file_actions.html.haml index 124dfde7b22..05af57acf03 100644 --- a/app/views/projects/merge_requests/conflicts/_file_actions.html.haml +++ b/app/views/projects/merge_requests/conflicts/_file_actions.html.haml @@ -1,5 +1,5 @@ .file-actions - .btn-group + .btn-group{"v-if" => "file.type === 'text'"} %button.btn{ ":class" => "{ 'active': file.resolveMode == 'interactive' }", '@click' => "onClickResolveModeButton(file, 'interactive')", type: 'button' } diff --git a/app/views/projects/merge_requests/conflicts/_inline_view.html.haml b/app/views/projects/merge_requests/conflicts/_inline_view.html.haml index 7120b6ff48d..60ac21d26c3 100644 --- a/app/views/projects/merge_requests/conflicts/_inline_view.html.haml +++ b/app/views/projects/merge_requests/conflicts/_inline_view.html.haml @@ -4,23 +4,26 @@ %i.fa.fa-fw{":class" => "file.iconClass"} %strong {{file.filePath}} = render partial: 'projects/merge_requests/conflicts/file_actions' - .diff-content.diff-wrap-lines - .diff-wrap-lines.code.file-content.js-syntax-highlight{ 'v-show' => "file.resolveMode === 'interactive'" } - %table - %tr.line_holder.diff-inline{"v-for" => "line in file.inlineLines"} - %template{"v-if" => "!line.isHeader"} - %td.diff-line-num.new_line{":class" => class_bindings} - %a {{line.new_line}} - %td.diff-line-num.old_line{":class" => class_bindings} - %a {{line.old_line}} - %td.line_content{":class" => class_bindings} - {{{line.richText}}} + %template{"v-if" => "file.type === 'text'"} + .diff-content.diff-wrap-lines + .diff-wrap-lines.code.file-content.js-syntax-highlight{ 'v-show' => "file.resolveMode === 'interactive'" } + %table + %tr.line_holder.diff-inline{"v-for" => "line in file.inlineLines"} + %template{"v-if" => "!line.isHeader"} + %td.diff-line-num.new_line{":class" => class_bindings} + %a {{line.new_line}} + %td.diff-line-num.old_line{":class" => class_bindings} + %a {{line.old_line}} + %td.line_content{":class" => class_bindings} + {{{line.richText}}} - %template{"v-if" => "line.isHeader"} - %td.diff-line-num.header{":class" => class_bindings} - %td.diff-line-num.header{":class" => class_bindings} - %td.line_content.header{":class" => class_bindings} - %strong {{{line.richText}}} - %button.btn{ "@click" => "handleSelected(file, line.id, line.section)" } - {{line.buttonTitle}} - = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.resolveMode === 'edit' && !isParallel" } + %template{"v-if" => "line.isHeader"} + %td.diff-line-num.header{":class" => class_bindings} + %td.diff-line-num.header{":class" => class_bindings} + %td.line_content.header{":class" => class_bindings} + %strong {{{line.richText}}} + %button.btn{ "@click" => "handleSelected(file, line.id, line.section)" } + {{line.buttonTitle}} + = render partial: 'projects/merge_requests/conflicts/diff_file_editor' + %template{"v-else" => true} + = render partial: 'projects/merge_requests/conflicts/diff_file_editor' diff --git a/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml b/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml index 18c830ddb17..7ed1485fc01 100644 --- a/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml +++ b/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml @@ -22,4 +22,4 @@ {{line.lineNumber}} %td.line_content.parallel{":class" => class_bindings} {{{line.richText}}} - = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.resolveMode === 'edit' && isParallel" } + = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.loadFile && isParallel" } diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb index f2ff000486b..0e5507a0210 100644 --- a/spec/features/merge_requests/conflicts_spec.rb +++ b/spec/features/merge_requests/conflicts_spec.rb @@ -37,7 +37,7 @@ feature 'Merge request conflict resolution', js: true, feature: true do end end - context 'when in inline mode' do + context 'when in inline mode' do it 'resolves files manually' do within find('.files-wrapper .diff-file.inline-view', text: 'files/ruby/popen.rb') do click_button 'Edit inline' @@ -66,6 +66,42 @@ feature 'Merge request conflict resolution', js: true, feature: true do end end + context 'when a merge request can be resolved in the UI' do + let(:merge_request) { create_merge_request('conflict-contains-conflict-markers') } + + before do + project.team << [user, :developer] + login_as(user) + + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + context 'a conflict contain markers' do + before { click_link('conflicts', href: /\/conflicts\Z/) } + + it 'resolves files manually' do + within find('.files-wrapper .diff-file.inline-view', text: 'files/markdown/ruby-style-guide.md') do + wait_for_ajax + execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[0]).setValue("Gregor Samsa woke from troubled dreams");') + end + + click_button 'Commit conflict resolution' + wait_for_ajax + + expect(page).to have_content('All merge conflicts were resolved') + + merge_request.reload_diff + + click_on 'Changes' + wait_for_ajax + find('.nothing-here-block', visible: true).click + wait_for_ajax + + expect(page).to have_content('Gregor Samsa woke from troubled dreams') + end + end + end + UNRESOLVABLE_CONFLICTS = { 'conflict-too-large' => 'when the conflicts contain a large file', 'conflict-binary-file' => 'when the conflicts contain a binary file', -- cgit v1.2.1 From cebad0fb60c90ff64cf16b022d262381aff5bf4f Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Fri, 30 Sep 2016 16:23:38 -0500 Subject: Do not show Diff view switcher if all files are can be only resolved with an editor --- .../javascripts/merge_conflicts/merge_conflict_store.js.es6 | 12 ++++++++++++ .../merge_conflicts/merge_conflicts_bundle.js.es6 | 3 ++- .../merge_requests/conflicts/_commit_stats.html.haml | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 index e1f862797f5..e31cac49d44 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 @@ -425,6 +425,18 @@ setSubmitState(state) { this.state.isSubmitting = state; + }, + + fileTextTypePresent() { + let found = false; + + this.state.conflictsData.files.find((f) => { + if (f.type === CONFLICT_TYPES.TEXT) { + return found = true; + } + }); + + return found; } }; diff --git a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 index b5123e22f7a..449b4d2209a 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 +++ b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 @@ -21,7 +21,8 @@ $(() => { computed: { conflictsCountText() { return mergeConflictsStore.getConflictsCountText() }, readyToCommit() { return mergeConflictsStore.isReadyToCommit() }, - commitButtonText() { return mergeConflictsStore.getCommitButtonText() } + commitButtonText() { return mergeConflictsStore.getCommitButtonText() }, + showDiffViewTypeSwitcher() { return mergeConflictsStore.fileTextTypePresent() } }, created() { mergeConflictsService diff --git a/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml b/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml index a3831d5a34e..7222b8ae4d3 100644 --- a/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml +++ b/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml @@ -1,5 +1,5 @@ .content-block.oneline-block.files-changed{"v-if" => "!isLoading && !hasError"} - .inline-parallel-buttons + .inline-parallel-buttons{"v-if" => "showDiffViewTypeSwitcher"} .btn-group %a.btn{ | ":class" => "{'active': !isParallel}", | -- cgit v1.2.1 From f947972dede2f8d68215555e4b4fd6e90a6bc437 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Fri, 30 Sep 2016 17:42:36 -0500 Subject: Improve diff view switching and components --- .../projects/merge_requests/conflicts.html.haml | 17 +++++++++++-- .../conflicts/_inline_view.html.haml | 29 ---------------------- .../conflicts/_parallel_view.html.haml | 25 ------------------- .../_resolve_mode_interactive_inline.html.haml | 17 +++++++++++++ .../_resolve_mode_interactive_parallel.html.haml | 17 +++++++++++++ spec/features/merge_requests/conflicts_spec.rb | 10 ++++---- 6 files changed, 54 insertions(+), 61 deletions(-) delete mode 100644 app/views/projects/merge_requests/conflicts/_inline_view.html.haml delete mode 100644 app/views/projects/merge_requests/conflicts/_parallel_view.html.haml create mode 100644 app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_inline.html.haml create mode 100644 app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_parallel.html.haml diff --git a/app/views/projects/merge_requests/conflicts.html.haml b/app/views/projects/merge_requests/conflicts.html.haml index 997f40c0588..5ff7befe0ca 100644 --- a/app/views/projects/merge_requests/conflicts.html.haml +++ b/app/views/projects/merge_requests/conflicts.html.haml @@ -27,9 +27,22 @@ = render partial: "projects/merge_requests/conflicts/commit_stats" .files-wrapper{"v-if" => "!isLoading && !hasError"} - = render partial: "projects/merge_requests/conflicts/inline_view", locals: { class_bindings: class_bindings } - = render partial: "projects/merge_requests/conflicts/parallel_view", locals: { class_bindings: class_bindings } + .files + .diff-file.file-holder.conflict{"v-for" => "file in conflictsData.files"} + .file-title + %i.fa.fa-fw{":class" => "file.iconClass"} + %strong {{file.filePath}} + = render partial: 'projects/merge_requests/conflicts/file_actions' + .diff-content.diff-wrap-lines + %div{"v-show" => "!isParallel && file.resolveMode === 'interactive' && file.type === 'text'" } + = render partial: "projects/merge_requests/conflicts/resolve_mode_interactive_inline", locals: { class_bindings: class_bindings } + %div{"v-show" => "isParallel && file.resolveMode === 'interactive' && file.type === 'text'" } + = render partial: "projects/merge_requests/conflicts/resolve_mode_interactive_parallel", locals: { class_bindings: class_bindings } + %div{"v-show" => " file.resolveMode === 'edit' || file.type === 'text-editor'"} + = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.loadFile && isParallel" } + = render partial: "projects/merge_requests/conflicts/submit_form" -# Components = render partial: 'projects/merge_requests/conflicts/components/diff_file_editor' + \ No newline at end of file diff --git a/app/views/projects/merge_requests/conflicts/_inline_view.html.haml b/app/views/projects/merge_requests/conflicts/_inline_view.html.haml deleted file mode 100644 index 60ac21d26c3..00000000000 --- a/app/views/projects/merge_requests/conflicts/_inline_view.html.haml +++ /dev/null @@ -1,29 +0,0 @@ -.files{"v-show" => "!isParallel"} - .diff-file.file-holder.conflict.inline-view{"v-for" => "file in conflictsData.files"} - .file-title - %i.fa.fa-fw{":class" => "file.iconClass"} - %strong {{file.filePath}} - = render partial: 'projects/merge_requests/conflicts/file_actions' - %template{"v-if" => "file.type === 'text'"} - .diff-content.diff-wrap-lines - .diff-wrap-lines.code.file-content.js-syntax-highlight{ 'v-show' => "file.resolveMode === 'interactive'" } - %table - %tr.line_holder.diff-inline{"v-for" => "line in file.inlineLines"} - %template{"v-if" => "!line.isHeader"} - %td.diff-line-num.new_line{":class" => class_bindings} - %a {{line.new_line}} - %td.diff-line-num.old_line{":class" => class_bindings} - %a {{line.old_line}} - %td.line_content{":class" => class_bindings} - {{{line.richText}}} - - %template{"v-if" => "line.isHeader"} - %td.diff-line-num.header{":class" => class_bindings} - %td.diff-line-num.header{":class" => class_bindings} - %td.line_content.header{":class" => class_bindings} - %strong {{{line.richText}}} - %button.btn{ "@click" => "handleSelected(file, line.id, line.section)" } - {{line.buttonTitle}} - = render partial: 'projects/merge_requests/conflicts/diff_file_editor' - %template{"v-else" => true} - = render partial: 'projects/merge_requests/conflicts/diff_file_editor' diff --git a/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml b/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml deleted file mode 100644 index 7ed1485fc01..00000000000 --- a/app/views/projects/merge_requests/conflicts/_parallel_view.html.haml +++ /dev/null @@ -1,25 +0,0 @@ -.files{"v-show" => "isParallel"} - .diff-file.file-holder.conflict.parallel-view{"v-for" => "file in conflictsData.files"} - .file-title - %i.fa.fa-fw{":class" => "file.iconClass"} - %strong {{file.filePath}} - = render partial: 'projects/merge_requests/conflicts/file_actions' - .diff-content.diff-wrap-lines - .diff-wrap-lines.code.file-content.js-syntax-highlight{ 'v-show' => "file.resolveMode === 'interactive'" } - %table - %tr.line_holder.parallel{"v-for" => "section in file.parallelLines"} - %template{"v-for" => "line in section"} - - %template{"v-if" => "line.isHeader"} - %td.diff-line-num.header{":class" => class_bindings} - %td.line_content.header{":class" => class_bindings} - %strong {{line.richText}} - %button.btn{"@click" => "handleSelected(file, line.id, line.section)"} - {{line.buttonTitle}} - - %template{"v-if" => "!line.isHeader"} - %td.diff-line-num.old_line{":class" => class_bindings} - {{line.lineNumber}} - %td.line_content.parallel{":class" => class_bindings} - {{{line.richText}}} - = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.loadFile && isParallel" } diff --git a/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_inline.html.haml b/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_inline.html.haml new file mode 100644 index 00000000000..b1d7a90eb74 --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_inline.html.haml @@ -0,0 +1,17 @@ +.diff-wrap-lines.code.file-content.js-syntax-highlight + %table + %tr.line_holder.diff-inline{"v-for" => "line in file.inlineLines"} + %template{"v-if" => "!line.isHeader"} + %td.diff-line-num.new_line{":class" => class_bindings} + %a {{line.new_line}} + %td.diff-line-num.old_line{":class" => class_bindings} + %a {{line.old_line}} + %td.line_content{":class" => class_bindings} + {{{line.richText}}} + %template{"v-if" => "line.isHeader"} + %td.diff-line-num.header{":class" => class_bindings} + %td.diff-line-num.header{":class" => class_bindings} + %td.line_content.header{":class" => class_bindings} + %strong {{{line.richText}}} + %button.btn{ "@click" => "handleSelected(file, line.id, line.section)" } + {{line.buttonTitle}} diff --git a/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_parallel.html.haml b/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_parallel.html.haml new file mode 100644 index 00000000000..f5af347f59b --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_parallel.html.haml @@ -0,0 +1,17 @@ +.diff-wrap-lines.code.file-content.js-syntax-highlight + %table + %tr.line_holder.parallel{"v-for" => "section in file.parallelLines"} + %template{"v-for" => "line in section"} + + %template{"v-if" => "line.isHeader"} + %td.diff-line-num.header{":class" => class_bindings} + %td.line_content.header{":class" => class_bindings} + %strong {{line.richText}} + %button.btn{"@click" => "handleSelected(file, line.id, line.section)"} + {{line.buttonTitle}} + + %template{"v-if" => "!line.isHeader"} + %td.diff-line-num.old_line{":class" => class_bindings} + {{line.lineNumber}} + %td.line_content.parallel{":class" => class_bindings} + {{{line.richText}}} diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb index 0e5507a0210..4cecc22aa6c 100644 --- a/spec/features/merge_requests/conflicts_spec.rb +++ b/spec/features/merge_requests/conflicts_spec.rb @@ -39,16 +39,16 @@ feature 'Merge request conflict resolution', js: true, feature: true do context 'when in inline mode' do it 'resolves files manually' do - within find('.files-wrapper .diff-file.inline-view', text: 'files/ruby/popen.rb') do + within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do click_button 'Edit inline' wait_for_ajax - execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[0]).setValue("One morning");') + execute_script('ace.edit($(".files-wrapper .diff-file pre")[0]).setValue("One morning");') end within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do click_button 'Edit inline' wait_for_ajax - execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[1]).setValue("Gregor Samsa woke from troubled dreams");') + execute_script('ace.edit($(".files-wrapper .diff-file pre")[1]).setValue("Gregor Samsa woke from troubled dreams");') end click_button 'Commit conflict resolution' @@ -80,9 +80,9 @@ feature 'Merge request conflict resolution', js: true, feature: true do before { click_link('conflicts', href: /\/conflicts\Z/) } it 'resolves files manually' do - within find('.files-wrapper .diff-file.inline-view', text: 'files/markdown/ruby-style-guide.md') do + within find('.files-wrapper .diff-file', text: 'files/markdown/ruby-style-guide.md') do wait_for_ajax - execute_script('ace.edit($(".files-wrapper .diff-file.inline-view pre")[0]).setValue("Gregor Samsa woke from troubled dreams");') + execute_script('ace.edit($(".files-wrapper .diff-file pre")[0]).setValue("Gregor Samsa woke from troubled dreams");') end click_button 'Commit conflict resolution' -- cgit v1.2.1 From 4f76ff0ecd0d3b278f9722042b66d1767078ddd0 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Tue, 4 Oct 2016 03:24:44 -0500 Subject: Use .some instead of .find for phantomjs compatibility --- .../javascripts/merge_conflicts/merge_conflict_store.js.es6 | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 index e31cac49d44..bdd6753f66a 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 @@ -428,15 +428,7 @@ }, fileTextTypePresent() { - let found = false; - - this.state.conflictsData.files.find((f) => { - if (f.type === CONFLICT_TYPES.TEXT) { - return found = true; - } - }); - - return found; + return this.state.conflictsData.files.some(f => f.type === CONFLICT_TYPES.TEXT); } }; -- cgit v1.2.1 From c4142cf9c0c0b217034c60a0a973d2e96b17a428 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 5 Oct 2016 07:57:22 -0500 Subject: Improve components for PhantomJs compatibility --- .../components/diff_file_editor.js.es6 | 14 ++++++++++--- .../components/inline_conflict_lines.js.es6 | 12 +++++++++++ .../components/parallel_conflict_line.js.es6 | 14 +++++++++++++ .../components/parallel_conflict_lines.js.es6 | 15 ++++++++++++++ .../merge_conflicts/merge_conflict_store.js.es6 | 6 +++--- .../merge_conflicts/merge_conflicts_bundle.js.es6 | 12 +++++++---- .../mixins/line_conflict_actions.js.es6 | 12 +++++++++++ .../mixins/line_conflict_utils.js.es6 | 18 ++++++++++++++++ .../projects/merge_requests/conflicts.html.haml | 24 ++++++++-------------- .../conflicts/_commit_stats.html.haml | 8 ++------ .../conflicts/_diff_file_editor.html.haml | 8 -------- .../_resolve_mode_interactive_inline.html.haml | 17 --------------- .../_resolve_mode_interactive_parallel.html.haml | 17 --------------- .../components/_diff_file_editor.html.haml | 10 +++++++-- .../components/_inline_conflict_lines.html.haml | 15 ++++++++++++++ .../components/_parallel_conflict_line.html.haml | 10 +++++++++ .../components/_parallel_conflict_lines.html.haml | 4 ++++ 17 files changed, 140 insertions(+), 76 deletions(-) create mode 100644 app/assets/javascripts/merge_conflicts/components/inline_conflict_lines.js.es6 create mode 100644 app/assets/javascripts/merge_conflicts/components/parallel_conflict_line.js.es6 create mode 100644 app/assets/javascripts/merge_conflicts/components/parallel_conflict_lines.js.es6 create mode 100644 app/assets/javascripts/merge_conflicts/mixins/line_conflict_actions.js.es6 create mode 100644 app/assets/javascripts/merge_conflicts/mixins/line_conflict_utils.js.es6 delete mode 100644 app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml delete mode 100644 app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_inline.html.haml delete mode 100644 app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_parallel.html.haml create mode 100644 app/views/projects/merge_requests/conflicts/components/_inline_conflict_lines.html.haml create mode 100644 app/views/projects/merge_requests/conflicts/components/_parallel_conflict_line.html.haml create mode 100644 app/views/projects/merge_requests/conflicts/components/_parallel_conflict_lines.html.haml diff --git a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 index 46aad9fe33c..417095a8db7 100644 --- a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 +++ b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 @@ -3,8 +3,11 @@ global.mergeConflicts = global.mergeConflicts || {}; global.mergeConflicts.diffFileEditor = Vue.extend({ - props: ['file', 'loadFile'], - template: '#diff-file-editor', + props: { + file: Object, + onCancelDiscardConfirmation: Function, + onAcceptDiscardConfirmation: Function + }, data() { return { saved: false, @@ -16,7 +19,6 @@ computed: { classObject() { return { - 'load-file': this.loadFile, 'saved': this.saved, 'is-loading': this.loading }; @@ -77,6 +79,12 @@ if (this.fileLoaded) { this.editor.setValue(this.originalContent, -1); } + }, + cancelDiscardConfirmation(file) { + this.onCancelDiscardConfirmation(file); + }, + acceptDiscardConfirmation(file) { + this.onAcceptDiscardConfirmation(file); } } }); diff --git a/app/assets/javascripts/merge_conflicts/components/inline_conflict_lines.js.es6 b/app/assets/javascripts/merge_conflicts/components/inline_conflict_lines.js.es6 new file mode 100644 index 00000000000..b4be1c8988d --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/components/inline_conflict_lines.js.es6 @@ -0,0 +1,12 @@ +((global) => { + + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.inlineConflictLines = Vue.extend({ + props: { + file: Object + }, + mixins: [global.mergeConflicts.utils, global.mergeConflicts.actions], + }); + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/components/parallel_conflict_line.js.es6 b/app/assets/javascripts/merge_conflicts/components/parallel_conflict_line.js.es6 new file mode 100644 index 00000000000..8b0a8ab2073 --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/components/parallel_conflict_line.js.es6 @@ -0,0 +1,14 @@ +((global) => { + + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.parallelConflictLine = Vue.extend({ + props: { + file: Object, + line: Object + }, + mixins: [global.mergeConflicts.utils, global.mergeConflicts.actions], + template: '#parallel-conflict-line' + }); + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/components/parallel_conflict_lines.js.es6 b/app/assets/javascripts/merge_conflicts/components/parallel_conflict_lines.js.es6 new file mode 100644 index 00000000000..eb4cc6a9dac --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/components/parallel_conflict_lines.js.es6 @@ -0,0 +1,15 @@ +((global) => { + + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.parallelConflictLines = Vue.extend({ + props: { + file: Object + }, + mixins: [global.mergeConflicts.utils], + components: { + 'parallel-conflict-line': gl.mergeConflicts.parallelConflictLine + } + }); + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 index bdd6753f66a..83bcc3f51aa 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 @@ -196,7 +196,7 @@ isHead: true, isSelected: false, isUnselected: false - } + }; }, decorateLineForInlineView(line, id, conflict) { @@ -227,7 +227,7 @@ richText: rich_text, isSelected: false, isUnselected: false - } + }; }, getOriginHeaderLine(id) { @@ -241,7 +241,7 @@ isOrigin: true, isSelected: false, isUnselected: false - } + }; }, getFilePath(file) { diff --git a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 index 449b4d2209a..12688365f19 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 +++ b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 @@ -1,7 +1,12 @@ //= require vue //= require ./merge_conflict_store //= require ./merge_conflict_service +//= require ./mixins/line_conflict_utils +//= require ./mixins/line_conflict_actions //= require ./components/diff_file_editor +//= require ./components/inline_conflict_lines +//= require ./components/parallel_conflict_line +//= require ./components/parallel_conflict_lines $(() => { const INTERACTIVE_RESOLVE_MODE = 'interactive'; @@ -16,7 +21,9 @@ $(() => { el: '#conflicts', data: mergeConflictsStore.state, components: { - 'diff-file-editor': gl.mergeConflicts.diffFileEditor + 'diff-file-editor': gl.mergeConflicts.diffFileEditor, + 'inline-conflict-lines': gl.mergeConflicts.inlineConflictLines, + 'parallel-conflict-lines': gl.mergeConflicts.parallelConflictLines }, computed: { conflictsCountText() { return mergeConflictsStore.getConflictsCountText() }, @@ -46,9 +53,6 @@ $(() => { }); }, methods: { - handleSelected(file, sectionId, selection) { - mergeConflictsStore.handleSelected(file, sectionId, selection); - }, handleViewTypeChange(viewType) { mergeConflictsStore.setViewType(viewType); }, diff --git a/app/assets/javascripts/merge_conflicts/mixins/line_conflict_actions.js.es6 b/app/assets/javascripts/merge_conflicts/mixins/line_conflict_actions.js.es6 new file mode 100644 index 00000000000..114a2c5b305 --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/mixins/line_conflict_actions.js.es6 @@ -0,0 +1,12 @@ +((global) => { + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.actions = { + methods: { + handleSelected(file, sectionId, selection) { + gl.mergeConflicts.mergeConflictsStore.handleSelected(file, sectionId, selection); + } + } + }; + +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_conflicts/mixins/line_conflict_utils.js.es6 b/app/assets/javascripts/merge_conflicts/mixins/line_conflict_utils.js.es6 new file mode 100644 index 00000000000..b846a90ab2a --- /dev/null +++ b/app/assets/javascripts/merge_conflicts/mixins/line_conflict_utils.js.es6 @@ -0,0 +1,18 @@ +((global) => { + global.mergeConflicts = global.mergeConflicts || {}; + + global.mergeConflicts.utils = { + methods: { + lineCssClass(line) { + return { + 'head': line.isHead, + 'origin': line.isOrigin, + 'match': line.hasMatch, + 'selected': line.isSelected, + 'unselected': line.isUnselected + }; + } + } + }; + +})(window.gl || (window.gl = {})); diff --git a/app/views/projects/merge_requests/conflicts.html.haml b/app/views/projects/merge_requests/conflicts.html.haml index 5ff7befe0ca..d9f74d2cbfb 100644 --- a/app/views/projects/merge_requests/conflicts.html.haml +++ b/app/views/projects/merge_requests/conflicts.html.haml @@ -1,10 +1,3 @@ -- class_bindings = "{ | - 'head': line.isHead, | - 'origin': line.isOrigin, | - 'match': line.hasMatch, | - 'selected': line.isSelected, | - 'unselected': line.isUnselected }" - - page_title "Merge Conflicts", "#{@merge_request.title} (#{@merge_request.to_reference}", "Merge Requests" - content_for :page_specific_javascripts do = page_specific_javascript_tag('merge_conflicts/merge_conflicts_bundle.js') @@ -34,15 +27,14 @@ %strong {{file.filePath}} = render partial: 'projects/merge_requests/conflicts/file_actions' .diff-content.diff-wrap-lines - %div{"v-show" => "!isParallel && file.resolveMode === 'interactive' && file.type === 'text'" } - = render partial: "projects/merge_requests/conflicts/resolve_mode_interactive_inline", locals: { class_bindings: class_bindings } - %div{"v-show" => "isParallel && file.resolveMode === 'interactive' && file.type === 'text'" } - = render partial: "projects/merge_requests/conflicts/resolve_mode_interactive_parallel", locals: { class_bindings: class_bindings } - %div{"v-show" => " file.resolveMode === 'edit' || file.type === 'text-editor'"} - = render partial: 'projects/merge_requests/conflicts/diff_file_editor', locals: { if_condition: "file.loadFile && isParallel" } + .diff-wrap-lines.code.file-content.js-syntax-highlight{"v-show" => "!isParallel && file.resolveMode === 'interactive' && file.type === 'text'" } + = render partial: "projects/merge_requests/conflicts/components/inline_conflict_lines" + .diff-wrap-lines.code.file-content.js-syntax-highlight{"v-show" => "isParallel && file.resolveMode === 'interactive' && file.type === 'text'" } + = render partial: "projects/merge_requests/conflicts/components/parallel_conflict_lines" + %div{"v-show" => "file.resolveMode === 'edit' || file.type === 'text-editor'"} + = render partial: "projects/merge_requests/conflicts/components/diff_file_editor" = render partial: "projects/merge_requests/conflicts/submit_form" --# Components -= render partial: 'projects/merge_requests/conflicts/components/diff_file_editor' - \ No newline at end of file +-# Components += render partial: 'projects/merge_requests/conflicts/components/parallel_conflict_line' diff --git a/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml b/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml index 7222b8ae4d3..5ab3cd96163 100644 --- a/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml +++ b/app/views/projects/merge_requests/conflicts/_commit_stats.html.haml @@ -1,13 +1,9 @@ .content-block.oneline-block.files-changed{"v-if" => "!isLoading && !hasError"} .inline-parallel-buttons{"v-if" => "showDiffViewTypeSwitcher"} .btn-group - %a.btn{ | - ":class" => "{'active': !isParallel}", | - "@click" => "handleViewTypeChange('inline')"} + %button.btn{":class" => "{'active': !isParallel}", "@click" => "handleViewTypeChange('inline')"} Inline - %a.btn{ | - ":class" => "{'active': isParallel}", | - "@click" => "handleViewTypeChange('parallel')"} + %button.btn{":class" => "{'active': isParallel}", "@click" => "handleViewTypeChange('parallel')"} Side-by-side .js-toggle-container diff --git a/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml deleted file mode 100644 index 497db7e3f9b..00000000000 --- a/app/views/projects/merge_requests/conflicts/_diff_file_editor.html.haml +++ /dev/null @@ -1,8 +0,0 @@ -.diff-editor-wrap{ "v-show" => "file.showEditor" } - .discard-changes-alert-wrap{ "v-if" => "file.promptDiscardConfirmation" } - .discard-changes-alert - Are you sure to discard your changes? - .discard-actions - %button.btn.btn-sm.btn-close{ "@click" => "acceptDiscardConfirmation(file)" } Discard changes - %button.btn.btn-sm{ "@click" => "cancelDiscardConfirmation(file)" } Cancel - %diff-file-editor{":file" => "file"} diff --git a/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_inline.html.haml b/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_inline.html.haml deleted file mode 100644 index b1d7a90eb74..00000000000 --- a/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_inline.html.haml +++ /dev/null @@ -1,17 +0,0 @@ -.diff-wrap-lines.code.file-content.js-syntax-highlight - %table - %tr.line_holder.diff-inline{"v-for" => "line in file.inlineLines"} - %template{"v-if" => "!line.isHeader"} - %td.diff-line-num.new_line{":class" => class_bindings} - %a {{line.new_line}} - %td.diff-line-num.old_line{":class" => class_bindings} - %a {{line.old_line}} - %td.line_content{":class" => class_bindings} - {{{line.richText}}} - %template{"v-if" => "line.isHeader"} - %td.diff-line-num.header{":class" => class_bindings} - %td.diff-line-num.header{":class" => class_bindings} - %td.line_content.header{":class" => class_bindings} - %strong {{{line.richText}}} - %button.btn{ "@click" => "handleSelected(file, line.id, line.section)" } - {{line.buttonTitle}} diff --git a/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_parallel.html.haml b/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_parallel.html.haml deleted file mode 100644 index f5af347f59b..00000000000 --- a/app/views/projects/merge_requests/conflicts/_resolve_mode_interactive_parallel.html.haml +++ /dev/null @@ -1,17 +0,0 @@ -.diff-wrap-lines.code.file-content.js-syntax-highlight - %table - %tr.line_holder.parallel{"v-for" => "section in file.parallelLines"} - %template{"v-for" => "line in section"} - - %template{"v-if" => "line.isHeader"} - %td.diff-line-num.header{":class" => class_bindings} - %td.line_content.header{":class" => class_bindings} - %strong {{line.richText}} - %button.btn{"@click" => "handleSelected(file, line.id, line.section)"} - {{line.buttonTitle}} - - %template{"v-if" => "!line.isHeader"} - %td.diff-line-num.old_line{":class" => class_bindings} - {{line.lineNumber}} - %td.line_content.parallel{":class" => class_bindings} - {{{line.richText}}} diff --git a/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml index 9965bdf9028..943ae6ee129 100644 --- a/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml +++ b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml @@ -1,5 +1,11 @@ -%template{ id: "diff-file-editor" } - %div +%diff-file-editor{"inline-template" => "true", ":file" => "file", ":on-cancel-discard-confirmation" => "cancelDiscardConfirmation", ":on-accept-discard-confirmation" => "acceptDiscardConfirmation"} + .diff-editor-wrap{ "v-show" => "file.showEditor" } + .discard-changes-alert-wrap{ "v-if" => "file.promptDiscardConfirmation" } + .discard-changes-alert + Are you sure to discard your changes? + .discard-actions + %button.btn.btn-sm.btn-close{ "@click" => "acceptDiscardConfirmation(file)" } Discard changes + %button.btn.btn-sm{ "@click" => "cancelDiscardConfirmation(file)" } Cancel .editor-wrap{ ":class" => "classObject" } .loading %i.fa.fa-spinner.fa-spin diff --git a/app/views/projects/merge_requests/conflicts/components/_inline_conflict_lines.html.haml b/app/views/projects/merge_requests/conflicts/components/_inline_conflict_lines.html.haml new file mode 100644 index 00000000000..f094df7fcaa --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/components/_inline_conflict_lines.html.haml @@ -0,0 +1,15 @@ +%inline-conflict-lines{ "inline-template" => "true", ":file" => "file"} + %table + %tr.line_holder.diff-inline{"v-for" => "line in file.inlineLines"} + %td.diff-line-num.new_line{":class" => "lineCssClass(line)", "v-if" => "!line.isHeader"} + %a {{line.new_line}} + %td.diff-line-num.old_line{":class" => "lineCssClass(line)", "v-if" => "!line.isHeader"} + %a {{line.old_line}} + %td.line_content{":class" => "lineCssClass(line)", "v-if" => "!line.isHeader"} + {{{line.richText}}} + %td.diff-line-num.header{":class" => "lineCssClass(line)", "v-if" => "line.isHeader"} + %td.diff-line-num.header{":class" => "lineCssClass(line)", "v-if" => "line.isHeader"} + %td.line_content.header{":class" => "lineCssClass(line)", "v-if" => "line.isHeader"} + %strong {{{line.richText}}} + %button.btn{ "@click" => "handleSelected(file, line.id, line.section)" } + {{line.buttonTitle}} diff --git a/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_line.html.haml b/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_line.html.haml new file mode 100644 index 00000000000..5690bf7419c --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_line.html.haml @@ -0,0 +1,10 @@ +%script{"id" => 'parallel-conflict-line', "type" => "text/x-template"} + %td.diff-line-num.header{":class" => "lineCssClass(line)", "v-if" => "line.isHeader"} + %td.line_content.header{":class" => "lineCssClass(line)", "v-if" => "line.isHeader"} + %strong {{line.richText}} + %button.btn{"@click" => "handleSelected(file, line.id, line.section)"} + {{line.buttonTitle}} + %td.diff-line-num.old_line{":class" => "lineCssClass(line)", "v-if" => "!line.isHeader"} + {{line.lineNumber}} + %td.line_content.parallel{":class" => "lineCssClass(line)", "v-if" => "!line.isHeader"} + {{{line.richText}}} diff --git a/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_lines.html.haml b/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_lines.html.haml new file mode 100644 index 00000000000..a8ecdf59393 --- /dev/null +++ b/app/views/projects/merge_requests/conflicts/components/_parallel_conflict_lines.html.haml @@ -0,0 +1,4 @@ +%parallel-conflict-lines{"inline-template" => "true", ":file" => "file"} + %table + %tr.line_holder.parallel{"v-for" => "section in file.parallelLines"} + %td{"is"=>"parallel-conflict-line", "v-for" => "line in section", ":line" => "line", ":file" => "file"} -- cgit v1.2.1 From 54bfe70795e289b86485b2a57d72b6711e4994bd Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 5 Oct 2016 07:57:57 -0500 Subject: Add more tests to check conflicts resolution --- .../merge_conflict_data_provider.js.es6 | 2 +- .../merge_conflicts/merge_conflict_store.js.es6 | 4 +- .../projects/merge_requests_controller_spec.rb | 1 + spec/features/merge_requests/conflicts_spec.rb | 145 ++++++++++++++------- 4 files changed, 101 insertions(+), 51 deletions(-) diff --git a/app/assets/javascripts/merge_conflict_data_provider.js.es6 b/app/assets/javascripts/merge_conflict_data_provider.js.es6 index 6d1d3f36b33..5877d2f1896 100644 --- a/app/assets/javascripts/merge_conflict_data_provider.js.es6 +++ b/app/assets/javascripts/merge_conflict_data_provider.js.es6 @@ -290,7 +290,7 @@ class MergeConflictDataProvider { isReadyToCommit() { const vi = this.vueInstance; const files = this.vueInstance.conflictsData.files; - const hasCommitMessage = $.trim(this.vueInstance.conflictsData.commitMessage).length; + const hasCommitMessage = this.vueInstance.conflictsData.commitMessage.trim(); let unresolved = 0; for (let i = 0, l = files.length; i < l; i++) { diff --git a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 index 83bcc3f51aa..5c5c65f29d4 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 +++ b/app/assets/javascripts/merge_conflicts/merge_conflict_store.js.es6 @@ -321,7 +321,8 @@ let numberConflicts = 0; let resolvedConflicts = Object.keys(file.resolutionData).length - // We only check if + // We only check for conflicts type 'text' + // since conflicts `text_editor` can´t be resolved in interactive mode if (file.type === CONFLICT_TYPES.TEXT) { for (let j = 0, k = file.sections.length; j < k; j++) { if (file.sections[j].conflict) { @@ -334,6 +335,7 @@ } } } else if (file.resolveMode === EDIT_RESOLVE_MODE) { + // Unlikely to happen since switching to Edit mode saves content automatically. // Checking anyway in case the save strategy changes in the future if (!file.content) { diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 06b37aa4997..31f43bdc89a 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -659,6 +659,7 @@ describe Projects::MergeRequestsController do id: merge_request.iid expect(merge_request.reload.title).to eq(merge_request.wipless_title) + end end describe 'GET conflict_for_path' do diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb index 4cecc22aa6c..d258ff52bbb 100644 --- a/spec/features/merge_requests/conflicts_spec.rb +++ b/spec/features/merge_requests/conflicts_spec.rb @@ -12,74 +12,121 @@ feature 'Merge request conflict resolution', js: true, feature: true do end end - context 'when a merge request can be resolved in the UI' do - let(:merge_request) { create_merge_request('conflict-resolvable') } + shared_examples "conflicts are resolved in Interactive mode" do + it 'conflicts are resolved in Interactive mode' do + within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do + click_button 'Use ours' + end - before do - project.team << [user, :developer] - login_as(user) + within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do + all('button', text: 'Use ours').each do |button| + button.click + end + end - visit namespace_project_merge_request_path(project.namespace, project, merge_request) - end + click_button 'Commit conflict resolution' + wait_for_ajax - it 'shows a link to the conflict resolution page' do - expect(page).to have_link('conflicts', href: /\/conflicts\Z/) - end + expect(page).to have_content('All merge conflicts were resolved') + merge_request.reload_diff - context 'visiting the conflicts resolution page' do - before { click_link('conflicts', href: /\/conflicts\Z/) } + click_on 'Changes' + wait_for_ajax - it 'shows the conflicts' do - begin - expect(find('#conflicts')).to have_content('popen.rb') - rescue Capybara::Poltergeist::JavascriptError - retry - end + within find('.diff-file', text: 'files/ruby/popen.rb') do + expect(page).to have_selector('.line_content.new', text: "vars = { 'PWD' => path }") + expect(page).to have_selector('.line_content.new', text: "options = { chdir: path }") end - context 'when in inline mode' do - it 'resolves files manually' do - within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do - click_button 'Edit inline' - wait_for_ajax - execute_script('ace.edit($(".files-wrapper .diff-file pre")[0]).setValue("One morning");') - end - - within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do - click_button 'Edit inline' - wait_for_ajax - execute_script('ace.edit($(".files-wrapper .diff-file pre")[1]).setValue("Gregor Samsa woke from troubled dreams");') - end - - click_button 'Commit conflict resolution' - wait_for_ajax - expect(page).to have_content('All merge conflicts were resolved') - merge_request.reload_diff + within find('.diff-file', text: 'files/ruby/regex.rb') do + expect(page).to have_selector('.line_content.new', text: "def username_regexp") + expect(page).to have_selector('.line_content.new', text: "def project_name_regexp") + expect(page).to have_selector('.line_content.new', text: "def path_regexp") + expect(page).to have_selector('.line_content.new', text: "def archive_formats_regexp") + expect(page).to have_selector('.line_content.new', text: "def git_reference_regexp") + expect(page).to have_selector('.line_content.new', text: "def default_regexp") + end + end + end - click_on 'Changes' - wait_for_ajax + shared_examples "conflicts are resolved in Edit inline mode" do + it 'conflicts are resolved in Edit inline mode' do + expect(find('#conflicts')).to have_content('popen.rb') - expect(page).to have_content('One morning') - expect(page).to have_content('Gregor Samsa woke from troubled dreams') - end + within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do + click_button 'Edit inline' + wait_for_ajax + execute_script('ace.edit($(".files-wrapper .diff-file pre")[0]).setValue("One morning");') end + + within find('.files-wrapper .diff-file', text: 'files/ruby/regex.rb') do + click_button 'Edit inline' + wait_for_ajax + execute_script('ace.edit($(".files-wrapper .diff-file pre")[1]).setValue("Gregor Samsa woke from troubled dreams");') + end + + click_button 'Commit conflict resolution' + wait_for_ajax + expect(page).to have_content('All merge conflicts were resolved') + merge_request.reload_diff + + click_on 'Changes' + wait_for_ajax + + expect(page).to have_content('One morning') + expect(page).to have_content('Gregor Samsa woke from troubled dreams') end end - context 'when a merge request can be resolved in the UI' do - let(:merge_request) { create_merge_request('conflict-contains-conflict-markers') } - + context 'can be resolved in the UI' do before do project.team << [user, :developer] login_as(user) + end - visit namespace_project_merge_request_path(project.namespace, project, merge_request) + context 'the conflicts are resolvable' do + let(:merge_request) { create_merge_request('conflict-resolvable') } + + before { visit namespace_project_merge_request_path(project.namespace, project, merge_request) } + + it 'shows a link to the conflict resolution page' do + expect(page).to have_link('conflicts', href: /\/conflicts\Z/) + end + + context 'in Inline view mode' do + before { click_link('conflicts', href: /\/conflicts\Z/) } + + include_examples "conflicts are resolved in Interactive mode" + include_examples "conflicts are resolved in Edit inline mode" + end + + context 'in Parallel view mode' do + before do + click_link('conflicts', href: /\/conflicts\Z/) + click_button 'Side-by-side' + end + + include_examples "conflicts are resolved in Interactive mode" + include_examples "conflicts are resolved in Edit inline mode" + end end - context 'a conflict contain markers' do - before { click_link('conflicts', href: /\/conflicts\Z/) } + context 'the conflict contain markers' do + let(:merge_request) { create_merge_request('conflict-contains-conflict-markers') } + + before do + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + click_link('conflicts', href: /\/conflicts\Z/) + end + + it 'conflicts can not be resolved in Interactive mode' do + within find('.files-wrapper .diff-file', text: 'files/markdown/ruby-style-guide.md') do + expect(page).not_to have_content 'Interactive mode' + expect(page).not_to have_content 'Edit inline' + end + end - it 'resolves files manually' do + it 'conflicts are resolved in Edit inline mode' do within find('.files-wrapper .diff-file', text: 'files/markdown/ruby-style-guide.md') do wait_for_ajax execute_script('ace.edit($(".files-wrapper .diff-file pre")[0]).setValue("Gregor Samsa woke from troubled dreams");') @@ -94,7 +141,7 @@ feature 'Merge request conflict resolution', js: true, feature: true do click_on 'Changes' wait_for_ajax - find('.nothing-here-block', visible: true).click + find('.click-to-expand').click wait_for_ajax expect(page).to have_content('Gregor Samsa woke from troubled dreams') -- cgit v1.2.1 From cc874d9167b7ba5531aeade09f69ecdc1057a0ee Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 5 Oct 2016 18:05:16 -0500 Subject: Fix column limit on mysql --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8708eae1b2a..2c2edd8e9e0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -22,7 +22,7 @@ before_script: - bundle --version - '[ "$USE_BUNDLE_INSTALL" != "true" ] || retry bundle install --without postgres production --jobs $(nproc) "${FLAGS[@]}"' - retry gem install knapsack - - '[ "$SETUP_DB" != "true" ] || bundle exec rake db:drop db:create db:schema:load db:migrate' + - '[ "$SETUP_DB" != "true" ] || bundle exec rake db:drop db:create db:schema:load db:migrate add_limits_mysql' stages: - prepare -- cgit v1.2.1 From 8fd7a5942286d69eb34406e06ba5263c3944012e Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 5 Oct 2016 20:49:16 -0500 Subject: Remove unused files --- .../merge_conflict_data_provider.js.es6 | 438 --------------------- .../javascripts/merge_conflict_resolver.js.es6 | 112 ------ 2 files changed, 550 deletions(-) delete mode 100644 app/assets/javascripts/merge_conflict_data_provider.js.es6 delete mode 100644 app/assets/javascripts/merge_conflict_resolver.js.es6 diff --git a/app/assets/javascripts/merge_conflict_data_provider.js.es6 b/app/assets/javascripts/merge_conflict_data_provider.js.es6 deleted file mode 100644 index 5877d2f1896..00000000000 --- a/app/assets/javascripts/merge_conflict_data_provider.js.es6 +++ /dev/null @@ -1,438 +0,0 @@ -const HEAD_HEADER_TEXT = 'HEAD//our changes'; -const ORIGIN_HEADER_TEXT = 'origin//their changes'; -const HEAD_BUTTON_TITLE = 'Use ours'; -const ORIGIN_BUTTON_TITLE = 'Use theirs'; -const INTERACTIVE_RESOLVE_MODE = 'interactive'; -const EDIT_RESOLVE_MODE = 'edit'; -const DEFAULT_RESOLVE_MODE = INTERACTIVE_RESOLVE_MODE; - -class MergeConflictDataProvider { - - getInitialData() { - // TODO: remove reliance on jQuery and DOM state introspection - const diffViewType = $.cookie('diff_view'); - const fixedLayout = $('.content-wrapper .container-fluid').hasClass('container-limited'); - - return { - isLoading : true, - hasError : false, - isParallel : diffViewType === 'parallel', - diffViewType : diffViewType, - fixedLayout : fixedLayout, - isSubmitting : false, - conflictsData : {} - } - } - - - decorateData(vueInstance, data) { - this.vueInstance = vueInstance; - - if (data.type === 'error') { - vueInstance.hasError = true; - data.errorMessage = data.message; - } - else { - data.shortCommitSha = data.commit_sha.slice(0, 7); - data.commitMessage = data.commit_message; - - this.decorateFiles(data); - this.setParallelLines(data); - this.setInlineLines(data); - } - - vueInstance.conflictsData = data; - vueInstance.isSubmitting = false; - - const conflictsText = this.getConflictsCount() > 1 ? 'conflicts' : 'conflict'; - vueInstance.conflictsData.conflictsText = conflictsText; - } - - decorateFiles(data) { - data.files.forEach((file) => { - file.content = ''; - file.resolutionData = {}; - file.promptDiscardConfirmation = false; - file.resolveMode = DEFAULT_RESOLVE_MODE; - }); - } - - - setParallelLines(data) { - data.files.forEach( (file) => { - file.filePath = this.getFilePath(file); - file.iconClass = `fa-${file.blob_icon}`; - file.blobPath = file.blob_path; - file.parallelLines = []; - const linesObj = { left: [], right: [] }; - - file.sections.forEach( (section) => { - const { conflict, lines, id } = section; - - if (conflict) { - linesObj.left.push(this.getOriginHeaderLine(id)); - linesObj.right.push(this.getHeadHeaderLine(id)); - } - - lines.forEach( (line) => { - const { type } = line; - - if (conflict) { - if (type === 'old') { - linesObj.left.push(this.getLineForParallelView(line, id, 'conflict')); - } - else if (type === 'new') { - linesObj.right.push(this.getLineForParallelView(line, id, 'conflict', true)); - } - } - else { - const lineType = type || 'context'; - - linesObj.left.push (this.getLineForParallelView(line, id, lineType)); - linesObj.right.push(this.getLineForParallelView(line, id, lineType, true)); - } - }); - - this.checkLineLengths(linesObj); - }); - - for (let i = 0, len = linesObj.left.length; i < len; i++) { - file.parallelLines.push([ - linesObj.right[i], - linesObj.left[i] - ]); - } - - }); - } - - - checkLineLengths(linesObj) { - let { left, right } = linesObj; - - if (left.length !== right.length) { - if (left.length > right.length) { - const diff = left.length - right.length; - for (let i = 0; i < diff; i++) { - right.push({ lineType: 'emptyLine', richText: '' }); - } - } - else { - const diff = right.length - left.length; - for (let i = 0; i < diff; i++) { - left.push({ lineType: 'emptyLine', richText: '' }); - } - } - } - } - - - setInlineLines(data) { - data.files.forEach( (file) => { - file.iconClass = `fa-${file.blob_icon}`; - file.blobPath = file.blob_path; - file.filePath = this.getFilePath(file); - file.inlineLines = [] - - file.sections.forEach( (section) => { - let currentLineType = 'new'; - const { conflict, lines, id } = section; - - if (conflict) { - file.inlineLines.push(this.getHeadHeaderLine(id)); - } - - lines.forEach( (line) => { - const { type } = line; - - if ((type === 'new' || type === 'old') && currentLineType !== type) { - currentLineType = type; - file.inlineLines.push({ lineType: 'emptyLine', richText: '' }); - } - - this.decorateLineForInlineView(line, id, conflict); - file.inlineLines.push(line); - }) - - if (conflict) { - file.inlineLines.push(this.getOriginHeaderLine(id)); - } - }); - }); - } - - - handleSelected(file, sectionId, selection) { - const vi = this.vueInstance; - let files = vi.conflictsData.files; - - vi.$set(`conflictsData.files[${files.indexOf(file)}].resolutionData['${sectionId}']`, selection); - - - files.forEach( (file) => { - file.inlineLines.forEach( (line) => { - if (line.id === sectionId && (line.hasConflict || line.isHeader)) { - this.markLine(line, selection); - } - }); - - file.parallelLines.forEach( (lines) => { - const left = lines[0]; - const right = lines[1]; - const hasSameId = right.id === sectionId || left.id === sectionId; - const isLeftMatch = left.hasConflict || left.isHeader; - const isRightMatch = right.hasConflict || right.isHeader; - - if (hasSameId && (isLeftMatch || isRightMatch)) { - this.markLine(left, selection); - this.markLine(right, selection); - } - }) - }); - } - - - updateViewType(newType) { - const vi = this.vueInstance; - - if (newType === vi.diffViewType || !(newType === 'parallel' || newType === 'inline')) { - return; - } - - vi.diffViewType = newType; - vi.isParallel = newType === 'parallel'; - $.cookie('diff_view', newType, { - path: (gon && gon.relative_url_root) || '/' - }); - $('.content-wrapper .container-fluid') - .toggleClass('container-limited', !vi.isParallel && vi.fixedLayout); - } - - setFileResolveMode(file, mode) { - const vi = this.vueInstance; - - // Restore Interactive mode when switching to Edit mode - if (mode === EDIT_RESOLVE_MODE) { - file.resolutionData = {}; - - this.restoreFileLinesState(file); - } - - file.resolveMode = mode; - } - - - restoreFileLinesState(file) { - file.inlineLines.forEach((line) => { - if (line.hasConflict || line.isHeader) { - line.isSelected = false; - line.isUnselected = false; - } - }); - - file.parallelLines.forEach((lines) => { - const left = lines[0]; - const right = lines[1]; - const isLeftMatch = left.hasConflict || left.isHeader; - const isRightMatch = right.hasConflict || right.isHeader; - - if (isLeftMatch || isRightMatch) { - left.isSelected = false; - left.isUnselected = false; - right.isSelected = false; - right.isUnselected = false; - } - }); - } - - - setPromptConfirmationState(file, state) { - file.promptDiscardConfirmation = state; - } - - - markLine(line, selection) { - if (selection === 'head' && line.isHead) { - line.isSelected = true; - line.isUnselected = false; - } - else if (selection === 'origin' && line.isOrigin) { - line.isSelected = true; - line.isUnselected = false; - } - else { - line.isSelected = false; - line.isUnselected = true; - } - } - - - getConflictsCount() { - if (!this.vueInstance.conflictsData.files) { - return 0; - } - - const files = this.vueInstance.conflictsData.files; - let count = 0; - - files.forEach((file) => { - file.sections.forEach((section) => { - if (section.conflict) { - count++; - } - }); - }); - - return count; - } - - - isReadyToCommit() { - const vi = this.vueInstance; - const files = this.vueInstance.conflictsData.files; - const hasCommitMessage = this.vueInstance.conflictsData.commitMessage.trim(); - let unresolved = 0; - - for (let i = 0, l = files.length; i < l; i++) { - let file = files[i]; - - if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { - let numberConflicts = 0; - let resolvedConflicts = Object.keys(file.resolutionData).length - - for (let j = 0, k = file.sections.length; j < k; j++) { - if (file.sections[j].conflict) { - numberConflicts++; - } - } - - if (resolvedConflicts !== numberConflicts) { - unresolved++; - } - } else if (file.resolveMode === EDIT_RESOLVE_MODE) { - // Unlikely to happen since switching to Edit mode saves content automatically. - // Checking anyway in case the save strategy changes in the future - if (!file.content) { - unresolved++; - continue; - } - } - } - - return !vi.isSubmitting && hasCommitMessage && !unresolved; - } - - - getCommitButtonText() { - const initial = 'Commit conflict resolution'; - const inProgress = 'Committing...'; - const vue = this.vueInstance; - - return vue ? vue.isSubmitting ? inProgress : initial : initial; - } - - - decorateLineForInlineView(line, id, conflict) { - const { type } = line; - line.id = id; - line.hasConflict = conflict; - line.isHead = type === 'new'; - line.isOrigin = type === 'old'; - line.hasMatch = type === 'match'; - line.richText = line.rich_text; - line.isSelected = false; - line.isUnselected = false; - } - - getLineForParallelView(line, id, lineType, isHead) { - const { old_line, new_line, rich_text } = line; - const hasConflict = lineType === 'conflict'; - - return { - id, - lineType, - hasConflict, - isHead : hasConflict && isHead, - isOrigin : hasConflict && !isHead, - hasMatch : lineType === 'match', - lineNumber : isHead ? new_line : old_line, - section : isHead ? 'head' : 'origin', - richText : rich_text, - isSelected : false, - isUnselected : false - } - } - - - getHeadHeaderLine(id) { - return { - id : id, - richText : HEAD_HEADER_TEXT, - buttonTitle : HEAD_BUTTON_TITLE, - type : 'new', - section : 'head', - isHeader : true, - isHead : true, - isSelected : false, - isUnselected: false - } - } - - - getOriginHeaderLine(id) { - return { - id : id, - richText : ORIGIN_HEADER_TEXT, - buttonTitle : ORIGIN_BUTTON_TITLE, - type : 'old', - section : 'origin', - isHeader : true, - isOrigin : true, - isSelected : false, - isUnselected: false - } - } - - - handleFailedRequest(vueInstance, data) { - vueInstance.hasError = true; - vueInstance.conflictsData.errorMessage = 'Something went wrong!'; - } - - - getCommitData() { - let conflictsData = this.vueInstance.conflictsData; - let commitData = {}; - - commitData = { - commitMessage: conflictsData.commitMessage, - files: [] - }; - - conflictsData.files.forEach((file) => { - let addFile; - - addFile = { - old_path: file.old_path, - new_path: file.new_path - }; - - // Submit only one data for type of editing - if (file.resolveMode === INTERACTIVE_RESOLVE_MODE) { - addFile.sections = file.resolutionData; - } else if (file.resolveMode === EDIT_RESOLVE_MODE) { - addFile.content = file.content; - } - - commitData.files.push(addFile); - }); - - return commitData; - } - - - getFilePath(file) { - const { old_path, new_path } = file; - return old_path === new_path ? new_path : `${old_path} → ${new_path}`; - } -} diff --git a/app/assets/javascripts/merge_conflict_resolver.js.es6 b/app/assets/javascripts/merge_conflict_resolver.js.es6 deleted file mode 100644 index c317a6521b5..00000000000 --- a/app/assets/javascripts/merge_conflict_resolver.js.es6 +++ /dev/null @@ -1,112 +0,0 @@ -//= require vue -//= require ./merge_conflicts/components/diff_file_editor - -const INTERACTIVE_RESOLVE_MODE = 'interactive'; -const EDIT_RESOLVE_MODE = 'edit'; - -class MergeConflictResolver { - - constructor() { - this.dataProvider = new MergeConflictDataProvider(); - this.initVue(); - } - - initVue() { - const that = this; - this.vue = new Vue({ - el : '#conflicts', - name : 'MergeConflictResolver', - data : this.dataProvider.getInitialData(), - created : this.fetchData(), - computed : this.setComputedProperties(), - methods : { - handleSelected(file, sectionId, selection) { - that.dataProvider.handleSelected(file, sectionId, selection); - }, - handleViewTypeChange(newType) { - that.dataProvider.updateViewType(newType); - }, - commit() { - that.commit(); - }, - onClickResolveModeButton(file, mode) { - that.toggleResolveMode(file, mode); - }, - acceptDiscardConfirmation(file) { - that.dataProvider.setPromptConfirmationState(file, false); - that.dataProvider.setFileResolveMode(file, INTERACTIVE_RESOLVE_MODE); - }, - cancelDiscardConfirmation(file) { - that.dataProvider.setPromptConfirmationState(file, false); - }, - }, - components: { - 'diff-file-editor': window.gl.diffFileEditor - } - }) - } - - - setComputedProperties() { - const dp = this.dataProvider; - - return { - conflictsCount() { return dp.getConflictsCount() }, - readyToCommit() { return dp.isReadyToCommit() }, - commitButtonText() { return dp.getCommitButtonText() } - } - } - - - fetchData() { - const dp = this.dataProvider; - - $.get($('#conflicts').data('conflictsPath')) - .done((data) => { - dp.decorateData(this.vue, data); - }) - .error((data) => { - dp.handleFailedRequest(this.vue, data); - }) - .always(() => { - this.vue.isLoading = false; - - this.vue.$nextTick(() => { - $('#conflicts .js-syntax-highlight').syntaxHighlight(); - }); - - $('.content-wrapper .container-fluid') - .toggleClass('container-limited', !this.vue.isParallel && this.vue.fixedLayout); - }) - } - - - commit() { - this.vue.isSubmitting = true; - - $.ajax({ - url: $('#conflicts').data('resolveConflictsPath'), - data: JSON.stringify(this.dataProvider.getCommitData()), - contentType: "application/json", - dataType: 'json', - method: 'POST' - }) - .done((data) => { - window.location.href = data.redirect_to; - }) - .error(() => { - this.vue.isSubmitting = false; - new Flash('Something went wrong!'); - }); - } - - - toggleResolveMode(file, mode) { - if (mode === INTERACTIVE_RESOLVE_MODE && file.resolveEditChanged) { - this.dataProvider.setPromptConfirmationState(file, true); - return; - } - - this.dataProvider.setFileResolveMode(file, mode); - } -} -- cgit v1.2.1 From 6141816c233d87888d1450d3bc48a28fc6044ec1 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 5 Oct 2016 20:49:43 -0500 Subject: Use plain JS to get elements and data and values --- .../javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 index 12688365f19..7fd3749b3e2 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 +++ b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js.es6 @@ -10,14 +10,14 @@ $(() => { const INTERACTIVE_RESOLVE_MODE = 'interactive'; - const $conflicts = $(document.getElementById('conflicts')); + const conflictsEl = document.querySelector('#conflicts'); const mergeConflictsStore = gl.mergeConflicts.mergeConflictsStore; const mergeConflictsService = new gl.mergeConflicts.mergeConflictsService({ - conflictsPath: $conflicts.data('conflictsPath'), - resolveConflictsPath: $conflicts.data('resolveConflictsPath') + conflictsPath: conflictsEl.dataset.conflictsPath, + resolveConflictsPath: conflictsEl.dataset.resolveConflictsPath }); - gl.MergeConflictsResolverApp = new Vue({ + gl.MergeConflictsResolverApp = new Vue({ el: '#conflicts', data: mergeConflictsStore.state, components: { @@ -48,7 +48,7 @@ $(() => { mergeConflictsStore.setLoadingState(false); this.$nextTick(() => { - $conflicts.find('.js-syntax-highlight').syntaxHighlight(); + $(conflictsEl.querySelectorAll('.js-syntax-highlight')).syntaxHighlight(); }); }); }, -- cgit v1.2.1 From edd30976bd8069a02e0674ee36d1455c06ea751f Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 12 Oct 2016 18:53:29 -0500 Subject: Update CHANGELOG --- CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 99bbd99726d..f0b250f365c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,8 @@ v 8.13.0 (unreleased) - Update runner version only when updating contacted_at - Add link from system note to compare with previous version - Use gitlab-shell v3.6.6 + - Ability to resolve merge request conflicts with editor !6374 + - Use gitlab-shell v3.6.2 (GIT TRACE logging) - Add `/projects/visible` API endpoint (Ben Boeckel) - Fix centering of custom header logos (Ashley Dumaine) - ExpireBuildArtifactsWorker query builds table without ordering enqueuing one job per build to cleanup -- cgit v1.2.1 From cdd0dd507d1d5a226f1f438c4bfa602a115caac3 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 12 Oct 2016 22:52:44 -0500 Subject: Fix discard message --- .../merge_requests/conflicts/components/_diff_file_editor.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml index 943ae6ee129..3c927d362c2 100644 --- a/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml +++ b/app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml @@ -2,7 +2,7 @@ .diff-editor-wrap{ "v-show" => "file.showEditor" } .discard-changes-alert-wrap{ "v-if" => "file.promptDiscardConfirmation" } .discard-changes-alert - Are you sure to discard your changes? + Are you sure you want to discard your changes? .discard-actions %button.btn.btn-sm.btn-close{ "@click" => "acceptDiscardConfirmation(file)" } Discard changes %button.btn.btn-sm{ "@click" => "cancelDiscardConfirmation(file)" } Cancel -- cgit v1.2.1 From 3764fd4b4166bdf869cc04e4e82558d6b80b4ca5 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 13 Oct 2016 10:34:15 +0100 Subject: Add blob_ace_mode to conflict content response --- lib/gitlab/conflict/file.rb | 5 +++++ spec/controllers/projects/merge_requests_controller_spec.rb | 1 + spec/lib/gitlab/conflict/file_spec.rb | 11 +++++++++++ 3 files changed, 17 insertions(+) diff --git a/lib/gitlab/conflict/file.rb b/lib/gitlab/conflict/file.rb index 661e43d3fa9..c843315782d 100644 --- a/lib/gitlab/conflict/file.rb +++ b/lib/gitlab/conflict/file.rb @@ -25,6 +25,10 @@ module Gitlab merge_file_result[:data] end + def our_blob + @our_blob ||= repository.blob_at(merge_request.diff_refs.head_sha, our_path) + end + def type lines unless @type @@ -209,6 +213,7 @@ module Gitlab json_hash.tap do |json_hash| if opts[:full_content] json_hash[:content] = content + json_hash[:blob_ace_mode] = our_blob && our_blob.language.try(:ace_mode) else json_hash[:sections] = sections if type.text? json_hash[:type] = type diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 31f43bdc89a..3fe90375b92 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -712,6 +712,7 @@ describe Projects::MergeRequestsController do 'new_path' => path, 'blob_icon' => 'file-text-o', 'blob_path' => a_string_ending_with(path), + 'blob_ace_mode' => 'ruby', 'content' => content) end end diff --git a/spec/lib/gitlab/conflict/file_spec.rb b/spec/lib/gitlab/conflict/file_spec.rb index 60020487061..648d342ecf8 100644 --- a/spec/lib/gitlab/conflict/file_spec.rb +++ b/spec/lib/gitlab/conflict/file_spec.rb @@ -257,5 +257,16 @@ FILE it 'includes the blob icon for the file' do expect(conflict_file.as_json[:blob_icon]).to eq('file-text-o') end + + context 'with the full_content option passed' do + it 'includes the full content of the conflict' do + expect(conflict_file.as_json(full_content: true)).to have_key(:content) + end + + it 'includes the detected language of the conflict file' do + expect(conflict_file.as_json(full_content: true)[:blob_ace_mode]). + to eq('ruby') + end + end end end -- cgit v1.2.1 From f5de2ce3fd74af214556f677f54efa3f4da13ed3 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 13 Oct 2016 12:32:35 -0500 Subject: Add syntax highlighting to files --- .../javascripts/merge_conflicts/components/diff_file_editor.js.es6 | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 index 417095a8db7..3379414343f 100644 --- a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 +++ b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 @@ -55,6 +55,7 @@ this.fileLoaded = true; this.editor = ace.edit(content); this.editor.$blockScrolling = Infinity; // Turn off annoying warning + this.editor.getSession().setMode(`ace/mode/${file.blob_ace_mode}`); this.editor.on('change', () => { this.saveDiffResolution(); }); -- cgit v1.2.1 From 26e327ea93140f6400b965b845958ff50461718c Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 13 Oct 2016 14:17:28 -0500 Subject: Update CHANGELOG --- CHANGELOG | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index f0b250f365c..e9f84503b2b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,7 +9,6 @@ v 8.13.0 (unreleased) - Add link from system note to compare with previous version - Use gitlab-shell v3.6.6 - Ability to resolve merge request conflicts with editor !6374 - - Use gitlab-shell v3.6.2 (GIT TRACE logging) - Add `/projects/visible` API endpoint (Ben Boeckel) - Fix centering of custom header logos (Ashley Dumaine) - ExpireBuildArtifactsWorker query builds table without ordering enqueuing one job per build to cleanup -- cgit v1.2.1 From 5b5c7e048b6e975a655478b0e5d421c2438276aa Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 14 Oct 2016 16:58:55 -0700 Subject: Fix trending projects Spinach failure The trending projects list is now pre-calculated. To make this work with the Spinach test, we have to manually refresh the list. Partial fix to #23378 --- features/explore/projects.feature | 1 + features/steps/shared/project.rb | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/features/explore/projects.feature b/features/explore/projects.feature index 092e18d1b86..4e0f4486ab7 100644 --- a/features/explore/projects.feature +++ b/features/explore/projects.feature @@ -128,6 +128,7 @@ Feature: Explore Projects And project "Archive" has comments And I sign in as a user And project "Community" has comments + And trending projects are refreshed When I visit the explore trending projects Then I should see project "Community" And I should not see project "Internal" diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index afbd8ef1233..cab85a48396 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -218,6 +218,10 @@ module SharedProject 2.times { create(:note_on_issue, project: project) } end + step 'trending projects are refreshed' do + TrendingProject.refresh! + end + step 'project "Shop" has labels: "bug", "feature", "enhancement"' do project = Project.find_by(name: "Shop") create(:label, project: project, title: 'bug') -- cgit v1.2.1 From 7e3ff1852340c33f5b8853190d6a24813d5920a2 Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Sun, 11 Sep 2016 20:38:09 +0430 Subject: Add RTL support to markdown renderer --- CHANGELOG | 1 + app/assets/stylesheets/framework/typography.scss | 13 +++++++++++++ lib/banzai/filter/set_direction_filter.rb | 15 +++++++++++++++ lib/banzai/pipeline/gfm_pipeline.rb | 4 +++- spec/features/atom/users_spec.rb | 2 +- spec/lib/banzai/object_renderer_spec.rb | 6 +++--- spec/lib/banzai/pipeline/description_pipeline_spec.rb | 12 +++++++++--- spec/models/concerns/cache_markdown_field_spec.rb | 2 +- 8 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 lib/banzai/filter/set_direction_filter.rb diff --git a/CHANGELOG b/CHANGELOG index e3201cd2250..76b483d9019 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -55,6 +55,7 @@ v 8.13.0 (unreleased) - Added soft wrap button to repository file/blob editor - Update namespace validation to forbid reserved names (.git and .atom) (Will Starms) - Show the time ago a merge request was deployed to an environment + - Add RTL support to markdown renderer (Ebrahim Byagowi) - Add word-wrap to issue title on issue and milestone boards (ClemMakesApps) - Fix todos page mobile viewport layout (ClemMakesApps) - Fix inconsistent highlighting of already selected activity nav-links (ClemMakesApps) diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 8df0067fac1..287653beac5 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -90,6 +90,11 @@ border-left: 3px solid #e7e9ed; } + blockquote:dir(rtl) { + border-left: 0; + border-right: 3px solid #e7e9ed; + } + blockquote p { color: #7f8fa4 !important; font-size: inherit; @@ -112,6 +117,10 @@ } } + table:dir(rtl) th { + text-align: right; + } + pre { margin: 12px 0; font-size: 13px; @@ -129,6 +138,10 @@ margin: 3px 0 3px 28px !important; } + ul:dir(rtl), ol:dir(rtl) { + margin: 3px 28px 3px 0 !important; + } + li { line-height: 1.6em; } diff --git a/lib/banzai/filter/set_direction_filter.rb b/lib/banzai/filter/set_direction_filter.rb new file mode 100644 index 00000000000..c2976aeb7c6 --- /dev/null +++ b/lib/banzai/filter/set_direction_filter.rb @@ -0,0 +1,15 @@ +module Banzai + module Filter + # HTML filter that sets dir="auto" for RTL languages support + class SetDirectionFilter < HTML::Pipeline::Filter + def call + # select these elements just on top level of the document + doc.xpath('p|h1|h2|h3|h4|h5|h6|ol|ul[not(@class="section-nav")]|blockquote|table').each do |el| + el['dir'] = 'auto' + end + + doc + end + end + end +end diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index 8d94b199c66..5da2d0b008c 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -25,7 +25,9 @@ module Banzai Filter::MilestoneReferenceFilter, Filter::TaskListFilter, - Filter::InlineDiffFilter + Filter::InlineDiffFilter, + + Filter::SetDirectionFilter ] end diff --git a/spec/features/atom/users_spec.rb b/spec/features/atom/users_spec.rb index a8833194421..f8c3ccb416b 100644 --- a/spec/features/atom/users_spec.rb +++ b/spec/features/atom/users_spec.rb @@ -53,7 +53,7 @@ describe "User Feed", feature: true do end it 'has XHTML summaries in issue descriptions' do - expect(body).to match /we have a bug!<\/p>\n\n
\n\n

I guess/ + expect(body).to match /we have a bug!<\/p>\n\n

\n\n

I guess/ end it 'has XHTML summaries in notes' do diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb index 90da78a67dd..6bcda87c999 100644 --- a/spec/lib/banzai/object_renderer_spec.rb +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -24,7 +24,7 @@ describe Banzai::ObjectRenderer do with(an_instance_of(Array)). and_call_original - expect(object).to receive(:redacted_note_html=).with('

hello

') + expect(object).to receive(:redacted_note_html=).with('

hello

') expect(object).to receive(:user_visible_reference_count=).with(0) renderer.render([object], :note) @@ -92,10 +92,10 @@ describe Banzai::ObjectRenderer do docs = renderer.render_attributes(objects, :note) expect(docs[0]).to be_an_instance_of(Nokogiri::HTML::DocumentFragment) - expect(docs[0].to_html).to eq('

hello

') + expect(docs[0].to_html).to eq('

hello

') expect(docs[1]).to be_an_instance_of(Nokogiri::HTML::DocumentFragment) - expect(docs[1].to_html).to eq('

bye

') + expect(docs[1].to_html).to eq('

bye

') end it 'returns when no objects to render' do diff --git a/spec/lib/banzai/pipeline/description_pipeline_spec.rb b/spec/lib/banzai/pipeline/description_pipeline_spec.rb index 76f42071810..8cce1b96698 100644 --- a/spec/lib/banzai/pipeline/description_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/description_pipeline_spec.rb @@ -4,11 +4,11 @@ describe Banzai::Pipeline::DescriptionPipeline do def parse(html) # When we pass HTML to Redcarpet, it gets wrapped in `p` tags... # ...except when we pass it pre-wrapped text. Rabble rabble. - unwrap = !html.start_with?('

') + unwrap = !html.start_with?('

(.*)

(.*)\z}, '\1\2') if unwrap + output.gsub!(%r{\A

(.*)

(.*)\z}, '\1\2') if unwrap output end @@ -27,11 +27,17 @@ describe Banzai::Pipeline::DescriptionPipeline do end end - %w(b i strong em a ins del sup sub p).each do |elem| + %w(b i strong em a ins del sup sub).each do |elem| it "still allows '#{elem}' elements" do exp = act = "<#{elem}>Description" expect(parse(act).strip).to eq exp end end + + it "still allows 'p' elements" do + exp = act = "

Description

" + + expect(parse(act).strip).to eq exp + end end diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 15cd3a7ed70..2e3702f7520 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -64,7 +64,7 @@ describe CacheMarkdownField do let(:html) { "

Foo

" } let(:updated_markdown) { "`Bar`" } - let(:updated_html) { "

Bar

" } + let(:updated_html) { "

Bar

" } subject { ThingWithMarkdownFields.new(foo: markdown, foo_html: html) } -- cgit v1.2.1 From 9c6c5c79f8d3a555ded0539e06a922bc058d5c20 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 14 Oct 2016 19:08:48 +0200 Subject: Add Pipeline metrics worker --- CHANGELOG.md | 1 + app/models/ci/pipeline.rb | 10 ++++++---- app/workers/pipeline_metrics_worker.rb | 15 +++++++++++++++ 3 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 app/workers/pipeline_metrics_worker.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 46f718fc88a..06269b79f14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Change user & group landing page routing from /u/:username to /:username - Prevent running GfmAutocomplete setup for each diff note !6569 - Added documentation for .gitattributes files + - Move Pipeline Metrics to separate worker - AbstractReferenceFilter caches project_refs on RequestStore when active - Replaced the check sign to arrow in the show build view. !6501 - Add a /wip slash command to toggle the Work In Progress status of a merge request. !6259 (tbalthazar) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 4fdb5fef4fb..c7b9d6cc223 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -49,6 +49,10 @@ module Ci transition any => :canceled end + # IMPORTANT + # Do not add any operations to this state_machine + # Create a separate worker for each new operation + before_transition [:created, :pending] => :running do |pipeline| pipeline.started_at = Time.now end @@ -62,13 +66,11 @@ module Ci end after_transition [:created, :pending] => :running do |pipeline| - MergeRequest::Metrics.where(merge_request_id: pipeline.merge_requests.map(&:id)). - update_all(latest_build_started_at: pipeline.started_at, latest_build_finished_at: nil) + pipeline.run_after_commit { PipelineMetricsWorker.perform_async(id) } end after_transition any => [:success] do |pipeline| - MergeRequest::Metrics.where(merge_request_id: pipeline.merge_requests.map(&:id)). - update_all(latest_build_finished_at: pipeline.finished_at) + pipeline.run_after_commit { PipelineMetricsWorker.perform_async(id) } end after_transition [:created, :pending, :running] => :success do |pipeline| diff --git a/app/workers/pipeline_metrics_worker.rb b/app/workers/pipeline_metrics_worker.rb new file mode 100644 index 00000000000..3d1cd770515 --- /dev/null +++ b/app/workers/pipeline_metrics_worker.rb @@ -0,0 +1,15 @@ +class PipelineMetricsWorker + include Sidekiq::Worker + + sidekiq_options queue: :default + + def perform(pipeline_id) + Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline| + merge_requests = pipeline.merge_requests.map(&:id) + + metrics = MergeRequest::Metrics.where(merge_request_id: merge_requests) + metrics.update_all(latest_build_started_at: pipeline.started_at) if pipeline.active? + metrics.update_all(latest_build_finished_at: pipeline.finished_at) if pipeline.success? + end + end +end -- cgit v1.2.1 From a497803072edb4c8edbf9f4daf3832c122ee50e2 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 17 Oct 2016 09:55:47 +0200 Subject: Improve spec for pipeline metrics worker --- app/workers/pipeline_metrics_worker.rb | 25 ++++++++++++--- spec/models/ci/pipeline_spec.rb | 27 ++++++---------- spec/workers/pipeline_metrics_worker_spec.rb | 46 ++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 23 deletions(-) create mode 100644 spec/workers/pipeline_metrics_worker_spec.rb diff --git a/app/workers/pipeline_metrics_worker.rb b/app/workers/pipeline_metrics_worker.rb index 3d1cd770515..7bb92df3bbd 100644 --- a/app/workers/pipeline_metrics_worker.rb +++ b/app/workers/pipeline_metrics_worker.rb @@ -5,11 +5,26 @@ class PipelineMetricsWorker def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline| - merge_requests = pipeline.merge_requests.map(&:id) - - metrics = MergeRequest::Metrics.where(merge_request_id: merge_requests) - metrics.update_all(latest_build_started_at: pipeline.started_at) if pipeline.active? - metrics.update_all(latest_build_finished_at: pipeline.finished_at) if pipeline.success? + update_metrics_for_active_pipeline(pipeline) if pipeline.active? + update_metrics_for_succeeded_pipeline(pipeline) if pipeline.success? end end + + private + + def update_metrics_for_active_pipeline(pipeline) + metrics(pipeline).update_all(latest_build_started_at: pipeline.started_at, latest_build_finished_at: nil) + end + + def update_metrics_for_succeeded_pipeline(pipeline) + metrics(pipeline).update_all(latest_build_started_at: pipeline.started_at, latest_build_finished_at: pipeline.finished_at) + end + + def metrics(pipeline) + MergeRequest::Metrics.where(merge_request_id: merge_requests(pipeline)) + end + + def merge_requests(pipeline) + pipeline.merge_requests.map(&:id) + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 550a890797e..163c0b5c516 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -187,33 +187,24 @@ describe Ci::Pipeline, models: true do end end - describe "merge request metrics" do + describe 'merge request metrics' do let(:project) { FactoryGirl.create :project } let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) } - context 'when transitioning to running' do - it 'records the build start time' do - time = Time.now - Timecop.freeze(time) { build.run } - - expect(merge_request.reload.metrics.latest_build_started_at).to be_within(1.second).of(time) - end - - it 'clears the build end time' do - build.run + before do + expect(PipelineMetricsWorker).to receive(:perform_async).with(pipeline.id) + end - expect(merge_request.reload.metrics.latest_build_finished_at).to be_nil + context 'when transitioning to running' do + it 'schedules metrics workers' do + pipeline.run end end context 'when transitioning to success' do - it 'records the build end time' do - build.run - time = Time.now - Timecop.freeze(time) { build.success } - - expect(merge_request.reload.metrics.latest_build_finished_at).to be_within(1.second).of(time) + it 'schedules metrics workers' do + pipeline.succeed end end end diff --git a/spec/workers/pipeline_metrics_worker_spec.rb b/spec/workers/pipeline_metrics_worker_spec.rb new file mode 100644 index 00000000000..f58df3f0d6e --- /dev/null +++ b/spec/workers/pipeline_metrics_worker_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe PipelineMetricsWorker do + let(:project) { create(:project) } + let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) } + + let(:pipeline) do + create(:ci_empty_pipeline, + status: status, + project: project, + ref: 'master', + sha: project.repository.commit('master').id, + started_at: 1.hour.ago, + finished_at: Time.now) + end + + describe '#perform' do + subject { described_class.new.perform(pipeline.id) } + + context 'when pipeline is running' do + let(:status) { 'running' } + + it 'records the build start time' do + subject + + expect(merge_request.reload.metrics.latest_build_started_at).to eq(pipeline.started_at) + end + + it 'clears the build end time' do + subject + + expect(merge_request.reload.metrics.latest_build_finished_at).to be_nil + end + end + + context 'when pipeline succeeded' do + let(:status) { 'success' } + + it 'records the build end time' do + subject + + expect(merge_request.reload.metrics.latest_build_finished_at).to eq(pipeline.finished_at) + end + end + end +end -- cgit v1.2.1 From 89bb889c4541a477aff4202fd6dad23ad3fe38d6 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Tue, 11 Oct 2016 10:18:36 +0200 Subject: Stop directly parsing due_date with Date.parse, prefer parsing implicitly. --- app/assets/javascripts/due_date_select.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/due_date_select.js b/app/assets/javascripts/due_date_select.js index bf68b7e3a9b..0fcf0823d4e 100644 --- a/app/assets/javascripts/due_date_select.js +++ b/app/assets/javascripts/due_date_select.js @@ -43,7 +43,7 @@ // Create the post date value = $("input[name='" + fieldName + "']").val(); if (value !== '') { - date = new Date(value.replace(new RegExp('-', 'g'), ',')); + date = new Date(value); mediumDate = $.datepicker.formatDate('M d, yy', date); } else { mediumDate = 'No due date'; @@ -64,7 +64,7 @@ $selectbox.hide(); } $value.css('display', ''); - cssClass = Date.parse(mediumDate) ? 'bold' : 'no-value'; + cssClass = new Date(mediumDate) ? 'bold' : 'no-value'; $valueContent.html("" + mediumDate + ""); $sidebarValue.html(mediumDate); if (value !== '') { -- cgit v1.2.1 From 039ccc169a4114a53acb16c0b031f704748c3cae Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Tue, 11 Oct 2016 10:19:18 +0200 Subject: Convert due_date_select.js filetype to es6. --- CHANGELOG.md | 1 + app/assets/javascripts/dispatcher.js.es6 | 2 +- app/assets/javascripts/due_date_select.js | 107 ----------------- app/assets/javascripts/due_date_select.js.es6 | 161 ++++++++++++++++++++++++++ app/views/shared/issuable/_sidebar.html.haml | 2 +- 5 files changed, 164 insertions(+), 109 deletions(-) delete mode 100644 app/assets/javascripts/due_date_select.js create mode 100644 app/assets/javascripts/due_date_select.js.es6 diff --git a/CHANGELOG.md b/CHANGELOG.md index 46f718fc88a..671bb01c003 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -116,6 +116,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Grouped pipeline dropdown is a scrollable container - Cleanup Ci::ApplicationController. !6757 (Takuya Noguchi) - Fixes padding in all clipboard icons that have .btn class + - Fix due date being displayed as NaN in Safari - Fix a typo in doc/api/labels.md - API: all unknown routing will be handled with 404 Not Found - Add docs for request profiling diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index f3957ed374b..5be35cf4b41 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/app/assets/javascripts/dispatcher.js.es6 @@ -50,7 +50,7 @@ case 'projects:milestones:new': case 'projects:milestones:edit': new ZenMode(); - new DueDateSelect(); + new gl.DueDateSelectors(); new GLForm($('.milestone-form')); break; case 'groups:milestones:new': diff --git a/app/assets/javascripts/due_date_select.js b/app/assets/javascripts/due_date_select.js deleted file mode 100644 index 0fcf0823d4e..00000000000 --- a/app/assets/javascripts/due_date_select.js +++ /dev/null @@ -1,107 +0,0 @@ -(function() { - this.DueDateSelect = (function() { - function DueDateSelect() { - var $datePicker, $dueDate, $loading; - // Milestone edit/new form - $datePicker = $('.datepicker'); - if ($datePicker.length) { - $dueDate = $('#milestone_due_date'); - $datePicker.datepicker({ - dateFormat: 'yy-mm-dd', - onSelect: function(dateText, inst) { - return $dueDate.val(dateText); - } - }).datepicker('setDate', $.datepicker.parseDate('yy-mm-dd', $dueDate.val())); - } - $('.js-clear-due-date').on('click', function(e) { - e.preventDefault(); - return $.datepicker._clearDate($datePicker); - }); - // Issuable sidebar - $loading = $('.js-issuable-update .due_date').find('.block-loading').hide(); - $('.js-due-date-select').each(function(i, dropdown) { - var $block, $dropdown, $dropdownParent, $selectbox, $sidebarValue, $value, $valueContent, abilityName, addDueDate, fieldName, issueUpdateURL; - $dropdown = $(dropdown); - $dropdownParent = $dropdown.closest('.dropdown'); - $datePicker = $dropdownParent.find('.js-due-date-calendar'); - $block = $dropdown.closest('.block'); - $selectbox = $dropdown.closest('.selectbox'); - $value = $block.find('.value'); - $valueContent = $block.find('.value-content'); - $sidebarValue = $('.js-due-date-sidebar-value', $block); - fieldName = $dropdown.data('field-name'); - abilityName = $dropdown.data('ability-name'); - issueUpdateURL = $dropdown.data('issue-update'); - $dropdown.glDropdown({ - hidden: function() { - $selectbox.hide(); - return $value.css('display', ''); - } - }); - addDueDate = function(isDropdown) { - var data, date, mediumDate, value; - // Create the post date - value = $("input[name='" + fieldName + "']").val(); - if (value !== '') { - date = new Date(value); - mediumDate = $.datepicker.formatDate('M d, yy', date); - } else { - mediumDate = 'No due date'; - } - data = {}; - data[abilityName] = {}; - data[abilityName].due_date = value; - return $.ajax({ - type: 'PUT', - url: issueUpdateURL, - data: data, - dataType: 'json', - beforeSend: function() { - var cssClass; - $loading.fadeIn(); - if (isDropdown) { - $dropdown.trigger('loading.gl.dropdown'); - $selectbox.hide(); - } - $value.css('display', ''); - cssClass = new Date(mediumDate) ? 'bold' : 'no-value'; - $valueContent.html("" + mediumDate + ""); - $sidebarValue.html(mediumDate); - if (value !== '') { - return $('.js-remove-due-date-holder').removeClass('hidden'); - } else { - return $('.js-remove-due-date-holder').addClass('hidden'); - } - } - }).done(function(data) { - if (isDropdown) { - $dropdown.trigger('loaded.gl.dropdown'); - $dropdown.dropdown('toggle'); - } - return $loading.fadeOut(); - }); - }; - $block.on('click', '.js-remove-due-date', function(e) { - e.preventDefault(); - $("input[name='" + fieldName + "']").val(''); - return addDueDate(false); - }); - return $datePicker.datepicker({ - dateFormat: 'yy-mm-dd', - defaultDate: $("input[name='" + fieldName + "']").val(), - altField: "input[name='" + fieldName + "']", - onSelect: function() { - return addDueDate(true); - } - }); - }); - $(document).off('click', '.ui-datepicker-header a').on('click', '.ui-datepicker-header a', function(e) { - return e.stopImmediatePropagation(); - }); - } - - return DueDateSelect; - - })(); - -}).call(this); diff --git a/app/assets/javascripts/due_date_select.js.es6 b/app/assets/javascripts/due_date_select.js.es6 new file mode 100644 index 00000000000..41925fcc8e3 --- /dev/null +++ b/app/assets/javascripts/due_date_select.js.es6 @@ -0,0 +1,161 @@ +(function(global) { + class DueDateSelect { + constructor({ $dropdown, $loading } = {}) { + const $dropdownParent = $dropdown.closest('.dropdown'); + const $block = $dropdown.closest('.block'); + this.$loading = $loading; + this.$dropdown = $dropdown; + this.$dropdownParent = $dropdownParent; + this.$datePicker = $dropdownParent.find('.js-due-date-calendar'); + this.$block = $block; + this.$selectbox = $dropdown.closest('.selectbox'); + this.$value = $block.find('.value'); + this.$valueContent = $block.find('.value-content'); + this.$sidebarValue = $('.js-due-date-sidebar-value', $block); + this.fieldName = $dropdown.data('field-name'), + this.abilityName = $dropdown.data('ability-name'), + this.issueUpdateURL = $dropdown.data('issue-update') + + this.rawSelectedDate = null; + this.displayedDate = null; + this.datePayload = null; + + this.initGlDropdown(); + this.initRemoveDueDate(); + this.initDatePicker(); + this.initStopPropagation(); + } + + initGlDropdown() { + this.$dropdown.glDropdown({ + hidden: () => { + this.$selectbox.hide(); + this.$value.css('display', ''); + } + }); + } + + initDatePicker() { + this.$datePicker.datepicker({ + dateFormat: 'yy-mm-dd', + defaultDate: $("input[name='" + this.fieldName + "']").val(), + altField: "input[name='" + this.fieldName + "']", + onSelect: () => { + return this.saveDueDate(true); + } + }); + } + + initRemoveDueDate() { + this.$block.on('click', '.js-remove-due-date', (e) => { + e.preventDefault(); + $("input[name='" + this.fieldName + "']").val(''); + return this.saveDueDate(false); + }); + } + + initStopPropagation() { + $(document).off('click', '.ui-datepicker-header a').on('click', '.ui-datepicker-header a', (e) => { + return e.stopImmediatePropagation(); + }); + } + + saveDueDate(isDropdown) { + this.parseSelectedDate(); + this.prepSelectedDate(); + this.submitSelectedDate(isDropdown); + } + + parseSelectedDate() { + this.rawSelectedDate = $("input[name='" + this.fieldName + "']").val(); + if (this.rawSelectedDate.length) { + let dateObj = new Date(this.rawSelectedDate); + this.displayedDate = $.datepicker.formatDate('M d, yy', dateObj); + } else { + this.displayedDate = 'No due date'; + } + } + + prepSelectedDate() { + const datePayload = {}; + datePayload[this.abilityName] = {}; + datePayload[this.abilityName].due_date = this.rawSelectedDate; + this.datePayload = datePayload; + } + + submitSelectedDate(isDropdown) { + return $.ajax({ + type: 'PUT', + url: this.issueUpdateURL, + data: this.datePayload, + dataType: 'json', + beforeSend: () => { + const selectedDateValue = this.datePayload[this.abilityName].due_date; + const displayedDateStyle = this.displayedDate !== 'No due date' ? 'bold' : 'no-value'; + + this.$loading.fadeIn(); + + if (isDropdown) { + this.$dropdown.trigger('loading.gl.dropdown'); + this.$selectbox.hide(); + } + + this.$value.css('display', ''); + this.$valueContent.html(`${this.displayedDate}`); + this.$sidebarValue.html(this.displayedDate); + + return selectedDateValue.length ? + $('.js-remove-due-date-holder').removeClass('hidden') : + $('.js-remove-due-date-holder').addClass('hidden'); + + } + }).done((data) => { + if (isDropdown) { + this.$dropdown.trigger('loaded.gl.dropdown'); + this.$dropdown.dropdown('toggle'); + } + return this.$loading.fadeOut(); + }); + } + } + + class DueDateSelectors { + constructor() { + this.initMilestoneDueDate(); + this.initIssuableSelect(); + } + + initMilestoneDueDate() { + const $datePicker = $('.datepicker'); + + if ($datePicker.length) { + const $dueDate = $('#milestone_due_date'); + $datePicker.datepicker({ + dateFormat: 'yy-mm-dd', + onSelect: (dateText, inst) => { + $dueDate.val(dateText); + } + }).datepicker('setDate', $.datepicker.parseDate('yy-mm-dd', $dueDate.val())); + } + $('.js-clear-due-date').on('click', (e) => { + e.preventDefault(); + $.datepicker._clearDate($datePicker); + }); + } + + initIssuableSelect() { + const $loading = $('.js-issuable-update .due_date').find('.block-loading').hide(); + + $('.js-due-date-select').each((i, dropdown) => { + const $dropdown = $(dropdown); + new DueDateSelect({ + $dropdown, + $loading + }); + }); + } + } + + global.DueDateSelectors = DueDateSelectors; + +})(window.gl || (window.gl = {})); diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index f8059988038..ba9f0c27661 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -171,5 +171,5 @@ new LabelsSelect(); new IssuableContext('#{escape_javascript(current_user.to_json(only: [:username, :id, :name]))}'); new Subscription('.subscription') - new DueDateSelect(); + new gl.DueDateSelectors(); sidebar = new Sidebar(); -- cgit v1.2.1 From aa76fa55f7ab2dfd1ecc3df5737d6b5f48ed5759 Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Sun, 16 Oct 2016 18:38:35 +0100 Subject: Remove carriage returns from commit description as summary is on a newline and will always include carriage returns --- features/steps/project/commits/commits.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index b08912de25f..c2a15c1a19a 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -21,7 +21,7 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps expect(response_headers['Content-Type']).to have_content("application/atom+xml") expect(body).to have_selector("title", text: "#{@project.name}:master commits") expect(body).to have_selector("author email", text: commit.author_email) - expect(body).to have_selector("entry summary", text: commit.description[0..10]) + expect(body).to have_selector("entry summary", text: commit.description[0..10].gsub("\r", "")) end step 'I click on tag link' do -- cgit v1.2.1 From 7df7a92b0a4a5d9731086177f0cb3f618578d122 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 17 Oct 2016 11:57:33 +0300 Subject: [Great spinach fix] Replace gsub with delete --- features/steps/project/commits/commits.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index c2a15c1a19a..244306e8464 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -21,7 +21,7 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps expect(response_headers['Content-Type']).to have_content("application/atom+xml") expect(body).to have_selector("title", text: "#{@project.name}:master commits") expect(body).to have_selector("author email", text: commit.author_email) - expect(body).to have_selector("entry summary", text: commit.description[0..10].gsub("\r", "")) + expect(body).to have_selector("entry summary", text: commit.description[0..10].delete("\r")) end step 'I click on tag link' do -- cgit v1.2.1 From 77bbd9857fefde0d2a89acd23463732240585f23 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 17 Oct 2016 13:59:59 +0300 Subject: Fix randomly crashing spinach test for merge request Signed-off-by: Dmitriy Zaporozhets --- features/steps/project/merge_requests.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index de065dffbc2..44346d99f44 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -512,6 +512,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'I should see new target branch changes' do expect(page).to have_content 'Request to merge fix into feature' expect(page).to have_content 'Target branch changed from merge-test to feature' + wait_for_ajax end step 'I click on "Email Patches"' do -- cgit v1.2.1 From 7762d8f5a350c92295fc11cdffa703b3cb498974 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 17 Oct 2016 14:10:04 +0300 Subject: Fix Test Env (proper error handling when gitlab-shell is not clonned) --- spec/support/test_env.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index ad8ae763f6d..725299031d0 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -98,7 +98,9 @@ module TestEnv def setup_gitlab_shell unless File.directory?(Gitlab.config.gitlab_shell.path) - `rake gitlab:shell:install` + unless system('rake', 'gitlab:shell:install') + raise 'Can`t clone gitlab-shell' + end end end -- cgit v1.2.1 From 009da2fc80924778ba61c6363b6e12d7daf1fa75 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 17 Oct 2016 14:31:43 +0200 Subject: Fix broken specs on MySQL after https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6896 --- spec/workers/pipeline_metrics_worker_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/workers/pipeline_metrics_worker_spec.rb b/spec/workers/pipeline_metrics_worker_spec.rb index f58df3f0d6e..232478c9735 100644 --- a/spec/workers/pipeline_metrics_worker_spec.rb +++ b/spec/workers/pipeline_metrics_worker_spec.rb @@ -23,7 +23,7 @@ describe PipelineMetricsWorker do it 'records the build start time' do subject - expect(merge_request.reload.metrics.latest_build_started_at).to eq(pipeline.started_at) + expect(merge_request.reload.metrics.latest_build_started_at).to be_within(1.second).of(pipeline.started_at) end it 'clears the build end time' do @@ -39,7 +39,7 @@ describe PipelineMetricsWorker do it 'records the build end time' do subject - expect(merge_request.reload.metrics.latest_build_finished_at).to eq(pipeline.finished_at) + expect(merge_request.reload.metrics.latest_build_finished_at).to be_within(1.second).of(pipeline.finished_at) end end end -- cgit v1.2.1 From 3fe36c5b5356ee76edc7010e4d91073b2063495a Mon Sep 17 00:00:00 2001 From: Matt Lee Date: Mon, 17 Oct 2016 10:55:35 -0400 Subject: Updated logo from @luke --- doc/raketasks/backup_hrz.png | Bin 8907 -> 31784 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/doc/raketasks/backup_hrz.png b/doc/raketasks/backup_hrz.png index 42084717ebe..287587609a1 100644 Binary files a/doc/raketasks/backup_hrz.png and b/doc/raketasks/backup_hrz.png differ -- cgit v1.2.1 From 855e8524b208e60612976aed0d66c52a51d965a6 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 17 Oct 2016 12:52:16 +0200 Subject: Enable activerecord_sane_schema_dumper for test --- Gemfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 5f754c1b66f..05166b6a828 100644 --- a/Gemfile +++ b/Gemfile @@ -262,8 +262,6 @@ group :development do # thin instead webrick gem 'thin', '~> 1.7.0' - - gem 'activerecord_sane_schema_dumper', '0.2' end group :development, :test do @@ -310,6 +308,8 @@ group :development, :test do gem 'license_finder', '~> 2.1.0', require: false gem 'knapsack', '~> 1.11.0' + + gem 'activerecord_sane_schema_dumper', '0.2' end group :test do -- cgit v1.2.1 From 6aca8b570c929e10f08a599f7bd4f3c7004f96d3 Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Sun, 16 Oct 2016 14:01:58 +0100 Subject: Added download-button class and applied button margin --- app/assets/stylesheets/pages/tree.scss | 4 ++++ app/views/projects/buttons/_download.html.haml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index 99c0f6362d0..6ea7a2b5498 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -169,4 +169,8 @@ margin-top: 11px; position: relative; z-index: 2; + + .download-button { + margin-left: $btn-side-margin; + } } diff --git a/app/views/projects/buttons/_download.html.haml b/app/views/projects/buttons/_download.html.haml index 9089586a89d..7e83a88913a 100644 --- a/app/views/projects/buttons/_download.html.haml +++ b/app/views/projects/buttons/_download.html.haml @@ -1,5 +1,5 @@ - if !project.empty_repo? && can?(current_user, :download_code, project) - %span{class: 'hidden-xs hidden-sm'} + %span{class: 'hidden-xs hidden-sm download-button'} .dropdown.inline %button.btn{ 'data-toggle' => 'dropdown' } = icon('download') -- cgit v1.2.1 From 0ff28c706d33c889348115cef6de6f779e1349da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 17 Oct 2016 17:32:50 +0200 Subject: Update CHANGELOG for 8.12.7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ci skip] Signed-off-by: Rémy Coutable --- CHANGELOG.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 447b7ffdfdf..fd37d9bcde6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,6 @@ Please view this file on the master branch, on stable branches it's out of date. - Clarify documentation for Runners API (Gennady Trafimenkov) - The instrumentation for Banzai::Renderer has been restored - Change user & group landing page routing from /u/:username to /:username - - Prevent running GfmAutocomplete setup for each diff note !6569 - Added documentation for .gitattributes files - Move Pipeline Metrics to separate worker - AbstractReferenceFilter caches project_refs on RequestStore when active @@ -40,7 +39,6 @@ Please view this file on the master branch, on stable branches it's out of date. - Update Gitlab Shell to fix some problems with moving projects between storages - Cache rendered markdown in the database, rather than Redis - Avoid database queries on Banzai::ReferenceParser::BaseParser for nodes without references - - Do not alter 'force_remove_source_branch' options on MergeRequest unless specified - Simplify Mentionable concern instance methods - API: Ability to retrieve version information (Robert Schilling) - Fix permission for setting an issue's due date @@ -77,14 +75,12 @@ Please view this file on the master branch, on stable branches it's out of date. - Only update issuable labels if they have been changed - Take filters in account in issuable counters. !6496 - Use custom Ruby images to test builds (registry.dev.gitlab.org/gitlab/gitlab-build-images:*) - - Prevent flash alert text from being obscured when container is fluid - Append issue template to existing description !6149 (Joseph Frazier) - Trending projects now only show public projects and the list of projects is cached for a day - Memoize Gitlab Shell's secret token (!6599, Justin DiPierro) - Revoke button in Applications Settings underlines on hover. - Use higher size on Gitlab::Redis connection pool on Sidekiq servers - Add missing values to linter !6276 (Katarzyna Kobierska Ula Budziszewska) - - Fix Long commit messages overflow viewport in file tree - Revert avoid touching file system on Build#artifacts? - Stop using a Redis lease when updating the project activity timestamp whenever a new event is created - Add disabled delete button to protected branches (ClemMakesApps) @@ -118,7 +114,6 @@ Please view this file on the master branch, on stable branches it's out of date. - Grouped pipeline dropdown is a scrollable container - Cleanup Ci::ApplicationController. !6757 (Takuya Noguchi) - Fixes padding in all clipboard icons that have .btn class - - Fix due date being displayed as NaN in Safari - Fix a typo in doc/api/labels.md - API: all unknown routing will be handled with 404 Not Found - Add docs for request profiling @@ -126,8 +121,15 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.12.7 - - Use gitlab-markup gem instead of github-markup to fix `.rst` file rendering. !6659 - - Fix GFM autocomplete setup being called several times + - Prevent running `GfmAutocomplete` setup for each diff note. !6569 + - Fix long commit messages overflow viewport in file tree. !6573 + - Use `gitlab-markup` gem instead of `github-markup` to fix `.rst` file rendering. !6659 + - Prevent flash alert text from being obscured when container is fluid. !6694 + - Fix due date being displayed as `NaN` in Safari. !6797 + - Fix JS bug with select2 because of missing `data-field` attribute in select box. !6812 + - Do not alter `force_remove_source_branch` options on MergeRequest unless specified. !6817 + - Fix GFM autocomplete setup being called several times. !6840 + - Handle case where deployment ref no longer exists. !6855 ## 8.12.6 -- cgit v1.2.1 From f91df171c53f230937b538ffdec04d12f7726ffc Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 17 Oct 2016 16:55:34 +0100 Subject: Fixed find file keyboard navigation Closes #23423 --- app/assets/javascripts/project_find_file.js | 9 +++++ .../projects/files/find_file_keyboard_spec.rb | 42 ++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 spec/features/projects/files/find_file_keyboard_spec.rb diff --git a/app/assets/javascripts/project_find_file.js b/app/assets/javascripts/project_find_file.js index 8e38ccf7e44..b8347367717 100644 --- a/app/assets/javascripts/project_find_file.js +++ b/app/assets/javascripts/project_find_file.js @@ -7,6 +7,7 @@ function ProjectFindFile(element1, options) { this.element = element1; this.options = options; + this.goToBlob = bind(this.goToBlob, this); this.goToTree = bind(this.goToTree, this); this.selectRowDown = bind(this.selectRowDown, this); this.selectRowUp = bind(this.selectRowUp, this); @@ -154,6 +155,14 @@ return location.href = this.options.treeUrl; }; + ProjectFindFile.prototype.goToBlob = function() { + var $link = this.element.find(".tree-item.selected .tree-item-file-name a"); + + if ($link.length) { + $link.get(0).click(); + } + }; + return ProjectFindFile; })(); diff --git a/spec/features/projects/files/find_file_keyboard_spec.rb b/spec/features/projects/files/find_file_keyboard_spec.rb new file mode 100644 index 00000000000..fc88fd74af8 --- /dev/null +++ b/spec/features/projects/files/find_file_keyboard_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +feature 'Find file keyboard shortcuts', feature: true, js: true do + include WaitForAjax + + let(:user) { create(:user) } + let(:project) { create(:project) } + + before do + project.team << [user, :master] + login_as user + + visit namespace_project_find_file_path(project.namespace, project, project.repository.root_ref) + + wait_for_ajax + end + + it 'opens file when pressing enter key' do + fill_in 'file_find', with: 'CHANGELOG' + + find('#file_find').native.send_keys(:enter) + + expect(page).to have_selector('.blob-content-holder') + + page.within('.file-title') do + expect(page).to have_content('CHANGELOG') + end + end + + it 'navigates files with arrow keys' do + fill_in 'file_find', with: 'application.' + + find('#file_find').native.send_keys(:down) + find('#file_find').native.send_keys(:enter) + + expect(page).to have_selector('.blob-content-holder') + + page.within('.file-title') do + expect(page).to have_content('application.js') + end + end +end -- cgit v1.2.1 From 9ff64b299bdcca767464f26e25c7e0017ec0fc38 Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Fri, 14 Oct 2016 18:13:03 +0100 Subject: Swapped button text manipulation outcomes for the toggle query --- app/assets/javascripts/pipelines.js.es6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/pipelines.js.es6 b/app/assets/javascripts/pipelines.js.es6 index 6bf63ee6979..a7624de6089 100644 --- a/app/assets/javascripts/pipelines.js.es6 +++ b/app/assets/javascripts/pipelines.js.es6 @@ -15,7 +15,7 @@ $($pipelineBtn).add($pipelineGraph).toggleClass('graph-collapsed'); - graphCollapsed ? $btnText.text('Expand') : $btnText.text('Hide') + graphCollapsed ? $btnText.text('Hide') : $btnText.text('Expand') } addMarginToBuildColumns() { -- cgit v1.2.1 From 501d1485edf321d9af242af4721dec604b8dfbcc Mon Sep 17 00:00:00 2001 From: tauriedavis Date: Fri, 14 Oct 2016 13:55:10 -0700 Subject: Apply better hierarchy to markdown headers and issue/mr titles --- app/assets/stylesheets/framework/typography.scss | 22 ++++++++++------------ app/assets/stylesheets/pages/detail_page.scss | 6 ++++-- app/assets/stylesheets/pages/merge_requests.scss | 7 ------- 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 287653beac5..8f8ad728269 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -45,40 +45,38 @@ } h1 { - font-size: 2em; + font-size: 1.75em; font-weight: 600; - margin: 1em 0 10px; + margin: 16px 0 10px; padding: 0 0 0.3em; border-bottom: 1px solid $btn-default-border; color: $gl-gray-dark; } h2 { - font-size: 1.6em; + font-size: 1.5em; font-weight: 600; - margin: 1em 0 10px; - padding-bottom: 0.3em; - border-bottom: 1px solid $btn-default-border; + margin: 16px 0 10px; color: $gl-gray-dark; } h3 { - margin: 1em 0 10px; - font-size: 1.4em; + margin: 16px 0 10px; + font-size: 1.3em; } h4 { - margin: 1em 0 10px; - font-size: 1.25em; + margin: 16px 0 10px; + font-size: 1.2em; } h5 { - margin: 1em 0 10px; + margin: 16px 0 10px; font-size: 1em; } h6 { - margin: 1em 0 10px; + margin: 16px 0 10px; font-size: 0.95em; } diff --git a/app/assets/stylesheets/pages/detail_page.scss b/app/assets/stylesheets/pages/detail_page.scss index 4d9c73c6840..7aa156bc47a 100644 --- a/app/assets/stylesheets/pages/detail_page.scss +++ b/app/assets/stylesheets/pages/detail_page.scss @@ -20,9 +20,11 @@ .detail-page-description { .title { - margin: 0; - font-size: 23px; + margin: 0 0 16px; + font-size: 2em; color: $gl-gray-dark; + padding: 0 0 0.3em; + border-bottom: 1px solid #e7e9ed; } .description { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index afc4e517fde..101472278e2 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -429,13 +429,6 @@ } } -.merge-request-details { - - .title { - margin-bottom: 20px; - } -} - .merge-request-tabs { background-color: #fff; -- cgit v1.2.1 From 28dbfd5c4c7793be4147ba217e7d9e162905088b Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 17 Oct 2016 12:43:17 -0500 Subject: Provide better error message to the user --- .../javascripts/merge_conflicts/components/diff_file_editor.js.es6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 index 3379414343f..5012bdfe997 100644 --- a/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 +++ b/app/assets/javascripts/merge_conflicts/components/diff_file_editor.js.es6 @@ -62,7 +62,7 @@ this.saveDiffResolution(); }) .fail(() => { - console.log('error'); + new Flash('Failed to load the file, please try again.'); }) .always(() => { this.loading = false; -- cgit v1.2.1 From a5f5c02598d189428c583572d42f38e478669771 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 13 Oct 2016 16:07:16 +0300 Subject: Add todo for deprecated user routes and more information about deprecation to changelog Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG | 2 +- config/routes/user.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 2cbf5bc3277..1e887de6867 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -78,6 +78,7 @@ v 8.13.0 (unreleased) - Replace bootstrap caret with fontawesome caret (ClemMakesApps) - Fix unnecessary escaping of reserved HTML characters in milestone title. !6533 - Add organization field to user profile + - Change user pages routing from /u/:username/PATH to /users/:username/PATH. Old routes will redirect to the new ones for the time being. - Fix enter key when navigating search site search dropdown. !6643 (Brennan Roberts) - Fix deploy status responsiveness error !6633 - Make searching for commits case insensitive @@ -102,7 +103,6 @@ v 8.13.0 (unreleased) - Fix a typo in doc/api/labels.md - API: all unknown routing will be handled with 404 Not Found - Make guests unable to view MRs on private projects - - Change user pages routing from /u/:username/PATH to /users/:username/PATH v 8.12.6 - Update mailroom to 0.8.1 in Gemfile.lock !6814 diff --git a/config/routes/user.rb b/config/routes/user.rb index ae15b9d02a3..53f9aafc107 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -34,7 +34,9 @@ scope(path: 'users/:username', end # Compatibility with old routing +# TODO (dzaporozhets): remove in 10.0 get '/u/:username', to: redirect('/%{username}'), constraints: { username: /[a-zA-Z.0-9_\-]+(? Date: Mon, 17 Oct 2016 13:23:28 -0700 Subject: change border color to variable --- app/assets/stylesheets/framework/typography.scss | 6 +++--- app/assets/stylesheets/pages/detail_page.scss | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 8f8ad728269..55de9053be5 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -49,7 +49,7 @@ font-weight: 600; margin: 16px 0 10px; padding: 0 0 0.3em; - border-bottom: 1px solid $btn-default-border; + border-bottom: 1px solid $white-dark; color: $gl-gray-dark; } @@ -85,12 +85,12 @@ font-size: inherit; padding: 8px 21px; margin: 12px 0; - border-left: 3px solid #e7e9ed; + border-left: 3px solid $white-dark; } blockquote:dir(rtl) { border-left: 0; - border-right: 3px solid #e7e9ed; + border-right: 3px solid $white-dark; } blockquote p { diff --git a/app/assets/stylesheets/pages/detail_page.scss b/app/assets/stylesheets/pages/detail_page.scss index 7aa156bc47a..2357671c2ae 100644 --- a/app/assets/stylesheets/pages/detail_page.scss +++ b/app/assets/stylesheets/pages/detail_page.scss @@ -24,7 +24,7 @@ font-size: 2em; color: $gl-gray-dark; padding: 0 0 0.3em; - border-bottom: 1px solid #e7e9ed; + border-bottom: 1px solid $white-dark; } .description { -- cgit v1.2.1 From 73988dd994d6c0d7f5cec24ca5c4c20137cc3844 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 17 Oct 2016 17:47:07 -0500 Subject: Update endpoint to username validator --- app/assets/javascripts/username_validator.js.es6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/username_validator.js.es6 b/app/assets/javascripts/username_validator.js.es6 index 2517f778365..bf4b2e320cd 100644 --- a/app/assets/javascripts/username_validator.js.es6 +++ b/app/assets/javascripts/username_validator.js.es6 @@ -76,7 +76,7 @@ this.renderState(); return $.ajax({ type: 'GET', - url: `/u/${username}/exists`, + url: `/users/${username}/exists`, dataType: 'json', success: (res) => this.setAvailabilityState(res.exists) }); -- cgit v1.2.1