From 0d9a6fe7b16e52cc4d5595d6b26552c39911cf07 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Mon, 4 Mar 2013 18:52:30 +0400 Subject: [PATCH] User's blocked field refactored to use state machine --- app/controllers/admin/users_controller.rb | 2 +- app/controllers/application_controller.rb | 4 ++-- app/models/user.rb | 34 ++++++++++++++++----------- app/views/admin/users/_form.html.haml | 2 +- app/views/admin/users/index.html.haml | 2 +- app/views/admin/users/show.html.haml | 2 +- app/views/team_members/_team_member.html.haml | 2 +- app/views/teams/members/_show.html.haml | 2 +- db/schema.rb | 5 ++-- lib/api/entities.rb | 4 ++-- lib/gitlab/auth.rb | 6 +++-- spec/models/user_spec.rb | 6 ++--- 12 files changed, 39 insertions(+), 32 deletions(-) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 2e7114e10..43e6f0990 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -45,7 +45,7 @@ class Admin::UsersController < Admin::ApplicationController end def unblock - if admin_user.update_attribute(:blocked, false) + if admin_user.activate redirect_to :back, alert: "Successfully unblocked" else redirect_to :back, alert: "Error occured. User was not unblocked" diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5b886227b..6b72f3252 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -30,7 +30,7 @@ class ApplicationController < ActionController::Base end def reject_blocked! - if current_user && current_user.blocked + if current_user && current_user.blocked? sign_out current_user flash[:alert] = "Your account is blocked. Retry when an admin unblock it." redirect_to new_user_session_path @@ -38,7 +38,7 @@ class ApplicationController < ActionController::Base end def after_sign_in_path_for resource - if resource.is_a?(User) && resource.respond_to?(:blocked) && resource.blocked + if resource.is_a?(User) && resource.respond_to?(:blocked?) && resource.blocked? sign_out resource flash[:alert] = "Your account is blocked. Retry when an admin unblock it." new_user_session_path diff --git a/app/models/user.rb b/app/models/user.rb index cd0754d78..9f5a68d50 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,7 +25,7 @@ # dark_scheme :boolean default(FALSE), not null # theme_id :integer default(1), not null # bio :string(255) -# blocked :boolean default(FALSE), not null +# state :string(255) # failed_attempts :integer default(0) # locked_at :datetime # extern_uid :string(255) @@ -87,10 +87,27 @@ class User < ActiveRecord::Base delegate :path, to: :namespace, allow_nil: true, prefix: true + state_machine :state, initial: :active do + after_transition any => :blocked do |user, transition| + # Remove user from all projects and + user.users_projects.find_each do |membership| + return false unless membership.destroy + end + end + + event :block do + transition active: :blocked + end + + event :activate do + transition blocked: :active + end + end + # Scopes scope :admins, -> { where(admin: true) } - scope :blocked, -> { where(blocked: true) } - scope :active, -> { where(blocked: false) } + scope :blocked, -> { with_state(:blocked) } + scope :active, -> { with_state(:active) } scope :alphabetically, -> { order('name ASC') } scope :in_team, ->(team){ where(id: team.member_ids) } scope :not_in_team, ->(team){ where('users.id NOT IN (:ids)', ids: team.member_ids) } @@ -260,17 +277,6 @@ class User < ActiveRecord::Base MergeRequest.cared(self) end - # Remove user from all projects and - # set blocked attribute to true - def block - users_projects.find_each do |membership| - return false unless membership.destroy - end - - self.blocked = true - save - end - def projects_limit_percent return 100 if projects_limit.zero? (personal_projects.count.to_f / projects_limit) * 100 diff --git a/app/views/admin/users/_form.html.haml b/app/views/admin/users/_form.html.haml index 48876338a..1d1fe341c 100644 --- a/app/views/admin/users/_form.html.haml +++ b/app/views/admin/users/_form.html.haml @@ -61,7 +61,7 @@ .span4 - unless @admin_user.new_record? .alert.alert-error - - if @admin_user.blocked + - if @admin_user.blocked? %p This user is blocked and is not able to login to GitLab = link_to 'Unblock User', unblock_admin_user_path(@admin_user), method: :put, class: "btn btn-small" - else diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml index f5bb8b068..9da2871e9 100644 --- a/app/views/admin/users/index.html.haml +++ b/app/views/admin/users/index.html.haml @@ -53,7 +53,7 @@   = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: "btn btn-small" - unless user == current_user - - if user.blocked + - if user.blocked? = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: "btn btn-small success" - else = link_to 'Block', block_admin_user_path(user), confirm: 'USER WILL BE BLOCKED! Are you sure?', method: :put, class: "btn btn-small btn-remove" diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index c5d601948..2129ceec5 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -3,7 +3,7 @@ %h3.page_title = image_tag gravatar_icon(@admin_user.email, 90), class: "avatar s90" = @admin_user.name - - if @admin_user.blocked + - if @admin_user.blocked? %span.cred (Blocked) - if @admin_user.admin %span.cred (Admin) diff --git a/app/views/team_members/_team_member.html.haml b/app/views/team_members/_team_member.html.haml index e7cba0b34..e0485f408 100644 --- a/app/views/team_members/_team_member.html.haml +++ b/app/views/team_members/_team_member.html.haml @@ -20,7 +20,7 @@ %span.label This is you! - if @project.namespace_owner == user %span.label Owner - - elsif user.blocked + - elsif user.blocked? %span.label Blocked - elsif allow_admin = link_to project_team_member_path(@project, user), confirm: remove_from_project_team_message(@project, user), method: :delete, class: "btn-tiny btn btn-remove" do diff --git a/app/views/teams/members/_show.html.haml b/app/views/teams/members/_show.html.haml index 3aa2db866..597581090 100644 --- a/app/views/teams/members/_show.html.haml +++ b/app/views/teams/members/_show.html.haml @@ -23,7 +23,7 @@ %span.btn.disabled This is you! - if @team.owner == user %span.btn.disabled Owner - - elsif user.blocked + - elsif user.blocked? %span.btn.disabled.blocked Blocked - elsif allow_admin = link_to team_member_path(@team, user), confirm: remove_from_user_team_message(@team, user), method: :delete, class: "btn-tiny btn btn-remove", title: "Remove from team" do diff --git a/db/schema.rb b/db/schema.rb index 04ed79846..2250f418b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20130220133245) do +ActiveRecord::Schema.define(:version => 20130304105317) do create_table "events", :force => true do |t| t.string "target_type" @@ -261,7 +261,6 @@ ActiveRecord::Schema.define(:version => 20130220133245) do t.boolean "dark_scheme", :default => false, :null => false t.integer "theme_id", :default => 1, :null => false t.string "bio" - t.boolean "blocked", :default => false, :null => false t.integer "failed_attempts", :default => 0 t.datetime "locked_at" t.string "extern_uid" @@ -269,10 +268,10 @@ ActiveRecord::Schema.define(:version => 20130220133245) do t.string "username" t.boolean "can_create_group", :default => true, :null => false t.boolean "can_create_team", :default => true, :null => false + t.string "state" end add_index "users", ["admin"], :name => "index_users_on_admin" - add_index "users", ["blocked"], :name => "index_users_on_blocked" add_index "users", ["email"], :name => "index_users_on_email", :unique => true add_index "users", ["extern_uid", "provider"], :name => "index_users_on_extern_uid_and_provider", :unique => true add_index "users", ["name"], :name => "index_users_on_name" diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 1cae1d337..088c99598 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -2,11 +2,11 @@ module Gitlab module Entities class User < Grape::Entity expose :id, :username, :email, :name, :bio, :skype, :linkedin, :twitter, - :dark_scheme, :theme_id, :blocked, :created_at, :extern_uid, :provider + :dark_scheme, :theme_id, :state, :created_at, :extern_uid, :provider end class UserBasic < Grape::Entity - expose :id, :username, :email, :name, :blocked, :created_at + expose :id, :username, :email, :name, :state, :created_at end class UserLogin < UserBasic diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index d0e792bef..0fee33dbe 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -41,10 +41,12 @@ module Gitlab password_confirmation: password, projects_limit: Gitlab.config.gitlab.default_projects_limit, }, as: :admin) + @user.save! + if Gitlab.config.omniauth['block_auto_created_users'] && !ldap - @user.blocked = true + @user.block end - @user.save! + @user end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 40047b351..cb39b6fc6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -25,7 +25,7 @@ # dark_scheme :boolean default(FALSE), not null # theme_id :integer default(1), not null # bio :string(255) -# blocked :boolean default(FALSE), not null +# state :string(255) default(FALSE), not null # failed_attempts :integer default(0) # locked_at :datetime # extern_uid :string(255) @@ -140,7 +140,7 @@ describe User do it "should block user" do user.block - user.blocked.should be_true + user.blocked?.should be_true end end @@ -149,7 +149,7 @@ describe User do User.delete_all @user = create :user @admin = create :user, admin: true - @blocked = create :user, blocked: true + @blocked = create :user, state: :blocked end it { User.filter("admins").should == [@admin] } -- 2.11.0