From e033b9a7f19223e4fbad6fa3c712c82f49e47244 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 4 Mar 2016 08:42:22 +0000 Subject: [PATCH 01/18] Added discard button to comment form Also changed the labels on the buttons to better match the action they are completing. Closes #8057 --- app/assets/javascripts/notes.js.coffee | 61 +++++++++++++++---- .../projects/issues/_discussion.html.haml | 4 +- .../merge_requests/_discussion.html.haml | 4 +- app/views/projects/notes/_form.html.haml | 5 +- 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index c95ead22e6c..3142afccd6e 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -51,6 +51,9 @@ class @Notes $(document).on "ajax:complete", ".js-main-target-form", @reenableTargetFormSubmitButton $(document).on "ajax:success", ".js-main-target-form", @resetMainTargetForm + # reset main target form when clicking discard + $(document).on "click", ".js-note-discard", @resetMainTargetForm + # update the file name when an attachment is selected $(document).on "change", ".js-note-attachment-input", @updateFormAttachment @@ -85,6 +88,7 @@ class @Notes $(document).off "keyup", ".js-note-text" $(document).off "click", ".js-note-target-reopen" $(document).off "click", ".js-note-target-close" + $(document).off "click", ".js-note-discard" $('.note .js-task-list-container').taskList('disable') $(document).off 'tasklist:changed', '.note .js-task-list-container' @@ -219,7 +223,7 @@ class @Notes Resets text and preview. Resets buttons. ### - resetMainTargetForm: -> + resetMainTargetForm: (e) => form = $(".js-main-target-form") # remove validation errors @@ -231,6 +235,8 @@ class @Notes form.find(".js-note-text").data("autosave").reset() + @updateTargetButtons(e) + reenableTargetFormSubmitButton: -> form = $(".js-main-target-form") @@ -274,8 +280,10 @@ class @Notes form.removeClass "js-new-note-form" form.find('.div-dropzone').remove() + # hide discard button + form.find('.js-note-discard').hide() + # setup preview buttons - form.find(".js-md-write-button, .js-md-preview-button").tooltip placement: "left" previewButton = form.find(".js-md-preview-button") textarea = form.find(".js-note-text") @@ -561,21 +569,52 @@ class @Notes updateCloseButton: (e) => textarea = $(e.target) form = textarea.parents('form') - form.find('.js-note-target-close').text('Close') + closebtn = form.find('.js-note-target-close') + closebtn.text(closebtn.data('original-text')) updateTargetButtons: (e) => textarea = $(e.target) form = textarea.parents('form') + reopenbtn = form.find('.js-note-target-reopen') + closebtn = form.find('.js-note-target-close') + discardbtn = form.find('.js-note-discard') + if textarea.val().trim().length > 0 - form.find('.js-note-target-reopen').text('Comment & reopen') - form.find('.js-note-target-close').text('Comment & close') - form.find('.js-note-target-reopen').addClass('btn-comment-and-reopen') - form.find('.js-note-target-close').addClass('btn-comment-and-close') + reopentext = reopenbtn.data('alternative-text') + closetext = closebtn.data('alternative-text') + + if reopenbtn.text() isnt reopentext + reopenbtn.text(reopentext) + + if closebtn.text() isnt closetext + closebtn.text(closetext) + + if reopenbtn.is(':not(.btn-comment-and-reopen)') + reopenbtn.addClass('btn-comment-and-reopen') + + if closebtn.is(':not(.btn-comment-and-close)') + closebtn.addClass('btn-comment-and-close') + + if discardbtn.is(':hidden') + discardbtn.show() else - form.find('.js-note-target-reopen').text('Reopen') - form.find('.js-note-target-close').text('Close') - form.find('.js-note-target-reopen').removeClass('btn-comment-and-reopen') - form.find('.js-note-target-close').removeClass('btn-comment-and-close') + reopentext = reopenbtn.data('original-text') + closetext = closebtn.data('original-text') + + if reopenbtn.text() isnt reopentext + reopenbtn.text(reopentext) + + if closebtn.text() isnt closetext + closebtn.text(closebtn.data('original-text')) + + if reopenbtn.is(':not(.btn-comment-and-reopen)') + reopenbtn.removeClass('btn-comment-and-reopen') + + if closebtn.is(':not(.btn-comment-and-close)') + closebtn.removeClass('btn-comment-and-close') + + if discardbtn.is(':visible') + discardbtn.hide() initTaskList: -> @enableTaskList() diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml index eb9c225df2f..b151393abab 100644 --- a/app/views/projects/issues/_discussion.html.haml +++ b/app/views/projects/issues/_discussion.html.haml @@ -1,7 +1,7 @@ - content_for :note_actions do - if can?(current_user, :update_issue, @issue) - = link_to 'Reopen issue', issue_path(@issue, issue: {state_event: :reopen}, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "btn btn-nr btn-grouped btn-reopen btn-comment js-note-target-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue' - = link_to 'Close issue', issue_path(@issue, issue: {state_event: :close}, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "btn btn-nr btn-grouped btn-close btn-comment js-note-target-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' + = link_to 'Reopen issue', issue_path(@issue, issue: {state_event: :reopen}, status_only: true, format: 'json'), data: {no_turbolink: true, original_text: "Reopen issue", alternative_text: "Comment & reopen issue"}, class: "btn btn-nr btn-grouped btn-reopen btn-comment js-note-target-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue' + = link_to 'Close issue', issue_path(@issue, issue: {state_event: :close}, status_only: true, format: 'json'), data: {no_turbolink: true, original_text: "Close issue", alternative_text: "Comment & close issue"}, class: "btn btn-nr btn-grouped btn-close btn-comment js-note-target-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' #notes = render 'projects/notes/notes_with_form' diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml index 1c7de94acfd..393998f15b9 100644 --- a/app/views/projects/merge_requests/_discussion.html.haml +++ b/app/views/projects/merge_requests/_discussion.html.haml @@ -1,8 +1,8 @@ - content_for :note_actions do - if can?(current_user, :update_merge_request, @merge_request) - if @merge_request.open? - = link_to 'Close', merge_request_path(@merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-nr btn-comment btn-grouped btn-close close-mr-link js-note-target-close", title: "Close merge request" + = link_to 'Close merge request', merge_request_path(@merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-nr btn-comment btn-grouped btn-close close-mr-link js-note-target-close", title: "Close merge request", data: {original_text: "Close merge request", alternative_text: "Comment & close merge request"} - if @merge_request.closed? - = link_to 'Reopen', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-nr btn-comment btn-grouped btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request" + = link_to 'Reopen merge request', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-nr btn-comment btn-grouped btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request", data: {original_text: "Reopen merge request", alternative_text: "Comment & reopen merge request"} #notes= render "projects/notes/notes_with_form" diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index 09740d8ea12..2b293e4770d 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -13,6 +13,7 @@ .error-alert .note-form-actions.clearfix - = f.submit 'Add Comment', class: "btn btn-nr btn-create comment-btn btn-grouped js-comment-button" + = f.submit 'Comment', class: "btn btn-nr btn-create comment-btn btn-grouped js-comment-button" = yield(:note_actions) - %a.btn.btn-nr.btn-cancel.js-close-discussion-note-form Cancel + %a.btn.btn-cancel.js-note-discard{role: "button"} + Discard draft From da6e38a889d4725a50fbe0bf8a3ca1d3e53d01c3 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 4 Mar 2016 10:44:04 +0000 Subject: [PATCH 02/18] Fixed tests for comment forms --- app/assets/javascripts/notes.js.coffee | 5 +++++ app/views/projects/notes/_form.html.haml | 2 +- features/steps/project/snippets.rb | 2 +- spec/features/notes_on_merge_requests_spec.rb | 6 +++--- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 3142afccd6e..eff8beb97e9 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -470,6 +470,11 @@ class @Notes form.find("#note_line_code").val dataHolder.data("lineCode") form.find("#note_noteable_type").val dataHolder.data("noteableType") form.find("#note_noteable_id").val dataHolder.data("noteableId") + form.find('.js-note-discard') + .show() + .removeClass('js-note-discard') + .addClass('js-close-discussion-note-form') + .text(form.find('.js-close-discussion-note-form').data('cancel-text')) @setupNoteForm form form.find(".js-note-text").focus() form.addClass "js-discussion-note-form" diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index 2b293e4770d..f675f092da1 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -15,5 +15,5 @@ .note-form-actions.clearfix = f.submit 'Comment', class: "btn btn-nr btn-create comment-btn btn-grouped js-comment-button" = yield(:note_actions) - %a.btn.btn-cancel.js-note-discard{role: "button"} + %a.btn.btn-cancel.js-note-discard{role: "button", data: {cancel_text: "Cancel"}} Discard draft diff --git a/features/steps/project/snippets.rb b/features/steps/project/snippets.rb index 504654f90dd..786a0cad975 100644 --- a/features/steps/project/snippets.rb +++ b/features/steps/project/snippets.rb @@ -77,7 +77,7 @@ class Spinach::Features::ProjectSnippets < Spinach::FeatureSteps step 'I leave a comment like "Good snippet!"' do page.within('.js-main-target-form') do fill_in "note_note", with: "Good snippet!" - click_button "Add Comment" + click_button "Comment" end end diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 1a360cd1ebc..d9a8058efd9 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -22,7 +22,7 @@ describe 'Comments', feature: true do it 'should be valid' do is_expected.to have_css('.js-main-target-form', visible: true, count: 1) expect(find('.js-main-target-form input[type=submit]').value). - to eq('Add Comment') + to eq('Comment') page.within('.js-main-target-form') do expect(page).not_to have_link('Cancel') end @@ -49,7 +49,7 @@ describe 'Comments', feature: true do page.within('.js-main-target-form') do fill_in 'note[note]', with: 'This is awsome!' find('.js-md-preview-button').click - click_button 'Add Comment' + click_button 'Comment' end end @@ -202,7 +202,7 @@ describe 'Comments', feature: true do before do page.within("tr[id='#{line_code_2}'] + .js-temp-notes-holder") do fill_in 'note[note]', with: 'Another comment on line 10' - click_button('Add Comment') + click_button('Comment') end end From b1264d86fa0772860b284893a3bf129bec35cb0d Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 4 Mar 2016 12:22:27 +0000 Subject: [PATCH 03/18] Fixed failing comment test --- features/steps/project/merge_requests.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index dde864f5180..b0c4668fc78 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -404,7 +404,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps page.within(".js-discussion-note-form") do fill_in "note_note", with: "Line is correct" - click_button "Add Comment" + click_button "Comment" end page.within ".files [id^=diff]:nth-child(2) .note-body > .note-text" do From f95f242cfad4fb0032caecea47964d78c1f652bc Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 4 Mar 2016 13:52:29 +0000 Subject: [PATCH 04/18] Updated tests --- features/steps/project/issues/award_emoji.rb | 2 +- features/steps/project/issues/issues.rb | 2 +- features/steps/project/merge_requests.rb | 4 ++-- features/steps/shared/diff_note.rb | 4 ++-- features/steps/shared/issuable.rb | 2 +- features/steps/shared/note.rb | 6 +++--- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/features/steps/project/issues/award_emoji.rb b/features/steps/project/issues/award_emoji.rb index 277c63914d1..937fbbd34eb 100644 --- a/features/steps/project/issues/award_emoji.rb +++ b/features/steps/project/issues/award_emoji.rb @@ -79,7 +79,7 @@ class Spinach::Features::AwardEmoji < Spinach::FeatureSteps step 'I leave comment with a single emoji' do page.within('.js-main-target-form') do fill_in 'note[note]', with: ':smile:' - click_button 'Add Comment' + click_button 'Comment' end end diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index 565bf088b41..3a189a9a1e9 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -267,7 +267,7 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps step 'I leave a comment with code block' do page.within(".js-main-target-form") do fill_in "note[note]", with: "```\nCommand [1]: /usr/local/bin/git , see [text](doc/text)\n```" - click_button "Add Comment" + click_button "Comment" sleep 0.05 end end diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index b0c4668fc78..8a420f9deb4 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -417,7 +417,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps page.within(".js-discussion-note-form") do fill_in "note_note", with: "Line is wrong on here" - click_button "Add Comment" + click_button "Comment" end end @@ -509,7 +509,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps def leave_comment(message) page.within(".js-discussion-note-form", visible: true) do fill_in "note_note", with: message - click_button "Add Comment" + click_button "Comment" end page.within(".notes_holder", visible: true) do expect(page).to have_content message diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index 06e69441894..906b66a4a63 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -93,14 +93,14 @@ module SharedDiffNote page.within("form[id$='#{sample_commit.line_code}']") do fill_in 'note[note]', with: ':smile:' - click_button('Add Comment') + click_button('Comment') end end end step 'I submit the diff comment' do page.within(diff_file_selector) do - click_button("Add Comment") + click_button("Comment") end end diff --git a/features/steps/shared/issuable.rb b/features/steps/shared/issuable.rb index ae10c6069a9..e59bfbea998 100644 --- a/features/steps/shared/issuable.rb +++ b/features/steps/shared/issuable.rb @@ -182,7 +182,7 @@ module SharedIssuable page.within('.js-main-target-form') do fill_in 'note[note]', with: "##{issuable.to_reference(project)}" - click_button 'Add Comment' + click_button 'Comment' end end diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb index eb6df61b8e6..6870d364ac0 100644 --- a/features/steps/shared/note.rb +++ b/features/steps/shared/note.rb @@ -17,7 +17,7 @@ module SharedNote step 'I leave a comment like "XML attached"' do page.within(".js-main-target-form") do fill_in "note[note]", with: "XML attached" - click_button "Add Comment" + click_button "Comment" end end @@ -30,7 +30,7 @@ module SharedNote step 'I submit the comment' do page.within(".js-main-target-form") do - click_button "Add Comment" + click_button "Comment" end end @@ -115,7 +115,7 @@ module SharedNote step 'I leave a comment with a header containing "Comment with a header"' do page.within(".js-main-target-form") do fill_in "note[note]", with: "# Comment with a header" - click_button "Add Comment" + click_button "Comment" sleep 0.05 end end From 344d6e6f89cceb33e371e10de8262154ee108fbe Mon Sep 17 00:00:00 2001 From: Pascal Bach Date: Wed, 24 Feb 2016 15:48:56 +0100 Subject: [PATCH 05/18] Support YAML alias/anchor usage in .gitlab-ci.yml This allows to reuse one job as a template for another one: ``` job1: &JOBTMPL script: execute-script-for-job job2: *JOBTMPL ``` This also helps to solve some of the issues in #342 Signed-off-by: Pascal Bach Signed-off-by: Fabio Huser --- lib/ci/gitlab_ci_yaml_processor.rb | 2 +- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 39 ++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 1a3f662811a..28e074cd289 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -10,7 +10,7 @@ module Ci attr_reader :before_script, :image, :services, :variables, :path, :cache def initialize(config, path = nil) - @config = YAML.safe_load(config, [Symbol]) + @config = YAML.safe_load(config, [Symbol], [], true) @path = path unless @config.is_a? Hash diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index f3394910c5b..1e98280d045 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -427,6 +427,45 @@ module Ci end end + describe "YAML Alias/Anchor" do + it "is correctly supported for jobs" do + config = < Date: Thu, 25 Feb 2016 13:51:17 -0500 Subject: [PATCH 06/18] Add error for ajax:error when submitting comments Fixes #13814 --- app/assets/javascripts/notes.js.coffee | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 863a4edfad7..cb6b7b29935 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -30,6 +30,9 @@ class @Notes $(document).on "ajax:success", ".js-main-target-form", @addNote $(document).on "ajax:success", ".js-discussion-note-form", @addDiscussionNote + # catch note ajax errors + $(document).on "ajax:error", ".js-main-target-form", @addNoteError + # change note in UI after update $(document).on "ajax:success", "form.edit-note", @updateNote @@ -309,6 +312,10 @@ class @Notes addNote: (xhr, note, status) => @renderNote(note) + addNoteError: (xhr, note, status) => + flash = new Flash('Your comment could not be submitted! Please check your network connection and try again.', 'alert') + flash.pinTo('.md-area') + ### Called in response to the new note form being submitted From 136ba502d3ad3378f7746a18c3045346a2f49081 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Mon, 29 Feb 2016 13:03:21 -0500 Subject: [PATCH 07/18] Change test in an attempt to pass. Thanks @rspeicher! --- features/steps/project/source/browse_files.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb index 51b15791674..243469b8e7d 100644 --- a/features/steps/project/source/browse_files.rb +++ b/features/steps/project/source/browse_files.rb @@ -361,7 +361,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps end step 'I can see the new rendered SVG image' do - expect(find('.file-content')).to have_css('img') + expect(page).to have_css('.file-content img') end private From bd411675d2e742b183737644e73212fec198db73 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 10 Mar 2016 17:34:06 +0000 Subject: [PATCH 08/18] Fixes issue with issue sidebar toggle button not working Closes #14195 --- app/assets/javascripts/application.js.coffee | 8 ++++---- app/assets/javascripts/merge_request_tabs.js.coffee | 3 +-- app/views/projects/issues/show.html.haml | 2 +- .../projects/merge_requests/show/_mr_title.html.haml | 2 +- app/views/shared/issuable/_sidebar.html.haml | 2 +- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/application.js.coffee b/app/assets/javascripts/application.js.coffee index 321da10a009..1212e89975b 100644 --- a/app/assets/javascripts/application.js.coffee +++ b/app/assets/javascripts/application.js.coffee @@ -220,17 +220,17 @@ $ -> .off 'breakpoint:change' .on 'breakpoint:change', (e, breakpoint) -> if breakpoint is 'sm' or breakpoint is 'xs' - $gutterIcon = $('aside .gutter-toggle').find('i') + $gutterIcon = $('.js-sidebar-toggle').find('i') if $gutterIcon.hasClass('fa-angle-double-right') $gutterIcon.closest('a').trigger('click') $(document) - .off 'click', 'aside .gutter-toggle' - .on 'click', 'aside .gutter-toggle', (e, triggered) -> + .off 'click', '.js-sidebar-toggle' + .on 'click', '.js-sidebar-toggle', (e, triggered) -> e.preventDefault() $this = $(this) $thisIcon = $this.find 'i' - $allGutterToggleIcons = $('.gutter-toggle i') + $allGutterToggleIcons = $('.js-sidebar-toggle i') if $thisIcon.hasClass('fa-angle-double-right') $allGutterToggleIcons .removeClass('fa-angle-double-right') diff --git a/app/assets/javascripts/merge_request_tabs.js.coffee b/app/assets/javascripts/merge_request_tabs.js.coffee index 58373ba87a5..8322b4c46ad 100644 --- a/app/assets/javascripts/merge_request_tabs.js.coffee +++ b/app/assets/javascripts/merge_request_tabs.js.coffee @@ -189,7 +189,7 @@ class @MergeRequestTabs $('.container-fluid').removeClass('container-limited') shrinkView: -> - $gutterIcon = $('.gutter-toggle i') + $gutterIcon = $('.js-sidebar-toggle i') # Wait until listeners are set setTimeout( -> @@ -197,4 +197,3 @@ class @MergeRequestTabs if $gutterIcon.is('.fa-angle-double-right') $gutterIcon.closest('a').trigger('click',[true]) , 0) - diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 617b0437807..2a7d0b0757d 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -18,7 +18,7 @@ %span.hidden-sm.hidden-md.hidden-lg = icon('circle-o') - %a.btn.btn-default.pull-right.hidden-sm.hidden-md.hidden-lg.gutter-toggle{ href: "#" } + %a.btn.btn-default.pull-right.visible-xs.gutter-toggle.js-sidebar-toggle{ href: "#" } = icon('angle-double-left') .issue-meta diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml index d24c12251f3..83b056eef66 100644 --- a/app/views/projects/merge_requests/show/_mr_title.html.haml +++ b/app/views/projects/merge_requests/show/_mr_title.html.haml @@ -4,7 +4,7 @@ = @merge_request.state_human_name %span.hidden-sm.hidden-md.hidden-lg = icon(@merge_request.state_icon_name) - %a.btn.btn-default.pull-right.hidden-sm.hidden-md.hidden-lg.gutter-toggle{ href: "#" } + %a.btn.btn-default.pull-right.visible-xs.gutter-toggle.js-sidebar-toggle{ href: "#" } = icon('angle-double-left') .issue-meta %strong.identifier diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 36f06377886..9020a1330a3 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -6,7 +6,7 @@ of = issuables_count(issuable) %span.pull-right - %a.gutter-toggle{href: '#'} + %a.gutter-toggle.js-sidebar-toggle{href: '#'} = sidebar_gutter_toggle_icon .issuable-nav.hide-collapsed.pull-right.btn-group{role: 'group', "aria-label" => '...'} - if prev_issuable = prev_issuable_for(issuable) From 07e1754c7c9e82bbd8cf65cb7dcccec2c8c758d5 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 11 Mar 2016 08:57:53 +0000 Subject: [PATCH 09/18] Removed deprecated bootstrap classes --- app/views/projects/issues/show.html.haml | 2 +- app/views/projects/merge_requests/show/_mr_title.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 2a7d0b0757d..f5bb5d998bb 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -18,7 +18,7 @@ %span.hidden-sm.hidden-md.hidden-lg = icon('circle-o') - %a.btn.btn-default.pull-right.visible-xs.gutter-toggle.js-sidebar-toggle{ href: "#" } + %a.btn.btn-default.pull-right.visible-xs-block.gutter-toggle.js-sidebar-toggle{ href: "#" } = icon('angle-double-left') .issue-meta diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml index 83b056eef66..a75c0d96c57 100644 --- a/app/views/projects/merge_requests/show/_mr_title.html.haml +++ b/app/views/projects/merge_requests/show/_mr_title.html.haml @@ -4,7 +4,7 @@ = @merge_request.state_human_name %span.hidden-sm.hidden-md.hidden-lg = icon(@merge_request.state_icon_name) - %a.btn.btn-default.pull-right.visible-xs.gutter-toggle.js-sidebar-toggle{ href: "#" } + %a.btn.btn-default.pull-right.visible-xs-block.gutter-toggle.js-sidebar-toggle{ href: "#" } = icon('angle-double-left') .issue-meta %strong.identifier From f48f51ac7e204aa174effcda7cc79e06e2bbaba0 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 29 Feb 2016 13:39:29 +0000 Subject: [PATCH 10/18] Account settings Closes #13854 --- app/assets/javascripts/profile.js.coffee | 7 +- app/assets/stylesheets/framework/common.scss | 4 +- .../stylesheets/framework/variables.scss | 4 +- app/assets/stylesheets/pages/profile.scss | 48 +++- .../profiles/accounts_controller.rb | 24 ++ app/views/profiles/accounts/show.html.haml | 237 ++++++++++-------- 6 files changed, 204 insertions(+), 120 deletions(-) diff --git a/app/assets/javascripts/profile.js.coffee b/app/assets/javascripts/profile.js.coffee index 9110b732adc..59d44c30bee 100644 --- a/app/assets/javascripts/profile.js.coffee +++ b/app/assets/javascripts/profile.js.coffee @@ -4,12 +4,13 @@ class @Profile $('.js-preferences-form').on 'change.preference', 'input[type=radio]', -> $(this).parents('form').submit() - $('.update-username form').on 'ajax:before', -> - $('.loading-gif').show() + $('.update-username').on 'ajax:before', -> + $('.loading-username').show() $(this).find('.update-success').hide() $(this).find('.update-failed').hide() - $('.update-username form').on 'ajax:complete', -> + $('.update-username').on 'ajax:complete', -> + $('.loading-username').hide() $(this).find('.btn-save').enable() $(this).find('.loading-gif').hide() diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index c98e43ad09f..ff551f151f1 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -12,11 +12,13 @@ .prepend-top-default { margin-top: $gl-padding !important; } .prepend-top-20 { margin-top:20px } .prepend-left-10 { margin-left:10px } -.prepend-left-default { margin-left:$gl-padding } +.prepend-left-default { margin-left: $gl-padding; } .prepend-left-20 { margin-left:20px } .append-right-5 { margin-right: 5px } .append-right-10 { margin-right:10px } +.append-right-default { margin-right: $gl-padding; } .append-right-20 { margin-right:20px } +.append-bottom-0 { margin-bottom:0 } .append-bottom-10 { margin-bottom:10px } .append-bottom-15 { margin-bottom:15px } .append-bottom-20 { margin-bottom:20px } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 6561b3de7c1..0fd2f74cf89 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -70,7 +70,7 @@ $orange-light: rgba(252, 109, 38, 0.80); $orange-normal: #E75E40; $orange-dark: #CE5237; -$red-light: #F43263; +$red-light: #F06559; $red-normal: #E52C5A; $red-dark: #D22852; @@ -94,7 +94,7 @@ $border-orange-light: #fc6d26; $border-orange-normal: #CE5237; $border-orange-dark: #C14E35; -$border-red-light: #E52C5A; +$border-red-light: #F24F41; $border-red-normal: #D22852; $border-red-dark: #CA264F; diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 4826b994e37..f8aeab6857f 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -1,7 +1,6 @@ -.account-page { - fieldset { - margin-bottom: 15px; - padding-bottom: 15px; +.profile-avatar-form-option { + hr { + margin: 10px 0; } } @@ -175,3 +174,44 @@ color: $profile-settings-link-color; } } + +.change-username-title { + color: #FC6D26; +} + +.remove-account-title { + color: #F00; +} + +.provider-btn-group { + display: inline-block; + margin-right: 10px; + border: 1px solid #E5E5E5; + border-radius: 3px; + + &:last-child { + margin-right: 0; + } +} + +.provider-btn-image { + display: inline-block; + padding: 5px 10px; + border-right: 1px solid #E5E5E5; + + > img { + width: 20px; + } +} + +.provider-btn { + display: inline-block; + padding: 5px 10px; + margin-left: -3px; + line-height: 22px; + background-color: $gray-light; + + &.not-active { + color: #4688F1; + } +} diff --git a/app/controllers/profiles/accounts_controller.rb b/app/controllers/profiles/accounts_controller.rb index 175afbf8425..669fe05e5c7 100644 --- a/app/controllers/profiles/accounts_controller.rb +++ b/app/controllers/profiles/accounts_controller.rb @@ -1,6 +1,18 @@ class Profiles::AccountsController < Profiles::ApplicationController def show + unless current_user.otp_secret + current_user.otp_secret = User.generate_otp_secret(32) + end + + unless current_user.otp_grace_period_started_at && two_factor_grace_period + current_user.otp_grace_period_started_at = Time.current + end + + current_user.save! if current_user.changed? + @user = current_user + + @qr_code = build_qr_code end def unlink @@ -8,4 +20,16 @@ class Profiles::AccountsController < Profiles::ApplicationController current_user.identities.find_by(provider: provider).destroy redirect_to profile_account_path end + + private + + def build_qr_code + issuer = "#{issuer_host} | #{current_user.email}" + uri = current_user.otp_provisioning_uri(current_user.email, issuer: issuer) + RQRCode::render_qrcode(uri, :svg, level: :m, unit: 3) + end + + def issuer_host + Gitlab.config.gitlab.host + end end diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 9fa96084f94..3e8f6606358 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -5,114 +5,131 @@ .alert.alert-info Some options are unavailable for LDAP accounts -.account-page.prepend-top-default - .panel.panel-default.update-token - .panel-heading - Reset Private token - .panel-body - = form_for @user, url: reset_private_token_profile_path, method: :put do |f| - .data - %p - Your private token is used to access application resources without authentication. - %br - It can be used for atom feeds or the API. - %span.cred - Keep it secret! - - %p.cgray - - if current_user.private_token - = text_field_tag "token", current_user.private_token, class: "form-control" - - else - %span You don`t have one yet. Click generate to fix it. - - .form-actions - - if current_user.private_token - = f.submit 'Reset private token', data: { confirm: "Are you sure?" }, class: "btn btn-default" - - else - = f.submit 'Generate', class: "btn btn-default" - - .panel.panel-default - .panel-heading - Two-factor Authentication - .panel-body - - if current_user.two_factor_enabled? - .pull-right - = link_to 'Disable Two-factor Authentication', profile_two_factor_auth_path, method: :delete, class: 'btn btn-close btn-sm', - data: { confirm: 'Are you sure?' } - %p.text-success - %strong - Two-factor Authentication is enabled - %p - If you lose your recovery codes you can - %strong - = succeed ',' do - = link_to 'generate new ones', codes_profile_two_factor_auth_path, method: :post, data: { confirm: 'Are you sure?' } - invalidating all previous codes. - - - else - %p - Increase your account's security by enabling two-factor authentication (2FA). - %p - Each time you log in you’ll be required to provide your username and - password as usual, plus a randomly-generated code from your phone. - - .form-actions - = link_to 'Enable Two-factor Authentication', new_profile_two_factor_auth_path, class: 'btn btn-success' - - - if button_based_providers.any? - .panel.panel-default - .panel-heading - Connected Accounts - .panel-body - .oauth-buttons.append-bottom-10 - %p Click on icon to activate signin with one of the following services - - button_based_providers.each do |provider| - .btn-group - = link_to provider_image_tag(provider), user_omniauth_authorize_path(provider), method: :post, class: "btn btn-lg #{'active' if auth_active?(provider)}", "data-no-turbolink" => "true" - - - if auth_active?(provider) - = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'btn btn-lg' do - = icon('close') - - - if current_user.can_change_username? - .panel.panel-warning.update-username - .panel-heading - Change Username - .panel-body - = form_for @user, url: update_username_profile_path, method: :put, remote: true do |f| - %p - Changing your username will change path to all personal projects! - %div - .input-group - .input-group-addon - = "#{root_url}u/" - = f.text_field :username, required: true, class: 'form-control' -   - .loading-gif.hide - %p - = icon('spinner spin') - Saving new username - .form-actions - = f.submit 'Save username', class: "btn btn-warning" - - - if signup_enabled? - .panel.panel-danger.remove-account - .panel-heading - Remove account - .panel-body - - if @user.can_be_removed? - %p Deleting an account has the following effects: - %ul - %li All user content like authored issues, snippets, comments will be removed - - rp = current_user.personal_projects.count - - unless rp.zero? - %li #{pluralize rp, 'personal project'} will be removed and cannot be restored - .form-actions - = link_to 'Delete account', user_registration_path, data: { confirm: "REMOVE #{current_user.name}? Are you sure?" }, method: :delete, class: "btn btn-remove" +.row.prepend-top-default + .col-lg-3.profile-settings-sidebar + %h4.prepend-top-0 + Private Token + %p + Your private token is used to access application resources without authentication. + .col-lg-9 + = form_for @user, url: reset_private_token_profile_path, method: :put do |f| + %p.cgray + - if current_user.private_token + = label_tag "token", "Private token", class: "label-light" + = text_field_tag "token", current_user.private_token, class: "form-control" - else - - if @user.solo_owned_groups.present? - %p - Your account is currently an owner in these groups: - %strong #{@user.solo_owned_groups.map(&:name).join(', ')} - %p - You must transfer ownership or delete these groups before you can delete your account. + %span You don`t have one yet. Click generate to fix it. + %p.help-block + It can be used for atom feeds or the API. Keep it secret! + .prepend-top-default + - if current_user.private_token + = f.submit 'Reset private token', data: { confirm: "Are you sure?" }, class: "btn btn-default" + - else + = f.submit 'Generate', class: "btn btn-default" +%hr +.row.prepend-top-default + .col-lg-3.profile-settings-sidebar + %h4.prepend-top-0 + Two-factor Authentication + %p + Increase your account's security by enabling two-factor authentication (2FA). + .col-lg-9 + %p + Status: #{current_user.two_factor_enabled? ? 'enabled' : 'disabled'} + - if !current_user.two_factor_enabled? + %p + Download the Google Authenticator application from App Store for iOS or Google Play for Android and scan this code. + More information is available in the #{link_to('documentation', help_page_path('profile', 'two_factor_authentication'))}. + .row.append-bottom-10 + .col-md-3 + = raw @qr_code + .col-md-9 + .account-well + %p.prepend-top-0.append-bottom-0 + Can't scan the code? + %p.prepend-top-0.append-bottom-0 + To add the entry manually, provide the following details to the application on your phone. + %p.prepend-top-0.append-bottom-0 + Account: + = current_user.email + %p.prepend-top-0.append-bottom-0 + Key: + = current_user.otp_secret.scan(/.{4}/).join(' ') + %p.two-factor-new-manual-content + Time based: Yes + = form_for @user, url: "", method: :put do |f| + .form-group + = label_tag :pin_code, nil, class: "label-light" + = text_field_tag :pin_code, nil, class: "form-control", required: true + .prepend-top-default + = submit_tag 'Enable two-factor authentication', class: 'btn btn-success' +%hr +- if button_based_providers.any? + .row.prepend-top-default + .col-lg-3.profile-settings-sidebar + %h4.prepend-top-0 + Social sign-in + %p + Activate signin with one of the following services + .col-lg-9 + %label.label-light + Connected Accounts + %p Click on icon to activate signin with one of the following services + - button_based_providers.each do |provider| + .provider-btn-group + .provider-btn-image + = provider_image_tag(provider) + - if auth_active?(provider) + = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'provider-btn' do + Disconnect + - else + = link_to user_omniauth_authorize_path(provider), method: :post, class: "provider-btn #{'not-active' if !auth_active?(provider)}", "data-no-turbolink" => "true" do + Connect + %hr +- if current_user.can_change_username? + .row.prepend-top-default + .col-lg-3.profile-settings-sidebar + %h4.prepend-top-0.change-username-title + Change username + %p + Changing your username will change path to all personal projects! + .col-lg-9 + = form_for @user, url: update_username_profile_path, method: :put, remote: true, html: {class: "update-username"} do |f| + .form-group + = f.label :username, "Path", class: "label-light" + .input-group + .input-group-addon + = "#{root_url}u/" + = f.text_field :username, required: true, class: 'form-control' + .help-block + Current path: + = "#{root_url}u/#{current_user.username}" + .prepend-top-default + = f.button class: "btn btn-warning", type: "submit" do + = icon "spinner spin", class: "hidden loading-username" + Update username + %hr + +- if signup_enabled? + .row.prepend-top-default + .col-lg-3.profile-settings-sidebar + %h4.prepend-top-0.remove-account-title + Remove account + .col-lg-9 + - if @user.can_be_removed? + %p + Deleting an account has the following effects: + %ul + %li All user content like authored issues, snippets, comments will be removed + - rp = current_user.personal_projects.count + - unless rp.zero? + %li #{pluralize rp, 'personal project'} will be removed and cannot be restored + = link_to 'Delete account', user_registration_path, data: { confirm: "REMOVE #{current_user.name}? Are you sure?" }, method: :delete, class: "btn btn-remove" + - else + - if @user.solo_owned_groups.present? + %p + Your account is currently an owner in these groups: + %strong #{@user.solo_owned_groups.map(&:name).join(', ')} + %p + You must transfer ownership or delete these groups before you can delete your account. +.append-bottom-default From d5f145bf8be6c787044726a8e77f928a78d0e100 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 1 Mar 2016 15:58:00 +0000 Subject: [PATCH 11/18] Fixed failing tests --- app/views/profiles/accounts/show.html.haml | 2 +- features/steps/profile/profile.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 3e8f6606358..9fe064e5883 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -12,7 +12,7 @@ %p Your private token is used to access application resources without authentication. .col-lg-9 - = form_for @user, url: reset_private_token_profile_path, method: :put do |f| + = form_for @user, url: reset_private_token_profile_path, method: :put, html: {class: "private-token"} do |f| %p.cgray - if current_user.private_token = label_tag "token", "Private token", class: "label-light" diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb index 0c60328583a..d9436e9e21a 100644 --- a/features/steps/profile/profile.rb +++ b/features/steps/profile/profile.rb @@ -99,9 +99,9 @@ class Spinach::Features::Profile < Spinach::FeatureSteps end step 'I reset my token' do - page.within '.update-token' do + page.within '.private-token' do @old_token = @user.private_token - click_button "Reset" + click_button "Reset private token" end end From 9dc71dcaa081592c226f7c6be574d24f0fadec1e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 1 Mar 2016 16:22:04 +0000 Subject: [PATCH 12/18] Updated CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 1847c5193ab..c1c90903d5d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -41,6 +41,7 @@ v 8.5.3 - Sort starred projects on dashboard based on last activity by default - Show commit message in JIRA mention comment - Makes issue page and merge request page usable on mobile browsers. + - Improved UI for profile settings v 8.5.2 - Fix sidebar overlapping content when screen width was below 1200px From 4d4573268e055606c90dfe1c433b1a331e696378 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 2 Mar 2016 10:34:25 +0000 Subject: [PATCH 13/18] Fixed heading weight issue Moved colours to variables --- app/assets/stylesheets/framework/variables.scss | 12 ++++++++++++ app/assets/stylesheets/pages/profile.scss | 10 +++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 0fd2f74cf89..d55d5cfbca5 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -41,6 +41,12 @@ $btn-transparent-color: #8F8F8F; $ssh-key-icon-color: #8F8F8F; $ssh-key-icon-size: 18px; +$change-username-title-color: #FC6D26; +$remove-account-title-color: #F00; + +$provider-btn-group-border: #E5E5E5; +$provider-btn-not-active-color: #4688F1; + /* * Color schema */ @@ -98,8 +104,14 @@ $border-red-light: #F24F41; $border-red-normal: #D22852; $border-red-dark: #CA264F; +<<<<<<< 454832ace49f1b1742b380441817663051ba8ac8 $help-well-bg: #FAFAFA; $help-well-border: #E5E5E5; +======= +$account-well-bg: #FAFAFA; +$account-well-border: #E5E5E5; +$account-well-radius: 3px; +>>>>>>> Fixed heading weight issue /* header */ $light-grey-header: #faf9f9; diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index f8aeab6857f..40037c548f3 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -176,17 +176,17 @@ } .change-username-title { - color: #FC6D26; + color: $change-username-title-color; } .remove-account-title { - color: #F00; + color: $remove-account-title-color; } .provider-btn-group { display: inline-block; margin-right: 10px; - border: 1px solid #E5E5E5; + border: 1px solid $provider-btn-group-border; border-radius: 3px; &:last-child { @@ -197,7 +197,7 @@ .provider-btn-image { display: inline-block; padding: 5px 10px; - border-right: 1px solid #E5E5E5; + border-right: 1px solid $provider-btn-group-border; > img { width: 20px; @@ -212,6 +212,6 @@ background-color: $gray-light; &.not-active { - color: #4688F1; + color: $provider-btn-not-active-color; } } From c4baf2417fec7b8a65b6cafe515f73ea1a3474f4 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 2 Mar 2016 17:34:31 +0000 Subject: [PATCH 14/18] Fixed issue with 2fa not enabling Added in disable button for 2fa --- app/controllers/application_controller.rb | 2 +- .../profiles/accounts_controller.rb | 11 ++++++++ .../profiles/two_factor_auths_controller.rb | 26 +++---------------- app/views/profiles/accounts/show.html.haml | 8 +++++- 4 files changed, 22 insertions(+), 25 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1f55b18e0b1..fef79cefc92 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -240,7 +240,7 @@ class ApplicationController < ActionController::Base def check_2fa_requirement if two_factor_authentication_required? && current_user && !current_user.two_factor_enabled && !skip_two_factor? - redirect_to new_profile_two_factor_auth_path + redirect_to profile_account_path end end diff --git a/app/controllers/profiles/accounts_controller.rb b/app/controllers/profiles/accounts_controller.rb index 669fe05e5c7..bd827f2ab1b 100644 --- a/app/controllers/profiles/accounts_controller.rb +++ b/app/controllers/profiles/accounts_controller.rb @@ -1,4 +1,6 @@ class Profiles::AccountsController < Profiles::ApplicationController + skip_before_action :check_2fa_requirement + def show unless current_user.otp_secret current_user.otp_secret = User.generate_otp_secret(32) @@ -10,6 +12,15 @@ class Profiles::AccountsController < Profiles::ApplicationController current_user.save! if current_user.changed? + if two_factor_authentication_required? + if two_factor_grace_period_expired? + flash.now[:alert] = 'You must enable Two-factor Authentication for your account.' + else + grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours + flash.now[:alert] = "You must enable Two-factor Authentication for your account before #{l(grace_period_deadline)}." + end + end + @user = current_user @qr_code = build_qr_code diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index 8f83fdd02bc..65ecee0746e 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -2,26 +2,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController skip_before_action :check_2fa_requirement def new - unless current_user.otp_secret - current_user.otp_secret = User.generate_otp_secret(32) - end - - unless current_user.otp_grace_period_started_at && two_factor_grace_period - current_user.otp_grace_period_started_at = Time.current - end - - current_user.save! if current_user.changed? - - if two_factor_authentication_required? - if two_factor_grace_period_expired? - flash.now[:alert] = 'You must enable Two-factor Authentication for your account.' - else - grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours - flash.now[:alert] = "You must enable Two-factor Authentication for your account before #{l(grace_period_deadline)}." - end - end - - @qr_code = build_qr_code + redirect_to profile_account_path end def create @@ -32,10 +13,9 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController render 'create' else - @error = 'Invalid pin code' - @qr_code = build_qr_code + error = 'Invalid pin code' - render 'new' + redirect_to profile_account_path, flash: { error: error } end end diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 9fe064e5883..a5417655001 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -57,12 +57,18 @@ = current_user.otp_secret.scan(/.{4}/).join(' ') %p.two-factor-new-manual-content Time based: Yes - = form_for @user, url: "", method: :put do |f| + = form_for @user, url: profile_two_factor_auth_path, method: :post do |f| + - if flash[:error] + .alert.alert-danger + = flash[:error] .form-group = label_tag :pin_code, nil, class: "label-light" = text_field_tag :pin_code, nil, class: "form-control", required: true .prepend-top-default = submit_tag 'Enable two-factor authentication', class: 'btn btn-success' + - else + = link_to 'Disable Two-factor Authentication', profile_two_factor_auth_path, method: :delete, class: 'btn btn-danger', + data: { confirm: 'Are you sure?' } %hr - if button_based_providers.any? .row.prepend-top-default From 218f3e702a19b8682219d0d8932e7b34986c00d4 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 3 Mar 2016 09:28:33 +0000 Subject: [PATCH 15/18] Moved 2fa into separate view --- app/controllers/application_controller.rb | 2 +- .../profiles/accounts_controller.rb | 35 --------- .../profiles/two_factor_auths_controller.rb | 26 ++++++- app/views/profiles/accounts/show.html.haml | 28 +------ .../profiles/two_factor_auths/new.html.haml | 77 +++++++++---------- 5 files changed, 64 insertions(+), 104 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index fef79cefc92..1f55b18e0b1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -240,7 +240,7 @@ class ApplicationController < ActionController::Base def check_2fa_requirement if two_factor_authentication_required? && current_user && !current_user.two_factor_enabled && !skip_two_factor? - redirect_to profile_account_path + redirect_to new_profile_two_factor_auth_path end end diff --git a/app/controllers/profiles/accounts_controller.rb b/app/controllers/profiles/accounts_controller.rb index bd827f2ab1b..175afbf8425 100644 --- a/app/controllers/profiles/accounts_controller.rb +++ b/app/controllers/profiles/accounts_controller.rb @@ -1,29 +1,6 @@ class Profiles::AccountsController < Profiles::ApplicationController - skip_before_action :check_2fa_requirement - def show - unless current_user.otp_secret - current_user.otp_secret = User.generate_otp_secret(32) - end - - unless current_user.otp_grace_period_started_at && two_factor_grace_period - current_user.otp_grace_period_started_at = Time.current - end - - current_user.save! if current_user.changed? - - if two_factor_authentication_required? - if two_factor_grace_period_expired? - flash.now[:alert] = 'You must enable Two-factor Authentication for your account.' - else - grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours - flash.now[:alert] = "You must enable Two-factor Authentication for your account before #{l(grace_period_deadline)}." - end - end - @user = current_user - - @qr_code = build_qr_code end def unlink @@ -31,16 +8,4 @@ class Profiles::AccountsController < Profiles::ApplicationController current_user.identities.find_by(provider: provider).destroy redirect_to profile_account_path end - - private - - def build_qr_code - issuer = "#{issuer_host} | #{current_user.email}" - uri = current_user.otp_provisioning_uri(current_user.email, issuer: issuer) - RQRCode::render_qrcode(uri, :svg, level: :m, unit: 3) - end - - def issuer_host - Gitlab.config.gitlab.host - end end diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index 65ecee0746e..8f83fdd02bc 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -2,7 +2,26 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController skip_before_action :check_2fa_requirement def new - redirect_to profile_account_path + unless current_user.otp_secret + current_user.otp_secret = User.generate_otp_secret(32) + end + + unless current_user.otp_grace_period_started_at && two_factor_grace_period + current_user.otp_grace_period_started_at = Time.current + end + + current_user.save! if current_user.changed? + + if two_factor_authentication_required? + if two_factor_grace_period_expired? + flash.now[:alert] = 'You must enable Two-factor Authentication for your account.' + else + grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours + flash.now[:alert] = "You must enable Two-factor Authentication for your account before #{l(grace_period_deadline)}." + end + end + + @qr_code = build_qr_code end def create @@ -13,9 +32,10 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController render 'create' else - error = 'Invalid pin code' + @error = 'Invalid pin code' + @qr_code = build_qr_code - redirect_to profile_account_path, flash: { error: error } + render 'new' end end diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index a5417655001..6efd119f260 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -40,32 +40,8 @@ %p Download the Google Authenticator application from App Store for iOS or Google Play for Android and scan this code. More information is available in the #{link_to('documentation', help_page_path('profile', 'two_factor_authentication'))}. - .row.append-bottom-10 - .col-md-3 - = raw @qr_code - .col-md-9 - .account-well - %p.prepend-top-0.append-bottom-0 - Can't scan the code? - %p.prepend-top-0.append-bottom-0 - To add the entry manually, provide the following details to the application on your phone. - %p.prepend-top-0.append-bottom-0 - Account: - = current_user.email - %p.prepend-top-0.append-bottom-0 - Key: - = current_user.otp_secret.scan(/.{4}/).join(' ') - %p.two-factor-new-manual-content - Time based: Yes - = form_for @user, url: profile_two_factor_auth_path, method: :post do |f| - - if flash[:error] - .alert.alert-danger - = flash[:error] - .form-group - = label_tag :pin_code, nil, class: "label-light" - = text_field_tag :pin_code, nil, class: "form-control", required: true - .prepend-top-default - = submit_tag 'Enable two-factor authentication', class: 'btn btn-success' + .append-bottom-10 + = link_to 'Enable two-factor authentication', new_profile_two_factor_auth_path, class: 'btn btn-success' - else = link_to 'Disable Two-factor Authentication', profile_two_factor_auth_path, method: :delete, class: 'btn btn-danger', data: { confirm: 'Are you sure?' } diff --git a/app/views/profiles/two_factor_auths/new.html.haml b/app/views/profiles/two_factor_auths/new.html.haml index b2830aa0834..c82c5962191 100644 --- a/app/views/profiles/two_factor_auths/new.html.haml +++ b/app/views/profiles/two_factor_auths/new.html.haml @@ -1,41 +1,40 @@ - page_title 'Two-factor Authentication', 'Account' -%h2.page-title Two-factor Authentication (2FA) -%p - Download the Google Authenticator application from App Store for iOS or Google - Play for Android and scan this code. - - More information is available in the #{link_to('documentation', help_page_path('profile', 'two_factor_authentication'))}. - -%hr - -= form_tag profile_two_factor_auth_path, method: :post, class: 'form-horizontal two-factor-new' do |f| - - if @error - .alert.alert-danger - = @error - .form-group - .col-lg-2.col-lg-offset-2 - = raw @qr_code - .col-lg-7.col-lg-offset-1.manual-instructions - %h3 Can't scan the code? - - %p - To add the entry manually, provide the following details to the - application on your phone. - - %dl - %dt Account - %dd= current_user.email - %dl - %dt Key - %dd= current_user.otp_secret.scan(/.{4}/).join(' ') - %dl - %dt Time based - %dd Yes - .form-group - = label_tag :pin_code, nil, class: "control-label" - .col-lg-10 - = text_field_tag :pin_code, nil, class: "form-control", required: true, autofocus: true - .form-actions - = submit_tag 'Submit', class: 'btn btn-success' - = link_to 'Configure it later', skip_profile_two_factor_auth_path, :method => :patch, class: 'btn btn-cancel' if two_factor_skippable? +.row.prepend-top-default + .col-lg-3 + %h4.prepend-top-0 + Two-factor Authentication (2FA) + %p + Increase your account's security by enabling two-factor authentication (2FA). + .col-lg-9 + %p + Status: #{current_user.two_factor_enabled? ? 'enabled' : 'disabled'} + %p + Download the Google Authenticator application from App Store for iOS or Google Play for Android and scan this code. + More information is available in the #{link_to('documentation', help_page_path('profile', 'two_factor_authentication'))}. + .row.append-bottom-10 + .col-md-3 + = raw @qr_code + .col-md-9 + .account-well + %p.prepend-top-0.append-bottom-0 + Can't scan the code? + %p.prepend-top-0.append-bottom-0 + To add the entry manually, provide the following details to the application on your phone. + %p.prepend-top-0.append-bottom-0 + Account: + = current_user.email + %p.prepend-top-0.append-bottom-0 + Key: + = current_user.otp_secret.scan(/.{4}/).join(' ') + %p.two-factor-new-manual-content + Time based: Yes + = form_tag profile_two_factor_auth_path, method: :post do |f| + - if @error + .alert.alert-danger + = @error + .form-group + = label_tag :pin_code, nil, class: "label-light" + = text_field_tag :pin_code, nil, class: "form-control", required: true + .prepend-top-default + = submit_tag 'Enable two-factor authentication', class: 'btn btn-success' From a9d97838a399f153a23c7a9a9656a3a0ed912dcc Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 3 Mar 2016 16:59:33 +0000 Subject: [PATCH 16/18] Added back 2fa configure later button --- app/views/profiles/two_factor_auths/new.html.haml | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/profiles/two_factor_auths/new.html.haml b/app/views/profiles/two_factor_auths/new.html.haml index c82c5962191..5d342ef58e5 100644 --- a/app/views/profiles/two_factor_auths/new.html.haml +++ b/app/views/profiles/two_factor_auths/new.html.haml @@ -38,3 +38,4 @@ = text_field_tag :pin_code, nil, class: "form-control", required: true .prepend-top-default = submit_tag 'Enable two-factor authentication', class: 'btn btn-success' + = link_to 'Configure it later', skip_profile_two_factor_auth_path, :method => :patch, class: 'btn btn-cancel' if two_factor_skippable? From c9b62662cc569af613b94e41c6d8de503eb2749a Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 4 Mar 2016 09:03:50 +0000 Subject: [PATCH 17/18] Used standard variable colours --- app/assets/stylesheets/framework/variables.scss | 10 ---------- app/assets/stylesheets/pages/profile.scss | 8 ++++---- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index d55d5cfbca5..835364b2990 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -34,16 +34,12 @@ $error-exclamation-point: #E62958; $border-radius-default: 3px; $list-title-color: #333333; $list-text-color: #555555; -$profile-settings-link-color: $md-link-color; $btn-transparent-color: #8F8F8F; $ssh-key-icon-color: #8F8F8F; $ssh-key-icon-size: 18px; -$change-username-title-color: #FC6D26; -$remove-account-title-color: #F00; - $provider-btn-group-border: #E5E5E5; $provider-btn-not-active-color: #4688F1; @@ -104,14 +100,8 @@ $border-red-light: #F24F41; $border-red-normal: #D22852; $border-red-dark: #CA264F; -<<<<<<< 454832ace49f1b1742b380441817663051ba8ac8 $help-well-bg: #FAFAFA; $help-well-border: #E5E5E5; -======= -$account-well-bg: #FAFAFA; -$account-well-border: #E5E5E5; -$account-well-radius: 3px; ->>>>>>> Fixed heading weight issue /* header */ $light-grey-header: #faf9f9; diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 40037c548f3..248c56e459d 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -19,7 +19,7 @@ .account-btn-link, .profile-settings-sidebar a { - color: $profile-settings-link-color; + color: $md-link-color; } .oauth-buttons { @@ -171,16 +171,16 @@ .profile-settings-content { a { - color: $profile-settings-link-color; + color: $md-link-color; } } .change-username-title { - color: $change-username-title-color; + color: $gl-warning; } .remove-account-title { - color: $remove-account-title-color; + color: $gl-danger; } .provider-btn-group { From 6690d8e25b0da2fa83e218d6f2411df7cec9ef32 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 11 Mar 2016 10:03:35 +0000 Subject: [PATCH 18/18] Correctly uses a variable with the text --- app/assets/javascripts/notes.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index eff8beb97e9..884abff120f 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -610,7 +610,7 @@ class @Notes reopenbtn.text(reopentext) if closebtn.text() isnt closetext - closebtn.text(closebtn.data('original-text')) + closebtn.text(closetext) if reopenbtn.is(':not(.btn-comment-and-reopen)') reopenbtn.removeClass('btn-comment-and-reopen')