• Joined on 2026-03-17
marcel commented on pull request marcel/familienarchiv#618 2026-05-18 21:58:58 +02:00
security(import): validate PDF magic bytes before S3 upload

📋 Elicit — Requirements Engineer & Business Analyst

Verdict: ⚠️ Approved with concerns

The implementation satisfies the core requirement (validate PDF magic bytes, report skipped…

marcel commented on pull request marcel/familienarchiv#618 2026-05-18 21:58:44 +02:00
security(import): validate PDF magic bytes before S3 upload

🔧 Tobias Wendt — DevOps & Platform Engineer

Verdict: Approved

This PR has no changes to infrastructure files, Docker Compose configuration, CI workflows, or environment variables.…

marcel commented on pull request marcel/familienarchiv#618 2026-05-18 21:58:34 +02:00
security(import): validate PDF magic bytes before S3 upload

👨‍💻 Felix Brandt — Senior Fullstack Developer

Verdict: ⚠️ Approved with concerns

The core feature is clean. isPdfMagicBytes is well-named, properly isolated, and the try-with-resourc…

marcel commented on pull request marcel/familienarchiv#618 2026-05-18 21:58:15 +02:00
security(import): validate PDF magic bytes before S3 upload

🏛️ Markus Keller — Senior Application Architect

Verdict: ⚠️ Approved with concerns

Good, focused PR. Magic byte validation belongs where it is — before S3 upload, inside the service…

marcel commented on pull request marcel/familienarchiv#622 2026-05-18 21:57:01 +02:00
perf(document): EAGER→LAZY migration with @EntityGraph + @BatchSize (#467)

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Verdict: Approved

This PR is exclusively backend — no Svelte components, no markup, no CSS, no routes, no frontend files were…

marcel commented on pull request marcel/familienarchiv#622 2026-05-18 21:56:56 +02:00
perf(document): EAGER→LAZY migration with @EntityGraph + @BatchSize (#467)

📋 Elicit — Requirements Engineer

Verdict: Approved

Reviewing against issue #467 requirements.

Acceptance Criteria Coverage

The PR description maps directly to the stated goal…

marcel commented on pull request marcel/familienarchiv#622 2026-05-18 21:56:44 +02:00
perf(document): EAGER→LAZY migration with @EntityGraph + @BatchSize (#467)

🧪 Sara Holt — Senior QA Engineer

Verdict: ⚠️ Approved with concerns

Good coverage strategy — both the repository-level query-count tests and the service-level LazyInitializationExcepti…

marcel commented on pull request marcel/familienarchiv#622 2026-05-18 21:56:27 +02:00
perf(document): EAGER→LAZY migration with @EntityGraph + @BatchSize (#467)

🔐 Nora "NullX" Steiner — Application Security Engineer

Verdict: Approved

This is a pure performance migration — no new endpoints, no auth changes, no data exposure changes. Nothing…

marcel commented on pull request marcel/familienarchiv#622 2026-05-18 21:56:15 +02:00
perf(document): EAGER→LAZY migration with @EntityGraph + @BatchSize (#467)

🚀 Tobias Wendt — DevOps & Platform Engineer

Verdict: Approved

No infrastructure changes in this PR — no Compose file modifications, no CI workflow changes, no new Docker services or…

marcel commented on pull request marcel/familienarchiv#622 2026-05-18 21:56:05 +02:00
perf(document): EAGER→LAZY migration with @EntityGraph + @BatchSize (#467)

👨‍💻 Felix Brandt — Senior Fullstack Developer

Verdict: ⚠️ Approved with concerns

Clean implementation. The EntityGraph definitions on the entity are correct, the two-graph strategy is…

marcel commented on pull request marcel/familienarchiv#622 2026-05-18 21:55:51 +02:00
perf(document): EAGER→LAZY migration with @EntityGraph + @BatchSize (#467)

🏛️ Markus Keller — Senior Application Architect

Verdict: ⚠️ Approved with concerns

This is a well-executed performance migration. The EntityGraph strategy is correct — two graphs…

29eaa253a6 fix(document): remove @BatchSize from @ManyToOne sender — not supported
marcel commented on pull request marcel/familienarchiv#622 2026-05-18 19:07:07 +02:00
perf(document): EAGER→LAZY migration with @EntityGraph + @BatchSize (#467)

Review concerns addressed

All open concerns from Felix, Markus, Sara, and Tobias resolved in 6 commits.


@Sara / @Tobias — Blocker 1: generate_statistics: true in `application-test.…

3358f0509f test(document): strengthen getRecentActivity smoke test for post-return access
667b237c33 docs(document): add WHY comments to @Transactional(readOnly=true) methods
e677756139 perf(document): add @BatchSize(50) to sender and trainingLabels
108ab1a288 perf(document): add @EntityGraph(Document.list) for findAll(Pageable)
5f83fa1bc2 test(document): add query-count assertion for findAll(Pageable) path
Compare 6 commits »
marcel commented on pull request marcel/familienarchiv#617 2026-05-18 19:05:47 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

🧪 Sara Holt (@saraholt) — QA Engineer & Test Strategist

Verdict: ⚠️ Approved with concerns


Concerns

**1. ReflectionTestUtils.setField() is a test smell — root cause is in…

marcel commented on pull request marcel/familienarchiv#617 2026-05-18 19:05:19 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

🎨 Leonie Voss (@leonievoss) — UX/UI Designer & Accessibility Lead

Verdict: Approved


What's done well

1. role="alert" added to both error paths — This is the critical…

marcel commented on pull request marcel/familienarchiv#618 2026-05-18 19:05:10 +02:00
security(import): validate PDF magic bytes before S3 upload

Round-3 concerns addressed — f144b025

All open reviewer concerns from the round-2 review have been resolved in commit f144b025.

marcel pushed to worktree-feat+issue-529-pdf-magic-bytes at marcel/familienarchiv 2026-05-18 19:05:00 +02:00
f144b025b0 fix(import): address round-3 review concerns
marcel commented on pull request marcel/familienarchiv#617 2026-05-18 19:05:00 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

📋 Elicit — Requirements Engineer

Verdict: Approved


Requirements coverage

Issue #524 specified three security capabilities. Checking each against the implementation:

marcel commented on pull request marcel/familienarchiv#617 2026-05-18 19:04:41 +02:00
feat(security): CSRF protection, session revocation, login rate limiting (#524)

🛠️ Tobias Wendt (@tobiwendt) — DevOps & Platform Engineer

Verdict: Approved


What's done well

No new infrastructure. The in-memory Caffeine rate limiter avoids a Redis…