Skip to content

feature/adding-Thymeleaf-frontend#12

Merged
VonAdamo merged 1 commit into
mainfrom
feature/adding-Thymeleaf-frontend
Apr 24, 2026
Merged

feature/adding-Thymeleaf-frontend#12
VonAdamo merged 1 commit into
mainfrom
feature/adding-Thymeleaf-frontend

Conversation

@VonAdamo

@VonAdamo VonAdamo commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Documentation

    • Removed MinIO startup instructions from troubleshooting guide.
  • Bug Fixes

    • Fixed product name spelling in Swedish UI text.
    • Removed debug output from document upload process.
  • New Features

    • Added required file input field to document upload form for improved validation.

@coderabbitai

coderabbitai Bot commented Apr 24, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR performs minor UI and documentation updates: removes MinIO startup instructions from the README, fixes a product name spelling from "MiniIO" to "MinIO" in the documents list template, removes debug logging from the ticket upload controller, and adds an explicit file input field to the ticket upload form.

Changes

Cohort / File(s) Summary
Documentation & Configuration
README.md
Removes the docker run snippet for local MinIO startup from the troubleshooting section, simplifying the documented local setup flow.
UI Templates
src/main/resources/templates/documents/list.html, src/main/resources/templates/tickets/detail.html
Fixes product name spelling from "MiniIO" to "MinIO" in the documents list page; adds an explicit type="file" input with required and data-document-file-input attributes to the ticket upload form for client-side file access.
Controller Cleanup
src/main/java/org/group1/projectbackend/controller/web/TicketViewController.java
Removes debug print statements (principal, ticket ID, file metadata) from the uploadDocument handler; reformats user lookup logic indentation without functional change.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

  • PR #10: Both modify the same web UI paths—TicketViewController and tickets/detail.html—suggesting alignment in the document upload feature development.
  • PR #11: Directly modifies the same upload-related code in TicketViewController, indicating continued refinement of the upload handler logic.

Poem

🐰 Whiskers twitching with delight,
Debug logs removed, our code runs light,
MiniIO → MinIO, spelling now quite right,
File inputs added, forms feel tight,
A cleanup hop, the PR's just right!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title 'feature/adding-Thymeleaf-frontend' is overly broad and does not accurately reflect the actual changes, which are primarily bug fixes and minor improvements (removing debug output, fixing typos, adding a file input) rather than a major frontend feature addition. Replace the title with a more specific summary of the actual changes, such as 'Fix debug output, correct MinIO typo, and add file input validation' or a similar descriptor that accurately represents the changeset.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/adding-Thymeleaf-frontend

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/main/resources/templates/tickets/detail.html (1)

116-116: Consider visually hiding the native file input to avoid duplicate UI affordances.

The required native <input type="file"> now sits alongside the custom dropzone, which gives users two visibly different "pick a file" controls. Since document-upload.js already populates fileInput.files via the DataTransfer API for drop/paste (see document-upload.js:30-39), the native input only needs to be reachable — not visually prominent. Visually hiding it (while keeping it focusable and form-submittable) typically yields a cleaner UX and avoids confusion, while required still works for native-picker, drop, and paste paths.

One common pattern:

♻️ Optional UX refinement
-                    <input id="file" name="file" type="file" required data-document-file-input>
+                    <input id="file" name="file" type="file" required data-document-file-input class="visually-hidden">

…paired with a .visually-hidden utility in app.css (or equivalent) and wiring the dropzone's click/keydown to call fileInput.click() so selection still opens the native picker.

Otherwise, the change itself is correct and aligns with the JS expectations (form.querySelector("[data-document-file-input]")) that would otherwise short-circuit init.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/templates/tickets/detail.html` at line 116, The native
file input (<input id="file" name="file" type="file" required
data-document-file-input>) is visually duplicating the custom dropzone; visually
hide it while keeping it focusable and form-submittable by adding a utility
class (e.g., "visually-hidden") to that element and defining the corresponding
CSS in app.css (hide visually but not from assistive tech or form submission).
Also ensure the dropzone click/keydown handlers in document-upload.js still call
fileInput.click() (the code that populates fileInput.files via DataTransfer
should continue to work) so the native picker remains reachable and the required
attribute still validates.
src/main/java/org/group1/projectbackend/controller/web/TicketViewController.java (1)

119-122: ex.printStackTrace() still present — inconsistent with PR summary, and should use a proper logger.

The summary states that runtime debug output was removed from uploadDocument, but ex.printStackTrace() on line 120 still dumps the stack to System.err. For a web controller, prefer an SLF4J logger so the error is captured in the application's structured logs (with correlation context) instead of raw stderr, and consider logging at warn/error with the exception as the last argument to preserve the stack trace.

♻️ Proposed change
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@@
 public class TicketViewController {
+    private static final Logger log = LoggerFactory.getLogger(TicketViewController.class);
@@
         } catch (RuntimeException ex) {
-            ex.printStackTrace();
+            log.warn("Failed to upload document for ticket {}", id, ex);
             redirectAttributes.addFlashAttribute("documentError", "Dokumentet kunde inte laddas upp: " + ex.getMessage());
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/group1/projectbackend/controller/web/TicketViewController.java`
around lines 119 - 122, Replace the direct stderr dump in the
TicketViewController catch block by removing ex.printStackTrace() and instead
log the failure via the class SLF4J logger (e.g. the Logger instance on
TicketViewController) at error/warn level, passing a clear message (including
any ticketId or context available in uploadDocument) and the exception as the
last argument so the stack trace is preserved; keep the
redirectAttributes.addFlashAttribute call intact. If the logger is not present,
add a private static final Logger using
LoggerFactory.getLogger(TicketViewController.class).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@src/main/java/org/group1/projectbackend/controller/web/TicketViewController.java`:
- Around line 119-122: Replace the direct stderr dump in the
TicketViewController catch block by removing ex.printStackTrace() and instead
log the failure via the class SLF4J logger (e.g. the Logger instance on
TicketViewController) at error/warn level, passing a clear message (including
any ticketId or context available in uploadDocument) and the exception as the
last argument so the stack trace is preserved; keep the
redirectAttributes.addFlashAttribute call intact. If the logger is not present,
add a private static final Logger using
LoggerFactory.getLogger(TicketViewController.class).

In `@src/main/resources/templates/tickets/detail.html`:
- Line 116: The native file input (<input id="file" name="file" type="file"
required data-document-file-input>) is visually duplicating the custom dropzone;
visually hide it while keeping it focusable and form-submittable by adding a
utility class (e.g., "visually-hidden") to that element and defining the
corresponding CSS in app.css (hide visually but not from assistive tech or form
submission). Also ensure the dropzone click/keydown handlers in
document-upload.js still call fileInput.click() (the code that populates
fileInput.files via DataTransfer should continue to work) so the native picker
remains reachable and the required attribute still validates.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 659a3d6f-d1a0-4712-84b5-49fbbd2bca50

📥 Commits

Reviewing files that changed from the base of the PR and between d627ec5 and e423be7.

📒 Files selected for processing (4)
  • README.md
  • src/main/java/org/group1/projectbackend/controller/web/TicketViewController.java
  • src/main/resources/templates/documents/list.html
  • src/main/resources/templates/tickets/detail.html
💤 Files with no reviewable changes (1)
  • README.md

@VonAdamo
VonAdamo merged commit de97871 into main Apr 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant