(feat) move external id to InvoicingProfile

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] }],

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)

app/frontend/src/javascript/components/user/user-profile-form.tsx

@@ -239,7 +239,7 @@ export const UserProfileForm: React.FC<UserProfileFormProps> = ({ action, size,
                      disabled={isDisabled}
                      formState={formState}
                      label={t('app.shared.user_profile_form.pseudonym')} />
-          {fieldsSettings.get('external_id') === 'true' && <FormInput id="external_id"
+          {fieldsSettings.get('external_id') === 'true' && <FormInput id="invoicing_profile_attributes.external_id"
                      register={register}
                      disabled={isDisabled}
                      formState={formState}

app/frontend/src/javascript/models/user.ts

@@ -12,7 +12,6 @@ type ProfileAttributesSocial = {
 export interface User {
   id: number,
   username?: string,
-  external_id?: string,
   email: string,
   group_id?: number,
   role?: UserRole
@@ -43,6 +42,7 @@ export interface User {
   },
   invoicing_profile_attributes?: {
     id?: number,
+    external_id?: string,
     address_attributes: {
       id?: number,
       address: string
@@ -132,4 +132,4 @@ export const UserFieldMapping = Object.assign({
   group_id: 'user.group_id'
 }, ...socialMappings);
 
-export const UserFieldsReservedForPrivileged = ['external_id'];
+export const UserFieldsReservedForPrivileged = ['invoicing_profile_attributes.external_id'];

app/models/invoicing_profile.rb

@@ -27,6 +27,8 @@ class InvoicingProfile < ApplicationRecord
 
   has_many :accounting_lines, dependent: :destroy
 
+  before_validation :set_external_id_nil
+  validates :external_id, uniqueness: true, allow_blank: true
   validates :address, presence: true, if: -> { 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

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

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)

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

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

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

db/migrate/20221206100225_add_external_id_to_user.rb

@@ -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

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

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;
+;;;;;;

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:

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:

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'