From 5926bbac12d5831e1ad90964272b96e152a72e34 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 31 Jul 2012 08:32:49 +0300 Subject: [PATCH] Backend Refactoring --- app/contexts/notes/create_context.rb | 12 ++++++ app/contexts/notes/load_context.rb | 34 +++++++++++++++ app/contexts/notes_load.rb | 32 -------------- app/contexts/test_hook_context.rb | 8 ++++ app/controllers/admin/projects_controller.rb | 31 +++++--------- app/controllers/hooks_controller.rb | 5 +-- app/controllers/notes_controller.rb | 8 +--- app/controllers/profile_controller.rb | 14 +++--- app/controllers/search_controller.rb | 11 ++--- app/roles/team.rb | 29 +++++++++++++ spec/models/project_spec.rb | 45 +++++++++++++++++--- 11 files changed, 151 insertions(+), 78 deletions(-) create mode 100644 app/contexts/notes/create_context.rb create mode 100644 app/contexts/notes/load_context.rb delete mode 100644 app/contexts/notes_load.rb create mode 100644 app/contexts/test_hook_context.rb diff --git a/app/contexts/notes/create_context.rb b/app/contexts/notes/create_context.rb new file mode 100644 index 00000000..d93adb83 --- /dev/null +++ b/app/contexts/notes/create_context.rb @@ -0,0 +1,12 @@ +module Notes + class CreateContext < BaseContext + def execute + note = project.notes.new(params[:note]) + note.author = current_user + note.notify = true if params[:notify] == '1' + note.notify_author = true if params[:notify_author] == '1' + note.save + note + end + end +end diff --git a/app/contexts/notes/load_context.rb b/app/contexts/notes/load_context.rb new file mode 100644 index 00000000..c89a7d19 --- /dev/null +++ b/app/contexts/notes/load_context.rb @@ -0,0 +1,34 @@ +module Notes + class LoadContext < BaseContext + def execute + target_type = params[:target_type] + target_id = params[:target_id] + first_id = params[:first_id] + last_id = params[:last_id] + + + @notes = case target_type + when "commit" + then project.commit_notes(project.commit(target_id)).fresh.limit(20) + when "snippet" + then project.snippets.find(target_id).notes + when "wall" + then project.common_notes.order("created_at DESC").fresh.limit(50) + when "issue" + then project.issues.find(target_id).notes.inc_author.order("created_at DESC").limit(20) + when "merge_request" + then project.merge_requests.find(target_id).notes.inc_author.order("created_at DESC").limit(20) + when "wiki" + then project.wikis.reverse.map {|w| w.notes.fresh }.flatten[0..20] + end + + @notes = if last_id + @notes.where("id > ?", last_id) + elsif first_id + @notes.where("id < ?", first_id) + else + @notes + end + end + end +end diff --git a/app/contexts/notes_load.rb b/app/contexts/notes_load.rb deleted file mode 100644 index cbb9f67f..00000000 --- a/app/contexts/notes_load.rb +++ /dev/null @@ -1,32 +0,0 @@ -class NotesLoad < BaseContext - def execute - target_type = params[:target_type] - target_id = params[:target_id] - first_id = params[:first_id] - last_id = params[:last_id] - - - @notes = case target_type - when "commit" - then project.commit_notes(project.commit(target_id)).fresh.limit(20) - when "snippet" - then project.snippets.find(target_id).notes - when "wall" - then project.common_notes.order("created_at DESC").fresh.limit(50) - when "issue" - then project.issues.find(target_id).notes.inc_author.order("created_at DESC").limit(20) - when "merge_request" - then project.merge_requests.find(target_id).notes.inc_author.order("created_at DESC").limit(20) - when "wiki" - then project.wikis.reverse.map {|w| w.notes.fresh }.flatten[0..20] - end - - @notes = if last_id - @notes.where("id > ?", last_id) - elsif first_id - @notes.where("id < ?", first_id) - else - @notes - end - end -end diff --git a/app/contexts/test_hook_context.rb b/app/contexts/test_hook_context.rb new file mode 100644 index 00000000..cba5d1f8 --- /dev/null +++ b/app/contexts/test_hook_context.rb @@ -0,0 +1,8 @@ +class TestHookContext < BaseContext + def execute + hook = project.hooks.find(params[:id]) + commits = project.commits(project.default_branch, nil, 3) + data = project.post_receive_data(commits.last.id, commits.first.id, "refs/heads/#{project.default_branch}", current_user) + hook.execute(data) + end +end diff --git a/app/controllers/admin/projects_controller.rb b/app/controllers/admin/projects_controller.rb index 0ff97bf2..17009946 100644 --- a/app/controllers/admin/projects_controller.rb +++ b/app/controllers/admin/projects_controller.rb @@ -2,6 +2,7 @@ class Admin::ProjectsController < ApplicationController layout "admin" before_filter :authenticate_user! before_filter :authenticate_admin! + before_filter :admin_project, :only => [:edit, :show, :update, :destroy, :team_update] def index @admin_projects = Project.scoped @@ -10,13 +11,9 @@ class Admin::ProjectsController < ApplicationController end def show - @admin_project = Project.find_by_code(params[:id]) - - @users = if @admin_project.users.empty? - User - else - User.not_in_project(@admin_project) - end.all + @users = User.scoped + @users = @users.not_in_project(@admin_project) if @admin_project.users.present? + @users = @users.all end def new @@ -24,19 +21,10 @@ class Admin::ProjectsController < ApplicationController end def edit - @admin_project = Project.find_by_code(params[:id]) end def team_update - @admin_project = Project.find_by_code(params[:id]) - - UsersProject.bulk_import( - @admin_project, - params[:user_ids], - params[:project_access] - ) - - @admin_project.update_repository + @admin_project.add_users_ids_to_team(params[:user_ids], params[:project_access]) redirect_to [:admin, @admin_project], notice: 'Project was successfully updated.' end @@ -53,8 +41,6 @@ class Admin::ProjectsController < ApplicationController end def update - @admin_project = Project.find_by_code(params[:id]) - owner_id = params[:project].delete(:owner_id) if owner_id @@ -69,9 +55,14 @@ class Admin::ProjectsController < ApplicationController end def destroy - @admin_project = Project.find_by_code(params[:id]) @admin_project.destroy redirect_to admin_projects_url, notice: 'Project was successfully deleted.' end + + private + + def admin_project + @admin_project = Project.find_by_code(params[:id]) + end end diff --git a/app/controllers/hooks_controller.rb b/app/controllers/hooks_controller.rb index ad2fb3ae..df22b0ba 100644 --- a/app/controllers/hooks_controller.rb +++ b/app/controllers/hooks_controller.rb @@ -28,10 +28,7 @@ class HooksController < ApplicationController end def test - @hook = @project.hooks.find(params[:id]) - commits = @project.commits(@project.default_branch, nil, 3) - data = @project.post_receive_data(commits.last.id, commits.first.id, "refs/heads/#{@project.default_branch}", current_user) - @hook.execute(data) + TestHookContext.new(project, current_user, params).execute redirect_to :back end diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 1c997e38..e8e4bb54 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -15,11 +15,7 @@ class NotesController < ApplicationController end def create - @note = @project.notes.new(params[:note]) - @note.author = current_user - @note.notify = true if params[:notify] == '1' - @note.notify_author = true if params[:notify_author] == '1' - @note.save + @note = Notes::CreateContext.new(project, current_user, params).execute respond_to do |format| format.html {redirect_to :back} @@ -40,6 +36,6 @@ class NotesController < ApplicationController protected def notes - @notes = NotesLoad.new(project, current_user, params).execute + @notes = Notes::LoadContext.new(project, current_user, params).execute end end diff --git a/app/controllers/profile_controller.rb b/app/controllers/profile_controller.rb index 0da46375..0322a039 100644 --- a/app/controllers/profile_controller.rb +++ b/app/controllers/profile_controller.rb @@ -1,30 +1,26 @@ class ProfileController < ApplicationController layout "profile" + before_filter :user + def show - @user = current_user end def design - @user = current_user end def update - @user = current_user @user.update_attributes(params[:user]) redirect_to :back end def token - @user = current_user end def password - @user = current_user end def password_update params[:user].reject!{ |k, v| k != "password" && k != "password_confirmation"} - @user = current_user if @user.update_attributes(params[:user]) flash[:notice] = "Password was successfully updated. Please login with it" @@ -38,4 +34,10 @@ class ProfileController < ApplicationController current_user.reset_authentication_token! redirect_to profile_token_path end + + private + + def user + @user = current_user + end end diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index dfc318c5..30bcb49b 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -1,11 +1,12 @@ class SearchController < ApplicationController def show query = params[:search] - if query.blank? - @projects = [] - @merge_requests = [] - @issues = [] - else + + @projects = [] + @merge_requests = [] + @issues = [] + + if query.present? @projects = current_user.projects.search(query).limit(10) @merge_requests = MergeRequest.where(:project_id => current_user.project_ids).search(query).limit(10) @issues = Issue.where(:project_id => current_user.project_ids).search(query).limit(10) diff --git a/app/roles/team.rb b/app/roles/team.rb index 2a477b6e..bf3a6702 100644 --- a/app/roles/team.rb +++ b/app/roles/team.rb @@ -4,7 +4,36 @@ module Team users_projects.find_by_user_id(user.id) if user end + # Get Team Member record by user id def team_member_by_id(user_id) users_projects.find_by_user_id(user_id) end + + # Add user to project + # with passed access role + def add_user_to_team(user, access_role) + add_user_id_to_team(user.id, access_role) + end + + # Add multiple users to project + # with same access role + def add_users_to_team(users, access_role) + add_users_ids_to_team(users.map(&:id), access_role) + end + + # Add user to project + # with passed access role by user id + def add_user_id_to_team(user_id, access_role) + users_projects.create( + :user_id => user_id, + :project_access => access_role + ) + end + + # Add multiple users to project + # with same access role by user ids + def add_users_ids_to_team(users_ids, access_role) + UsersProject.bulk_import(self, users_ids, access_role) + self.update_repository + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 351f5748..2c8ddd5c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -22,21 +22,56 @@ describe Project do end describe "Respond to" do - it { should respond_to(:repository_writers) } - it { should respond_to(:add_access) } - it { should respond_to(:reset_access) } - it { should respond_to(:update_repository) } - it { should respond_to(:destroy_repository) } it { should respond_to(:public?) } it { should respond_to(:private?) } it { should respond_to(:url_to_repo) } it { should respond_to(:path_to_repo) } it { should respond_to(:valid_repo?) } it { should respond_to(:repo_exists?) } + + # Repository Role + it { should respond_to(:tree) } + it { should respond_to(:root_ref) } it { should respond_to(:repo) } it { should respond_to(:tags) } it { should respond_to(:commit) } + it { should respond_to(:commits) } it { should respond_to(:commits_between) } + it { should respond_to(:commits_with_refs) } + it { should respond_to(:commits_since) } + it { should respond_to(:commits_between) } + it { should respond_to(:write_hooks) } + it { should respond_to(:satellite) } + it { should respond_to(:update_repository) } + it { should respond_to(:destroy_repository) } + it { should respond_to(:archive_repo) } + + # Authority Role + it { should respond_to(:add_access) } + it { should respond_to(:reset_access) } + it { should respond_to(:repository_writers) } + it { should respond_to(:repository_masters) } + it { should respond_to(:repository_readers) } + it { should respond_to(:allow_read_for?) } + it { should respond_to(:guest_access_for?) } + it { should respond_to(:report_access_for?) } + it { should respond_to(:dev_access_for?) } + it { should respond_to(:master_access_for?) } + + # Team Role + it { should respond_to(:team_member_by_name_or_email) } + it { should respond_to(:team_member_by_id) } + it { should respond_to(:add_user_to_team) } + it { should respond_to(:add_users_to_team) } + it { should respond_to(:add_user_id_to_team) } + it { should respond_to(:add_users_ids_to_team) } + + # Project Push Role + it { should respond_to(:observe_push) } + it { should respond_to(:update_merge_requests) } + it { should respond_to(:execute_hooks) } + it { should respond_to(:post_receive_data) } + it { should respond_to(:trigger_post_receive) } end it "should not allow 'gitolite-admin' as repo name" do