Add incident document support with MinIO integration

[SandraNelj]

• Implement file upload for incident attachments
• Store files in MinIO with unique file keys
• Save document metadata in database (fileName, fileKey, contentType)
• Add API endpoint to serve files from MinIO
• Fix frontend rendering of multiple images per incident
• Add image gallery with thumbnails and click-to-open in new tab
• Support mixed file types (images + documents)

Summary by CodeRabbit

Release Notes

• New Features
  • Implemented direct file download capability for all documents
  • Documents now display within incident views with image previews and file links
  • Incidents show visual indicators when documents are attached

[coderabbitai]

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/java/org/example/team6backend/incident/service/IncidentService.java (1)

134-144: ⚠️ Potential issue | 🔴 Critical

Move MinIO cleanup out of the transaction's critical path.

These deletes happen before incidentRepository.delete(incident) and before the transaction is known to have committed. If the DB delete or commit fails after some minioService.deleteFile(...) calls, the incident can roll back while its attachments are already gone.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/team6backend/incident/service/IncidentService.java`
around lines 134 - 144, The MinIO deletes currently run inside the transaction
before calling incidentRepository.delete(incident), risking orphaned DB
rollbacks after files are removed; instead, collect the document file keys from
incident.getDocuments(), perform the incidentRepository.delete(incident) inside
the transaction, and register an after-commit task to call
minioService.deleteFile(...) for each key (use
TransactionSynchronizationManager.registerSynchronization or an
`@TransactionalEventListener`) so file deletions occur only after successful
commit; ensure you still catch/log exceptions from minioService.deleteFile to
avoid crashing the after-commit hook.

🧹 Nitpick comments (2)

src/main/java/org/example/team6backend/document/service/MinioService.java (1)

62-68: Collapse this duplicate download path.

getFile(...) and downloadFile(...) now do the same getObject(...) call, so they can drift for no benefit. Delegate to the existing method or rename it instead of keeping both.

♻️ Small cleanup

 public InputStream getFile(String fileKey) {
-	try {
-		return minioClient.getObject(GetObjectArgs.builder().bucket(bucketName).object(fileKey).build());
-	} catch (Exception e) {
-		throw new RuntimeException("Failed to fetch file " + fileKey, e);
-	}
+	return downloadFile(fileKey);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/team6backend/document/service/MinioService.java`
around lines 62 - 68, MinioService currently has duplicate logic: both
getFile(String fileKey) and downloadFile(String fileKey) call
minioClient.getObject(GetObjectArgs.builder()....) — collapse this by delegating
one to the other to avoid drift. Edit MinioService so downloadFile(String
fileKey) simply returns getFile(fileKey) (or vice versa), preserving the
existing exception handling/RuntimeException wrapping in the delegated method
and keeping references to minioClient.getObject and GetObjectArgs only in one
place.

src/main/java/org/example/team6backend/incident/repository/IncidentRepository.java (1)

25-26: Use DISTINCT in the fetch-join-by-id query for safer provider behavior.

This helps prevent duplicate root-row edge cases with collection fetch joins across JPA providers.

Proposed query tweak

-@Query("SELECT i FROM Incident i LEFT JOIN FETCH i.documents WHERE i.id = :id")
+@Query("SELECT DISTINCT i FROM Incident i LEFT JOIN FETCH i.documents WHERE i.id = :id")
 Optional<Incident> findByIdWithDocuments(`@Param`("id") Long id);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/team6backend/incident/repository/IncidentRepository.java`
around lines 25 - 26, The fetch-join query in IncidentRepository's
findByIdWithDocuments can produce duplicate root rows with some JPA providers;
update the JPQL to select distinct root entities by adding DISTINCT after SELECT
(i.e., make the query select distinct Incident entities when LEFT JOIN FETCHing
i.documents) so the method returns a single unique Incident without duplicate
rows from collection joins.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/org/example/team6backend/config/MinioConfig.java`:
- Around line 10-12: The bean MinioConfig.minioClient() currently hardcodes
endpoint and credentials; change it to read values from configuration properties
(minio.endpoint, minio.accessKey, minio.secretKey) instead. Inject the
properties into the MinioConfig class (via `@Value` or a `@ConfigurationProperties`
POJO) and use those values when building the MinioClient in minioClient() so
deployments use src/main/resources/application.yml settings rather than
localhost/admin defaults.

In
`@src/main/java/org/example/team6backend/document/controller/DocumentController.java`:
- Around line 40-44: DocumentController.getFile currently always returns
MediaType.APPLICATION_OCTET_STREAM and treats any error as 500 because
MinioService.getFile wraps missing keys in RuntimeException; update the flow so
that MinioService.getFile throws a specific NotFoundException (or returns
Optional/InputStream with null) for missing objects instead of RuntimeException,
and in DocumentController.getFile detect the object's content type (e.g., via
the Minio object's metadata or by using
MediaTypeFactory/URLConnection.guessContentTypeFromName on the fileKey) and set
the ResponseEntity Content-Type accordingly (fall back to
application/octet-stream only if unknown); also catch the not-found condition
from minioService and return ResponseEntity.notFound().build() to map missing
files to 404.
- Around line 38-45: The getFile handler in DocumentController currently streams
directly by fileKey (minioService.getFile) and needs object-level authorization
to prevent IDOR: before calling minioService.getFile in getFile(`@PathVariable`
String fileKey), look up the owning document/incident for that fileKey (e.g.,
via a DocumentService or metadata lookup method such as
documentService.findByFileKey or minioService.getMetadata) and verify the
current principal's access using the existing auth/check methods (e.g.,
SecurityContextHolder or documentService.userHasAccessToIncident). If the check
fails, return ResponseEntity.status(HttpStatus.FORBIDDEN); otherwise proceed to
stream the file. Ensure you reference getFile, minioService.getFile, and the
document/incident lookup method you add to enforce authorization.

In
`@src/main/java/org/example/team6backend/document/service/DocumentService.java`:
- Around line 59-61: The deleteFile(Document) method currently calls
minioService.deleteFile(document.getFileKey()) before deleting the Document row,
risking dangling DB references if the repository delete fails; change this so
the DB delete occurs inside a transaction (e.g., annotate deleteFile or the
enclosing service with `@Transactional` and call
documentRepository.delete(document)), and register the MinIO cleanup to run
after commit (e.g., via
TransactionSynchronizationManager.registerSynchronization or a
`@TransactionalEventListener`) to call
minioService.deleteFile(document.getFileKey()) only when the transaction
successfully commits.

In `@src/main/java/org/example/team6backend/incident/dto/IncidentResponse.java`:
- Around line 35-42: The code currently calls incident.getDocuments().stream()
before checking for null; update the mapping so you null-check
incident.getDocuments() first (or default to Collections.emptyList()) when
calling response.setDocuments(...) and derive response.setHasDocuments(...) from
the same null-safe check; locate and change the lines that call
response.setDocuments(...) and response.setHasDocuments(...) in IncidentResponse
(referencing incident.getDocuments(), response.setDocuments, and
response.setHasDocuments) so the documents mapping uses an empty list when null
and setHasDocuments uses the same null/empty logic.

In
`@src/main/java/org/example/team6backend/incident/repository/IncidentRepository.java`:
- Around line 16-23: The pageable repository methods in IncidentRepository
(findAll, findByCreatedBy, findByAssignedTo) currently include the to-many
"documents" in their `@EntityGraph` which breaks pagination; remove "documents"
from the attributePaths on those pageable methods so only to-one associations
("createdBy", "assignedTo") are eagerly loaded, and implement the two-query
pattern: first page parent Incident IDs (or use existing pageable queries
without documents), then fetch full Incident entities with documents in a second
query (e.g., a non-paged `@EntityGraph` or a repository method like
findByIdInWithDocuments) to load the documents for the page results. Ensure
symbols to update: IncidentRepository, the three methods findAll(Pageable),
findByCreatedBy(AppUser, Pageable) and findByAssignedTo(AppUser, Pageable), and
add a helper method (e.g., findByIdInWithDocuments) to retrieve incidents with
documents for the selected IDs.

In `@src/main/resources/templates/viewincident.html`:
- Around line 203-217: Add authorization to the public document-by-fileKey flow
and harden attachment links: update the controller method that handles GET
"/{fileKey}" (e.g., getFile in DocumentController) to look up the Document via
documentService (add repository method DocumentRepository.findByFileKey /
documentService.getDocumentByFileKey), verify the returned Document and its
Incident are not null, call
incidentService.getById(document.getIncident().getId(), user) or otherwise
verify the authenticated user's access, then stream the file via
minioService.getFile(fileKey) and return it with proper Content-Disposition and
content type; also modify viewincident.html attachment <a> tags to include
rel="noopener noreferrer" on external/file links to prevent tabnabbing.

---

Outside diff comments:
In
`@src/main/java/org/example/team6backend/incident/service/IncidentService.java`:
- Around line 134-144: The MinIO deletes currently run inside the transaction
before calling incidentRepository.delete(incident), risking orphaned DB
rollbacks after files are removed; instead, collect the document file keys from
incident.getDocuments(), perform the incidentRepository.delete(incident) inside
the transaction, and register an after-commit task to call
minioService.deleteFile(...) for each key (use
TransactionSynchronizationManager.registerSynchronization or an
`@TransactionalEventListener`) so file deletions occur only after successful
commit; ensure you still catch/log exceptions from minioService.deleteFile to
avoid crashing the after-commit hook.

---

Nitpick comments:
In `@src/main/java/org/example/team6backend/document/service/MinioService.java`:
- Around line 62-68: MinioService currently has duplicate logic: both
getFile(String fileKey) and downloadFile(String fileKey) call
minioClient.getObject(GetObjectArgs.builder()....) — collapse this by delegating
one to the other to avoid drift. Edit MinioService so downloadFile(String
fileKey) simply returns getFile(fileKey) (or vice versa), preserving the
existing exception handling/RuntimeException wrapping in the delegated method
and keeping references to minioClient.getObject and GetObjectArgs only in one
place.

In
`@src/main/java/org/example/team6backend/incident/repository/IncidentRepository.java`:
- Around line 25-26: The fetch-join query in IncidentRepository's
findByIdWithDocuments can produce duplicate root rows with some JPA providers;
update the JPQL to select distinct root entities by adding DISTINCT after SELECT
(i.e., make the query select distinct Incident entities when LEFT JOIN FETCHing
i.documents) so the method returns a single unique Incident without duplicate
rows from collection joins.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 885ccea3-43a5-4400-a9fc-50160fc55941

📥 Commits

Reviewing files that changed from the base of the PR and between a8387a2 and 911be54.

📒 Files selected for processing (10)

• src/main/java/org/example/team6backend/config/MinioConfig.java
• src/main/java/org/example/team6backend/document/controller/DocumentController.java
• src/main/java/org/example/team6backend/document/dto/DocumentDTO.java
• src/main/java/org/example/team6backend/document/service/DocumentService.java
• src/main/java/org/example/team6backend/document/service/MinioService.java
• src/main/java/org/example/team6backend/incident/dto/IncidentResponse.java
• src/main/java/org/example/team6backend/incident/repository/IncidentRepository.java
• src/main/java/org/example/team6backend/incident/service/IncidentService.java
• src/main/resources/templates/incidents.html
• src/main/resources/templates/viewincident.html