From e59b5a2b85fe6045a5fad77c3c297c20524a3a59 Mon Sep 17 00:00:00 2001 From: Denis Talakevich Date: Sat, 9 Nov 2019 12:55:02 +0200 Subject: [PATCH 1/3] improve api controllers --- .rubocop.yml | 3 +++ app/controllers/api/eduroam/accounting_controller.rb | 3 --- app/controllers/api_controller.rb | 3 ++- app/lib/exception_handler.rb | 4 ++++ 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 34b6a34..efb9615 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -35,6 +35,9 @@ Style/FrozenStringLiteralComment: Style/Lambda: EnforcedStyle: literal +Style/GuardClause: + Enabled: false + Lint/AmbiguousBlockAssociation: Exclude: - spec/**/*.rb diff --git a/app/controllers/api/eduroam/accounting_controller.rb b/app/controllers/api/eduroam/accounting_controller.rb index b01a361..6cfc38c 100644 --- a/app/controllers/api/eduroam/accounting_controller.rb +++ b/app/controllers/api/eduroam/accounting_controller.rb @@ -1,9 +1,6 @@ module Api module Eduroam class AccountingController < ApiController - # fix for Can't verify CSRF token authenticity. - skip_before_action :verify_authenticity_token - def create record = WifiSessionForm.new(permitted_params) if record.save diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 4b49eef..87bae1b 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -1,4 +1,5 @@ -class ApiController < ApplicationController +class ApiController < ActionController::API + include ExceptionHandler rescue_from StandardError, with: :server_error def server_error(err) diff --git a/app/lib/exception_handler.rb b/app/lib/exception_handler.rb index 81de08b..ec54e0e 100644 --- a/app/lib/exception_handler.rb +++ b/app/lib/exception_handler.rb @@ -8,6 +8,10 @@ def log_error(error, backtrace: true) msg << "\n#{error.backtrace.join("\n")}" if backtrace msg end + if error.cause && error != error.cause + logger.error { 'Caused by:' } + log_error(error.cause, backtrace: backtrace) + end end end From 6c1c94dfaedaf616d3c7b8c3fb6174d705ae46bc Mon Sep 17 00:00:00 2001 From: Denis Talakevich Date: Sat, 9 Nov 2019 13:05:55 +0200 Subject: [PATCH 2/3] add model tests for wifi session --- spec/models/login_record_spec.rb | 24 ------------------------ spec/models/wifi_session_spec.rb | 32 +++++++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 25 deletions(-) delete mode 100644 spec/models/login_record_spec.rb diff --git a/spec/models/login_record_spec.rb b/spec/models/login_record_spec.rb deleted file mode 100644 index a38e98a..0000000 --- a/spec/models/login_record_spec.rb +++ /dev/null @@ -1,24 +0,0 @@ -# == Schema Information -# -# Table name: login_records -# -# id :bigint(8) not null, primary key -# allowed_services :integer default([]), not null, is an Array -# login :string not null -# login_entity_type :string not null -# password :string not null -# created_at :datetime not null -# updated_at :datetime not null -# login_entity_id :integer not null -# -# Indexes -# -# index_login_records_on_login (login) UNIQUE -# index_login_records_on_login_entity_id_and_login_entity_type (login_entity_id,login_entity_type) UNIQUE -# - -require 'rails_helper' - -RSpec.describe LoginRecord, type: :model do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/models/wifi_session_spec.rb b/spec/models/wifi_session_spec.rb index 1e1a9f7..2f873ed 100644 --- a/spec/models/wifi_session_spec.rb +++ b/spec/models/wifi_session_spec.rb @@ -28,5 +28,35 @@ require 'rails_helper' RSpec.describe WifiSession, type: :model do - pending "add some examples to (or delete) #{__FILE__}" + describe '.create' do + subject do + described_class.create(create_params) + end + + let(:create_params) { { session_id: '123' } } + + include_examples :creates_record + include_examples :changes_records_count_of, described_class, by: 1 + + context 'with optional attributes' do + let(:create_params) do + super().merge connect_info: 'connect_info', + bytes_rx: '123', + bytes_tx: '456', + duration: '789', + nas_identifier: 'nas_identifier', + nas_ip_address: '127.0.0.1', + nas_port_type: 'nas_port_type', + packets_rx: '1234', + packets_tx: '5678', + terminate_cause: 'terminate_cause', + username: 'username', + called_station_id: 'called_station_id', + calling_station_id: 'calling_station_id' + end + + include_examples :creates_record + include_examples :changes_records_count_of, described_class, by: 1 + end + end end From 6895d4dcb062d9ae7cb8cf4075070e3787bfcc33 Mon Sep 17 00:00:00 2001 From: Denis Talakevich Date: Sat, 9 Nov 2019 13:06:52 +0200 Subject: [PATCH 3/3] add attributes to students and employees --- app/admin/employees.rb | 35 +++++++++-- app/admin/students.rb | 39 +++++++++++-- app/forms/set_role_form.rb | 2 +- app/models/employee.rb | 21 +++++-- app/models/student.rb | 23 ++++++-- ...dd_attributes_to_students_and_employees.rb | 43 ++++++++++++++ db/structure.sql | 58 ++++++++++++++++++- spec/factories/employees.rb | 20 +++++-- spec/factories/students.rb | 22 +++++-- spec/models/employee_spec.rb | 57 ++++++++++++++++-- spec/models/student_spec.rb | 50 ++++++++++++++-- 11 files changed, 326 insertions(+), 44 deletions(-) create mode 100644 db/migrate/20191109103205_add_attributes_to_students_and_employees.rb diff --git a/app/admin/employees.rb b/app/admin/employees.rb index bbcccbc..78512d6 100644 --- a/app/admin/employees.rb +++ b/app/admin/employees.rb @@ -2,8 +2,9 @@ decorate_with EmployeeDecorator config.current_filters = false - filter :first_name filter :last_name + filter :first_name + filter :middle_name filter_select :login_record_allowed_services_arr_contains, label: 'Allowed Services', @@ -18,22 +19,35 @@ 'data-chosen-opts': { disable_search: true }.to_json } + filter :inn + filter :passport_number + filter :phone_number + filter :email filter :login_record_login, label: 'Login' filter :created_at filter :updated_at extended_batch_destroy! - includes(:login_record) + controller do + def scoped_collection + super.preload(:login_record) + end + end index do selectable_column id_column actions - column :first_name column :last_name + column :first_name + column :middle_name column :allowed_services, :service_tags column :login + column :inn + column :passport_number + column :phone_number + column :email column :created_at column :updated_at end @@ -43,8 +57,13 @@ column do attributes_table do row :id - row :first_name row :last_name + row :first_name + row :middle_name + row :inn + row :passport_number + row :phone_number + row :email row :created_at row :updated_at end @@ -68,14 +87,20 @@ end permit_params :first_name, :last_name, + :inn, :passport_number, :middle_name, :phone_number, :email, login_record_attributes: [:id, :login, :password, allowed_services: []] form do |f| f.semantic_errors(*f.object.errors.keys) f.inputs do - f.input :first_name f.input :last_name + f.input :first_name + f.input :middle_name + f.input :inn + f.input :passport_number + f.input :phone_number + f.input :email end f.inputs for: [:login_record, f.object.login_record || LoginRecord.new] do |c| diff --git a/app/admin/students.rb b/app/admin/students.rb index f304a71..e439933 100644 --- a/app/admin/students.rb +++ b/app/admin/students.rb @@ -2,8 +2,9 @@ decorate_with StudentDecorator config.current_filters = false - filter :first_name filter :last_name + filter :first_name + filter :middle_name filter_select :login_record_allowed_services_arr_contains, label: 'Allowed Services', @@ -18,22 +19,37 @@ 'data-chosen-opts': { disable_search: true }.to_json } + filter :inn + filter :passport_number + filter :ticket_number + filter :phone_number + filter :email filter :login_record_login, label: 'Login' filter :created_at filter :updated_at extended_batch_destroy! - includes(:login_record) + controller do + def scoped_collection + super.preload(:login_record) + end + end index do selectable_column id_column actions - column :first_name column :last_name + column :first_name + column :middle_name column :allowed_services, :service_tags column :login + column :inn + column :passport_number + column :ticket_number + column :phone_number + column :email column :created_at column :updated_at end @@ -43,8 +59,14 @@ column do attributes_table do row :id - row :first_name row :last_name + row :first_name + row :middle_name + row :inn + row :passport_number + row :ticket_number + row :phone_number + row :email row :created_at row :updated_at end @@ -68,14 +90,21 @@ end permit_params :first_name, :last_name, + :inn, :passport_number, :ticket_number, :middle_name, :phone_number, :email, login_record_attributes: [:id, :login, :password, allowed_services: []] form do |f| f.semantic_errors(*f.object.errors.keys) f.inputs do - f.input :first_name f.input :last_name + f.input :first_name + f.input :middle_name + f.input :inn + f.input :passport_number + f.input :ticket_number + f.input :phone_number + f.input :email end f.inputs for: [:login_record, f.object.login_record || LoginRecord.new] do |c| diff --git a/app/forms/set_role_form.rb b/app/forms/set_role_form.rb index 69de5ac..4144bf5 100644 --- a/app/forms/set_role_form.rb +++ b/app/forms/set_role_form.rb @@ -19,7 +19,7 @@ def validate_root_role errors.add(:role, "not root admin user can't set root role to someone") end - if !set_role_to_root? && model.root? && !initiator.root? # rubocop:disable Style/GuardClause + if !set_role_to_root? && model.root? && !initiator.root? errors.add(:role, "not root admin user can't remove root role from another admin user") end end diff --git a/app/models/employee.rb b/app/models/employee.rb index 0b27e7e..a94b1af 100644 --- a/app/models/employee.rb +++ b/app/models/employee.rb @@ -2,17 +2,28 @@ # # Table name: employees # -# id :bigint(8) not null, primary key -# first_name :string not null -# last_name :string not null -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# email :string +# first_name :string not null +# inn :string +# last_name :string not null +# middle_name :string +# passport_number :string +# phone_number :string +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# employees_inn_key (inn) UNIQUE +# employees_passport_number_key (passport_number) UNIQUE # class Employee < ApplicationRecord include ActsAsLoginable validates :first_name, :last_name, presence: true + validates :inn, :passport_number, uniqueness: true, allow_blank: true def full_name "#{first_name} #{last_name}" diff --git a/app/models/student.rb b/app/models/student.rb index 558f581..cf27321 100644 --- a/app/models/student.rb +++ b/app/models/student.rb @@ -2,17 +2,30 @@ # # Table name: students # -# id :bigint(8) not null, primary key -# first_name :string not null -# last_name :string not null -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# email :string +# first_name :string not null +# inn :string +# last_name :string not null +# middle_name :string +# passport_number :string +# phone_number :string +# ticket_number :string +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# students_inn_key (inn) UNIQUE +# students_passport_number_key (passport_number) UNIQUE +# students_ticket_number_key (ticket_number) UNIQUE # class Student < ApplicationRecord include ActsAsLoginable validates :first_name, :last_name, presence: true + validates :inn, :passport_number, :ticket_number, uniqueness: true, allow_blank: true def full_name "#{first_name} #{last_name}" diff --git a/db/migrate/20191109103205_add_attributes_to_students_and_employees.rb b/db/migrate/20191109103205_add_attributes_to_students_and_employees.rb new file mode 100644 index 0000000..45fd7c9 --- /dev/null +++ b/db/migrate/20191109103205_add_attributes_to_students_and_employees.rb @@ -0,0 +1,43 @@ +class AddAttributesToStudentsAndEmployees < ActiveRecord::Migration[5.2] + def up + execute <<-SQL + ALTER TABLE students + ADD middle_name varchar, + ADD ticket_number varchar UNIQUE, + ADD passport_number varchar UNIQUE, + ADD inn varchar UNIQUE, + ADD phone_number varchar, + ADD email varchar; + SQL + + execute <<-SQL + ALTER TABLE employees + ADD middle_name varchar, + ADD passport_number varchar UNIQUE, + ADD inn varchar UNIQUE, + ADD phone_number varchar, + ADD email varchar; + SQL + end + + def down + execute <<-SQL + ALTER TABLE employees + DROP middle_name, + DROP passport_number, + DROP inn, + DROP phone_number, + DROP email; + SQL + + execute <<-SQL + ALTER TABLE students + DROP middle_name, + DROP ticket_number, + DROP passport_number, + DROP inn, + DROP phone_number, + DROP email; + SQL + end +end diff --git a/db/structure.sql b/db/structure.sql index 69f7992..8eae5d7 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -116,7 +116,12 @@ CREATE TABLE public.employees ( first_name character varying NOT NULL, last_name character varying NOT NULL, created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL + updated_at timestamp without time zone NOT NULL, + middle_name character varying, + passport_number character varying, + inn character varying, + phone_number character varying, + email character varying ); @@ -223,7 +228,13 @@ CREATE TABLE public.students ( first_name character varying NOT NULL, last_name character varying NOT NULL, created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL + updated_at timestamp without time zone NOT NULL, + middle_name character varying, + ticket_number character varying, + passport_number character varying, + inn character varying, + phone_number character varying, + email character varying ); @@ -363,6 +374,22 @@ ALTER TABLE ONLY public.ar_internal_metadata ADD CONSTRAINT ar_internal_metadata_pkey PRIMARY KEY (key); +-- +-- Name: employees employees_inn_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.employees + ADD CONSTRAINT employees_inn_key UNIQUE (inn); + + +-- +-- Name: employees employees_passport_number_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.employees + ADD CONSTRAINT employees_passport_number_key UNIQUE (passport_number); + + -- -- Name: employees employees_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -395,6 +422,22 @@ ALTER TABLE ONLY public.services ADD CONSTRAINT services_pkey PRIMARY KEY (id); +-- +-- Name: students students_inn_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.students + ADD CONSTRAINT students_inn_key UNIQUE (inn); + + +-- +-- Name: students students_passport_number_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.students + ADD CONSTRAINT students_passport_number_key UNIQUE (passport_number); + + -- -- Name: students students_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -403,6 +446,14 @@ ALTER TABLE ONLY public.students ADD CONSTRAINT students_pkey PRIMARY KEY (id); +-- +-- Name: students students_ticket_number_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.students + ADD CONSTRAINT students_ticket_number_key UNIQUE (ticket_number); + + -- -- Name: wifi_sessions wifi_sessions_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -475,6 +526,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20180627193100'), ('20180628083820'), ('20180914155932'), -('20190502185343'); +('20190502185343'), +('20191109103205'); diff --git a/spec/factories/employees.rb b/spec/factories/employees.rb index 9d05241..518a55c 100644 --- a/spec/factories/employees.rb +++ b/spec/factories/employees.rb @@ -2,11 +2,21 @@ # # Table name: employees # -# id :bigint(8) not null, primary key -# first_name :string not null -# last_name :string not null -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# email :string +# first_name :string not null +# inn :string +# last_name :string not null +# middle_name :string +# passport_number :string +# phone_number :string +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# employees_inn_key (inn) UNIQUE +# employees_passport_number_key (passport_number) UNIQUE # FactoryBot.define do diff --git a/spec/factories/students.rb b/spec/factories/students.rb index 467c1de..ca4a4c5 100644 --- a/spec/factories/students.rb +++ b/spec/factories/students.rb @@ -2,11 +2,23 @@ # # Table name: students # -# id :bigint(8) not null, primary key -# first_name :string not null -# last_name :string not null -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# email :string +# first_name :string not null +# inn :string +# last_name :string not null +# middle_name :string +# passport_number :string +# phone_number :string +# ticket_number :string +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# students_inn_key (inn) UNIQUE +# students_passport_number_key (passport_number) UNIQUE +# students_ticket_number_key (ticket_number) UNIQUE # FactoryBot.define do diff --git a/spec/models/employee_spec.rb b/spec/models/employee_spec.rb index 83d197b..bf69730 100644 --- a/spec/models/employee_spec.rb +++ b/spec/models/employee_spec.rb @@ -2,11 +2,21 @@ # # Table name: employees # -# id :bigint(8) not null, primary key -# first_name :string not null -# last_name :string not null -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# email :string +# first_name :string not null +# inn :string +# last_name :string not null +# middle_name :string +# passport_number :string +# phone_number :string +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# employees_inn_key (inn) UNIQUE +# employees_passport_number_key (passport_number) UNIQUE # require 'rails_helper' @@ -29,12 +39,49 @@ } end + it 'creates login record with correct attributes' do + expect(subject.login_record).to be_persisted + expect(subject.login_record).to have_attributes( + login: 'john.doe', + password: 'password123', + allowed_services: match_array([1, 2]) + ) + end + include_examples :creates_record do let(:expected_record_attrs) do create_params.except(:login_record).merge(allowed_services: match_array([1, 2])) end end include_examples :changes_records_count_of, described_class, by: 1 + include_examples :changes_records_count_of, LoginRecord, by: 1 + + context 'with optional attributes' do + let(:create_params) do + super().merge middle_name: 'Jamesovich', + email: 'john.doe@example.com', + inn: '1234567890', + passport_number: 'AZ123456', + phone_number: '+987654321' + end + + it 'creates login record with correct attributes' do + expect(subject.login_record).to be_persisted + expect(subject.login_record).to have_attributes( + login: 'john.doe', + password: 'password123', + allowed_services: match_array([1, 2]) + ) + end + + include_examples :creates_record do + let(:expected_record_attrs) do + create_params.except(:login_record).merge(allowed_services: match_array([1, 2])) + end + end + include_examples :changes_records_count_of, described_class, by: 1 + include_examples :changes_records_count_of, LoginRecord, by: 1 + end context 'without first_name' do let(:create_params) { super().merge first_name: '' } diff --git a/spec/models/student_spec.rb b/spec/models/student_spec.rb index b6cfff9..16df139 100644 --- a/spec/models/student_spec.rb +++ b/spec/models/student_spec.rb @@ -2,11 +2,23 @@ # # Table name: students # -# id :bigint(8) not null, primary key -# first_name :string not null -# last_name :string not null -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint(8) not null, primary key +# email :string +# first_name :string not null +# inn :string +# last_name :string not null +# middle_name :string +# passport_number :string +# phone_number :string +# ticket_number :string +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# students_inn_key (inn) UNIQUE +# students_passport_number_key (passport_number) UNIQUE +# students_ticket_number_key (ticket_number) UNIQUE # require 'rails_helper' @@ -44,6 +56,34 @@ include_examples :changes_records_count_of, described_class, by: 1 include_examples :changes_records_count_of, LoginRecord, by: 1 + context 'with optional attributes' do + let(:create_params) do + super().merge middle_name: 'Jamesovich', + email: 'john.doe@example.com', + inn: '1234567890', + passport_number: 'AZ123456', + phone_number: '+987654321', + ticket_number: '6781245774357' + end + + it 'creates login record with correct attributes' do + expect(subject.login_record).to be_persisted + expect(subject.login_record).to have_attributes( + login: 'john.doe', + password: 'password123', + allowed_services: match_array([1, 2]) + ) + end + + include_examples :creates_record do + let(:expected_record_attrs) do + create_params.except(:login_record).merge(allowed_services: match_array([1, 2])) + end + end + include_examples :changes_records_count_of, described_class, by: 1 + include_examples :changes_records_count_of, LoginRecord, by: 1 + end + context 'without first_name' do let(:create_params) { super().merge first_name: '' }