From b0eb6798b0cb09cc30e98ff253a67e5c258d90f6 Mon Sep 17 00:00:00 2001 From: Martin Lensment Date: Tue, 20 Jan 2015 17:49:14 +0200 Subject: [PATCH] Refactor roles --- app/controllers/admin/users_controller.rb | 3 +- app/models/ability.rb | 91 +++------ app/models/right.rb | 5 - app/models/role.rb | 12 -- app/models/user.rb | 3 +- app/views/admin/users/_form.haml | 4 +- app/views/admin/users/index.haml | 4 +- app/views/admin/users/show.haml | 4 +- db/migrate/20150120140346_refactor_roles.rb | 16 ++ db/schema.rb | 211 +++++++++----------- db/seeds.rb | 6 +- spec/fabricators/role_fabricator.rb | 3 - spec/fabricators/user_fabricator.rb | 2 +- spec/models/right_spec.rb | 5 - spec/models/role_spec.rb | 5 - spec/models/user_spec.rb | 4 +- 16 files changed, 155 insertions(+), 223 deletions(-) delete mode 100644 app/models/right.rb delete mode 100644 app/models/role.rb create mode 100644 db/migrate/20150120140346_refactor_roles.rb delete mode 100644 spec/fabricators/role_fabricator.rb delete mode 100644 spec/models/right_spec.rb delete mode 100644 spec/models/role_spec.rb diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 5d7f961dc..5d53aac6a 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -54,7 +54,6 @@ class Admin::UsersController < AdminController end def user_params - params.require(:user).permit(:username, :password, :identity_code, :email, - :role_id, :country_id) + params.require(:user).permit(:username, :password, :identity_code, :email, :country_id, { roles: [] }) end end diff --git a/app/models/ability.rb b/app/models/ability.rb index 1a758b73e..6d389484c 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,72 +1,41 @@ class Ability include CanCan::Ability - # rubocop: disable Metrics/MethodLength - # rubocop: disable Metrics/CyclomaticComplexity def initialize(user) alias_action :create, :read, :update, :destroy, to: :crud - user ||= User.new + @user = user || User.new + @user.roles.each { |role| send(role) } if @user.roles - admin_role = (user.role.try(:code) == 'admin') - user_role = (user.role.try(:code) == 'user') - customer_service_role = (user.role.try(:code) == 'customer_service') - no_role = user.role.nil? - - if admin_role - can :manage, Domain - can :manage, Contact - can :manage, Registrar - can :manage, Setting - can :manage, ZonefileSetting - can :manage, DomainVersion - can :manage, User - can :manage, EppUser - can :manage, Keyrelay - can :manage, LegalDocument - can :read, ApiLog::EppLog - can :read, ApiLog::ReppLog - can :index, :delayed_job - can :create, :zonefile - can :access, :settings_menu - elsif customer_service_role - can :manage, Domain - can :manage, Contact - can :manage, Registrar - elsif user_role - elsif no_role + if @user.roles.nil? || @user.roles.empty? can :show, :dashboard end - - can :show, :dashboard if user.persisted? - - # Define abilities for the passed in user here. For example: - # - # user ||= User.new # guest user (not logged in) - # if user.admin? - # can :manage, :all - # else - # can :read, :all - # end - # - # The first argument to `can` is the action you are giving the user - # permission to do. - # If you pass :manage it will apply to every action. Other common actions - # here are :read, :create, :update and :destroy. - # - # The second argument is the resource the user can perform the action on. - # If you pass :all it will apply to every resource. Otherwise pass a Ruby - # class of the resource. - # - # The third argument is an optional hash of conditions to further filter the - # objects. - # For example, here the user can only update published articles. - # - # can :update, Article, :published => true - # - # See the wiki for details: - # https://github.com/ryanb/cancan/wiki/Defining-Abilities end - # rubocop: enable Metrics/MethodLength - # rubocop: enable Metrics/CyclomaticComplexity + + def user + can :show, :dashboard + end + + def customer_service + user + can :manage, Domain + can :manage, Contact + can :manage, Registrar + end + + def admin + customer_service + can :manage, Setting + can :manage, ZonefileSetting + can :manage, DomainVersion + can :manage, User + can :manage, EppUser + can :manage, Keyrelay + can :manage, LegalDocument + can :read, ApiLog::EppLog + can :read, ApiLog::ReppLog + can :index, :delayed_job + can :create, :zonefile + can :access, :settings_menu + end end diff --git a/app/models/right.rb b/app/models/right.rb deleted file mode 100644 index 79f8ebe5c..000000000 --- a/app/models/right.rb +++ /dev/null @@ -1,5 +0,0 @@ -class Right < ActiveRecord::Base - # rubocop: disable Rails/HasAndBelongsToMany - has_and_belongs_to_many :roles - # rubocop: enable Rails/HasAndBelongsToMany -end diff --git a/app/models/role.rb b/app/models/role.rb deleted file mode 100644 index f886c2e23..000000000 --- a/app/models/role.rb +++ /dev/null @@ -1,12 +0,0 @@ -class Role < ActiveRecord::Base - has_many :users - # rubocop: disable Rails/HasAndBelongsToMany - has_and_belongs_to_many :rights - # rubocop: enbale Rails/HasAndBelongsToMany - - validates :code, uniqueness: true - - def to_s - code - end -end diff --git a/app/models/user.rb b/app/models/user.rb index 09ae10d69..04abe0300 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -6,7 +6,6 @@ class User < ActiveRecord::Base # After activisation, system should require to change temp password. # TODO: Estonian id validation - belongs_to :role belongs_to :country validates :username, :password, presence: true @@ -16,6 +15,8 @@ class User < ActiveRecord::Base validate :validate_identity_code + ROLES = ['user', 'customer_service', 'admin'] + def to_s username end diff --git a/app/views/admin/users/_form.haml b/app/views/admin/users/_form.haml index 75a1d41e1..0c3239cfc 100644 --- a/app/views/admin/users/_form.haml +++ b/app/views/admin/users/_form.haml @@ -26,8 +26,8 @@ = f.label :email = f.text_field(:email, class: 'form-control') .form-group - = f.label :role_id - = f.select(:role_id, Role.all.map {|x| [t(x.code), x.id] }, {}, { class: 'form-control selectize' }) + = f.label :role + = select_tag 'user[roles][]', options_for_select(User::ROLES.map {|x| [t(x), x] }, @user.roles.try(:first)), class: 'form-control selectize' %hr .row diff --git a/app/views/admin/users/index.haml b/app/views/admin/users/index.haml index 6740c1db9..374bbdcda 100644 --- a/app/views/admin/users/index.haml +++ b/app/views/admin/users/index.haml @@ -25,8 +25,8 @@ %td= link_to(x, [:admin, x]) %td= x.email %td= x.identity_code - - if x.role - %td= t(x.role) + - if x.roles + %td= t(x.roles.first) - else %td .row diff --git a/app/views/admin/users/show.haml b/app/views/admin/users/show.haml index 99235f79a..29126fc77 100644 --- a/app/views/admin/users/show.haml +++ b/app/views/admin/users/show.haml @@ -40,7 +40,7 @@ %dd= @user.email %dt= t('role') - - if @user.role - %dd= t(@user.role) + - if @user.roles + %dd= t(@user.roles.first) - else %dd diff --git a/db/migrate/20150120140346_refactor_roles.rb b/db/migrate/20150120140346_refactor_roles.rb new file mode 100644 index 000000000..f8c8606de --- /dev/null +++ b/db/migrate/20150120140346_refactor_roles.rb @@ -0,0 +1,16 @@ +class RefactorRoles < ActiveRecord::Migration + def change + add_column :users, :roles, :string, array: true + + User.all.each do |x| + x.roles = [x.role.code] + x.save + end + + remove_column :users, :role_id + + drop_table :roles + drop_table :rights + drop_table :rights_roles + end +end diff --git a/db/schema.rb b/db/schema.rb index 2b3e6e73e..36415a165 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,16 +11,16 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150109081914) do +ActiveRecord::Schema.define(version: 20150120140346) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" create_table "address_versions", force: :cascade do |t| - t.string "item_type", null: false - t.integer "item_id", null: false - t.string "event", null: false - t.string "whodunnit" + t.string "item_type", limit: 255, null: false + t.integer "item_id", null: false + t.string "event", limit: 255, null: false + t.string "whodunnit", limit: 255 t.text "object" t.datetime "created_at" end @@ -30,13 +30,13 @@ ActiveRecord::Schema.define(version: 20150109081914) do create_table "addresses", force: :cascade do |t| t.integer "contact_id" t.integer "country_id" - t.string "city" - t.string "street" - t.string "zip" + t.string "city", limit: 255 + t.string "street", limit: 255 + t.string "zip", limit: 255 t.datetime "created_at" t.datetime "updated_at" - t.string "street2" - t.string "street3" + t.string "street2", limit: 255 + t.string "street3", limit: 255 end create_table "cached_nameservers", id: false, force: :cascade do |t| @@ -60,18 +60,18 @@ ActiveRecord::Schema.define(version: 20150109081914) do end create_table "contact_statuses", force: :cascade do |t| - t.string "value" - t.string "description" + t.string "value", limit: 255 + t.string "description", limit: 255 t.integer "contact_id" t.datetime "created_at" t.datetime "updated_at" end create_table "contact_versions", force: :cascade do |t| - t.string "item_type", null: false - t.integer "item_id", null: false - t.string "event", null: false - t.string "whodunnit" + t.string "item_type", limit: 255, null: false + t.integer "item_id", null: false + t.string "event", limit: 255, null: false + t.string "whodunnit", limit: 255 t.text "object" t.datetime "created_at" end @@ -79,41 +79,41 @@ ActiveRecord::Schema.define(version: 20150109081914) do add_index "contact_versions", ["item_type", "item_id"], name: "index_contact_versions_on_item_type_and_item_id", using: :btree create_table "contacts", force: :cascade do |t| - t.string "code" - t.string "type" - t.string "reg_no" - t.string "phone" - t.string "email" - t.string "fax" + t.string "code", limit: 255 + t.string "type", limit: 255 + t.string "reg_no", limit: 255 + t.string "phone", limit: 255 + t.string "email", limit: 255 + t.string "fax", limit: 255 t.datetime "created_at" t.datetime "updated_at" - t.string "ident" - t.string "ident_type" + t.string "ident", limit: 255 + t.string "ident_type", limit: 255 t.integer "created_by_id" t.integer "updated_by_id" - t.string "auth_info" - t.string "name" - t.string "org_name" + t.string "auth_info", limit: 255 + t.string "name", limit: 255 + t.string "org_name", limit: 255 t.integer "registrar_id" end create_table "countries", force: :cascade do |t| - t.string "iso" - t.string "name" + t.string "iso", limit: 255 + t.string "name", limit: 255 t.datetime "created_at" t.datetime "updated_at" end create_table "delayed_jobs", force: :cascade do |t| - t.integer "priority", default: 0, null: false - t.integer "attempts", default: 0, null: false - t.text "handler", null: false + t.integer "priority", default: 0, null: false + t.integer "attempts", default: 0, null: false + t.text "handler", null: false t.text "last_error" t.datetime "run_at" t.datetime "locked_at" t.datetime "failed_at" - t.string "locked_by" - t.string "queue" + t.string "locked_by", limit: 255 + t.string "queue", limit: 255 t.datetime "created_at" t.datetime "updated_at" end @@ -122,10 +122,10 @@ ActiveRecord::Schema.define(version: 20150109081914) do create_table "delegation_signers", force: :cascade do |t| t.integer "domain_id" - t.string "key_tag" + t.string "key_tag", limit: 255 t.integer "alg" t.integer "digest_type" - t.string "digest" + t.string "digest", limit: 255 end create_table "dnskeys", force: :cascade do |t| @@ -135,26 +135,26 @@ ActiveRecord::Schema.define(version: 20150109081914) do t.integer "alg" t.text "public_key" t.integer "delegation_signer_id" - t.string "ds_key_tag" + t.string "ds_key_tag", limit: 255 t.integer "ds_alg" t.integer "ds_digest_type" - t.string "ds_digest" + t.string "ds_digest", limit: 255 end create_table "domain_contacts", force: :cascade do |t| t.integer "contact_id" t.integer "domain_id" - t.string "contact_type" + t.string "contact_type", limit: 255 t.datetime "created_at" t.datetime "updated_at" - t.string "contact_code_cache" + t.string "contact_code_cache", limit: 255 end create_table "domain_status_versions", force: :cascade do |t| - t.string "item_type", null: false - t.integer "item_id", null: false - t.string "event", null: false - t.string "whodunnit" + t.string "item_type", limit: 255, null: false + t.integer "item_id", null: false + t.string "event", limit: 255, null: false + t.string "whodunnit", limit: 255 t.text "object" t.datetime "created_at" end @@ -163,13 +163,13 @@ ActiveRecord::Schema.define(version: 20150109081914) do create_table "domain_statuses", force: :cascade do |t| t.integer "domain_id" - t.string "description" - t.string "value" + t.string "description", limit: 255 + t.string "value", limit: 255 end create_table "domain_transfers", force: :cascade do |t| t.integer "domain_id" - t.string "status" + t.string "status", limit: 255 t.datetime "transfer_requested_at" t.datetime "transferred_at" t.integer "transfer_from_id" @@ -180,10 +180,10 @@ ActiveRecord::Schema.define(version: 20150109081914) do end create_table "domain_versions", force: :cascade do |t| - t.string "item_type", null: false - t.integer "item_id", null: false - t.string "event", null: false - t.string "whodunnit" + t.string "item_type", limit: 255, null: false + t.integer "item_id", null: false + t.string "event", limit: 255, null: false + t.string "whodunnit", limit: 255 t.text "object" t.datetime "created_at" t.text "snapshot" @@ -192,24 +192,24 @@ ActiveRecord::Schema.define(version: 20150109081914) do add_index "domain_versions", ["item_type", "item_id"], name: "index_domain_versions_on_item_type_and_item_id", using: :btree create_table "domains", force: :cascade do |t| - t.string "name" + t.string "name", limit: 255 t.integer "registrar_id" t.datetime "registered_at" - t.string "status" + t.string "status", limit: 255 t.datetime "valid_from" t.datetime "valid_to" t.integer "owner_contact_id" - t.string "auth_info" + t.string "auth_info", limit: 255 t.datetime "created_at" t.datetime "updated_at" - t.string "name_dirty" - t.string "name_puny" + t.string "name_dirty", limit: 255 + t.string "name_puny", limit: 255 t.integer "period" t.string "period_unit", limit: 1 end create_table "epp_sessions", force: :cascade do |t| - t.string "session_id" + t.string "session_id", limit: 255 t.text "data" t.datetime "created_at" t.datetime "updated_at" @@ -220,9 +220,9 @@ ActiveRecord::Schema.define(version: 20150109081914) do create_table "epp_users", force: :cascade do |t| t.integer "registrar_id" - t.string "username" - t.string "password" - t.boolean "active", default: false + t.string "username", limit: 255 + t.string "password", limit: 255 + t.boolean "active", default: false t.text "csr" t.text "crt" t.datetime "created_at" @@ -232,12 +232,12 @@ ActiveRecord::Schema.define(version: 20150109081914) do create_table "keyrelays", force: :cascade do |t| t.integer "domain_id" t.datetime "pa_date" - t.string "key_data_flags" - t.string "key_data_protocol" - t.string "key_data_alg" + t.string "key_data_flags", limit: 255 + t.string "key_data_protocol", limit: 255 + t.string "key_data_alg", limit: 255 t.text "key_data_public_key" - t.string "auth_info_pw" - t.string "expiry_relative" + t.string "auth_info_pw", limit: 255 + t.string "expiry_relative", limit: 255 t.datetime "expiry_absolute" t.integer "requester_id" t.integer "accepter_id" @@ -246,29 +246,29 @@ ActiveRecord::Schema.define(version: 20150109081914) do end create_table "legal_documents", force: :cascade do |t| - t.string "document_type" + t.string "document_type", limit: 255 t.text "body" t.integer "documentable_id" - t.string "documentable_type" + t.string "documentable_type", limit: 255 t.datetime "created_at" t.datetime "updated_at" end create_table "messages", force: :cascade do |t| t.integer "registrar_id" - t.string "body" - t.string "attached_obj_type" - t.string "attached_obj_id" + t.string "body", limit: 255 + t.string "attached_obj_type", limit: 255 + t.string "attached_obj_id", limit: 255 t.boolean "queued" t.datetime "created_at" t.datetime "updated_at" end create_table "nameserver_versions", force: :cascade do |t| - t.string "item_type", null: false - t.integer "item_id", null: false - t.string "event", null: false - t.string "whodunnit" + t.string "item_type", limit: 255, null: false + t.integer "item_id", null: false + t.string "event", limit: 255, null: false + t.string "whodunnit", limit: 255 t.text "object" t.datetime "created_at" end @@ -276,50 +276,33 @@ ActiveRecord::Schema.define(version: 20150109081914) do add_index "nameserver_versions", ["item_type", "item_id"], name: "index_nameserver_versions_on_item_type_and_item_id", using: :btree create_table "nameservers", force: :cascade do |t| - t.string "hostname" - t.string "ipv4" + t.string "hostname", limit: 255 + t.string "ipv4", limit: 255 t.datetime "created_at" t.datetime "updated_at" - t.string "ipv6" + t.string "ipv6", limit: 255 t.integer "domain_id" end create_table "registrars", force: :cascade do |t| - t.string "name" - t.string "reg_no" - t.string "vat_no" - t.string "address" + t.string "name", limit: 255 + t.string "reg_no", limit: 255 + t.string "vat_no", limit: 255 + t.string "address", limit: 255 t.integer "country_id" - t.string "billing_address" + t.string "billing_address", limit: 255 t.datetime "created_at" t.datetime "updated_at" end create_table "reserved_domains", force: :cascade do |t| - t.string "name" - t.datetime "created_at" - t.datetime "updated_at" - end - - create_table "rights", force: :cascade do |t| - t.string "code" - t.datetime "created_at" - t.datetime "updated_at" - end - - create_table "rights_roles", force: :cascade do |t| - t.integer "right_id" - t.integer "role_id" - end - - create_table "roles", force: :cascade do |t| - t.string "code" + t.string "name", limit: 255 t.datetime "created_at" t.datetime "updated_at" end create_table "settings", force: :cascade do |t| - t.string "var", null: false + t.string "var", limit: 255, null: false t.text "value" t.integer "thing_id" t.string "thing_type", limit: 30 @@ -330,26 +313,26 @@ ActiveRecord::Schema.define(version: 20150109081914) do add_index "settings", ["thing_type", "thing_id", "var"], name: "index_settings_on_thing_type_and_thing_id_and_var", unique: true, using: :btree create_table "users", force: :cascade do |t| - t.string "username" - t.string "password" - t.integer "role_id" + t.string "username", limit: 255 + t.string "password", limit: 255 t.datetime "created_at" t.datetime "updated_at" - t.string "email" - t.integer "sign_in_count", default: 0, null: false + t.string "email", limit: 255 + t.integer "sign_in_count", default: 0, null: false t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" t.inet "current_sign_in_ip" t.inet "last_sign_in_ip" - t.string "identity_code" + t.string "identity_code", limit: 255 t.integer "country_id" + t.string "roles", array: true end create_table "versions", force: :cascade do |t| - t.string "item_type", null: false - t.integer "item_id", null: false - t.string "event", null: false - t.string "whodunnit" + t.string "item_type", limit: 255, null: false + t.integer "item_id", null: false + t.string "event", limit: 255, null: false + t.string "whodunnit", limit: 255 t.text "object" t.datetime "created_at" end @@ -357,14 +340,14 @@ ActiveRecord::Schema.define(version: 20150109081914) do add_index "versions", ["item_type", "item_id"], name: "index_versions_on_item_type_and_item_id", using: :btree create_table "zonefile_settings", force: :cascade do |t| - t.string "origin" + t.string "origin", limit: 255 t.integer "ttl" t.integer "refresh" t.integer "retry" t.integer "expire" t.integer "minimum_ttl" - t.string "email" - t.string "master_nameserver" + t.string "email", limit: 255 + t.string "master_nameserver", limit: 255 t.datetime "created_at" t.datetime "updated_at" end diff --git a/db/seeds.rb b/db/seeds.rb index 19680cbe1..3f3bb03d8 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -61,9 +61,5 @@ User.where( country: Country.where(name: 'Estonia').first ).first_or_create -Role.create(code: 'admin') -Role.create(code: 'user') -Role.create(code: 'customer_service') - -User.update_all(role_id: Role.first.id) +User.update_all(roles: ['admin']) # Setting.whois_enabled = true only uncomment this if you wish whois diff --git a/spec/fabricators/role_fabricator.rb b/spec/fabricators/role_fabricator.rb deleted file mode 100644 index e084a0dcc..000000000 --- a/spec/fabricators/role_fabricator.rb +++ /dev/null @@ -1,3 +0,0 @@ -Fabricator(:role) do - code 'admin' -end diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index 57776f677..d3635dfbb 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/spec/fabricators/user_fabricator.rb @@ -4,5 +4,5 @@ Fabricator(:user) do email 'info@gitlab.eu' identity_code '37810013108' country - role + roles ['admin'] end diff --git a/spec/models/right_spec.rb b/spec/models/right_spec.rb deleted file mode 100644 index dc4e46d81..000000000 --- a/spec/models/right_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require 'rails_helper' - -describe Right do - it { should have_and_belong_to_many(:roles) } -end diff --git a/spec/models/role_spec.rb b/spec/models/role_spec.rb deleted file mode 100644 index ff9a22703..000000000 --- a/spec/models/role_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require 'rails_helper' - -describe Role do - it { should have_and_belong_to_many(:rights) } -end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d0c764f0c..e2bbc8134 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2,8 +2,6 @@ require 'rails_helper' require 'cancan/matchers' describe User do - it { should belong_to(:role) } - describe 'abilities' do subject(:ability) { Ability.new(user) } let(:user) { nil } @@ -29,7 +27,7 @@ describe User do end context 'when user is customer service' do - let(:user) { Fabricate(:user, role: Role.new(code: 'customer_service')) } + let(:user) { Fabricate(:user, roles: ['customer_service']) } it { should be_able_to(:manage, Domain.new) } it { should be_able_to(:manage, Contact.new) }