Merge branch 'refactor/profile_account' of /home/git/repositories/gitlab/gitlabhq

app/assets/stylesheets/common.scss

@@ -271,7 +271,6 @@ li.note {
 }
 
 .oauth_select_holder {
-  padding: 20px;
   img {
     padding: 5px;
     margin-right: 10px;

app/controllers/profiles/accounts_controller.rb

@@ -0,0 +1,7 @@
+class Profiles::AccountsController < ApplicationController
+  layout "profile"
+
+  def show
+    @user = current_user
+  end
+end

app/controllers/profiles/passwords_controller.rb

@@ -1,10 +1,11 @@
 class Profiles::PasswordsController < ApplicationController
-  layout 'navless'
+  layout :determine_layout
 
-  skip_before_filter :check_password_expiration
+  skip_before_filter :check_password_expiration, only: [:new, :create]
 
   before_filter :set_user
   before_filter :set_title
+  before_filter :authorize_change_password!
 
   def new
   end
@@ -26,6 +27,32 @@ class Profiles::PasswordsController < ApplicationController
     end
   end
 
+  def edit
+  end
+
+  def update
+    password_attributes = params[:user].select do |key, value|
+      %w(password password_confirmation).include?(key.to_s)
+    end
+
+    unless @user.valid_password?(params[:user][:current_password])
+      redirect_to edit_profile_password_path, alert: 'You must provide a valid current password'
+      return
+    end
+
+    if @user.update_attributes(password_attributes)
+      flash[:notice] = "Password was successfully updated. Please login with it"
+      redirect_to new_user_session_path
+    else
+      render 'edit'
+    end
+  end
+
+  def reset
+    current_user.send_reset_password_instructions
+    redirect_to edit_profile_password_path, notice: 'We sent you an email with reset password instructions'
+  end
+
   private
 
   def set_user
@@ -35,4 +62,16 @@ class Profiles::PasswordsController < ApplicationController
   def set_title
     @title = "New password"
   end
+
+  def determine_layout
+    if [:new, :create].include?(action_name.to_sym)
+      'navless'
+    else
+      'profile'
+    end
+  end
+
+  def authorize_change_password!
+    return render_404 if @user.ldap_user?
+  end
 end

app/controllers/profiles_controller.rb

@@ -2,7 +2,6 @@ class ProfilesController < ApplicationController
   include ActionView::Helpers::SanitizeHelper
 
   before_filter :user
-  before_filter :authorize_change_password!, only: :update_password
   before_filter :authorize_change_username!, only: :update_username
 
   layout 'profile'
@@ -13,9 +12,6 @@ class ProfilesController < ApplicationController
   def design
   end
 
-  def account
-  end
-
   def update
     if @user.update_attributes(params[:user])
       flash[:notice] = "Profile was successfully updated"
@@ -29,33 +25,12 @@ class ProfilesController < ApplicationController
     end
   end
 
-  def token
-  end
-
-  def update_password
-    password_attributes = params[:user].select do |key, value|
-      %w(password password_confirmation).include?(key.to_s)
-    end
-
-    unless @user.valid_password?(params[:user][:current_password])
-      redirect_to account_profile_path, alert: 'You must provide a valid current password'
-      return
-    end
-
-    if @user.update_attributes(password_attributes)
-      flash[:notice] = "Password was successfully updated. Please login with it"
-      redirect_to new_user_session_path
-    else
-      render 'account'
-    end
-  end
-
   def reset_private_token
     if current_user.reset_authentication_token!
       flash[:notice] = "Token was successfully updated"
     end
 
-    redirect_to account_profile_path
+    redirect_to profile_account_path
   end
 
   def history
@@ -76,10 +51,6 @@ class ProfilesController < ApplicationController
     @user = current_user
   end
 
-  def authorize_change_password!
-    return render_404 if @user.ldap_user?
-  end
-
   def authorize_change_username!
     return render_404 unless @user.can_change_username?
   end

app/views/layouts/nav/_profile.html.haml

@@ -2,8 +2,11 @@
   = nav_link(path: 'profiles#show', html_options: {class: 'home'}) do
     = link_to profile_path, title: "Profile" do
       %i.icon-home
-  = nav_link(path: 'profiles#account') do
+  = nav_link(controller: :accounts) do
-    = link_to "Account", account_profile_path
+    = link_to "Account", profile_account_path
+  - unless current_user.ldap_user?
+    = nav_link(controller: :passwords) do
+      = link_to "Password", edit_profile_password_path
   = nav_link(controller: :notifications) do
     = link_to "Notifications", profile_notifications_path
   = nav_link(controller: :keys) do

app/views/profiles/account.html.haml

@@ -1,141 +0,0 @@
-%h3.page-title
-  Account settings
-%p.light
-  You can change your password, username and private token here.
-  - if current_user.ldap_user?
-    Some options are unavailable for LDAP accounts
-%hr
-
-
-.row
-  .span2
-    %ul.nav.nav-pills.nav-stacked.nav-stacked-menu
-      %li.active
-        = link_to '#tab-token', 'data-toggle' => 'tab' do
-          Private Token
-      %li
-        = link_to '#tab-password', 'data-toggle' => 'tab' do
-          Password
-
-      - if show_profile_social_tab?
-        %li
-          = link_to '#tab-social', 'data-toggle' => 'tab' do
-            Social Accounts
-
-      - if show_profile_username_tab?
-        %li
-          = link_to '#tab-username', 'data-toggle' => 'tab' do
-            Change Username
-
-      - if show_profile_remove_tab?
-        %li
-          = link_to '#tab-remove', 'data-toggle' => 'tab' do
-            Remove Account
-  .span10
-    .tab-content
-      .tab-pane.active#tab-token
-        %fieldset.update-token
-          %legend
-            Private token
-            %span.cred.pull-right
-              keep it secret!
-          %div
-            = form_for @user, url: reset_private_token_profile_path, method: :put do |f|
-              .data
-                %p.slead
-                  Your private token is used to access application resources without authentication.
-                  %br
-                  It can be used for atom feeds or the API.
-                %p.cgray
-                  - if current_user.private_token
-                    = text_field_tag "token", current_user.private_token, class: "input-xxlarge large_text input-xpadding"
-                    = f.submit 'Reset', confirm: "Are you sure?", class: "btn btn-primary btn-build-token"
-                  - else
-                    %span You don`t have one yet. Click generate to fix it.
-                    = f.submit 'Generate', class: "btn success btn-build-token"
-
-      .tab-pane#tab-password
-        %fieldset.update-password
-          %legend Password
-          - if current_user.ldap_user?
-            %h3.nothing_here_message Not available for LDAP user
-          - else
-            = form_for @user, url: update_password_profile_path, method: :put do |f|
-              %div
-                %p.slead
-                  You must provide current password in order to change it.
-                  %br
-                  After a successful password update you will be redirected to login page where you should login with your new password
-                -if @user.errors.any?
-                  .alert.alert-error
-                    %ul
-                      - @user.errors.full_messages.each do |msg|
-                        %li= msg
-                .control-group
-                  = f.label :current_password, class: 'cgreen'
-                  .controls= f.password_field :current_password, required: true
-                .control-group
-                  = f.label :password, 'New password'
-                  .controls= f.password_field :password, required: true
-                .control-group
-                  = f.label :password_confirmation
-                  .controls
-                    = f.password_field :password_confirmation, required: true
-                .control-group
-                  .controls
-                    = f.submit 'Save password', class: "btn btn-save"
-
-      - if show_profile_social_tab?
-        .tab-pane#tab-social
-          %fieldset
-            %legend Social Accounts
-            .oauth_select_holder
-              %p.hint Tip: Click on icon to activate signin with one of the following services
-              - enabled_social_providers.each do |provider|
-                %span{class: oauth_active_class(provider) }
-                  = link_to authbutton(provider, 32), omniauth_authorize_path(User, provider)
-
-      - if show_profile_username_tab?
-        .tab-pane#tab-username
-          %fieldset.update-username
-            %legend
-              Username
-              %small.cred.pull-right
-                Changing your username can have unintended side effects!
-            = form_for @user, url: update_username_profile_path,  method: :put, remote: true do |f|
-              %div
-              .control-group
-                = f.label :username
-                .controls
-                  = f.text_field :username, required: true
-                   
-                  %span.loading-gif.hide= image_tag "ajax_loader.gif"
-                  %span.update-success.cgreen.hide
-                    %i.icon-ok
-                    Saved
-                  %span.update-failed.cred.hide
-                    %i.icon-remove
-                    Failed
-                  %ul.cred
-                    %li This will change the web URL for personal projects.
-                    %li This will change the git path to repositories for personal projects.
-                .controls
-                  = f.submit 'Save username', class: "btn btn-save"
-
-      - if show_profile_remove_tab?
-        .tab-pane#tab-remove
-          %fieldset.remove-account
-            %legend
-              Remove account
-            %div
-              %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
-                - if current_user.solo_owned_groups.present?
-                  %li
-                    Next groups will be abandoned. You should transfer or remove them:
-                    %strong #{current_user.solo_owned_groups.map(&:name).join(', ')}
-              = link_to 'Delete account', user_registration_path, confirm: "REMOVE #{current_user.name}? Are you sure?", method: :delete, class: "btn btn-remove"

app/views/profiles/accounts/show.html.haml

@@ -0,0 +1,73 @@
+%h3.page-title
+  Account settings
+%p.light
+  You can change your username and private token here.
+  - if current_user.ldap_user?
+    Some options are unavailable for LDAP accounts
+%hr
+
+
+%div
+  %fieldset.update-token
+    %legend
+      Private token
+    %div
+      = 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: "input-xlarge input-xpadding pull-left"
+              = f.submit 'Reset', confirm: "Are you sure?", class: "btn btn-primary btn-build-token prepend-left-10"
+            - else
+              %span You don`t have one yet. Click generate to fix it.
+              = f.submit 'Generate', class: "btn success btn-build-token"
+
+
+  - if show_profile_social_tab?
+    %fieldset
+      %legend Social Accounts
+      .oauth_select_holder
+        %p Click on icon to activate signin with one of the following services
+        - enabled_social_providers.each do |provider|
+          %span{class: oauth_active_class(provider) }
+            = link_to authbutton(provider, 32), omniauth_authorize_path(User, provider)
+
+  - if show_profile_username_tab?
+    %fieldset.update-username
+      %legend
+        Username
+      = form_for @user, url: update_username_profile_path,  method: :put, remote: true do |f|
+        %p
+          Changing your username will change path to all personl projects!
+        %div
+          = f.text_field :username, required: true, class: 'input-xlarge input-xpadding'
+           
+          %span.loading-gif.hide= image_tag "ajax_loader.gif"
+        %p.light
+          = user_url(@user)
+        %div
+          = f.submit 'Save username', class: "btn btn-save"
+
+  - if show_profile_remove_tab?
+    %fieldset.remove-account
+      %legend
+        Remove account
+      %div
+        %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
+          - if current_user.solo_owned_groups.present?
+            %li
+              Next groups will be abandoned. You should transfer or remove them:
+              %strong #{current_user.solo_owned_groups.map(&:name).join(', ')}
+        = link_to 'Delete account', user_registration_path, confirm: "REMOVE #{current_user.name}? Are you sure?", method: :delete, class: "btn btn-remove"

app/views/profiles/passwords/edit.html.haml

@@ -0,0 +1,32 @@
+%h3.page-title Password
+%p.light
+  Change your password or recover your current one.
+%hr
+.update-password
+  = form_for @user, url: profile_password_path, method: :put do |f|
+    %div
+      %p.slead
+        You must provide current password in order to change it.
+        %br
+        After a successful password update you will be redirected to login page where you should login with your new password
+      -if @user.errors.any?
+        .alert.alert-error
+          %ul
+            - @user.errors.full_messages.each do |msg|
+              %li= msg
+      .control-group
+        = f.label :current_password
+        .controls
+          = f.password_field :current_password, required: true
+          %div
+            = link_to "Forgot your password?", reset_profile_password_path, method: :put
+
+      .control-group
+        = f.label :password, 'New password'
+        .controls= f.password_field :password, required: true
+      .control-group
+        = f.label :password_confirmation
+        .controls
+          = f.password_field :password_confirmation, required: true
+      .form-actions
+        = f.submit 'Save password', class: "btn btn-save"

app/views/profiles/update_username.js.haml

@@ -1,6 +1,6 @@
 - if @user.valid?
   :plain
-    $('.update-username .update-success').show();
+    new Flash("Username sucessfully changed", "notice")
 - else
   :plain
-    $('.update-username .update-failed').show();
+    new Flash("Username change failed - #{@user.errors.full_messages.first}", "alert")

config/routes.rb

@@ -99,19 +99,21 @@ Gitlab::Application.routes.draw do
   #
   resource :profile, only: [:show, :update] do
     member do
-      get :account
       get :history
-      get :token
       get :design
 
-      put :update_password
       put :reset_private_token
       put :update_username
     end
 
     scope module: :profiles do
+      resource :account, only: [:show, :update]
       resource :notifications, only: [:show, :update]
-      resource :password, only: [:new, :create]
+      resource :password, only: [:new, :create, :edit, :update] do
+        member do
+          put :reset
+        end
+      end
       resources :keys
       resources :groups, only: [:index] do
         member do

features/profile/profile.feature

@@ -12,13 +12,13 @@ Feature: Profile
     And I should see new contact info
 
   Scenario: I change my password without old one
-    Given I visit profile account page
+    Given I visit profile password page
     When I try change my password w/o old one
     Then I should see a missing password error message
-    And I should be redirected to account page
+    And I should be redirected to password page
 
   Scenario: I change my password
-    Given I visit profile account page
+    Given I visit profile password page
     Then I change my password
     And I should be redirected to sign in page
 
@@ -30,13 +30,13 @@ Feature: Profile
   Scenario: My password is expired
     Given my password is expired
     And I am not an ldap user
-    And I visit profile account page
+    Given I visit profile password page
     Then I redirected to expired password page
     And I submit new password
     And I redirected to sign in page
 
   Scenario: I unsuccessfully change my password
-    Given I visit profile account page
+    Given I visit profile password page
     When I unsuccessfully change my password
     Then I should see a password error message
 

features/steps/profile/profile.rb

@@ -133,8 +133,12 @@ class Profile < Spinach::FeatureSteps
     current_path.should == new_user_session_path
   end
 
+  step 'I should be redirected to password page' do
+    current_path.should == edit_profile_password_path
+  end
+
   step 'I should be redirected to account page' do
-    current_path.should == account_profile_path
+    current_path.should == profile_account_path
   end
 
   step 'I click on my profile picture' do

features/steps/shared/paths.rb

@@ -65,8 +65,12 @@ module SharedPaths
     visit profile_path
   end
 
+  step 'I visit profile password page' do
+    visit edit_profile_password_path
+  end
+
   step 'I visit profile account page' do
-    visit account_profile_path
+    visit profile_account_path
   end
 
   step 'I visit profile SSH keys page' do

spec/features/profile_spec.rb

@@ -12,7 +12,7 @@ describe "Profile account page" do
   describe "when signup is enabled" do
     before do
       Gitlab.config.gitlab.stub(:signup_enabled).and_return(true)
-      visit account_profile_path
+      visit profile_account_path
     end
 
     it { page.should have_content("Remove account") }
@@ -26,12 +26,12 @@ describe "Profile account page" do
   describe "when signup is disabled" do
     before do
       Gitlab.config.gitlab.stub(:signup_enabled).and_return(false)
-      visit account_profile_path
+      visit profile_account_path
     end
 
     it "should not have option to remove account" do
       page.should_not have_content("Remove account")
-      current_path.should == account_profile_path
+      current_path.should == profile_account_path
     end
   end
 end

spec/features/security/profile_access_spec.rb

@@ -29,7 +29,7 @@ describe "Users Security" do
     end
 
     describe "GET /profile/account" do
-      subject { account_profile_path }
+      subject { profile_account_path }
 
       it { should be_allowed_for @u1 }
       it { should be_allowed_for :admin }

spec/routing/routing_spec.rb

@@ -128,7 +128,7 @@ end
 #              profile_update PUT    /profile/update(.:format)              profile#update
 describe ProfilesController, "routing" do
   it "to #account" do
-    get("/profile/account").should route_to('profiles#account')
+    get("/profile/account").should route_to('profiles/accounts#show')
   end
 
   it "to #history" do