Merge branch 'master' of git://github.com/edavis10/redmine

app/controllers/groups_controller.rb

@@ -141,14 +141,22 @@ class GroupsController < ApplicationController
     @membership = Member.edit_membership(params[:membership_id], params[:membership], @group)
     @membership.save if request.post?
     respond_to do |format|
-       format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'memberships' }
-       format.js { 
-         render(:update) {|page| 
-           page.replace_html "tab-content-memberships", :partial => 'groups/memberships'
-           page.visual_effect(:highlight, "member-#{@membership.id}")
-         }
-       }
-     end
+      if @membership.valid?
+        format.html { redirect_to :controller => 'groups', :action => 'edit', :id => @group, :tab => 'memberships' }
+        format.js {
+          render(:update) {|page|
+            page.replace_html "tab-content-memberships", :partial => 'groups/memberships'
+            page.visual_effect(:highlight, "member-#{@membership.id}")
+          }
+        }
+      else
+        format.js {
+          render(:update) {|page|
+            page.alert(l(:notice_failed_to_save_members, :errors => @membership.errors.full_messages.join(', ')))
+          }
+        }
+      end
+    end
   end
   
   def destroy_membership

app/controllers/issues_controller.rb

@@ -251,13 +251,14 @@ class IssuesController < ApplicationController
   end
 
   def move
-    @issues.sort!
-    @copy = params[:copy_options] && params[:copy_options][:copy]
-    @allowed_projects = Issue.allowed_target_projects_on_move
-    @target_project = @allowed_projects.detect {|p| p.id.to_s == params[:new_project_id]} if params[:new_project_id]
-    @target_project ||= @project    
-    @trackers = @target_project.trackers
-    @available_statuses = Workflow.available_statuses(@project)
+    prepare_for_issue_move
+    render :layout => false if request.xhr?
+  end
+
+  # TODO: more descriptive name? move to separate controller like IssueMovesController?
+  def perform_move
+    prepare_for_issue_move
+
     if request.post?
       new_tracker = params[:new_tracker_id].blank? ? nil : @target_project.trackers.find_by_id(params[:new_tracker_id])
       unsaved_issue_ids = []
@@ -285,12 +286,6 @@ class IssuesController < ApplicationController
       end
       return
     end
-    render :layout => false if request.xhr?
-  end
-
-  # TODO: more descriptive name? move to separate controller like IssueMovesController?
-  def perform_move
-    move
   end
   
   def destroy
@@ -467,6 +462,16 @@ private
     @allowed_statuses = @issue.new_statuses_allowed_to(User.current, true)
   end
 
+  def prepare_for_issue_move
+    @issues.sort!
+    @copy = params[:copy_options] && params[:copy_options][:copy]
+    @allowed_projects = Issue.allowed_target_projects_on_move
+    @target_project = @allowed_projects.detect {|p| p.id.to_s == params[:new_project_id]} if params[:new_project_id]
+    @target_project ||= @project    
+    @trackers = @target_project.trackers
+    @available_statuses = Workflow.available_statuses(@project)
+  end
+
   def set_flash_from_bulk_issue_save(issues, unsaved_issue_ids)
     if unsaved_issue_ids.empty?
       flash[:notice] = l(:notice_successful_update) unless issues.empty?

app/controllers/timelog_controller.rb

@@ -55,8 +55,7 @@ class TimelogController < ApplicationController
 
       sql = "SELECT #{sql_select}, tyear, tmonth, tweek, spent_on, SUM(hours) AS hours"
       sql << " FROM #{TimeEntry.table_name}"
-      sql << " LEFT JOIN #{Issue.table_name} ON #{TimeEntry.table_name}.issue_id = #{Issue.table_name}.id"
-      sql << " LEFT JOIN #{Project.table_name} ON #{TimeEntry.table_name}.project_id = #{Project.table_name}.id"
+      sql << time_report_joins
       sql << " WHERE"
       sql << " (%s) AND" % sql_condition
       sql << " (spent_on BETWEEN '%s' AND '%s')" % [ActiveRecord::Base.connection.quoted_date(@from), ActiveRecord::Base.connection.quoted_date(@to)]
@@ -314,4 +313,12 @@ private
     call_hook(:controller_timelog_available_criterias, { :available_criterias => @available_criterias, :project => @project })
     @available_criterias
   end
+
+  def time_report_joins
+    sql = ''
+    sql << " LEFT JOIN #{Issue.table_name} ON #{TimeEntry.table_name}.issue_id = #{Issue.table_name}.id"
+    sql << " LEFT JOIN #{Project.table_name} ON #{TimeEntry.table_name}.project_id = #{Project.table_name}.id"
+    call_hook(:controller_timelog_time_report_joins, {:sql => sql} )
+    sql
+  end
 end

app/controllers/users_controller.rb

@@ -53,10 +53,8 @@ class UsersController < ApplicationController
     @user = User.find(params[:id])
     @custom_values = @user.custom_values
     
-    # show only public projects and private projects that the logged in user is also a member of
-    @memberships = @user.memberships.select do |membership|
-      membership.project.is_public? || (User.current.member_of?(membership.project))
-    end
+    # show projects based on current user visibility
+    @memberships = @user.memberships.all(:conditions => Project.visible_by(User.current))
     
     events = Redmine::Activity::Fetcher.new(User.current, :author => @user).events(nil, nil, :limit => 10)
     @events_by_day = events.group_by(&:event_date)
@@ -123,14 +121,22 @@ class UsersController < ApplicationController
     @membership = Member.edit_membership(params[:membership_id], params[:membership], @user)
     @membership.save if request.post?
     respond_to do |format|
-       format.html { redirect_to :controller => 'users', :action => 'edit', :id => @user, :tab => 'memberships' }
-       format.js { 
-         render(:update) {|page| 
-           page.replace_html "tab-content-memberships", :partial => 'users/memberships'
-           page.visual_effect(:highlight, "member-#{@membership.id}")
-         }
-       }
-     end
+      if @membership.valid?
+        format.html { redirect_to :controller => 'users', :action => 'edit', :id => @user, :tab => 'memberships' }
+        format.js {
+          render(:update) {|page|
+            page.replace_html "tab-content-memberships", :partial => 'users/memberships'
+            page.visual_effect(:highlight, "member-#{@membership.id}")
+          }
+        }
+      else
+        format.js {
+          render(:update) {|page|
+            page.alert(l(:notice_failed_to_save_members, :errors => @membership.errors.full_messages.join(', ')))
+          }
+        }
+      end
+    end
   end
   
   def destroy_membership

app/models/changeset.rb

@@ -76,7 +76,6 @@ class Changeset < ActiveRecord::Base
   def after_create
     scan_comment_for_issue_ids
   end
-  require 'pp'
   
   def scan_comment_for_issue_ids
     return if comments.blank?

app/models/member.rb

@@ -82,7 +82,7 @@ class Member < ActiveRecord::Base
   protected
   
   def validate
-    errors.add_to_base "Role can't be blank" if member_roles.empty? && roles.empty?
+    errors.add_on_empty :role if member_roles.empty? && roles.empty?
   end
   
   private

test/functional/account_controller_test.rb

@@ -67,6 +67,13 @@ class AccountControllerTest < ActionController::TestCase
     assert_redirected_to 'my/page'
   end
 
+  def test_login_with_invalid_openid_provider
+    Setting.self_registration = '0'
+    Setting.openid = '1'
+    post :login, :openid_url => 'http;//openid.example.com/good_user'
+    assert_redirected_to home_url
+  end
+  
   def test_login_with_openid_for_existing_non_active_user
     Setting.self_registration = '2'
     Setting.openid = '1'

test/functional/users_controller_test.rb

@@ -96,6 +96,16 @@ class UsersControllerTest < ActionController::TestCase
     assert_response 200
     assert_not_nil assigns(:user)
   end
+  
+  def test_show_displays_memberships_based_on_project_visibility
+    @request.session[:user_id] = 1
+    get :show, :id => 2
+    assert_response :success
+    memberships = assigns(:memberships)
+    assert_not_nil memberships
+    project_ids = memberships.map(&:project_id)
+    assert project_ids.include?(2) #private project admin can see
+  end
 
   def test_edit
     ActionMailer::Base.deliveries.clear

vendor/plugins/open_id_authentication/lib/open_id_authentication.rb

@@ -89,7 +89,7 @@ module OpenIdAuthentication
 
       begin
         uri = URI.parse(identifier)
-        uri.scheme = uri.scheme.downcase  # URI should do this
+        uri.scheme = uri.scheme.downcase if uri.scheme # URI should do this
         identifier = uri.normalize.to_s
       rescue URI::InvalidURIError
         raise InvalidOpenId.new("#{identifier} is not an OpenID identifier")