From c969ed0cc6e77ef43f3b7804aa04a2c6a2c0f17b Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Wed, 3 Jul 2024 15:51:43 +0200 Subject: [PATCH 01/32] Remove unwanted things from delayed job and start writing updating and creating of people --- app/controllers/people_controller.rb | 41 +++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 6d1fed1b8..5f91ded87 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -27,14 +27,23 @@ def show @person = Person.includes(projects: :project_technologies, person_roles: [:role, :person_role_level]).find(params.fetch(:id)) + + update_person_data super end def new - super - %w[DE EN FR].each do |language| - @person.language_skills.push(LanguageSkill.new({ language: language })) - end + ptime_employee_id = params[:ptime_employee_id] + person = Person.find_by(ptime_employee_id: ptime_employee_id) if ptime_employee_id + redirect_to action: :index unless person.nil? + new_person = Person.new + new_person.ptime_employee_id = ptime_employee_id + @person = new_person + redirect_to action: :show + # super + # %w[DE EN FR].each do |language| + # @person.language_skills.push(LanguageSkill.new({ language: language })) + # end end def create @@ -69,4 +78,28 @@ def fetch_entries def person @person ||= Person.find(params[:person_id]) end + + def update_person_data + attribute_mapping = { shortname: :shortname, email: :email, marital_status: :marital_status, + graduation: :title, company: :company, birthdate: :birthdate, + location: :location, nationality: :nationality }.freeze + + begin + ptime_employee = Ptime::Client.new.get("employees/#{@person.ptime_employee_id}")['data'] + rescue RestClient::InternalServerError, RestClient::NotFoundError + render(:index, status: :ok) + else + ptime_employee_firstname = ptime_employee['attributes']['firstname'] + ptime_employee_lastname = ptime_employee['attributes']['lastname'] + ptime_employee_name = "#{ptime_employee_firstname} #{ptime_employee_lastname}" + @person.name = ptime_employee_name + @person.ptime_employee_id ||= ptime_employee['id'] + ptime_employee['attributes'].each do |key, value| + if key.to_sym.in?(attribute_mapping.keys) + @person[attribute_mapping[key.to_sym]] = value + end + end + @person.save! + end + end end From 622913d8d9925c5fdc01c40130865772da818667 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Thu, 4 Jul 2024 08:23:11 +0200 Subject: [PATCH 02/32] Make show method use new record if necessary --- app/controllers/people_controller.rb | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 5f91ded87..0502dc1b0 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -25,21 +25,25 @@ def index def show return export if format_odt? - @person = Person.includes(projects: :project_technologies, - person_roles: [:role, :person_role_level]).find(params.fetch(:id)) - + unless @person.new_record? + @person = Person.includes(projects: :project_technologies, + person_roles: [:role, :person_role_level]).find(params.fetch(:id)) + end update_person_data super end def new - ptime_employee_id = params[:ptime_employee_id] - person = Person.find_by(ptime_employee_id: ptime_employee_id) if ptime_employee_id - redirect_to action: :index unless person.nil? - new_person = Person.new - new_person.ptime_employee_id = ptime_employee_id - @person = new_person - redirect_to action: :show + ptime_employee_id = params.fetch(:ptime_employee_id) + if ptime_employee_id + person = Person.find_by(ptime_employee_id: ptime_employee_id) + redirect_to action: :index unless person.nil? + new_person = Person.includes(projects: :project_technologies, + person_roles: [:role, :person_role_level]).new + new_person.ptime_employee_id = ptime_employee_id + @person = new_person + redirect_to action: :show + end # super # %w[DE EN FR].each do |language| # @person.language_skills.push(LanguageSkill.new({ language: language })) @@ -93,7 +97,6 @@ def update_person_data ptime_employee_lastname = ptime_employee['attributes']['lastname'] ptime_employee_name = "#{ptime_employee_firstname} #{ptime_employee_lastname}" @person.name = ptime_employee_name - @person.ptime_employee_id ||= ptime_employee['id'] ptime_employee['attributes'].each do |key, value| if key.to_sym.in?(attribute_mapping.keys) @person[attribute_mapping[key.to_sym]] = value From 80d6b896b482d9257c42d95aaa9e9c051dfa265b Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Thu, 4 Jul 2024 11:39:56 +0200 Subject: [PATCH 03/32] Move new create and update logic to new class and make create redirect to people if they already exist --- app/controllers/people_controller.rb | 39 +++---------------------- app/domain/ptime/update_person_data.rb | 40 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 35 deletions(-) create mode 100644 app/domain/ptime/update_person_data.rb diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 0502dc1b0..489f80289 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -27,23 +27,15 @@ def show unless @person.new_record? @person = Person.includes(projects: :project_technologies, - person_roles: [:role, :person_role_level]).find(params.fetch(:id)) + person_roles: [:role, :person_role_level]).find(@person.id) end - update_person_data + Ptime::UpdatePersonData.new.update_person_data(@person) super end def new - ptime_employee_id = params.fetch(:ptime_employee_id) - if ptime_employee_id - person = Person.find_by(ptime_employee_id: ptime_employee_id) - redirect_to action: :index unless person.nil? - new_person = Person.includes(projects: :project_technologies, - person_roles: [:role, :person_role_level]).new - new_person.ptime_employee_id = ptime_employee_id - @person = new_person - redirect_to action: :show - end + @person = Ptime::UpdatePersonData.new.create_person(params[:ptime_employee_id]) + redirect_to @person # super # %w[DE EN FR].each do |language| # @person.language_skills.push(LanguageSkill.new({ language: language })) @@ -82,27 +74,4 @@ def fetch_entries def person @person ||= Person.find(params[:person_id]) end - - def update_person_data - attribute_mapping = { shortname: :shortname, email: :email, marital_status: :marital_status, - graduation: :title, company: :company, birthdate: :birthdate, - location: :location, nationality: :nationality }.freeze - - begin - ptime_employee = Ptime::Client.new.get("employees/#{@person.ptime_employee_id}")['data'] - rescue RestClient::InternalServerError, RestClient::NotFoundError - render(:index, status: :ok) - else - ptime_employee_firstname = ptime_employee['attributes']['firstname'] - ptime_employee_lastname = ptime_employee['attributes']['lastname'] - ptime_employee_name = "#{ptime_employee_firstname} #{ptime_employee_lastname}" - @person.name = ptime_employee_name - ptime_employee['attributes'].each do |key, value| - if key.to_sym.in?(attribute_mapping.keys) - @person[attribute_mapping[key.to_sym]] = value - end - end - @person.save! - end - end end diff --git a/app/domain/ptime/update_person_data.rb b/app/domain/ptime/update_person_data.rb new file mode 100644 index 000000000..45ca7e4ac --- /dev/null +++ b/app/domain/ptime/update_person_data.rb @@ -0,0 +1,40 @@ +module Ptime + class UpdatePersonData + def create_person(ptime_employee_id) + raise 'No ptime_employee_id provided' unless ptime_employee_id + + person = Person.find_by(ptime_employee_id: ptime_employee_id) + return person unless person.nil? + + new_person = Person.includes(projects: :project_technologies, + person_roles: [:role, :person_role_level]).new + new_person.ptime_employee_id = ptime_employee_id + new_person + end + + def update_person_data(person) + attribute_mapping = { shortname: :shortname, email: :email, marital_status: :marital_status, + graduation: :title, company: :company, birthdate: :birthdate, + location: :location, nationality: :nationality }.freeze + + begin + ptime_employee = Ptime::Client.new.get("employees/#{person.ptime_employee_id}")['data'] + rescue RestClient::NotFound + raise "Ptime_employee with id #{person.ptime_employee_id} not found" + rescue StandardError + nil + else + ptime_employee_firstname = ptime_employee['attributes']['firstname'] + ptime_employee_lastname = ptime_employee['attributes']['lastname'] + ptime_employee_name = "#{ptime_employee_firstname} #{ptime_employee_lastname}" + person.name = ptime_employee_name + ptime_employee['attributes'].each do |key, value| + if key.to_sym.in?(attribute_mapping.keys) + person[attribute_mapping[key.to_sym]] = value + end + end + person.save! + end + end + end +end From 0d08852a38b08e2efeac636c5a7afa61c5c163d4 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Thu, 4 Jul 2024 14:45:14 +0200 Subject: [PATCH 04/32] Make newly created people have default languages, refactor code --- app/controllers/people_controller.rb | 19 ++++++++----------- app/domain/ptime/update_person_data.rb | 11 ++++++----- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 489f80289..a62da0c4a 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -8,8 +8,6 @@ class PeopleController < CrudController self.permitted_attrs = [:birthdate, :location, :marital_status, :updated_by, :name, :nationality, :nationality2, :title, :competence_notes, :company_id, :email, :department_id, :shortname, :picture, :picture_cache, - { person_roles_attributes: [:role_id, :person_role_level_id, - :percent, :id, :_destroy] }, { person_roles_attributes: [:role_id, :person_role_level_id, :percent, :id, :_destroy] }, { language_skills_attributes: @@ -25,21 +23,20 @@ def index def show return export if format_odt? - unless @person.new_record? - @person = Person.includes(projects: :project_technologies, - person_roles: [:role, :person_role_level]).find(@person.id) - end + @person = Person.includes(projects: :project_technologies, + person_roles: [:role, :person_role_level]).find(@person.id) + Ptime::UpdatePersonData.new.update_person_data(@person) super end def new @person = Ptime::UpdatePersonData.new.create_person(params[:ptime_employee_id]) - redirect_to @person - # super - # %w[DE EN FR].each do |language| - # @person.language_skills.push(LanguageSkill.new({ language: language })) - # end + %w[DE EN FR].each do |language| + @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) + end + redirect_to(@person) + #super end def create diff --git a/app/domain/ptime/update_person_data.rb b/app/domain/ptime/update_person_data.rb index 45ca7e4ac..77b67bfb9 100644 --- a/app/domain/ptime/update_person_data.rb +++ b/app/domain/ptime/update_person_data.rb @@ -6,10 +6,9 @@ def create_person(ptime_employee_id) person = Person.find_by(ptime_employee_id: ptime_employee_id) return person unless person.nil? - new_person = Person.includes(projects: :project_technologies, - person_roles: [:role, :person_role_level]).new - new_person.ptime_employee_id = ptime_employee_id - new_person + Person.create!(name: 'Default name', company: Company.first, birthdate: '1.1.1970', + nationality: 'CH', location: 'Bern', title: 'Default title', + email: 'default@example.com', ptime_employee_id: ptime_employee_id) end def update_person_data(person) @@ -18,6 +17,8 @@ def update_person_data(person) location: :location, nationality: :nationality }.freeze begin + return unless person.ptime_employee_id + ptime_employee = Ptime::Client.new.get("employees/#{person.ptime_employee_id}")['data'] rescue RestClient::NotFound raise "Ptime_employee with id #{person.ptime_employee_id} not found" @@ -30,7 +31,7 @@ def update_person_data(person) person.name = ptime_employee_name ptime_employee['attributes'].each do |key, value| if key.to_sym.in?(attribute_mapping.keys) - person[attribute_mapping[key.to_sym]] = value + person[attribute_mapping[key.to_sym]] = (value.presence || '-') end end person.save! From c071a85dfea11caa9e2d796d7a1a79962181a9c8 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Thu, 4 Jul 2024 16:15:15 +0200 Subject: [PATCH 05/32] Start writing tests for updating person data on visit and remove specs and code from old updating version with delayed job --- app/domain/ptime/update_person_data.rb | 12 +-- spec/features/update_person_data_spec.rb | 125 +++++++++++++++++++++++ 2 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 spec/features/update_person_data_spec.rb diff --git a/app/domain/ptime/update_person_data.rb b/app/domain/ptime/update_person_data.rb index 77b67bfb9..0601d8e86 100644 --- a/app/domain/ptime/update_person_data.rb +++ b/app/domain/ptime/update_person_data.rb @@ -11,9 +11,10 @@ def create_person(ptime_employee_id) email: 'default@example.com', ptime_employee_id: ptime_employee_id) end + # rubocop:disable Metrics def update_person_data(person) - attribute_mapping = { shortname: :shortname, email: :email, marital_status: :marital_status, - graduation: :title, company: :company, birthdate: :birthdate, + attribute_mapping = { full_name: :name, shortname: :shortname, email: :email, marital_status: + :marital_status, graduation: :title, birthdate: :birthdate, location: :location, nationality: :nationality }.freeze begin @@ -25,17 +26,16 @@ def update_person_data(person) rescue StandardError nil else - ptime_employee_firstname = ptime_employee['attributes']['firstname'] - ptime_employee_lastname = ptime_employee['attributes']['lastname'] - ptime_employee_name = "#{ptime_employee_firstname} #{ptime_employee_lastname}" - person.name = ptime_employee_name ptime_employee['attributes'].each do |key, value| if key.to_sym.in?(attribute_mapping.keys) person[attribute_mapping[key.to_sym]] = (value.presence || '-') end end + ptime_employee_employed = ptime_employee['attributes']['is_employed'] + person.company = Company.find_by(name: ptime_employee_employed ? 'Firma' : 'Ex-Mitarbeiter') person.save! end end + # rubocop:enable Metrics end end diff --git a/spec/features/update_person_data_spec.rb b/spec/features/update_person_data_spec.rb new file mode 100644 index 000000000..ffddb1de4 --- /dev/null +++ b/spec/features/update_person_data_spec.rb @@ -0,0 +1,125 @@ +require 'rails_helper' + +ptime_base_test_url = "www.ptime.example.com" +ptime_api_test_username = "test username" +ptime_api_test_password = "test password" +ENV["PTIME_BASE_URL"] = ptime_base_test_url +ENV["PTIME_API_USERNAME"] = ptime_api_test_username +ENV["PTIME_API_PASSWORD"] = ptime_api_test_password + +employees = { + 'data': [ + { + 'id': 33, + 'type': 'employee', + 'attributes': { + 'shortname': 'LSM', + 'firstname': 'Longmax', + 'lastname': 'Smith', + 'email': 'longmax@example.com', + 'marital_status': 'single', + 'nationalities': [ + 'ZW' + ], + 'graduation': 'BSc in Architecture', + 'department_shortname': 'SYS', + 'employment_roles': [] + } + }, + { + 'id': 21, + 'type': 'employee', + 'attributes': { + 'shortname': 'AMA', + 'firstname': 'Alice', + 'lastname': 'Mante', + 'email': 'alice@example.com', + 'marital_status': 'single', + 'nationalities': [ + 'AU' + ], + 'graduation': 'MSc in writing', + 'department_shortname': 'SYS', + 'employment_roles': [] + } + }, + { + 'id': 45, + 'type': 'employee', + 'attributes': { + 'shortname': 'CFO', + 'firstname': 'Charlie', + 'lastname': 'Ford', + 'email': 'charlie@example.com', + 'marital_status': 'married', + 'nationalities': [ + 'GB' + ], + 'graduation': 'MSc in networking', + 'department_shortname': 'SYS', + 'employment_roles': [] + } + }, + { + 'id': 50, + 'type': 'employee', + 'attributes': { + 'shortname': 'WAL', + 'firstname': 'Wally', + 'lastname': 'Allround', + 'email': 'wally@example.com', + 'marital_status': 'married', + 'nationalities': [ + 'US' + ], + 'graduation': 'Full-Stack Developer', + 'department_shortname': 'SYS', + 'employment_roles': [] + } + }, + ] +} + +describe Ptime::UpdatePersonData do + it 'should update person when visited' do + updated_wally = { + 'id': 50, + 'type': 'employee', + 'attributes': { + 'shortname': 'CAL', + 'firstname': 'Changed Wally', + 'lastname': 'Allround', + 'fullname': 'Changed Wally Allround', + 'email': 'changedwally@example.com', + 'marital_status': 'single', + 'nationalities': [ + 'US' + ], + 'graduation': 'Quarter-Stack Developer', + 'department_shortname': 'SYS', + 'employment_roles': [], + is_employed: false, + birthdate: '1.1.2000', + location: 'Bern', + nationality: 'CH' + }} + + stub_request(:get, "#{ptime_base_test_url}/api/v1/employees?per_page=1000"). + to_return(body: updated_wally.to_json, headers: { 'content-type': "application/vnd.api+json; charset=utf-8" }, status: 200) + .with(basic_auth: [ptime_api_test_username, ptime_api_test_password]) + + person_wally = people(:wally) + expect(person_wally.name).to eq('Wally Allround') + expect(person_wally.shortname).to eq('WAL') + visit person_path(person_wally) + person_wally.reload + expect(person_wally.name).to eq('Changed Wally Allround') + expect(person_wally.shortname).to eq('CAL') + expect(person_wally.email).to eq('changedwally@example.com') + expect(person_wally.marital_status).to eq('single') + expect(person_wally.title).to eq('Quarter-Stack Developer') + expect(person_wally.birthdate).to eq('1.1.2000') + expect(person_wally.location).to eq('Bern') + expect(person_wally.nationality).to eq('CH') + end +end \ No newline at end of file From b4d0f788b9ad68ec595429a90595c1a268283a9e Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 5 Jul 2024 11:50:58 +0200 Subject: [PATCH 06/32] Fix assigning and updating of nationalities and write test for creating a person --- app/domain/ptime/update_person_data.rb | 7 +- spec/features/update_person_data_spec.rb | 130 ++++++++++++++++------- 2 files changed, 98 insertions(+), 39 deletions(-) diff --git a/app/domain/ptime/update_person_data.rb b/app/domain/ptime/update_person_data.rb index 0601d8e86..7383de95f 100644 --- a/app/domain/ptime/update_person_data.rb +++ b/app/domain/ptime/update_person_data.rb @@ -15,13 +15,14 @@ def create_person(ptime_employee_id) def update_person_data(person) attribute_mapping = { full_name: :name, shortname: :shortname, email: :email, marital_status: :marital_status, graduation: :title, birthdate: :birthdate, - location: :location, nationality: :nationality }.freeze + location: :location }.freeze begin return unless person.ptime_employee_id ptime_employee = Ptime::Client.new.get("employees/#{person.ptime_employee_id}")['data'] rescue RestClient::NotFound + person.destroy! if person.created_at == person.updated_at raise "Ptime_employee with id #{person.ptime_employee_id} not found" rescue StandardError nil @@ -33,6 +34,10 @@ def update_person_data(person) end ptime_employee_employed = ptime_employee['attributes']['is_employed'] person.company = Company.find_by(name: ptime_employee_employed ? 'Firma' : 'Ex-Mitarbeiter') + ptime_employee_nationalities = ptime_employee['attributes']['nationalities'] + person.nationality = ptime_employee_nationalities[0] + person.nationality2 = ptime_employee_nationalities[1] + person.destroy! if !person.valid? && person.created_at == person.updated_at person.save! end end diff --git a/spec/features/update_person_data_spec.rb b/spec/features/update_person_data_spec.rb index ffddb1de4..dacf914aa 100644 --- a/spec/features/update_person_data_spec.rb +++ b/spec/features/update_person_data_spec.rb @@ -81,45 +81,99 @@ } describe Ptime::UpdatePersonData do - it 'should update person when visited' do - updated_wally = { - 'id': 50, - 'type': 'employee', - 'attributes': { - 'shortname': 'CAL', - 'firstname': 'Changed Wally', - 'lastname': 'Allround', - 'fullname': 'Changed Wally Allround', - 'email': 'changedwally@example.com', - 'marital_status': 'single', - 'nationalities': [ - 'US' - ], - 'graduation': 'Quarter-Stack Developer', - 'department_shortname': 'SYS', - 'employment_roles': [], - is_employed: false, - birthdate: '1.1.2000', - location: 'Bern', - nationality: 'CH' - }} + describe 'Update or create person', type: :feature, js: true do + before(:each) do + sign_in auth_users(:user), scope: :auth_user + end + + it 'should update person when visited' do + updated_wally = { + 'data': { + 'id': 50, + 'type': 'employee', + 'attributes': { + 'shortname': 'CAL', + 'firstname': 'Changed Wally', + 'lastname': 'Allround', + 'full_name': 'Changed Wally Allround', + 'email': 'changedwally@example.com', + 'marital_status': 'single', + 'nationalities': %w[DE DK], + 'graduation': 'Quarter-Stack Developer', + 'department_shortname': 'SYS', + 'employment_roles': [], + 'is_employed': false, + 'birthdate': '1.1.2000', + 'location': 'Basel', + }} + } + + + stub_request(:get, "#{ptime_base_test_url}/api/v1/employees/50"). + to_return(body: updated_wally.to_json, headers: { 'content-type': "application/vnd.api+json; charset=utf-8" }, status: 200) + .with(basic_auth: [ptime_api_test_username, ptime_api_test_password]) + + Company.create!(name: "Ex-Mitarbeiter") + person_wally = people(:wally) + person_wally.ptime_employee_id = 50 + person_wally.save! + + expect(person_wally.name).to eq('Wally Allround') + expect(person_wally.email).to eq('wally@example.com') + visit person_path(person_wally) + person_wally.reload + expect(person_wally.name).to eq('Changed Wally Allround') + expect(person_wally.shortname).to eq('CAL') + expect(person_wally.email).to eq('changedwally@example.com') + expect(person_wally.marital_status).to eq('single') + expect(person_wally.title).to eq('Quarter-Stack Developer') + expect(person_wally.birthdate).to eq('1.1.2000') + expect(person_wally.location).to eq('Basel') + expect(person_wally.nationality).to eq('DE') + expect(person_wally.nationality2).to eq('DK') + end + + it 'should create person when visited' do + updated_wally = { + 'data': { + 'id': 50, + 'type': 'employee', + 'attributes': { + 'shortname': 'CAL', + 'firstname': 'Changed Wally', + 'lastname': 'Allround', + 'full_name': 'Changed Wally Allround', + 'email': 'changedwally@example.com', + 'marital_status': 'single', + 'nationalities': %w[DE EG], + 'graduation': 'Quarter-Stack Developer', + 'department_shortname': 'SYS', + 'employment_roles': [], + 'is_employed': true, + 'birthdate': '1.1.2000', + 'location': 'Basel', + }} + } - stub_request(:get, "#{ptime_base_test_url}/api/v1/employees?per_page=1000"). - to_return(body: updated_wally.to_json, headers: { 'content-type': "application/vnd.api+json; charset=utf-8" }, status: 200) - .with(basic_auth: [ptime_api_test_username, ptime_api_test_password]) + stub_request(:get, "#{ptime_base_test_url}/api/v1/employees/50"). + to_return(body: updated_wally.to_json, headers: { 'content-type': "application/vnd.api+json; charset=utf-8" }, status: 200) + .with(basic_auth: [ptime_api_test_username, ptime_api_test_password]) + Company.create!(name: "Ex-Mitarbeiter") - person_wally = people(:wally) - expect(person_wally.name).to eq('Wally Allround') - expect(person_wally.shortname).to eq('WAL') - visit person_path(person_wally) - person_wally.reload - expect(person_wally.name).to eq('Changed Wally Allround') - expect(person_wally.shortname).to eq('CAL') - expect(person_wally.email).to eq('changedwally@example.com') - expect(person_wally.marital_status).to eq('single') - expect(person_wally.title).to eq('Quarter-Stack Developer') - expect(person_wally.birthdate).to eq('1.1.2000') - expect(person_wally.location).to eq('Bern') - expect(person_wally.nationality).to eq('CH') + person_wally = people(:wally) + person_wally.destroy! + expect(Person.find_by(name: "Wally Allround")).to be_nil + visit new_person_path(ptime_employee_id: 50) + new_wally = Person.find_by(name: "Changed Wally Allround") + expect(new_wally.name).to eq('Changed Wally Allround') + expect(new_wally.shortname).to eq('CAL') + expect(new_wally.email).to eq('changedwally@example.com') + expect(new_wally.marital_status).to eq('single') + expect(new_wally.title).to eq('Quarter-Stack Developer') + expect(new_wally.birthdate).to eq('1.1.2000') + expect(new_wally.location).to eq('Basel') + expect(new_wally.nationality).to eq('DE') + expect(new_wally.nationality2).to eq('EG') + end end end \ No newline at end of file From 12ab636e70c41eb1858cd013b22d0248e62def16 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 5 Jul 2024 15:32:05 +0200 Subject: [PATCH 07/32] Write a spec that tests if create_person can return an already existing person --- spec/domain/ptime/update_person_data_spec.rb | 25 ++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 spec/domain/ptime/update_person_data_spec.rb diff --git a/spec/domain/ptime/update_person_data_spec.rb b/spec/domain/ptime/update_person_data_spec.rb new file mode 100644 index 000000000..7e9acfab4 --- /dev/null +++ b/spec/domain/ptime/update_person_data_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +describe Ptime::UpdatePersonData do + it 'should raise error when no ptime_employee_id is passed to new action' do + expect{ Ptime::UpdatePersonData.new.create_person(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') + end + + it 'should create default person if ptime_employee_id doesnt already exist' do + default_person = Person.create(name: 'Default name', company: Company.first, birthdate: '1.1.1970', + nationality: 'CH', location: 'Bern', title: 'Default title', + email: 'default@example.com', ptime_employee_id: 123) + + new_person = Ptime::UpdatePersonData.new.create_person(123) + expect(default_person.attributes.except(*%w[created_at updated_at])).to eql(new_person.attributes.except(*%w[created_at updated_at])) + end + + it 'should return person if it has the given ptime_employee_id' do + person_wally = people(:wally) + person_wally.ptime_employee_id = 123 + person_wally.save! + + new_person = Ptime::UpdatePersonData.new.create_person(person_wally.ptime_employee_id) + expect(person_wally.attributes.except(*%w[created_at updated_at])).to eql(new_person.attributes.except(*%w[created_at updated_at])) + end +end \ No newline at end of file From ec2bb14a1862498f8e4ece72539d0df2767f91d8 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 5 Jul 2024 15:52:33 +0200 Subject: [PATCH 08/32] Write spec that tests ensures an error is raised when the person is not found in the ptime api --- spec/domain/ptime/update_person_data_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spec/domain/ptime/update_person_data_spec.rb b/spec/domain/ptime/update_person_data_spec.rb index 7e9acfab4..4b90900e4 100644 --- a/spec/domain/ptime/update_person_data_spec.rb +++ b/spec/domain/ptime/update_person_data_spec.rb @@ -1,5 +1,12 @@ require 'rails_helper' +ptime_base_test_url = "www.ptime.example.com" +ptime_api_test_username = "test username" +ptime_api_test_password = "test password" +ENV["PTIME_BASE_URL"] = ptime_base_test_url +ENV["PTIME_API_USERNAME"] = ptime_api_test_username +ENV["PTIME_API_PASSWORD"] = ptime_api_test_password + describe Ptime::UpdatePersonData do it 'should raise error when no ptime_employee_id is passed to new action' do expect{ Ptime::UpdatePersonData.new.create_person(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') @@ -22,4 +29,16 @@ new_person = Ptime::UpdatePersonData.new.create_person(person_wally.ptime_employee_id) expect(person_wally.attributes.except(*%w[created_at updated_at])).to eql(new_person.attributes.except(*%w[created_at updated_at])) end + + it 'should raise error when person is not found in ptime api' do + stub_request(:get, "#{ptime_base_test_url}/api/v1/employees/50"). + to_return(headers: { 'content-type': "application/vnd.api+json; charset=utf-8" }, status: 404) + .with(basic_auth: [ptime_api_test_username, ptime_api_test_password]) + + person_wally = people(:wally) + person_wally.ptime_employee_id = 50 + person_wally.save! + + expect{ Ptime::UpdatePersonData.new.update_person_data(person_wally) }.to raise_error(RuntimeError, 'Ptime_employee with id 50 not found') + end end \ No newline at end of file From 193f80a2cb60672cdd63177d46d669dd2f1a83c1 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Mon, 29 Jul 2024 11:34:45 +0200 Subject: [PATCH 09/32] Make create_person method directly create the correct person without creating a default person --- app/controllers/people_controller.rb | 4 +++- app/domain/ptime/update_person_data.rb | 13 ++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index a62da0c4a..d6c3a8d5e 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -33,7 +33,9 @@ def show def new @person = Ptime::UpdatePersonData.new.create_person(params[:ptime_employee_id]) %w[DE EN FR].each do |language| - @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) + unless @person.language_skills.pluck(:language).include?(language.to_s) + @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) + end end redirect_to(@person) #super diff --git a/app/domain/ptime/update_person_data.rb b/app/domain/ptime/update_person_data.rb index 7383de95f..b0436d81c 100644 --- a/app/domain/ptime/update_person_data.rb +++ b/app/domain/ptime/update_person_data.rb @@ -6,9 +6,7 @@ def create_person(ptime_employee_id) person = Person.find_by(ptime_employee_id: ptime_employee_id) return person unless person.nil? - Person.create!(name: 'Default name', company: Company.first, birthdate: '1.1.1970', - nationality: 'CH', location: 'Bern', title: 'Default title', - email: 'default@example.com', ptime_employee_id: ptime_employee_id) + update_person_data(person) end # rubocop:disable Metrics @@ -18,14 +16,11 @@ def update_person_data(person) location: :location }.freeze begin - return unless person.ptime_employee_id + raise 'Person has no ptime_employee_id' unless person.ptime_employee_id ptime_employee = Ptime::Client.new.get("employees/#{person.ptime_employee_id}")['data'] rescue RestClient::NotFound - person.destroy! if person.created_at == person.updated_at - raise "Ptime_employee with id #{person.ptime_employee_id} not found" - rescue StandardError - nil + raise "Ptime_employee with ptime_employee_id #{person.ptime_employee_id} not found" else ptime_employee['attributes'].each do |key, value| if key.to_sym.in?(attribute_mapping.keys) @@ -37,8 +32,8 @@ def update_person_data(person) ptime_employee_nationalities = ptime_employee['attributes']['nationalities'] person.nationality = ptime_employee_nationalities[0] person.nationality2 = ptime_employee_nationalities[1] - person.destroy! if !person.valid? && person.created_at == person.updated_at person.save! + person end end # rubocop:enable Metrics From 61541a7df14d90c05077e64b98e18d109819a1c2 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Mon, 29 Jul 2024 13:27:33 +0200 Subject: [PATCH 10/32] Clean up and fix specs, fix create_person method in update_person_data class --- app/domain/ptime/update_person_data.rb | 2 +- spec/domain/ptime/update_person_data_spec.rb | 28 +++--- spec/features/update_person_data_spec.rb | 89 ++------------------ 3 files changed, 20 insertions(+), 99 deletions(-) diff --git a/app/domain/ptime/update_person_data.rb b/app/domain/ptime/update_person_data.rb index b0436d81c..e296db278 100644 --- a/app/domain/ptime/update_person_data.rb +++ b/app/domain/ptime/update_person_data.rb @@ -6,7 +6,7 @@ def create_person(ptime_employee_id) person = Person.find_by(ptime_employee_id: ptime_employee_id) return person unless person.nil? - update_person_data(person) + update_person_data(Person.new(ptime_employee_id: ptime_employee_id)) end # rubocop:disable Metrics diff --git a/spec/domain/ptime/update_person_data_spec.rb b/spec/domain/ptime/update_person_data_spec.rb index 4b90900e4..c3095609f 100644 --- a/spec/domain/ptime/update_person_data_spec.rb +++ b/spec/domain/ptime/update_person_data_spec.rb @@ -1,24 +1,17 @@ require 'rails_helper' -ptime_base_test_url = "www.ptime.example.com" -ptime_api_test_username = "test username" -ptime_api_test_password = "test password" -ENV["PTIME_BASE_URL"] = ptime_base_test_url -ENV["PTIME_API_USERNAME"] = ptime_api_test_username -ENV["PTIME_API_PASSWORD"] = ptime_api_test_password - describe Ptime::UpdatePersonData do - it 'should raise error when no ptime_employee_id is passed to new action' do - expect{ Ptime::UpdatePersonData.new.create_person(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') + ptime_base_test_url = "www.ptime.example.com" + ptime_api_test_username = "test username" + ptime_api_test_password = "test password" + before(:each) do + ENV["PTIME_BASE_URL"] = ptime_base_test_url + ENV["PTIME_API_USERNAME"] = ptime_api_test_username + ENV["PTIME_API_PASSWORD"] = ptime_api_test_password end - it 'should create default person if ptime_employee_id doesnt already exist' do - default_person = Person.create(name: 'Default name', company: Company.first, birthdate: '1.1.1970', - nationality: 'CH', location: 'Bern', title: 'Default title', - email: 'default@example.com', ptime_employee_id: 123) - - new_person = Ptime::UpdatePersonData.new.create_person(123) - expect(default_person.attributes.except(*%w[created_at updated_at])).to eql(new_person.attributes.except(*%w[created_at updated_at])) + it 'should raise error when no ptime_employee_id is passed to new action' do + expect{ Ptime::UpdatePersonData.new.create_person(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') end it 'should return person if it has the given ptime_employee_id' do @@ -38,7 +31,6 @@ person_wally = people(:wally) person_wally.ptime_employee_id = 50 person_wally.save! - - expect{ Ptime::UpdatePersonData.new.update_person_data(person_wally) }.to raise_error(RuntimeError, 'Ptime_employee with id 50 not found') + expect{ Ptime::UpdatePersonData.new.update_person_data(person_wally) }.to raise_error(RuntimeError, 'Ptime_employee with ptime_employee_id 50 not found') end end \ No newline at end of file diff --git a/spec/features/update_person_data_spec.rb b/spec/features/update_person_data_spec.rb index dacf914aa..7b6335a7b 100644 --- a/spec/features/update_person_data_spec.rb +++ b/spec/features/update_person_data_spec.rb @@ -1,86 +1,15 @@ require 'rails_helper' -ptime_base_test_url = "www.ptime.example.com" -ptime_api_test_username = "test username" -ptime_api_test_password = "test password" -ENV["PTIME_BASE_URL"] = ptime_base_test_url -ENV["PTIME_API_USERNAME"] = ptime_api_test_username -ENV["PTIME_API_PASSWORD"] = ptime_api_test_password - -employees = { - 'data': [ - { - 'id': 33, - 'type': 'employee', - 'attributes': { - 'shortname': 'LSM', - 'firstname': 'Longmax', - 'lastname': 'Smith', - 'email': 'longmax@example.com', - 'marital_status': 'single', - 'nationalities': [ - 'ZW' - ], - 'graduation': 'BSc in Architecture', - 'department_shortname': 'SYS', - 'employment_roles': [] - } - }, - { - 'id': 21, - 'type': 'employee', - 'attributes': { - 'shortname': 'AMA', - 'firstname': 'Alice', - 'lastname': 'Mante', - 'email': 'alice@example.com', - 'marital_status': 'single', - 'nationalities': [ - 'AU' - ], - 'graduation': 'MSc in writing', - 'department_shortname': 'SYS', - 'employment_roles': [] - } - }, - { - 'id': 45, - 'type': 'employee', - 'attributes': { - 'shortname': 'CFO', - 'firstname': 'Charlie', - 'lastname': 'Ford', - 'email': 'charlie@example.com', - 'marital_status': 'married', - 'nationalities': [ - 'GB' - ], - 'graduation': 'MSc in networking', - 'department_shortname': 'SYS', - 'employment_roles': [] - } - }, - { - 'id': 50, - 'type': 'employee', - 'attributes': { - 'shortname': 'WAL', - 'firstname': 'Wally', - 'lastname': 'Allround', - 'email': 'wally@example.com', - 'marital_status': 'married', - 'nationalities': [ - 'US' - ], - 'graduation': 'Full-Stack Developer', - 'department_shortname': 'SYS', - 'employment_roles': [] - } - }, - ] -} - describe Ptime::UpdatePersonData do + ptime_base_test_url = "www.ptime.example.com" + ptime_api_test_username = "test username" + ptime_api_test_password = "test password" + before(:each) do + ENV["PTIME_BASE_URL"] = ptime_base_test_url + ENV["PTIME_API_USERNAME"] = ptime_api_test_username + ENV["PTIME_API_PASSWORD"] = ptime_api_test_password + end + describe 'Update or create person', type: :feature, js: true do before(:each) do sign_in auth_users(:user), scope: :auth_user From c06233def5f9a33ee01d43a6d610bad85fccfa0d Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Mon, 29 Jul 2024 14:04:57 +0200 Subject: [PATCH 11/32] Start updating default language logic --- app/controllers/people_controller.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index d6c3a8d5e..3c8317cc4 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -32,12 +32,14 @@ def show def new @person = Ptime::UpdatePersonData.new.create_person(params[:ptime_employee_id]) - %w[DE EN FR].each do |language| - unless @person.language_skills.pluck(:language).include?(language.to_s) - @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) - end - end + # (%w[DE EN FR] - @person.language_skills.pluck(:language)).each do |language| + # @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) + # end redirect_to(@person) + + # %w[DE EN FR].each do |language| + # @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) + # end #super end From 58a9824603aec76959ee3399ccb535e47ccd3464 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Mon, 29 Jul 2024 14:25:47 +0200 Subject: [PATCH 12/32] Rename update_person_data from ptime namespace to people_employees --- app/controllers/people_controller.rb | 4 ++-- .../ptime/{update_person_data.rb => people_employees.rb} | 2 +- ...pdate_person_data_spec.rb => people_employees_spec.rb} | 8 ++++---- ...pdate_person_data_spec.rb => people_employees_spec.rb} | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) rename app/domain/ptime/{update_person_data.rb => people_employees.rb} (98%) rename spec/domain/ptime/{update_person_data_spec.rb => people_employees_spec.rb} (75%) rename spec/features/{update_person_data_spec.rb => people_employees_spec.rb} (99%) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 3c8317cc4..493eba348 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -26,12 +26,12 @@ def show @person = Person.includes(projects: :project_technologies, person_roles: [:role, :person_role_level]).find(@person.id) - Ptime::UpdatePersonData.new.update_person_data(@person) + Ptime::PeopleEmployees.new.update_person_data(@person) super end def new - @person = Ptime::UpdatePersonData.new.create_person(params[:ptime_employee_id]) + @person = Ptime::PeopleEmployees.new.create_person(params[:ptime_employee_id]) # (%w[DE EN FR] - @person.language_skills.pluck(:language)).each do |language| # @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) # end diff --git a/app/domain/ptime/update_person_data.rb b/app/domain/ptime/people_employees.rb similarity index 98% rename from app/domain/ptime/update_person_data.rb rename to app/domain/ptime/people_employees.rb index e296db278..1a6a7217d 100644 --- a/app/domain/ptime/update_person_data.rb +++ b/app/domain/ptime/people_employees.rb @@ -1,5 +1,5 @@ module Ptime - class UpdatePersonData + class PeopleEmployees def create_person(ptime_employee_id) raise 'No ptime_employee_id provided' unless ptime_employee_id diff --git a/spec/domain/ptime/update_person_data_spec.rb b/spec/domain/ptime/people_employees_spec.rb similarity index 75% rename from spec/domain/ptime/update_person_data_spec.rb rename to spec/domain/ptime/people_employees_spec.rb index c3095609f..b30d2752a 100644 --- a/spec/domain/ptime/update_person_data_spec.rb +++ b/spec/domain/ptime/people_employees_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe Ptime::UpdatePersonData do +describe Ptime::PeopleEmployees do ptime_base_test_url = "www.ptime.example.com" ptime_api_test_username = "test username" ptime_api_test_password = "test password" @@ -11,7 +11,7 @@ end it 'should raise error when no ptime_employee_id is passed to new action' do - expect{ Ptime::UpdatePersonData.new.create_person(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') + expect{ Ptime::PeopleEmployees.new.create_person(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') end it 'should return person if it has the given ptime_employee_id' do @@ -19,7 +19,7 @@ person_wally.ptime_employee_id = 123 person_wally.save! - new_person = Ptime::UpdatePersonData.new.create_person(person_wally.ptime_employee_id) + new_person = Ptime::PeopleEmployees.new.create_person(person_wally.ptime_employee_id) expect(person_wally.attributes.except(*%w[created_at updated_at])).to eql(new_person.attributes.except(*%w[created_at updated_at])) end @@ -31,6 +31,6 @@ person_wally = people(:wally) person_wally.ptime_employee_id = 50 person_wally.save! - expect{ Ptime::UpdatePersonData.new.update_person_data(person_wally) }.to raise_error(RuntimeError, 'Ptime_employee with ptime_employee_id 50 not found') + expect{ Ptime::PeopleEmployees.new.update_person_data(person_wally) }.to raise_error(RuntimeError, 'Ptime_employee with ptime_employee_id 50 not found') end end \ No newline at end of file diff --git a/spec/features/update_person_data_spec.rb b/spec/features/people_employees_spec.rb similarity index 99% rename from spec/features/update_person_data_spec.rb rename to spec/features/people_employees_spec.rb index 7b6335a7b..ea88eb471 100644 --- a/spec/features/update_person_data_spec.rb +++ b/spec/features/people_employees_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe Ptime::UpdatePersonData do +describe Ptime::PeopleEmployees do ptime_base_test_url = "www.ptime.example.com" ptime_api_test_username = "test username" ptime_api_test_password = "test password" From a724c4514db55e5906cffeda88190343e3170790 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Mon, 29 Jul 2024 14:31:12 +0200 Subject: [PATCH 13/32] Rename create_person method to create_or_find --- app/controllers/people_controller.rb | 2 +- app/domain/ptime/people_employees.rb | 2 +- spec/domain/ptime/people_employees_spec.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 493eba348..ea165865f 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -31,7 +31,7 @@ def show end def new - @person = Ptime::PeopleEmployees.new.create_person(params[:ptime_employee_id]) + @person = Ptime::PeopleEmployees.new.create_or_find(params[:ptime_employee_id]) # (%w[DE EN FR] - @person.language_skills.pluck(:language)).each do |language| # @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) # end diff --git a/app/domain/ptime/people_employees.rb b/app/domain/ptime/people_employees.rb index 1a6a7217d..103efb68c 100644 --- a/app/domain/ptime/people_employees.rb +++ b/app/domain/ptime/people_employees.rb @@ -1,6 +1,6 @@ module Ptime class PeopleEmployees - def create_person(ptime_employee_id) + def create_or_find(ptime_employee_id) raise 'No ptime_employee_id provided' unless ptime_employee_id person = Person.find_by(ptime_employee_id: ptime_employee_id) diff --git a/spec/domain/ptime/people_employees_spec.rb b/spec/domain/ptime/people_employees_spec.rb index b30d2752a..b1cc07ab9 100644 --- a/spec/domain/ptime/people_employees_spec.rb +++ b/spec/domain/ptime/people_employees_spec.rb @@ -11,7 +11,7 @@ end it 'should raise error when no ptime_employee_id is passed to new action' do - expect{ Ptime::PeopleEmployees.new.create_person(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') + expect{ Ptime::PeopleEmployees.new.create_or_find(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') end it 'should return person if it has the given ptime_employee_id' do @@ -19,7 +19,7 @@ person_wally.ptime_employee_id = 123 person_wally.save! - new_person = Ptime::PeopleEmployees.new.create_person(person_wally.ptime_employee_id) + new_person = Ptime::PeopleEmployees.new.create_or_find(person_wally.ptime_employee_id) expect(person_wally.attributes.except(*%w[created_at updated_at])).to eql(new_person.attributes.except(*%w[created_at updated_at])) end From 7037d7f824a0af581b2d04f2daa9fb213e91008b Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Tue, 30 Jul 2024 09:13:53 +0200 Subject: [PATCH 14/32] Change way in which default languages are set --- app/controllers/people_controller.rb | 5 +---- app/domain/ptime/people_employees.rb | 6 +++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index ea165865f..88484af68 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -32,15 +32,12 @@ def show def new @person = Ptime::PeopleEmployees.new.create_or_find(params[:ptime_employee_id]) - # (%w[DE EN FR] - @person.language_skills.pluck(:language)).each do |language| - # @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) - # end redirect_to(@person) # %w[DE EN FR].each do |language| # @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) # end - #super + # super end def create diff --git a/app/domain/ptime/people_employees.rb b/app/domain/ptime/people_employees.rb index 103efb68c..96a2e645e 100644 --- a/app/domain/ptime/people_employees.rb +++ b/app/domain/ptime/people_employees.rb @@ -6,7 +6,11 @@ def create_or_find(ptime_employee_id) person = Person.find_by(ptime_employee_id: ptime_employee_id) return person unless person.nil? - update_person_data(Person.new(ptime_employee_id: ptime_employee_id)) + new_person = Person.new(ptime_employee_id: ptime_employee_id) + %w[DE EN FR].each do |language| + new_person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) + end + update_person_data(new_person) end # rubocop:disable Metrics From 9f018bcfb9725e1e854b584a429ad1cc8da4bb45 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Tue, 30 Jul 2024 10:32:50 +0200 Subject: [PATCH 15/32] Update tests with new helpers --- app/domain/ptime/people_employees.rb | 10 ++-- spec/domain/ptime/people_employees_spec.rb | 13 +---- spec/features/people_employees_spec.rb | 61 ++-------------------- spec/fixtures/files/json/wally.json | 23 ++++++++ 4 files changed, 32 insertions(+), 75 deletions(-) create mode 100644 spec/fixtures/files/json/wally.json diff --git a/app/domain/ptime/people_employees.rb b/app/domain/ptime/people_employees.rb index 96a2e645e..125534e6d 100644 --- a/app/domain/ptime/people_employees.rb +++ b/app/domain/ptime/people_employees.rb @@ -22,18 +22,18 @@ def update_person_data(person) begin raise 'Person has no ptime_employee_id' unless person.ptime_employee_id - ptime_employee = Ptime::Client.new.get("employees/#{person.ptime_employee_id}")['data'] - rescue RestClient::NotFound + ptime_employee = Ptime::Client.new.request(:get, "employees/#{person.ptime_employee_id}") + rescue CustomExceptions::PTimeClientError raise "Ptime_employee with ptime_employee_id #{person.ptime_employee_id} not found" else - ptime_employee['attributes'].each do |key, value| + ptime_employee[:attributes].each do |key, value| if key.to_sym.in?(attribute_mapping.keys) person[attribute_mapping[key.to_sym]] = (value.presence || '-') end end - ptime_employee_employed = ptime_employee['attributes']['is_employed'] + ptime_employee_employed = ptime_employee[:attributes][:is_employeed] person.company = Company.find_by(name: ptime_employee_employed ? 'Firma' : 'Ex-Mitarbeiter') - ptime_employee_nationalities = ptime_employee['attributes']['nationalities'] + ptime_employee_nationalities = ptime_employee[:attributes][:nationalities] person.nationality = ptime_employee_nationalities[0] person.nationality2 = ptime_employee_nationalities[1] person.save! diff --git a/spec/domain/ptime/people_employees_spec.rb b/spec/domain/ptime/people_employees_spec.rb index b1cc07ab9..e8a9a5a82 100644 --- a/spec/domain/ptime/people_employees_spec.rb +++ b/spec/domain/ptime/people_employees_spec.rb @@ -1,15 +1,6 @@ require 'rails_helper' describe Ptime::PeopleEmployees do - ptime_base_test_url = "www.ptime.example.com" - ptime_api_test_username = "test username" - ptime_api_test_password = "test password" - before(:each) do - ENV["PTIME_BASE_URL"] = ptime_base_test_url - ENV["PTIME_API_USERNAME"] = ptime_api_test_username - ENV["PTIME_API_PASSWORD"] = ptime_api_test_password - end - it 'should raise error when no ptime_employee_id is passed to new action' do expect{ Ptime::PeopleEmployees.new.create_or_find(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') end @@ -24,9 +15,7 @@ end it 'should raise error when person is not found in ptime api' do - stub_request(:get, "#{ptime_base_test_url}/api/v1/employees/50"). - to_return(headers: { 'content-type': "application/vnd.api+json; charset=utf-8" }, status: 404) - .with(basic_auth: [ptime_api_test_username, ptime_api_test_password]) + stub_ptime_request('', "employees/50", 404) person_wally = people(:wally) person_wally.ptime_employee_id = 50 diff --git a/spec/features/people_employees_spec.rb b/spec/features/people_employees_spec.rb index ea88eb471..6900e6840 100644 --- a/spec/features/people_employees_spec.rb +++ b/spec/features/people_employees_spec.rb @@ -1,46 +1,13 @@ require 'rails_helper' describe Ptime::PeopleEmployees do - ptime_base_test_url = "www.ptime.example.com" - ptime_api_test_username = "test username" - ptime_api_test_password = "test password" - before(:each) do - ENV["PTIME_BASE_URL"] = ptime_base_test_url - ENV["PTIME_API_USERNAME"] = ptime_api_test_username - ENV["PTIME_API_PASSWORD"] = ptime_api_test_password - end - describe 'Update or create person', type: :feature, js: true do before(:each) do sign_in auth_users(:user), scope: :auth_user end it 'should update person when visited' do - updated_wally = { - 'data': { - 'id': 50, - 'type': 'employee', - 'attributes': { - 'shortname': 'CAL', - 'firstname': 'Changed Wally', - 'lastname': 'Allround', - 'full_name': 'Changed Wally Allround', - 'email': 'changedwally@example.com', - 'marital_status': 'single', - 'nationalities': %w[DE DK], - 'graduation': 'Quarter-Stack Developer', - 'department_shortname': 'SYS', - 'employment_roles': [], - 'is_employed': false, - 'birthdate': '1.1.2000', - 'location': 'Basel', - }} - } - - - stub_request(:get, "#{ptime_base_test_url}/api/v1/employees/50"). - to_return(body: updated_wally.to_json, headers: { 'content-type': "application/vnd.api+json; charset=utf-8" }, status: 200) - .with(basic_auth: [ptime_api_test_username, ptime_api_test_password]) + stub_ptime_request(fixture_data("wally").to_json, "employees/50", 200) Company.create!(name: "Ex-Mitarbeiter") person_wally = people(:wally) @@ -63,30 +30,8 @@ end it 'should create person when visited' do - updated_wally = { - 'data': { - 'id': 50, - 'type': 'employee', - 'attributes': { - 'shortname': 'CAL', - 'firstname': 'Changed Wally', - 'lastname': 'Allround', - 'full_name': 'Changed Wally Allround', - 'email': 'changedwally@example.com', - 'marital_status': 'single', - 'nationalities': %w[DE EG], - 'graduation': 'Quarter-Stack Developer', - 'department_shortname': 'SYS', - 'employment_roles': [], - 'is_employed': true, - 'birthdate': '1.1.2000', - 'location': 'Basel', - }} - } + stub_ptime_request(fixture_data("wally").to_json, "employees/50", 200) - stub_request(:get, "#{ptime_base_test_url}/api/v1/employees/50"). - to_return(body: updated_wally.to_json, headers: { 'content-type': "application/vnd.api+json; charset=utf-8" }, status: 200) - .with(basic_auth: [ptime_api_test_username, ptime_api_test_password]) Company.create!(name: "Ex-Mitarbeiter") person_wally = people(:wally) @@ -102,7 +47,7 @@ expect(new_wally.birthdate).to eq('1.1.2000') expect(new_wally.location).to eq('Basel') expect(new_wally.nationality).to eq('DE') - expect(new_wally.nationality2).to eq('EG') + expect(new_wally.nationality2).to eq('DK') end end end \ No newline at end of file diff --git a/spec/fixtures/files/json/wally.json b/spec/fixtures/files/json/wally.json new file mode 100644 index 000000000..582642fcd --- /dev/null +++ b/spec/fixtures/files/json/wally.json @@ -0,0 +1,23 @@ +{ + "data": { + "id": 50, + "type": "employee", + "attributes": { + "shortname": "CAL", + "firstname": "Changed Wally", + "lastname": "Allround", + "full_name": "Changed Wally Allround", + "email": "changedwally@example.com", + "marital_status": "single", + "nationalities": [ + "DE", "DK" + ], + "graduation": "Quarter-Stack Developer", + "department_shortname": "SYS", + "employment_roles2": [], + "is_employed": false, + "birthdate": "1.1.2000", + "location": "Basel" + } + } +} \ No newline at end of file From 5a812135e56af2bc5e837bca294bc6b7ca49e855 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Tue, 30 Jul 2024 15:59:44 +0200 Subject: [PATCH 16/32] Make person controller work without time again, start writing initializer that will enable the ptime connection when wanted, fix docker compose --- app/controllers/people_controller.rb | 12 ++++-------- config/application.rb | 2 +- config/initializers/ptime_connector.rb | 1 + docker-compose.yml | 2 +- lib/ptime/ptime_connection.rb | 18 ++++++++++++++++++ 5 files changed, 25 insertions(+), 10 deletions(-) create mode 100644 config/initializers/ptime_connector.rb create mode 100644 lib/ptime/ptime_connection.rb diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 88484af68..eb9e3641d 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -26,18 +26,14 @@ def show @person = Person.includes(projects: :project_technologies, person_roles: [:role, :person_role_level]).find(@person.id) - Ptime::PeopleEmployees.new.update_person_data(@person) super end def new - @person = Ptime::PeopleEmployees.new.create_or_find(params[:ptime_employee_id]) - redirect_to(@person) - - # %w[DE EN FR].each do |language| - # @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) - # end - # super + %w[DE EN FR].each do |language| + @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) + end + super end def create diff --git a/config/application.rb b/config/application.rb index d25c78c3a..2284f8e1a 100644 --- a/config/application.rb +++ b/config/application.rb @@ -14,7 +14,7 @@ module Skills def self.ptime_available? - ActiveModel::Type::Boolean.new.cast(ENV.fetch('PTIME_API_ACCESSIBLE', true)) + %w[true True 1].include?(ENV.fetch('PTIME_API_ACCESSIBLE', false)) end class Application < Rails::Application diff --git a/config/initializers/ptime_connector.rb b/config/initializers/ptime_connector.rb new file mode 100644 index 000000000..ee7a57e28 --- /dev/null +++ b/config/initializers/ptime_connector.rb @@ -0,0 +1 @@ +PeopleController.include Ptime::PtimeConnection if Skills.ptime_available? \ No newline at end of file diff --git a/docker-compose.yml b/docker-compose.yml index c54c7c2ce..8568cb03a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -41,7 +41,7 @@ services: /bin/bash -c " curl -fsSL https://deb.nodesource.com/setup_18.x | bash - && apt-get install -y nodejs && - npm install -g yarn && yarn add nodemon esbuild && bin/assets && + npm install -g yarn && bin/assets && sleep infinity" volumes: - ./:/myapp diff --git a/lib/ptime/ptime_connection.rb b/lib/ptime/ptime_connection.rb new file mode 100644 index 000000000..fc3ca37d3 --- /dev/null +++ b/lib/ptime/ptime_connection.rb @@ -0,0 +1,18 @@ +module Ptime + module PtimeConnection + def show + return export if format_odt? + + @person = Person.includes(projects: :project_technologies, + person_roles: [:role, :person_role_level]).find(@person.id) + + Ptime::PeopleEmployees.new.update_person_data(@person) + super + end + + def new + @person = Ptime::PeopleEmployees.new.create_or_find(params[:ptime_employee_id]) + redirect_to(@person) + end + end +end From b9c7959e21c4df9b9ba403026b64122598ea7a0e Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Wed, 31 Jul 2024 11:09:41 +0200 Subject: [PATCH 17/32] Move setting of default languages to after_initialize and cleanup code --- app/controllers/people_controller.rb | 7 ----- app/domain/ptime/people_employees.rb | 43 ++++++++++++---------------- app/models/person.rb | 8 ++++++ 3 files changed, 27 insertions(+), 31 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index eb9e3641d..ae246705f 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -29,13 +29,6 @@ def show super end - def new - %w[DE EN FR].each do |language| - @person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) - end - super - end - def create set_nationality2 super diff --git a/app/domain/ptime/people_employees.rb b/app/domain/ptime/people_employees.rb index 125534e6d..4e3af2105 100644 --- a/app/domain/ptime/people_employees.rb +++ b/app/domain/ptime/people_employees.rb @@ -1,5 +1,8 @@ module Ptime class PeopleEmployees + ATTRIBUTES_MAPPING = { full_name: :name, shortname: :shortname, email: :email, marital_status: + :marital_status, graduation: :title, birthdate: :birthdate, + location: :location }.freeze def create_or_find(ptime_employee_id) raise 'No ptime_employee_id provided' unless ptime_employee_id @@ -7,38 +10,30 @@ def create_or_find(ptime_employee_id) return person unless person.nil? new_person = Person.new(ptime_employee_id: ptime_employee_id) - %w[DE EN FR].each do |language| - new_person.language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) - end + update_person_data(new_person) end # rubocop:disable Metrics def update_person_data(person) - attribute_mapping = { full_name: :name, shortname: :shortname, email: :email, marital_status: - :marital_status, graduation: :title, birthdate: :birthdate, - location: :location }.freeze - - begin - raise 'Person has no ptime_employee_id' unless person.ptime_employee_id + raise 'Person has no ptime_employee_id' unless person.ptime_employee_id - ptime_employee = Ptime::Client.new.request(:get, "employees/#{person.ptime_employee_id}") - rescue CustomExceptions::PTimeClientError - raise "Ptime_employee with ptime_employee_id #{person.ptime_employee_id} not found" - else - ptime_employee[:attributes].each do |key, value| - if key.to_sym.in?(attribute_mapping.keys) - person[attribute_mapping[key.to_sym]] = (value.presence || '-') - end + ptime_employee = Ptime::Client.new.request(:get, "employees/#{person.ptime_employee_id}") + rescue CustomExceptions::PTimeTemporarlyUnavailableError + nil + else + ptime_employee[:attributes].each do |key, value| + if key.to_sym.in?(ATTRIBUTES_MAPPING.keys) + person[ATTRIBUTES_MAPPING[key.to_sym]] = (value.presence || '-') end - ptime_employee_employed = ptime_employee[:attributes][:is_employeed] - person.company = Company.find_by(name: ptime_employee_employed ? 'Firma' : 'Ex-Mitarbeiter') - ptime_employee_nationalities = ptime_employee[:attributes][:nationalities] - person.nationality = ptime_employee_nationalities[0] - person.nationality2 = ptime_employee_nationalities[1] - person.save! - person end + ptime_employee_employed = ptime_employee[:attributes][:is_employeed] + person.company = Company.find_by(name: ptime_employee_employed ? 'Firma' : 'Ex-Mitarbeiter') + ptime_employee_nationalities = ptime_employee[:attributes][:nationalities] + person.nationality = ptime_employee_nationalities[0] + person.nationality2 = ptime_employee_nationalities[1] + person.save! + person end # rubocop:enable Metrics end diff --git a/app/models/person.rb b/app/models/person.rb index 09a88f47c..a8e2f5c3d 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -27,6 +27,8 @@ class Person < ApplicationRecord include PgSearch::Model + after_initialize :set_default_languages + belongs_to :company belongs_to :department, optional: true @@ -103,4 +105,10 @@ def picture_size errors.add(:picture, 'grösse kann maximal 10MB sein') end + + def set_default_languages + %w[DE EN FR].each do |language| + language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) + end + end end From 3874b7c3101ac1d978501ba11a06401965ec1731 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Wed, 31 Jul 2024 11:27:46 +0200 Subject: [PATCH 18/32] Move ptime connection to concerns and rename it to people_controller --- .../controllers/concerns/ptime/people_controller.rb | 2 +- app/domain/ptime/people_employees.rb | 2 +- app/exceptions/custom_exceptions.rb | 1 + config/initializers/ptime.rb | 3 +++ config/initializers/ptime_connector.rb | 1 - 5 files changed, 6 insertions(+), 3 deletions(-) rename lib/ptime/ptime_connection.rb => app/controllers/concerns/ptime/people_controller.rb (94%) create mode 100644 config/initializers/ptime.rb delete mode 100644 config/initializers/ptime_connector.rb diff --git a/lib/ptime/ptime_connection.rb b/app/controllers/concerns/ptime/people_controller.rb similarity index 94% rename from lib/ptime/ptime_connection.rb rename to app/controllers/concerns/ptime/people_controller.rb index fc3ca37d3..c9c75d3ac 100644 --- a/lib/ptime/ptime_connection.rb +++ b/app/controllers/concerns/ptime/people_controller.rb @@ -1,5 +1,5 @@ module Ptime - module PtimeConnection + module PeopleController def show return export if format_odt? diff --git a/app/domain/ptime/people_employees.rb b/app/domain/ptime/people_employees.rb index 4e3af2105..4999d3618 100644 --- a/app/domain/ptime/people_employees.rb +++ b/app/domain/ptime/people_employees.rb @@ -19,7 +19,7 @@ def update_person_data(person) raise 'Person has no ptime_employee_id' unless person.ptime_employee_id ptime_employee = Ptime::Client.new.request(:get, "employees/#{person.ptime_employee_id}") - rescue CustomExceptions::PTimeTemporarlyUnavailableError + rescue CustomExceptions::PTimeTemporarilyUnavailableError nil else ptime_employee[:attributes].each do |key, value| diff --git a/app/exceptions/custom_exceptions.rb b/app/exceptions/custom_exceptions.rb index b59a9ef87..e33678fbd 100644 --- a/app/exceptions/custom_exceptions.rb +++ b/app/exceptions/custom_exceptions.rb @@ -1,5 +1,6 @@ module CustomExceptions class PTimeClientError < StandardError; end + class PTimeTemporarilyUnavailableError < StandardError; end end diff --git a/config/initializers/ptime.rb b/config/initializers/ptime.rb new file mode 100644 index 000000000..b115f68cb --- /dev/null +++ b/config/initializers/ptime.rb @@ -0,0 +1,3 @@ +Rails.application.config.to_prepare do + PeopleController.include Ptime::PeopleController if Skills.ptime_available? +end \ No newline at end of file diff --git a/config/initializers/ptime_connector.rb b/config/initializers/ptime_connector.rb deleted file mode 100644 index ee7a57e28..000000000 --- a/config/initializers/ptime_connector.rb +++ /dev/null @@ -1 +0,0 @@ -PeopleController.include Ptime::PtimeConnection if Skills.ptime_available? \ No newline at end of file From f5c5af3c247c736640e1e8fc0e80d385bfc82e76 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Wed, 31 Jul 2024 11:45:29 +0200 Subject: [PATCH 19/32] Rename create_or_find to find_or_create --- app/controllers/concerns/ptime/people_controller.rb | 2 +- app/domain/ptime/people_employees.rb | 4 ++-- spec/domain/ptime/people_employees_spec.rb | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/concerns/ptime/people_controller.rb b/app/controllers/concerns/ptime/people_controller.rb index c9c75d3ac..24f6fb1b2 100644 --- a/app/controllers/concerns/ptime/people_controller.rb +++ b/app/controllers/concerns/ptime/people_controller.rb @@ -11,7 +11,7 @@ def show end def new - @person = Ptime::PeopleEmployees.new.create_or_find(params[:ptime_employee_id]) + @person = Ptime::PeopleEmployees.new.find_or_create(params[:ptime_employee_id]) redirect_to(@person) end end diff --git a/app/domain/ptime/people_employees.rb b/app/domain/ptime/people_employees.rb index 4999d3618..9b14f9eee 100644 --- a/app/domain/ptime/people_employees.rb +++ b/app/domain/ptime/people_employees.rb @@ -3,7 +3,7 @@ class PeopleEmployees ATTRIBUTES_MAPPING = { full_name: :name, shortname: :shortname, email: :email, marital_status: :marital_status, graduation: :title, birthdate: :birthdate, location: :location }.freeze - def create_or_find(ptime_employee_id) + def find_or_create(ptime_employee_id) raise 'No ptime_employee_id provided' unless ptime_employee_id person = Person.find_by(ptime_employee_id: ptime_employee_id) @@ -20,7 +20,7 @@ def update_person_data(person) ptime_employee = Ptime::Client.new.request(:get, "employees/#{person.ptime_employee_id}") rescue CustomExceptions::PTimeTemporarilyUnavailableError - nil + return else ptime_employee[:attributes].each do |key, value| if key.to_sym.in?(ATTRIBUTES_MAPPING.keys) diff --git a/spec/domain/ptime/people_employees_spec.rb b/spec/domain/ptime/people_employees_spec.rb index e8a9a5a82..6a5ce7824 100644 --- a/spec/domain/ptime/people_employees_spec.rb +++ b/spec/domain/ptime/people_employees_spec.rb @@ -2,7 +2,7 @@ describe Ptime::PeopleEmployees do it 'should raise error when no ptime_employee_id is passed to new action' do - expect{ Ptime::PeopleEmployees.new.create_or_find(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') + expect{ Ptime::PeopleEmployees.new.find_or_create(nil) }.to raise_error(RuntimeError, 'No ptime_employee_id provided') end it 'should return person if it has the given ptime_employee_id' do @@ -10,11 +10,11 @@ person_wally.ptime_employee_id = 123 person_wally.save! - new_person = Ptime::PeopleEmployees.new.create_or_find(person_wally.ptime_employee_id) + new_person = Ptime::PeopleEmployees.new.find_or_create(person_wally.ptime_employee_id) expect(person_wally.attributes.except(*%w[created_at updated_at])).to eql(new_person.attributes.except(*%w[created_at updated_at])) end - it 'should raise error when person is not found in ptime api' do + xit 'should raise error when person is not found in ptime api' do stub_ptime_request('', "employees/50", 404) person_wally = people(:wally) From 684816ea4eab89795b4b65890b98dd8882d8892e Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Wed, 31 Jul 2024 12:59:59 +0200 Subject: [PATCH 20/32] Fix a bug where default languages where added every time person was visited --- app/models/person.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/person.rb b/app/models/person.rb index a8e2f5c3d..6ba682c40 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -107,8 +107,10 @@ def picture_size end def set_default_languages - %w[DE EN FR].each do |language| - language_skills.push(LanguageSkill.new({ language: language, level: 'A1' })) + if new_record? + %w[DE EN FR].each do |language| + language_skills.push(LanguageSkill.new({ language: language, level: 'Keine' })) + end end end end From 54925b5c81602747ca1d8d66ff6ea2e04ae35615 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Wed, 31 Jul 2024 13:27:46 +0200 Subject: [PATCH 21/32] Move some code from the update_person_data method to another method for better readabilty --- app/domain/ptime/people_employees.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/domain/ptime/people_employees.rb b/app/domain/ptime/people_employees.rb index 9b14f9eee..a4d7aee72 100644 --- a/app/domain/ptime/people_employees.rb +++ b/app/domain/ptime/people_employees.rb @@ -27,14 +27,18 @@ def update_person_data(person) person[ATTRIBUTES_MAPPING[key.to_sym]] = (value.presence || '-') end end + set_additional_attributes(person, ptime_employee) + person.save! + person + end + # rubocop:enable Metrics + + def set_additional_attributes(person, ptime_employee) ptime_employee_employed = ptime_employee[:attributes][:is_employeed] person.company = Company.find_by(name: ptime_employee_employed ? 'Firma' : 'Ex-Mitarbeiter') ptime_employee_nationalities = ptime_employee[:attributes][:nationalities] person.nationality = ptime_employee_nationalities[0] person.nationality2 = ptime_employee_nationalities[1] - person.save! - person end - # rubocop:enable Metrics end end From b5f1053cc9d62983d9c02ff3dfe5850d2d62c940 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Wed, 31 Jul 2024 13:31:48 +0200 Subject: [PATCH 22/32] Fix typo in people_employees --- app/domain/ptime/people_employees.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/domain/ptime/people_employees.rb b/app/domain/ptime/people_employees.rb index a4d7aee72..9c9cb9172 100644 --- a/app/domain/ptime/people_employees.rb +++ b/app/domain/ptime/people_employees.rb @@ -34,7 +34,7 @@ def update_person_data(person) # rubocop:enable Metrics def set_additional_attributes(person, ptime_employee) - ptime_employee_employed = ptime_employee[:attributes][:is_employeed] + ptime_employee_employed = ptime_employee[:attributes][:is_employed] person.company = Company.find_by(name: ptime_employee_employed ? 'Firma' : 'Ex-Mitarbeiter') ptime_employee_nationalities = ptime_employee[:attributes][:nationalities] person.nationality = ptime_employee_nationalities[0] From 027e2f9a3d292d39a02be1d0d24d2c6f576e5e8f Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Wed, 31 Jul 2024 13:59:46 +0200 Subject: [PATCH 23/32] Write spec to test if default languages are created correctly --- spec/models/person_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 6246e060d..4bd807888 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -113,4 +113,12 @@ expect(person.errors[:shortname].first).to eq('ist zu lang (mehr als 100 Zeichen)') end end + + context 'language_skills' do + it 'should automatically add default language on person creation' do + new_person = Person.new + expect(new_person.language_skills.length).to eq(3) + expect(new_person.language_skills.map(&:language)).to eql(%w[DE EN FR]) + end + end end From 2bcde3798b1df586c2d30c1793123be0ea06544f Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Wed, 31 Jul 2024 14:20:29 +0200 Subject: [PATCH 24/32] Change insert to prepend in ptime initializer --- config/initializers/ptime.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/ptime.rb b/config/initializers/ptime.rb index b115f68cb..73becbc3d 100644 --- a/config/initializers/ptime.rb +++ b/config/initializers/ptime.rb @@ -1,3 +1,3 @@ Rails.application.config.to_prepare do - PeopleController.include Ptime::PeopleController if Skills.ptime_available? + PeopleController.prepend Ptime::PeopleController if Skills.ptime_available? end \ No newline at end of file From 8cd170868400a0bd8ae46837cf78d7483346c5e3 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 2 Aug 2024 09:15:09 +0200 Subject: [PATCH 25/32] Simplify people_controller in ptime namespace --- app/controllers/concerns/ptime/people_controller.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/controllers/concerns/ptime/people_controller.rb b/app/controllers/concerns/ptime/people_controller.rb index 24f6fb1b2..500d4c8cf 100644 --- a/app/controllers/concerns/ptime/people_controller.rb +++ b/app/controllers/concerns/ptime/people_controller.rb @@ -1,13 +1,8 @@ module Ptime module PeopleController def show - return export if format_odt? - - @person = Person.includes(projects: :project_technologies, - person_roles: [:role, :person_role_level]).find(@person.id) - - Ptime::PeopleEmployees.new.update_person_data(@person) super + Ptime::PeopleEmployees.new.update_person_data(@person) end def new From 0277ae5dbdd46b0c5b1dad02674413548187d31b Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 2 Aug 2024 10:55:55 +0200 Subject: [PATCH 26/32] Rewrite people_employees feature spec as request spec --- spec/features/people_employees_spec.rb | 53 -------------------------- spec/rails_helper.rb | 1 + spec/requests/people_employees_spec.rb | 53 ++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 53 deletions(-) delete mode 100644 spec/features/people_employees_spec.rb create mode 100644 spec/requests/people_employees_spec.rb diff --git a/spec/features/people_employees_spec.rb b/spec/features/people_employees_spec.rb deleted file mode 100644 index 6900e6840..000000000 --- a/spec/features/people_employees_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -require 'rails_helper' - -describe Ptime::PeopleEmployees do - describe 'Update or create person', type: :feature, js: true do - before(:each) do - sign_in auth_users(:user), scope: :auth_user - end - - it 'should update person when visited' do - stub_ptime_request(fixture_data("wally").to_json, "employees/50", 200) - - Company.create!(name: "Ex-Mitarbeiter") - person_wally = people(:wally) - person_wally.ptime_employee_id = 50 - person_wally.save! - - expect(person_wally.name).to eq('Wally Allround') - expect(person_wally.email).to eq('wally@example.com') - visit person_path(person_wally) - person_wally.reload - expect(person_wally.name).to eq('Changed Wally Allround') - expect(person_wally.shortname).to eq('CAL') - expect(person_wally.email).to eq('changedwally@example.com') - expect(person_wally.marital_status).to eq('single') - expect(person_wally.title).to eq('Quarter-Stack Developer') - expect(person_wally.birthdate).to eq('1.1.2000') - expect(person_wally.location).to eq('Basel') - expect(person_wally.nationality).to eq('DE') - expect(person_wally.nationality2).to eq('DK') - end - - it 'should create person when visited' do - stub_ptime_request(fixture_data("wally").to_json, "employees/50", 200) - - Company.create!(name: "Ex-Mitarbeiter") - - person_wally = people(:wally) - person_wally.destroy! - expect(Person.find_by(name: "Wally Allround")).to be_nil - visit new_person_path(ptime_employee_id: 50) - new_wally = Person.find_by(name: "Changed Wally Allround") - expect(new_wally.name).to eq('Changed Wally Allround') - expect(new_wally.shortname).to eq('CAL') - expect(new_wally.email).to eq('changedwally@example.com') - expect(new_wally.marital_status).to eq('single') - expect(new_wally.title).to eq('Quarter-Stack Developer') - expect(new_wally.birthdate).to eq('1.1.2000') - expect(new_wally.location).to eq('Basel') - expect(new_wally.nationality).to eq('DE') - expect(new_wally.nationality2).to eq('DK') - end - end -end \ No newline at end of file diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index d57d07b37..43ba3a7e7 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -78,6 +78,7 @@ # Helpers from gems config.include(Devise::Test::IntegrationHelpers, type: :feature) + config.include(Devise::Test::IntegrationHelpers, type: :request) config.include(Devise::Test::ControllerHelpers, type: :controller) config.include(ActionView::RecordIdentifier, type: :feature) diff --git a/spec/requests/people_employees_spec.rb b/spec/requests/people_employees_spec.rb new file mode 100644 index 000000000..f8dde026d --- /dev/null +++ b/spec/requests/people_employees_spec.rb @@ -0,0 +1,53 @@ +require "rails_helper" + +describe 'Update or create person' do + before(:each) do + sign_in auth_users(:user), scope: :auth_user + end + it 'should update person when visited' do + stub_ptime_request(fixture_data("wally").to_json, "employees/50", 200) + + Company.create!(name: "Ex-Mitarbeiter") + person_wally = people(:wally) + person_wally.ptime_employee_id = 50 + person_wally.save! + expect(person_wally.name).to eq('Wally Allround') + expect(person_wally.email).to eq('wally@example.com') + + get "/people/#{person_wally.id}" + expect(response).to render_template(:show) + + person_wally.reload + expect(person_wally.name).to eq('Changed Wally Allround') + expect(person_wally.shortname).to eq('CAL') + expect(person_wally.email).to eq('changedwally@example.com') + expect(person_wally.marital_status).to eq('single') + expect(person_wally.title).to eq('Quarter-Stack Developer') + expect(person_wally.birthdate).to eq('1.1.2000') + expect(person_wally.location).to eq('Basel') + expect(person_wally.nationality).to eq('DE') + expect(person_wally.nationality2).to eq('DK') + end + + it 'should create person when visited' do + stub_ptime_request(fixture_data("wally").to_json, "employees/50", 200) + + Company.create!(name: "Ex-Mitarbeiter") + person_wally = people(:wally) + person_wally.destroy! + expect(Person.find_by(name: "Wally Allround")).to be_nil + + get "/people/new?ptime_employee_id=50" + + new_wally = Person.find_by(name: "Changed Wally Allround") + expect(new_wally.name).to eq('Changed Wally Allround') + expect(new_wally.shortname).to eq('CAL') + expect(new_wally.email).to eq('changedwally@example.com') + expect(new_wally.marital_status).to eq('single') + expect(new_wally.title).to eq('Quarter-Stack Developer') + expect(new_wally.birthdate).to eq('1.1.2000') + expect(new_wally.location).to eq('Basel') + expect(new_wally.nationality).to eq('DE') + expect(new_wally.nationality2).to eq('DK') + end +end \ No newline at end of file From 4946cc9480636dbd26827449375701d9657da414 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 2 Aug 2024 13:32:07 +0200 Subject: [PATCH 27/32] Fix ptime specs --- spec/requests/people_employees_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/requests/people_employees_spec.rb b/spec/requests/people_employees_spec.rb index f8dde026d..0b3f14d36 100644 --- a/spec/requests/people_employees_spec.rb +++ b/spec/requests/people_employees_spec.rb @@ -4,6 +4,16 @@ before(:each) do sign_in auth_users(:user), scope: :auth_user end + + before(:all) do + PeopleController.prepend(Ptime::PeopleController) + end + + after(:all) do + Ptime::PeopleController.remove_method(:new) + Ptime::PeopleController.remove_method(:show) + end + it 'should update person when visited' do stub_ptime_request(fixture_data("wally").to_json, "employees/50", 200) From b96835108346eaf0adc7f18e6d6c15c53f1285e5 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 2 Aug 2024 13:51:09 +0200 Subject: [PATCH 28/32] Make rubocop happy --- app/domain/ptime/people_employees.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/domain/ptime/people_employees.rb b/app/domain/ptime/people_employees.rb index 9c9cb9172..6195b4ce9 100644 --- a/app/domain/ptime/people_employees.rb +++ b/app/domain/ptime/people_employees.rb @@ -20,7 +20,7 @@ def update_person_data(person) ptime_employee = Ptime::Client.new.request(:get, "employees/#{person.ptime_employee_id}") rescue CustomExceptions::PTimeTemporarilyUnavailableError - return + # Ignored else ptime_employee[:attributes].each do |key, value| if key.to_sym.in?(ATTRIBUTES_MAPPING.keys) From 6eb889e1e5aed29352595821ed6ce1926b46506d Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 2 Aug 2024 14:31:27 +0200 Subject: [PATCH 29/32] Make controller read person id from params and remove unwanted spec --- app/controllers/people_controller.rb | 2 +- spec/domain/ptime/people_employees_spec.rb | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index ae246705f..ac1f70999 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -24,7 +24,7 @@ def show return export if format_odt? @person = Person.includes(projects: :project_technologies, - person_roles: [:role, :person_role_level]).find(@person.id) + person_roles: [:role, :person_role_level]).find(params.fetch(:id)) super end diff --git a/spec/domain/ptime/people_employees_spec.rb b/spec/domain/ptime/people_employees_spec.rb index 6a5ce7824..07da0800a 100644 --- a/spec/domain/ptime/people_employees_spec.rb +++ b/spec/domain/ptime/people_employees_spec.rb @@ -13,13 +13,4 @@ new_person = Ptime::PeopleEmployees.new.find_or_create(person_wally.ptime_employee_id) expect(person_wally.attributes.except(*%w[created_at updated_at])).to eql(new_person.attributes.except(*%w[created_at updated_at])) end - - xit 'should raise error when person is not found in ptime api' do - stub_ptime_request('', "employees/50", 404) - - person_wally = people(:wally) - person_wally.ptime_employee_id = 50 - person_wally.save! - expect{ Ptime::PeopleEmployees.new.update_person_data(person_wally) }.to raise_error(RuntimeError, 'Ptime_employee with ptime_employee_id 50 not found') - end end \ No newline at end of file From 2001e79e5dd1cc4011d7e0782c3ae21fb1a7d5e5 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 2 Aug 2024 15:03:18 +0200 Subject: [PATCH 30/32] Make ptime client correctly throw PTimeTemporarilyUnavailableError and write spec to test it --- app/domain/ptime/client.rb | 2 +- spec/requests/people_employees_spec.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/domain/ptime/client.rb b/app/domain/ptime/client.rb index 00edffe95..584207ddc 100644 --- a/app/domain/ptime/client.rb +++ b/app/domain/ptime/client.rb @@ -15,7 +15,7 @@ def request(method, endpoint, params = {}) if last_ptime_error_more_than_5_minutes_ago? send_request_and_parse_response(method, url, params) else - raise CustomExceptions::PTimeClientError, 'Error' + raise CustomExceptions::PTimeTemporarilyUnavailableError, 'PTime is temporarily unavailable' end end diff --git a/spec/requests/people_employees_spec.rb b/spec/requests/people_employees_spec.rb index 0b3f14d36..a5f08669f 100644 --- a/spec/requests/people_employees_spec.rb +++ b/spec/requests/people_employees_spec.rb @@ -60,4 +60,18 @@ expect(new_wally.nationality).to eq('DE') expect(new_wally.nationality2).to eq('DK') end + + it 'should not update but still show person if ptime temporarily unavailable' do + stub_env_var('LAST_PTIME_ERROR', 1.minute.ago) + stub_ptime_request(fixture_data("wally").to_json, "employees/50", 500) + + Company.create!(name: "Ex-Mitarbeiter") + person_wally = people(:wally) + person_wally.ptime_employee_id = 50 + person_wally.save! + + get "/people/#{person_wally.id}" + + expect(person_wally.attributes.except(*%w[created_at updated_at])).to eq(Person.find(person_wally.id).attributes.except(*%w[created_at updated_at])) + end end \ No newline at end of file From f621ac0d874fad4f2963ffe2a3f589540e40985b Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 2 Aug 2024 15:32:09 +0200 Subject: [PATCH 31/32] Check if correct template is rendered in ptime temporarely unavailable test --- spec/requests/people_employees_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/requests/people_employees_spec.rb b/spec/requests/people_employees_spec.rb index a5f08669f..b52e8a59a 100644 --- a/spec/requests/people_employees_spec.rb +++ b/spec/requests/people_employees_spec.rb @@ -72,6 +72,8 @@ get "/people/#{person_wally.id}" + expect(response).to render_template(:show) + expect(person_wally.attributes.except(*%w[created_at updated_at])).to eq(Person.find(person_wally.id).attributes.except(*%w[created_at updated_at])) end end \ No newline at end of file From 6d5eb50fc94b008ffbe652c3d233bbf27def486c Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 2 Aug 2024 15:42:52 +0200 Subject: [PATCH 32/32] Replace config.to_prepare with config.after_initialize in ptime initializer --- config/initializers/ptime.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/ptime.rb b/config/initializers/ptime.rb index 73becbc3d..ba93a57b1 100644 --- a/config/initializers/ptime.rb +++ b/config/initializers/ptime.rb @@ -1,3 +1,3 @@ -Rails.application.config.to_prepare do +Rails.application.config.after_initialize do PeopleController.prepend Ptime::PeopleController if Skills.ptime_available? end \ No newline at end of file