Skip to content

Fix/id docs ldap join and approval status - #7995

Merged
vitormattos merged 4 commits into
LibreSign:mainfrom
lfals:fix/id-docs-ldap-join-and-approval-status
Aug 22, 2026
Merged

Fix/id docs ldap join and approval status#7995
vitormattos merged 4 commits into
LibreSign:mainfrom
lfals:fix/id-docs-ldap-join-and-approval-status

Conversation

@lfals

@lfals lfals commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Resolves: #7861
Resolves: #7860

📝 Summary

Identification documents from LDAP/SSO accounts disappeared from the account page because IdDocsMapper used an inner join on oc_users. Those backends do not create a row there, so list() dropped the files even when filtering by userId.

Approving an identification document also never reached SIGNED. The uploader gets a DRAFT SignRequest so the account page can show a display name; evaluateStatusFromSigners() counted that placeholder as a required signature, so the file stayed PARTIAL_SIGNED after the approver signed.

This change left-joins users when listing identification documents, and excludes only that uploader placeholder from status evaluation. Sequential-signing DRAFT signers on normal files are unchanged.

🧪 How to test

  1. Enable Identification documents flow in LibreSign admin settings and set admin as an approver.
  2. Open the LibreSign account page (/apps/libresign/f/account) and confirm the Identification documents section is visible.
  3. Upload a test PDF with Upload file (tests/php/fixtures/pdfs/small_valid.pdf).
  4. Confirm the card no longer shows Not sent yet and that Delete file is available. Reload the page: the document must still be listed.
  5. Open Document Validation (/apps/libresign/f/docs/id-docs/validation). The row should show waiting for approval and a Sign action.
  6. Create a certificate for admin if needed, then click Sign and complete the signature.
  7. After signing, the list status must become approved (not stuck as partially signed). The Sign action is replaced by Validate.
  8. LDAP-specific check for ID documents of LDAP users are invisible on the account page (INNER JOIN on oc_users) #7861: repeat steps 2–4 as an LDAP user who has no row in oc_users. The uploaded document must remain visible on that account.
composer test:unit -- --filter IdDocsMapperTest
composer test:unit -- --filter testIdDocApprovalReachesSignedWhenUploaderPlaceholderIsUnsigned
composer test:unit -- --filter testDraftSignersStillCountWhenFileIsNotAnIdentificationDocument
npx playwright test playwright/e2e/id-docs-visual.spec.ts

⚙️ API / Back‑end changes

  • IdDocsMapper::getQueryBuilder() always leftJoins users, so listing by userId still returns documents when the account is missing from oc_users.
  • SignFileService::evaluateStatusFromSigners() ignores the identification-document uploader placeholder (id_docs.sign_request_id) so approval can reach SIGNED.
  • Unit and/or integration tests added – required for backend changes

✅ Checklist

  • I have read and followed the contribution guide.
  • Focused PHPUnit coverage added for the LDAP list join and ID-doc approval status.
  • Playwright coverage added for upload, account listing, and the approval queue.

🤖 AI (if applicable)

  • The content of this PR was partially or fully generated using AI

@welcome

welcome Bot commented Aug 19, 2026

Copy link
Copy Markdown

Thanks for opening your first pull request in this repository! ✌️

@github-project-automation github-project-automation Bot moved this to 0. Backlog in Roadmap Aug 19, 2026
@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
see 180 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

lfals added 3 commits August 21, 2026 15:54
LDAP accounts are missing from oc_users, so an inner join hid their
identification documents. The uploader placeholder sign request also
blocked approval from reaching SIGNED.

Fixes LibreSign#7861
Fixes LibreSign#7860

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Restore the system policy after the spec and match the English
Documents Validation sidebar label so later Playwright tests are not
affected.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
@lfals
lfals force-pushed the fix/id-docs-ldap-join-and-approval-status branch from f40fb01 to d9f6c6f Compare August 21, 2026 18:54
Comment on lines +1125 to +1129
try {
$idDocs = $this->idDocsMapper->getByFileId($fileId);
} catch (\Throwable) {
return null;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we catch only DoesNotExistException here instead of \Throwable?

A missing IdDocs entry is expected for regular files, but an unexpected database/programming error should probably not be silently treated as “not an identification document”.

Comment thread playwright/e2e/id-docs-visual.spec.ts Outdated
Comment thread playwright/e2e/id-docs-visual.spec.ts Outdated
@github-project-automation github-project-automation Bot moved this from 0. Backlog to 1. to do in Roadmap Aug 21, 2026
Catch only DoesNotExistException for missing id docs, store visual screenshots under build/, and keep e2e selectors English-only.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@lfals
lfals requested a review from vitormattos August 21, 2026 22:05
@vitormattos

Copy link
Copy Markdown
Member

/backport to stable34

@vitormattos

Copy link
Copy Markdown
Member

/backport to stable33

@vitormattos

Copy link
Copy Markdown
Member

/backport to stable32

@vitormattos
vitormattos merged commit f43caa3 into LibreSign:main Aug 22, 2026
72 checks passed
@github-project-automation github-project-automation Bot moved this from 1. to do to 4. to release in Roadmap Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 4. to release

Development

Successfully merging this pull request may close these issues.

ID documents of LDAP users are invisible on the account page (INNER JOIN on oc_users) Passport uploaded dockument does not reach SIGNED status

3 participants