From 90dc38ed10359131f23e72c04dd481b9a2fc99db Mon Sep 17 00:00:00 2001 From: Sylvain Date: Fri, 9 Dec 2022 10:56:39 +0100 Subject: [PATCH] (feat) move external id to InvoicingProfile --- app/controllers/api/members_controller.rb | 3 +-- app/controllers/open_api/v1/users_controller.rb | 2 +- .../components/user/user-profile-form.tsx | 2 +- app/frontend/src/javascript/models/user.ts | 4 ++-- app/models/invoicing_profile.rb | 8 ++++++++ app/models/user.rb | 6 ------ app/services/members/import_service.rb | 3 ++- app/views/api/members/_member.json.jbuilder | 4 ++-- app/views/open_api/v1/users/_user.json.jbuilder | 10 +++++++--- ...06100225_add_external_id_to_invoicing_profile.rb | 9 +++++++++ .../20221206100225_add_external_id_to_user.rb | 9 --------- db/schema.rb | 4 ++-- test/fixtures/files/members.csv | 13 +++++++------ test/fixtures/invoicing_profiles.yml | 9 +++++++++ test/fixtures/users.yml | 10 ---------- test/integration/members/import_test.rb | 7 +++++-- 16 files changed, 56 insertions(+), 47 deletions(-) create mode 100644 db/migrate/20221206100225_add_external_id_to_invoicing_profile.rb delete mode 100644 db/migrate/20221206100225_add_external_id_to_user.rb diff --git a/app/controllers/api/members_controller.rb b/app/controllers/api/members_controller.rb index b53016202..514983374 100644 --- a/app/controllers/api/members_controller.rb +++ b/app/controllers/api/members_controller.rb @@ -232,14 +232,13 @@ class API::MembersController < API::ApiController elsif current_user.admin? || current_user.manager? params.require(:user).permit(:username, :email, :password, :password_confirmation, :is_allow_contact, :is_allow_newsletter, :group_id, - :external_id, tag_ids: [], profile_attributes: [:id, :first_name, :last_name, :phone, :interest, :software_mastered, :website, :job, :facebook, :twitter, :google_plus, :viadeo, :linkedin, :instagram, :youtube, :vimeo, :dailymotion, :github, :echosciences, :pinterest, :lastfm, :flickr, { user_avatar_attributes: %i[id attachment destroy] }], invoicing_profile_attributes: [ - :id, :organization, + :id, :organization, :external_id, { address_attributes: %i[id address], organization_attributes: [:id, :name, { address_attributes: %i[id address] }], diff --git a/app/controllers/open_api/v1/users_controller.rb b/app/controllers/open_api/v1/users_controller.rb index a3141dbf7..0b3f491b8 100644 --- a/app/controllers/open_api/v1/users_controller.rb +++ b/app/controllers/open_api/v1/users_controller.rb @@ -7,7 +7,7 @@ class OpenAPI::V1::UsersController < OpenAPI::V1::BaseController expose_doc def index - @users = User.order(created_at: :desc).includes(:group, :profile) + @users = User.order(created_at: :desc).includes(:group, :profile, :invoicing_profile) if params[:email].present? email_param = params[:email].is_a?(String) ? params[:email].downcase : params[:email].map(&:downcase) diff --git a/app/frontend/src/javascript/components/user/user-profile-form.tsx b/app/frontend/src/javascript/components/user/user-profile-form.tsx index 6069aa9cf..b5c66d4ae 100644 --- a/app/frontend/src/javascript/components/user/user-profile-form.tsx +++ b/app/frontend/src/javascript/components/user/user-profile-form.tsx @@ -239,7 +239,7 @@ export const UserProfileForm: React.FC = ({ action, size, disabled={isDisabled} formState={formState} label={t('app.shared.user_profile_form.pseudonym')} /> - {fieldsSettings.get('external_id') === 'true' && { Setting.get('address_required') } def full_name @@ -43,4 +45,10 @@ class InvoicingProfile < ApplicationRecord '' end end + + private + + def set_external_id_nil + self.external_id = nil if external_id.blank? + end end diff --git a/app/models/user.rb b/app/models/user.rb index 28878bc13..74ddc19c6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -57,7 +57,6 @@ class User < ApplicationRecord email&.downcase! end - before_validation :set_external_id_nil before_create :assign_default_role after_create :init_dependencies after_update :update_invoicing_profile, if: :invoicing_data_was_modified? @@ -82,7 +81,6 @@ class User < ApplicationRecord validate :cgu_must_accept, if: :new_record? validates :username, presence: true, uniqueness: true, length: { maximum: 30 } - validates :external_id, uniqueness: true, allow_blank: true validate :password_complexity scope :active, -> { where(is_active: true) } @@ -173,10 +171,6 @@ class User < ApplicationRecord private - def set_external_id_nil - self.external_id = nil if external_id.blank? - end - def assign_default_role add_role(:member) if roles.blank? end diff --git a/app/services/members/import_service.rb b/app/services/members/import_service.rb index f5ab29d1d..b91137f86 100644 --- a/app/services/members/import_service.rb +++ b/app/services/members/import_service.rb @@ -52,7 +52,6 @@ class Members::ImportService res.merge! hashify(row, 'id') res.merge! hashify(row, 'username') res.merge! hashify(row, 'email') - res.merge! hashify(row, 'external_id') res.merge! hashify(row, 'password', value: password) res.merge! hashify(row, 'password', key: :password_confirmation, value: password) res.merge! hashify(row, 'allow_contact', value: row['allow_contact'] == 'yes', key: :is_allow_contact) @@ -113,6 +112,8 @@ class Members::ImportService def invoicing_profile(row, user) res = {} + res.merge! hashify(row, 'external_id') + res[:id] = user.invoicing_profile.id if user&.invoicing_profile address_attributes = address(row, user) diff --git a/app/views/api/members/_member.json.jbuilder b/app/views/api/members/_member.json.jbuilder index 5ae5c350e..82a4ce915 100644 --- a/app/views/api/members/_member.json.jbuilder +++ b/app/views/api/members/_member.json.jbuilder @@ -1,6 +1,6 @@ # frozen_string_literal: true -json.extract! member, :id, :username, :email, :group_id, :external_id +json.extract! member, :id, :username, :email, :group_id json.role member.roles.first.name json.name member.profile.full_name json.need_completion member.need_completion? @@ -20,7 +20,7 @@ json.profile_attributes do end json.invoicing_profile_attributes do - json.id member.invoicing_profile.id + json.extract! member.invoicing_profile, :id, :external_id if member.invoicing_profile.address json.address_attributes do json.id member.invoicing_profile.address.id diff --git a/app/views/open_api/v1/users/_user.json.jbuilder b/app/views/open_api/v1/users/_user.json.jbuilder index dfa45f3a8..9c523f291 100644 --- a/app/views/open_api/v1/users/_user.json.jbuilder +++ b/app/views/open_api/v1/users/_user.json.jbuilder @@ -1,10 +1,14 @@ # frozen_string_literal: true -json.extract! user, :id, :email, :created_at, :external_id +json.extract! user, :id, :email, :created_at json.extract! user.profile, :full_name, :first_name, :last_name if user.association(:profile).loaded? json.gender user.statistic_profile.gender ? 'man' : 'woman' -json.organization !user.invoicing_profile.organization.nil? -json.address user.invoicing_profile.invoicing_address + +if user.association(:invoicing_profile).loaded? + json.external_id user.invoicing_profile.external_id + json.organization !user.invoicing_profile.organization.nil? + json.address user.invoicing_profile.invoicing_address +end if user.association(:group).loaded? json.group do diff --git a/db/migrate/20221206100225_add_external_id_to_invoicing_profile.rb b/db/migrate/20221206100225_add_external_id_to_invoicing_profile.rb new file mode 100644 index 000000000..e469a1f54 --- /dev/null +++ b/db/migrate/20221206100225_add_external_id_to_invoicing_profile.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +# From this migration users can be identified by an unique external ID +class AddExternalIdToInvoicingProfile < ActiveRecord::Migration[5.2] + def change + add_column :invoicing_profiles, :external_id, :string, null: true + add_index :invoicing_profiles, :external_id, unique: true, where: '(external_id IS NOT NULL)', name: 'unique_not_null_external_id' + end +end diff --git a/db/migrate/20221206100225_add_external_id_to_user.rb b/db/migrate/20221206100225_add_external_id_to_user.rb deleted file mode 100644 index a791eaa85..000000000 --- a/db/migrate/20221206100225_add_external_id_to_user.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -# From this migration users can be identified by an unique external ID -class AddExternalIdToUser < ActiveRecord::Migration[5.2] - def change - add_column :users, :external_id, :string, null: true - add_index :users, :external_id, unique: true, where: '(external_id IS NOT NULL)', name: 'unique_not_null_external_id' - end -end diff --git a/db/schema.rb b/db/schema.rb index e08fbd013..8311e496b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -371,6 +371,8 @@ ActiveRecord::Schema.define(version: 2022_12_06_100225) do t.string "email" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "external_id" + t.index ["external_id"], name: "unique_not_null_external_id", unique: true, where: "(external_id IS NOT NULL)" t.index ["user_id"], name: "index_invoicing_profiles_on_user_id" end @@ -1150,11 +1152,9 @@ ActiveRecord::Schema.define(version: 2022_12_06_100225) do t.inet "last_sign_in_ip" t.string "mapped_from_sso" t.datetime "validated_at" - t.string "external_id" t.index ["auth_token"], name: "index_users_on_auth_token" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["email"], name: "index_users_on_email", unique: true - t.index ["external_id"], name: "unique_not_null_external_id", unique: true, where: "(external_id IS NOT NULL)" t.index ["group_id"], name: "index_users_on_group_id" t.index ["provider"], name: "index_users_on_provider" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true diff --git a/test/fixtures/files/members.csv b/test/fixtures/files/members.csv index da887eb74..b6fe590d6 100644 --- a/test/fixtures/files/members.csv +++ b/test/fixtures/files/members.csv @@ -1,6 +1,7 @@ -id;gender;first_name;last_name;username;email;password;birthdate;address;phone;group;tags;trainings;website;job;interests;softwares;allow_contact;allow_newsletter;organization_name;organization_address;facebook;twitter;googleplus;viadeo;linkedin;instagram;youtube;vimeo;dailymotion;github;echosciences;pinterest;lastfm;flickr -;male;victor;hugo;vhugo;victor.hugo@wanadoo.fr;;1802-02-26;140 Grande Rue - 25000 Besançon;0381614214;standard;1;1,2,5;http://www.victor-hugo.info;Poète;Hacking, DIY;SolidWorks, Inkspace;yes;yes;;;http://www.facebook.com/victor-hugo;;;;;;;;;http://github.com/vhugo;;;; -;female;louise;michel;lmichel;louise.michel@gresille.org;;1830-03-29;8 rue de l'église - 52240 Vroncourt-la-Côte;0324491826;standard;2;5;https://rebellyon.info;Institutrice;militantisme;AutoCAD, LibreCAD;yes;yes;;;;;;;;;;;;http://github.com/louisemichel;https://www.echosciences-grenoble.fr/membres/louise-michel;;; -;male;ambroise;croizat;acroizat;acroizat@msn.com;;1801-01-28;cité des Maisonnettes - 73260 Notre-Dame-de-Briançon;0473147852;standard;;5;;Ouvrier;métallurgie;;no;no;;;;;;;;;https://www.youtube.com/user/croizat;;;;;;; -;female;rirette;maîtrejean;;;;1887-08-14;19330 Saint-Mexant;0555124789;standard;;;;;;;;yes;no -2;;;;;jean.dupond@yahoo.fr +id;gender;first_name;last_name;username;email;password;external_id;birthdate;address;phone;group;tags;trainings;website;job;interests;softwares;allow_contact;allow_newsletter;organization_name;organization_address;facebook;twitter;googleplus;viadeo;linkedin;instagram;youtube;vimeo;dailymotion;github;echosciences;pinterest;lastfm;flickr +;male;victor;hugo;vhugo;victor.hugo@wanadoo.fr;;VH1802;1802-02-26;140 Grande Rue - 25000 Besançon;0381614214;standard;1;1,2,5;http://www.victor-hugo.info;Poète;Hacking, DIY;SolidWorks, Inkspace;yes;yes;;;http://www.facebook.com/victor-hugo;;;;;;;;;http://github.com/vhugo;;;; +;female;louise;michel;lmichel;louise.michel@gresille.org;;LM1830;1830-03-29;8 rue de l'église - 52240 Vroncourt-la-Côte;0324491826;standard;2;5;https://rebellyon.info;Institutrice;militantisme;AutoCAD, LibreCAD;yes;yes;;;;;;;;;;;;http://github.com/louisemichel;https://www.echosciences-grenoble.fr/membres/louise-michel;;; +;male;ambroise;croizat;acroizat;acroizat@msn.com;;AC1801;1801-01-28;cité des Maisonnettes - 73260 Notre-Dame-de-Briançon;0473147852;standard;;5;;Ouvrier;métallurgie;;no;no;;;;;;;;;https://www.youtube.com/user/croizat;;;;;;; +;female;rirette;maîtrejean;;;;RM1887;1887-08-14;19330 Saint-Mexant;0555124789;standard;;;;;;;;yes;no +2;;;;;jean.dupond@yahoo.fr;;DJ1980; +;;;;;; diff --git a/test/fixtures/invoicing_profiles.yml b/test/fixtures/invoicing_profiles.yml index ae29e2035..8546577dd 100644 --- a/test/fixtures/invoicing_profiles.yml +++ b/test/fixtures/invoicing_profiles.yml @@ -4,6 +4,7 @@ admin: first_name: admin last_name: admin email: admin@fab-manager.com + external_id: J5821-4 jdupont: id: 2 @@ -11,6 +12,7 @@ jdupont: first_name: Jean last_name: Dupont email: jean.dupond@gmail.com + external_id: J5846-4 kdumas: id: 4 @@ -18,6 +20,7 @@ kdumas: first_name: Kevin last_name: Dumas email: kevin.dumas@orange.fr + external_id: J5900-1 vlonchamp: id: 5 @@ -25,6 +28,7 @@ vlonchamp: first_name: Vanessa last_name: Lonchamp email: vanessa.lonchamp@sfr.fr + external_id: P4172-4 gpartenaire: id: 6 @@ -32,6 +36,7 @@ gpartenaire: first_name: Gilbert last_name: Partenaire email: gilbert.partenaire@nicolas.com + external_id: J5500-4 pdurand: id: 3 @@ -39,6 +44,7 @@ pdurand: first_name: Paulette last_name: Durand email: paulette.durand@hotmail.fr + external_id: lseguin: id: 7 @@ -53,6 +59,7 @@ atiermoulin: first_name: Amandine last_name: Tiermoulin email: a.tiermoulin@mail.fr + external_id: proudhon: id: 9 @@ -60,6 +67,7 @@ proudhon: first_name: Pierre-Joseph last_name: Proudhon email: pj.proudhon@la-propriete.org + external_id: acamus: id: 10 @@ -67,3 +75,4 @@ acamus: first_name: Albert last_name: Camus email: albert.camus@letranger.org + external_id: diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index fbb3d0cb6..dc1a21bc7 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -29,7 +29,6 @@ user_1: auth_token: merged_at: is_allow_newsletter: true - external_id: J5821-4 user_2: id: 2 @@ -62,7 +61,6 @@ user_2: auth_token: merged_at: is_allow_newsletter: true - external_id: J5846-4 user_3: id: 3 @@ -95,7 +93,6 @@ user_3: auth_token: merged_at: is_allow_newsletter: false - external_id: J5900-1 user_4: id: 4 @@ -128,7 +125,6 @@ user_4: auth_token: merged_at: is_allow_newsletter: false - external_id: P4172-4 user_5: id: 5 @@ -161,7 +157,6 @@ user_5: auth_token: merged_at: is_allow_newsletter: true - external_id: J5500-4 user_6: id: 6 @@ -194,7 +189,6 @@ user_6: auth_token: merged_at: is_allow_newsletter: true - external_id: user_7: id: 7 @@ -227,7 +221,6 @@ user_7: auth_token: merged_at: is_allow_newsletter: false - external_id: user_8: id: 8 @@ -260,7 +253,6 @@ user_8: auth_token: merged_at: is_allow_newsletter: false - external_id: user_9: id: 9 @@ -293,7 +285,6 @@ user_9: auth_token: merged_at: is_allow_newsletter: true - external_id: user_10: id: 10 @@ -326,4 +317,3 @@ user_10: auth_token: merged_at: is_allow_newsletter: true - external_id: diff --git a/test/integration/members/import_test.rb b/test/integration/members/import_test.rb index d93cef82f..a5ec2677c 100644 --- a/test/integration/members/import_test.rb +++ b/test/integration/members/import_test.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'test_helper' + class ImportTest < ActionDispatch::IntegrationTest # Called before every test method runs. Can be used # to set up fixture information. @@ -42,6 +44,7 @@ class ImportTest < ActionDispatch::IntegrationTest assert_equal 'create', res[1][:status], 'wrong operation: victor hugo should have been created' assert res[1][:result], 'wrong result: operation should have succeeded' assert_equal 1, User.where(id: res[1][:user]).count, 'victor hugo was not found in database' + assert_equal res[0][:row]['external_id'], User.find(res[1][:user]).invoicing_profile.external_id, 'victor hugo has a wrong external ID' assert_not_nil res[3][:user], 'wrong user: louise michel is expected to have been created in database' assert_equal 'create', res[3][:status], 'wrong operation: louise michel should have been created' @@ -58,8 +61,8 @@ class ImportTest < ActionDispatch::IntegrationTest assert_not res[7][:result], 'wrong result: operation should have failed' assert_equal 0, Profile.where(last_name: res[6][:row]['last_name']).count, 'rirette maitrejean was found in database' - assert_match /can't be blank/, res[8][:email].to_json - assert_match /can't be blank/, res[8][:username].to_json + assert_match(/can't be blank/, res[8][:email].to_json) + assert_match(/can't be blank/, res[8][:username].to_json) assert_not_nil res[10][:user], 'wrong user: jean dupont is expected to exists in database' assert_equal 'update', res[10][:status], 'wrong operation: jean dupont should have been updated'