diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 4bd732fc1..29e4e4b07 100644 --- a/app/controllers/groups_controller.rb +++ b/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 diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 7fb4148ed..46c61ea0f 100644 --- a/app/controllers/issues_controller.rb +++ b/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? diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index 7addff78c..e234848d0 100644 --- a/app/controllers/timelog_controller.rb +++ b/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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f19cd7831..0354d165d 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/app/models/changeset.rb b/app/models/changeset.rb index 3bd26b111..063a4a48c 100644 --- a/app/models/changeset.rb +++ b/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? diff --git a/app/models/member.rb b/app/models/member.rb index 94751efb2..d840cfc3e 100644 --- a/app/models/member.rb +++ b/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 diff --git a/test/functional/account_controller_test.rb b/test/functional/account_controller_test.rb index 2a8c9c66b..1d66d01d3 100644 --- a/test/functional/account_controller_test.rb +++ b/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' diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index d178f8f85..640ce8685 100644 --- a/test/functional/users_controller_test.rb +++ b/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 diff --git a/vendor/plugins/open_id_authentication/lib/open_id_authentication.rb b/vendor/plugins/open_id_authentication/lib/open_id_authentication.rb index 22481136e..70418fde7 100644 --- a/vendor/plugins/open_id_authentication/lib/open_id_authentication.rb +++ b/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")