[Candidate Identifier (Rework)] Domain Models and Identifier Logic by HachemJ · Pull Request #11212 · aces/Loris

HachemJ · 2026-09-22T20:03:57Z

Brief summary of changes

The goal is to support multiple identifier types with different formats, validation rules, permissions, and project-specific requirements.

The rework separates identifier configuration from the values assigned to candidates, providing a more flexible architecture that can be integrated across LORIS modules while maintaining backward compatibility.

This PR depends on #11209 and focuses on the domain models, persistence, identifier generation, and validation, while the database structure and configuration migration are handled in the preceding PR.

Changes

Domain models and persistence

Added IdentifierType and CandidateIdentifier domain models.
Added IdentifierTypeController to manage identifier type configurations and project associations.
Added CandidateIdentifierController to handle identifier creation, retrieval, updates, deletion, and validation.
Implemented application-level enforcement of uniqueness and multiple-value rules.

Identifier generation

Added CandidateIdentifierGenerator as a generalized generator supporting sequential and random identifier generation through configurable templates.
It currently duplicates some functionality from SiteIDGenerator, which is used by PSCIDGenerator and ExternalIDGenerator.
The goal is to eventually replace PSCIDGenerator and ExternalIDGenerator with the new configurable generator, removing the need for separate generation implementations.
The existing generators remain in place until the new identifier framework is integrated into candidate workflows.

The REST API endpoints for managing candidate identifiers are implemented separately in PR #11253.

Tests

Added unit tests for IdentifierTypeController.
Added unit tests for CandidateIdentifierController.
Added unit tests for CandidateIdentifierGenerator.
Updated existing identifier-related tests to support the template-string changes.

Remaining work

The next stages of the rework will focus on:

Project configuration: Build on the existing project associations to enforce project-specific identifier requirements and validation.
Architecture review: Validate the design and implementation against the requirements, conduct integration testing, and incorporate feedback.
Application integration: Integrate identifiers into candidate registration and management workflows, including search, lookup, identifier creation UI, and permission-based visibility.
Administration and migration: Implement the administration UI for managing identifier types, complete migration validation and regression testing, and ensure backward compatibility.
Documentation and release readiness: Complete developer and configuration documentation, address remaining issues, and prepare the framework for inclusion in LORIS core.

driusan · 2026-09-25T17:20:27Z

+  `Name` varchar(100) NOT NULL,
+  `Format` varchar(255) NOT NULL,
+  `IsRequired` boolean NOT NULL DEFAULT 1,
+  `IsUnique` boolean NOT NULL DEFAULT 1,


Commenting here because it's hard to comment on the PR description..

I don't see why "IsUnique" is required. What is a "non-unique" identifier example? An identifier that isn't unique can't be used to identify the candidate, which is the point of an identifier.

I have sad news for you Dave... 1 word: scanner hahah

The truth is other than PSCID not being unique, we have several other identifiers that could be shared, potentially something like "Family ID" but on CBIG we also want to allow projects to add their own internal identifiers where in some cases they are not unique...

The main driver for this however was PSCID= scanner

oh and GUID doesnt have to be unique either, and before you spell out GUID for me... GUID should be unique to the participant but the participant can have multiple entries on the same platform by design (coming from separate databases or sources)

Maybe this would be a good opportunity to just get rid of the "PSCID=scanner" hack and just.. don't assign scanners a PSCID.

As for GUIDs, if I understand correctly that's what the AllowMultiple flag is for in this description (not the IsUnique one).

Maybe this would be a good opportunity to just get rid of the "PSCID=scanner" hack and just.. don't assign scanners a PSCID.

Love the idea but not in the context of this PR, its a whole mountain of work on its own.

As for GUIDs, if I understand correctly that's what the AllowMultiple flag is for in this description (not the IsUnique one).

AllowMultiple: A Participant can have more than one RedCapID (just an example)
isUnique: 4 Different participants can have the same GUID because in reality they are the same participant recruited separately for 4 different studies and neither of those studies knows of this participant's involvement in the others

driusan · 2026-09-25T17:24:02Z

An identifier type can have at most one permission, and a permission can only be associated with one identifier type. [..]

What is the reasoning for this?

I get that one permission makes it easier to implement which makes sense, but if that's a permanent design decision then why have a separate rel table instead of just a column in the identifier type table to list the one permission?

As for a permission only being allowed to be associated with one identifier type.. why? Multiple different related identifier types having the same permission seems reasonable and I can't think of any way that adding this restriction would simplify the implementation.

HachemJ · 2026-09-25T18:31:23Z

An identifier type can have at most one permission, and a permission can only be associated with one identifier type. [..]

What is the reasoning for this?

I get that one permission makes it easier to implement which makes sense, but if that's a permanent design decision then why have a separate rel table instead of just a column in the identifier type table to list the one permission?

As for a permission only being allowed to be associated with one identifier type.. why? Multiple different related identifier types having the same permission seems reasonable and I can't think of any way that adding this restriction would simplify the implementation.

Yes, thank you for bringing it up. What I meant is that an identifier type can have at most one permission, but the same permission can be used by multiple identifier types.

With that relationship, I think your point about the separate relation table makes sense too. Since it’s many to one, permID could just be a nullable foreign key on the identifier type table. The relation table would only really be needed if we wanted multiple permissions per identifier type later.

`CandID` is the candidate identifier the application keys on, but the `candidate` table doesn't carry an index for it. It indexes `PSCID`, `ExternalID`, `Active`, `RegistrationCenterID`, `flagged_reason`, `Sex` and `ProbandSex`, but not `CandID`. So every lookup by CandID is a full table scan. That stays invisible until something does a lot of them at once. The dataquery run endpoint resolves CandID to `candidate.ID` once per result row, batched 2000 rows at a time, so each batch does 2000 scans. On a 32,487 candidate instance that is 65 million row reads per batch. Measured there, a batch went from 74.89 s to 0.32 s. The index is unique, which is what `CandIDGenerator` already assumes. It picks a random ID, checks the table for a collision, and returns it for insertion. The check and the insert are separate statements, so what stopped two concurrent registrations landing on the same CandID was the primary key the table used to carry on that column. Since aces#9556 moved the primary key to `candidate.ID`, nothing has.

Bumps [smarty/smarty](https://github.com/smarty-php/smarty) from 4.5.6 to 4.5.7. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/smarty-php/smarty/releases">smarty/smarty's releases</a>.</em></p> <blockquote> <h2>v4.5.7</h2> <h2>What's Changed</h2> <ul> <li>Port security fixes from v5.8.2 to Smarty v4. by <a href="https://github.com/wisskid"><code>@wisskid</code></a> in <a href="https://redirect.github.com/smarty-php/smarty/pull/1194">smarty-php/smarty#1194</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/smarty-php/smarty/compare/v4.5.6...v4.5.7">https://github.com/smarty-php/smarty/compare/v4.5.6...v4.5.7</a></p> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/smarty-php/smarty/blob/v4.5.7/CHANGELOG.md">smarty/smarty's changelog</a>.</em></p> <blockquote> <h2>[4.5.7] - 2026-06-29</h2> <ul> <li>Security: prevent symlinks inside a trusted <code>secure_dir</code>/template directory from being used to read files outside of it (CWE-22 path traversal), affecting <code>{include}</code> and <code>{fetch}</code> of local files</li> <li>Security: <code>{html_image}</code> now escapes the <code>file</code>, <code>path_prefix</code>, <code>href</code>/<code>link</code>, <code>width</code> and <code>height</code> attributes (it already escaped <code>alt</code> and pass-through attributes), and <code>{html_select_date}</code> casts <code>day_size</code>/<code>month_size</code>/<code>year_size</code> to int (matching <code>{html_select_time}</code>), preventing untrusted values passed into these attributes from breaking out of the generated HTML (CWE-79)</li> <li>Security: <code>{fetch}</code> no longer follows HTTP redirects for remote resources while a security policy is active, preventing an open redirect on a trusted host from bypassing <code>trusted_uri</code> (CWE-918 server-side request forgery)</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/smarty-php/smarty/commit/838d6ad1c635f8a6bafc08d791b8af18d8304b3a"><code>838d6ad</code></a> Merge branch 'release/4.5.7' into support/4</li> <li><a href="https://github.com/smarty-php/smarty/commit/187f7b6246de8195398241332ce8fa69c719ff4e"><code>187f7b6</code></a> version bump</li> <li><a href="https://github.com/smarty-php/smarty/commit/a1ccdb0518021a559b4066c37b76a42c86bbce90"><code>a1ccdb0</code></a> Port security fixes from v5.8.2 to Smarty v4. (<a href="https://redirect.github.com/smarty-php/smarty/issues/1194">#1194</a>)</li> <li><a href="https://github.com/smarty-php/smarty/commit/c285f84fc66212c0e1c73780bbebe07771e6e5bf"><code>c285f84</code></a> Reproduce block override leakage in template inheritance</li> <li>See full diff in <a href="https://github.com/smarty-php/smarty/compare/v4.5.6...v4.5.7">compare view</a></li> </ul> </details> <br /> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…ierGeneratorTest meant to replace it later on

…ReworkCandidateIdentifiers

HachemJ added 17 commits September 8, 2026 12:17

Refactor PSCID structure to use template strings

fdbb640

fix linter

ca346de

trying to fix integration test

f2e1b17

fix failing test

2dba502

fix failing unit test

e1e5183

fix linter

c310af3

Update PSCID template regex tests

75e5019

fix typo

5891de6

Add unit tests for PSCID template generation

cda36d2

lint

84e35fe

revert structureToPCRE

9334e78

revert docstring change

7f84da6

Rename PSCIDGeneratorTest to SiteIDGeneratorTest

37146f1

add more descriptive errors

da268fa

fix failing tests + lint

f0107a5

fix indentation

819570c

add min and max to identifier validation errors

19866c7

github-actions Bot added the Language: SQL PR or issue that update SQL code label Sep 22, 2026

HachemJ force-pushed the ReworkCandidateIdentifiers branch from cb0c78e to 649dd44 Compare September 24, 2026 16:24

driusan reviewed Sep 25, 2026

View reviewed changes

github-actions Bot added the Language: PHP PR or issue that update PHP code label Sep 25, 2026

HachemJ force-pushed the ReworkCandidateIdentifiers branch from a617af0 to 93e2302 Compare September 30, 2026 19:10

HachemJ added the State: Blocked PR or issue awaiting an external event such as the merge or another PR to proceed label Oct 7, 2026

github-actions Bot added the Module: api PR or issue related to api module label Oct 8, 2026

HachemJ changed the title ~~Add candidate identifier tables~~ [Candidate Identifiers] Introduce configurable candidate identifier framework Oct 9, 2026

HachemJ and others added 3 commits October 12, 2026 19:13

Add candidate identifier tables

01c8e85

HachemJ added 6 commits October 13, 2026 14:21

Support validation only formats for manual candidate identifiers

b938d01

lint

35787e7

checkstatic failing

7eee011

Add migration for legacy candidate identifier configuration

1f92209

Merge branch 'ReworkIdentifierStructure' into ReworkCandidateIdentifiers

214e8b8

Move candidate identifier REST API changes to separate PR

3be0b7f

HachemJ mentioned this pull request Oct 14, 2026

[Candidate Identifier (Rework)] Add candidate identifier REST API endpoints #11253

Open

HachemJ changed the title ~~[Candidate Identifiers] Introduce configurable candidate identifier framework~~ [Candidate Identifier Rework] Domain Models and Identifier Logic Oct 14, 2026

HachemJ mentioned this pull request Oct 15, 2026

[Candidate Identifier (Rework)] Database Schema and Configuration Migration #11209

Open

Read candidate identifier settings from database

8d2b75b

github-actions Bot added the Module: new_profile PR or issue related to new_profile module label Oct 16, 2026

HachemJ changed the title ~~[Candidate Identifier Rework] Domain Models and Identifier Logic~~ [Candidate Identifier (Rework)] Domain Models and Identifier Logic Oct 16, 2026

HachemJ added 6 commits October 16, 2026 12:47

fix lint

e0893b5

Add unit tests for identifier type lookup by name

21dfd73

Remove SiteIDGeneratorTest, redundant because of new CandidateIdentif…

bef7e7e

…ierGeneratorTest meant to replace it later on

Update PSCID validation test to use database identifier settings

ba4ff60

default PSCID identifier type for fresh installations

3f0b63f

default PSCID identifier type for fresh installations

c5e34be

HachemJ marked this pull request as ready for review October 19, 2026 18:43

HachemJ added 5 commits October 20, 2026 11:18

Fix missing default PSCID insert in candidate identifiers patch

74765c4

Revert unintended changes

d626c6b

Restore lockfiles to upstream versions

37dd3c8

Add method to retrieve all candidate identifier types

8b948d1

Merge remote-tracking branch 'origin/ReworkIdentifierStructure' into …

72fd2ef

…ReworkCandidateIdentifiers

HachemJ added the State: Blocking PR should be prioritized because it is blocking the progress of another task label Oct 22, 2026

This was referenced Oct 22, 2026

[Candidate Identifier (Rework)] Add candidate identifier type configuration #11261

Draft

[Candidate Identifier (Rework)] Generalized Candidate Identifier Framework #11270

Open

HachemJ added 3 commits October 23, 2026 12:29

Update candidate identifier schema defaults and timestamps

609b334

Merge branch 'ReworkIdentifierStructure' into ReworkCandidateIdentifiers

818cef9

Expose candidate identifier timestamps

c17c694

Navigation Menu

Repository navigation

[Candidate Identifier (Rework)] Domain Models and Identifier Logic - #11212

[Candidate Identifier (Rework)] Domain Models and Identifier Logic#11212
HachemJ wants to merge 63 commits into
aces:mainfrom
HachemJ:ReworkCandidateIdentifiers

HachemJ commented Sep 22, 2026 •

edited

Loading

Uh oh!

driusan Sep 25, 2026

Uh oh!

ridz1208 Sep 25, 2026

Uh oh!

ridz1208 Sep 25, 2026

Uh oh!

driusan Sep 25, 2026

Uh oh!

ridz1208 Sep 28, 2026

Uh oh!

driusan commented Sep 25, 2026

Uh oh!

HachemJ commented Sep 25, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

HachemJ commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Brief summary of changes

Changes

Domain models and persistence

Identifier generation

Tests

Remaining work

Uh oh!

driusan Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

ridz1208 Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

ridz1208 Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

driusan Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

ridz1208 Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

driusan commented Sep 25, 2026

Uh oh!

HachemJ commented Sep 25, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HachemJ commented Sep 22, 2026 •

edited

Loading