Add roles to external-ids, find and create external-ids for specific roles (#1693)

* Add roles to external-ids, find and create external-ids for specific roles

* Also handle role in the find user API

* Fixed failing tests

* Added a routine to infer roles for external ids

---------

Co-authored-by: Michael Volo <volo@rice.edu>

Author: Dantemss

app/controllers/api/v1/users_controller.rb

@@ -184,7 +184,9 @@ def find
     payload = consume!(Hashie::Mash.new, represent_with: Api::V1::FindUserRepresenter)
 
     user = if payload.external_id.present?
-      User.joins(:external_ids).find_by!(external_ids: { external_id: payload.external_id })
+      external_id = ExternalId.find_by_external_id_and_role(payload.external_id, payload.role)
+      raise ActiveRecord::RecordNotFound unless external_id
+      external_id.user
     elsif payload.uuid.present?
       User.find_by!(uuid: payload.uuid)
     else

app/models/external_id.rb

@@ -1,6 +1,26 @@
 class ExternalId < ApplicationRecord
   belongs_to :user, inverse_of: :external_ids
 
+  enum(role: User::VALID_ROLES)
+
   validates :user, presence: true
-  validates :external_id, presence: true, uniqueness: true
+  validates :external_id, presence: true, uniqueness: { scope: :role }
+
+  # Find an ExternalId by external_id and role, with backwards compatibility
+  # for unknown_role records when a specific role is provided
+  def self.find_by_external_id_and_role(external_id, role = nil)
+    query = { external_id: external_id }
+    query[:role] = [role, 'unknown_role'] unless role.nil?
+
+    external_ids = where(query).to_a
+
+    if role.nil?
+      external_ids.first
+    else
+      # First try to find ExternalId with matching role
+      result = external_ids.find { |ext_id| ext_id.role == role.to_s }
+      # If no result, try to find ExternalId with unknown_role for backwards compatibility
+      result || external_ids.find(&:unknown_role?)
+    end
+  end
 end

app/representers/api/v1/external_id_representer.rb

@@ -11,5 +11,10 @@ class ExternalIdRepresenter < Roar::Decorator
              type: String,
              readable: true,
              writeable: true
+
+    property :role,
+             type: String,
+             readable: true,
+             writeable: true
   end
 end

app/representers/api/v1/find_or_create_user_representer.rb

@@ -121,7 +121,7 @@ class FindOrCreateUserRepresenter < Roar::Decorator
              readable: false,
              writeable: true,
              schema_info: {
-                description: "The role to assign to the newly created user, one of #{
+                description: "The role to assign to the newly created user and external_id, one of #{
                   User.roles.keys.map(&:to_s).inspect
                 }"
              }

app/representers/api/v1/find_user_representer.rb

@@ -20,6 +20,16 @@ class FindUserRepresenter < Roar::Decorator
                description: "External ID to search by"
              }
 
+    property :role,
+             type: String,
+             readable: false,
+             writeable: true,
+             schema_info: {
+               description: "Role to search by (optional), one of #{
+                 User.roles.keys.map(&:to_s).inspect
+               }"
+             }
+
     collection :external_ids,
                type: String,
                readable: true,

app/routines/create_user.rb

@@ -20,7 +20,7 @@ def exec(state:, external_id: nil, username: nil,
 
     outputs[:user] = User.send(create_method) do |user|
       user.state = state
-      user.external_ids << ExternalId.new(external_id: external_id) unless external_id.nil?
+      user.external_ids << ExternalId.new(external_id: external_id, role: role) unless external_id.nil?
       user.username = username
       user.first_name = first_name
       user.last_name = last_name

app/routines/find_or_create_user.rb

@@ -25,7 +25,7 @@ def exec(options)
   # Attempt to find a user by either the external_id, username, or email address
   def find_user(options)
     if options[:external_id].present?
-      User.joins(:external_ids).find_by(external_ids: { external_id: options[:external_id] })
+      ExternalId.find_by_external_id_and_role(options[:external_id], options[:role])&.user
     elsif options[:username].present?
       User.find_by(username: options[:username])
     elsif options[:email].present?

app/routines/infer_external_id_roles.rb

@@ -0,0 +1,24 @@
+class InferExternalIdRoles
+  def self.call
+    new.call
+  end
+
+  def call
+    ExternalId.unknown_role.preload(:user).find_in_batches do |batch|
+      ActiveRecord::Base.transaction do
+        batch.each do |external_id|
+          external_id.role = external_id.user.no_faculty_info? ? :student : :instructor
+        end
+
+        ExternalId.import(
+          batch,
+          on_duplicate_key_update: {
+            conflict_target: [:id],
+            columns: [:role]
+          },
+          validate: false
+        )
+      end
+    end
+  end
+end

db/migrate/20260128161718_add_role_to_external_ids.rb

@@ -0,0 +1,9 @@
+class AddRoleToExternalIds < ActiveRecord::Migration[6.1]
+  def change
+    add_column :external_ids, :role, :integer, null: false, default: 0
+
+    add_index :external_ids, [:external_id, :role], unique: true
+
+    remove_index :external_ids, :external_id
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2024_11_07_193019) do
+ActiveRecord::Schema.define(version: 2026_01_28_161718) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
@@ -175,7 +175,8 @@
     t.string "external_id", null: false
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.index ["external_id"], name: "index_external_ids_on_external_id", unique: true
+    t.integer "role", default: 0, null: false
+    t.index ["external_id", "role"], name: "index_external_ids_on_external_id_and_role", unique: true
     t.index ["user_id"], name: "index_external_ids_on_user_id"
   end