jro_apps/redmine Commit - r411:e227b9297252

Various code cleaning, mainly on User, Permission and IssueStatus models....

Jean-Philippe Lang -

r411:e227b9297252

parent child

Context file:

r411:e227b9297252

Collapse all files

app/controllers/application.rb +2 -2

                  # admin is always authorized
                  return true if self.logged_in_user.admin?
                  # if not admin, check membership permission
-                 @user_membership ||= Member.find(:first, :conditions => ["user_id=? and project_id=?", self.logged_in_user.id, @project.id])
-                 if @user_membership and Permission.allowed_to_role( "%s/%s" % [ ctrl, action ], @user_membership.role_id )
+                 @user_membership ||= logged_in_user.role_for_project(@project)
+                 if @user_membership and Permission.allowed_to_role( "%s/%s" % [ ctrl, action ], @user_membership )
                    return true
                  end
                  render :nothing => true, :status => 403

app/controllers/feeds_controller.rb +1 -1

                    # project feed
                    # check if project is public or if the user is a member
                    @project = Project.find(params[:project_id])
-                   render(:nothing => true, :status => 403) and return false unless @project.is_public? || (@user && @user.role_for_project(@project.id))
+                   render(:nothing => true, :status => 403) and return false unless @project.is_public? || (@user && @user.role_for_project(@project))
                    scope = ["#{Project.table_name}.id=?", params[:project_id].to_i]
                  else
                    # global feed

app/controllers/issues_controller.rb +2 -12

                include IfpdfHelper
                def show
-                 @status_options = ([@issue.status] + @issue.status.workflows.find(:all, :order => 'position', :include => :new_status, :conditions => ["role_id=? and tracker_id=?", self.logged_in_user.role_for_project(@project.id), @issue.tracker.id]).collect{ |w| w.new_status }) if self.logged_in_user
+                 @status_options = @issue.status.find_new_statuses_allowed_to(logged_in_user.role_for_project(@project), @issue.tracker) if logged_in_user
                  @custom_values = @issue.custom_values.find(:all, :include => :custom_field)
                  @journals_count = @issue.journals.count
                  @journals = @issue.journals.find(:all, :include => [:user, :details], :limit => 15, :order => "#{Journal.table_name}.created_on desc")
                def add_note
                  unless params[:notes].empty?
                    journal = @issue.init_journal(self.logged_in_user, params[:notes])
-                   #@history = @issue.histories.build(params[:history])
-                   #@history.author_id = self.logged_in_user.id if self.logged_in_user
-                   #@history.status = @issue.status
                    if @issue.save
                      flash[:notice] = l(:notice_successful_update)
                      Mailer.deliver_issue_edit(journal) if Permission.find_by_controller_and_action(params[:controller], params[:action]).mail_enabled?
                end
                def change_status
-                 #@history = @issue.histories.build(params[:history])
-                 @status_options = ([@issue.status] + @issue.status.workflows.find(:all, :order => 'position', :include => :new_status, :conditions => ["role_id=? and tracker_id=?", self.logged_in_user.role_for_project(@project.id), @issue.tracker.id]).collect{ |w| w.new_status }) if self.logged_in_user
+                 @status_options = @issue.status.find_new_statuses_allowed_to(logged_in_user.role_for_project(@project), @issue.tracker) if logged_in_user
                  @new_status = IssueStatus.find(params[:new_status_id])
                  if params[:confirm]
                    begin
-                     #@history.author_id = self.logged_in_user.id if self.logged_in_user
-                     #@issue.status = @history.status
-                     #@issue.fixed_version_id = (params[:issue][:fixed_version_id])
-                     #@issue.assigned_to_id = (params[:issue][:assigned_to_id])
-                     #@issue.done_ratio = (params[:issue][:done_ratio])
-                     #@issue.lock_version = (params[:issue][:lock_version])
                      journal = @issue.init_journal(self.logged_in_user, params[:notes])
                      @issue.status = @new_status
                      if @issue.update_attributes(params[:issue])

app/controllers/projects_controller.rb +2 -3

                  default_status = IssueStatus.default
                  @issue = Issue.new(:project => @project, :tracker => @tracker, :status => default_status)
-                 @allowed_statuses = [default_status] + default_status.workflows.find(:all, :order => 'position', :include => :new_status, :conditions => ["role_id=? and tracker_id=?", self.logged_in_user.role_for_project(@project.id), @issue.tracker.id]).collect{ |w| w.new_status }
+                 @allowed_statuses = default_status.find_new_statuses_allowed_to(logged_in_user.role_for_project(@project), @issue.tracker) if logged_in_user
                  if request.get?
                    @issue.start_date = Date.today
                    @custom_values = @project.custom_fields_for_issues(@tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue) }
                  redirect_to :action => 'list_issues', :id => @project and return unless @issues
                  @projects = []
                  # find projects to which the user is allowed to move the issue
-                 @logged_in_user.memberships.each {|m| @projects << m.project if Permission.allowed_to_role("projects/move_issues", m.role_id)}
+                 @logged_in_user.memberships.each {|m| @projects << m.project if Permission.allowed_to_role("projects/move_issues", m.role)}
                  # issue can be moved to any tracker
                  @trackers = Tracker.find(:all)
                  if request.post? and params[:new_project_id] and params[:new_tracker_id]

app/helpers/application_helper.rb +1 -1

                    return true
                  end
                  # check if user is authorized
-                 if @logged_in_user and (@logged_in_user.admin? or Permission.allowed_to_role( "%s/%s" % [ controller, action ], @logged_in_user.role_for_project(@project.id)  )  )
+                 if @logged_in_user and (@logged_in_user.admin? or Permission.allowed_to_role( "%s/%s" % [ controller, action ], @logged_in_user.role_for_project(@project)  )  )
                    return true
                  end
                  return false

app/models/issue_status.rb +12 -5

                end
                # Returns an array of all statuses the given role can switch to
+               # Uses association cache when called more than one time
                def new_statuses_allowed_to(role, tracker)
-                 statuses = []
-                 for workflow in self.workflows
-                   statuses << workflow.new_status if workflow.role_id == role.id and workflow.tracker_id == tracker.id
-                 end unless role.nil? or tracker.nil?
-                 statuses
+                 new_statuses = [self] + workflows.select {|w| w.role_id == role.id && w.tracker_id == tracker.id}.collect{|w| w.new_status}
+                 new_statuses.sort{|x, y| x.position <=> y.position }
+               end
+               # Same thing as above but uses a database query
+               # More efficient than the previous method if called just once
+               def find_new_statuses_allowed_to(role, tracker)
+                 new_statuses = [self] + workflows.find(:all,
+                                                        :include => :new_status,
+                                                        :conditions => ["role_id=? and tracker_id=?", role.id, tracker.id]).collect{ |w| w.new_status }
+                 new_statuses.sort{|x, y| x.position <=> y.position }
                end
              private

app/models/permission.rb +1 -1

                      find(:all, :include => :roles).each {|p| perms.store "#{p.controller}/#{p.action}", p.roles.collect {|r| r.id } }
                      perms
                    end
-                 allowed_to_public(action) or (@@cached_perms_for_roles[action] and @@cached_perms_for_roles[action].include? role)
+                 allowed_to_public(action) or (role && @@cached_perms_for_roles[action] && @@cached_perms_for_roles[action].include?(role.id))
                end
                def self.allowed_to_role_expired

app/models/user.rb +2 -8

                  User.hash_password(clear_password) == self.hashed_password
                end
-               def role_for_project(project_id)
-                 @role_for_projects ||=
-                   begin
-                     roles = {}
-                     self.memberships.each { |m| roles.store m.project_id, m.role_id }
-                     roles
-                   end
-                 @role_for_projects[project_id]
+               def role_for_project(project)
+                 memberships.detect {|m| m.project_id == project.id}
                end
                def pref

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages