From 451ce1ab22a0226705eac0995411a880d4fe5bf7 Mon Sep 17 00:00:00 2001 From: Ivo Oskamp Date: Tue, 10 Feb 2026 13:28:41 +0100 Subject: [PATCH] Add screenshot attachment support to Feedback/Bug system User request: Allow screenshots to be attached to bug reports and feature requests for better documentation and reproduction. Database: - New model: FeedbackAttachment (file_data BYTEA, filename, mime_type, file_size) - Links to feedback_item_id (required) and feedback_reply_id (optional) - Migration: auto-creates table with indexes on startup - Cascading deletes when item or reply is deleted Backend (routes_feedback.py): - Helper function: _validate_image_file() for security - Validates file type using imghdr (not just extension) - Enforces size limit (5MB per file) - Secure filename handling with werkzeug - Allowed: PNG, JPG, GIF, WEBP - Updated feedback_new: accepts multiple file uploads - Updated feedback_reply: accepts multiple file uploads - Updated feedback_detail: fetches attachments for item + replies - New route: /feedback/attachment/ to serve images Frontend: - feedback_new.html: file input with multiple selection - feedback_detail.html: - Shows item screenshots as clickable thumbnails (max 300x200) - Shows reply screenshots as clickable thumbnails (max 200x150) - File upload in reply form - All images open full-size in new tab Security: - Access control: only authenticated users with feedback roles - Image type verification using imghdr (header inspection) - File size limit enforced (5MB) - Secure filename sanitization - Deleted items hide their attachments (404) Co-Authored-By: Claude Sonnet 4.5 --- .../src/backend/app/main/routes_feedback.py | 154 +++++++++++++++++- .../src/backend/app/migrations.py | 44 +++++ .../backupchecks/src/backend/app/models.py | 17 ++ .../src/templates/main/feedback_detail.html | 40 ++++- .../src/templates/main/feedback_new.html | 7 +- docs/changelog-claude.md | 11 ++ 6 files changed, 270 insertions(+), 3 deletions(-) diff --git a/containers/backupchecks/src/backend/app/main/routes_feedback.py b/containers/backupchecks/src/backend/app/main/routes_feedback.py index 804a549..7744496 100644 --- a/containers/backupchecks/src/backend/app/main/routes_feedback.py +++ b/containers/backupchecks/src/backend/app/main/routes_feedback.py @@ -1,5 +1,53 @@ from .routes_shared import * # noqa: F401,F403 from .routes_shared import _format_datetime +from werkzeug.utils import secure_filename +import imghdr + + +# Allowed image extensions and max file size +ALLOWED_EXTENSIONS = {'png', 'jpg', 'jpeg', 'gif', 'webp'} +MAX_FILE_SIZE = 5 * 1024 * 1024 # 5 MB + + +def _validate_image_file(file): + """Validate uploaded image file. + + Returns (is_valid, error_message, mime_type) + """ + if not file or not file.filename: + return False, "No file selected", None + + # Check file size + file.seek(0, 2) # Seek to end + size = file.tell() + file.seek(0) # Reset to beginning + + if size > MAX_FILE_SIZE: + return False, f"File too large (max {MAX_FILE_SIZE // (1024*1024)}MB)", None + + if size == 0: + return False, "Empty file", None + + # Check extension + filename = secure_filename(file.filename) + if '.' not in filename: + return False, "File must have an extension", None + + ext = filename.rsplit('.', 1)[1].lower() + if ext not in ALLOWED_EXTENSIONS: + return False, f"Only images allowed ({', '.join(ALLOWED_EXTENSIONS)})", None + + # Verify it's actually an image by reading header + file_data = file.read() + file.seek(0) + + image_type = imghdr.what(None, h=file_data) + if image_type is None: + return False, "Invalid image file", None + + mime_type = f"image/{image_type}" + + return True, None, mime_type @main_bp.route("/feedback") @@ -135,6 +183,31 @@ def feedback_new(): created_by_user_id=int(current_user.id), ) db.session.add(item) + db.session.flush() # Get item.id for attachments + + # Handle file uploads (multiple files allowed) + files = request.files.getlist('screenshots') + for file in files: + if file and file.filename: + is_valid, error_msg, mime_type = _validate_image_file(file) + if not is_valid: + db.session.rollback() + flash(f"Screenshot error: {error_msg}", "danger") + return redirect(url_for("main.feedback_new")) + + filename = secure_filename(file.filename) + file_data = file.read() + + attachment = FeedbackAttachment( + feedback_item_id=item.id, + feedback_reply_id=None, + filename=filename, + file_data=file_data, + mime_type=mime_type, + file_size=len(file_data), + ) + db.session.add(attachment) + db.session.commit() flash("Feedback item created.", "success") @@ -174,13 +247,41 @@ def feedback_detail(item_id: int): resolved_by = User.query.get(item.resolved_by_user_id) resolved_by_name = resolved_by.username if resolved_by else "" - + # Get attachments for the main item (not linked to a reply) + item_attachments = ( + FeedbackAttachment.query.filter( + FeedbackAttachment.feedback_item_id == item.id, + FeedbackAttachment.feedback_reply_id.is_(None), + ) + .order_by(FeedbackAttachment.created_at.asc()) + .all() + ) + replies = ( FeedbackReply.query.filter(FeedbackReply.feedback_item_id == item.id) .order_by(FeedbackReply.created_at.asc()) .all() ) + # Get attachments for each reply + reply_ids = [r.id for r in replies] + reply_attachments_list = [] + if reply_ids: + reply_attachments_list = ( + FeedbackAttachment.query.filter( + FeedbackAttachment.feedback_reply_id.in_(reply_ids) + ) + .order_by(FeedbackAttachment.created_at.asc()) + .all() + ) + + # Map reply_id -> list of attachments + reply_attachments_map = {} + for att in reply_attachments_list: + if att.feedback_reply_id not in reply_attachments_map: + reply_attachments_map[att.feedback_reply_id] = [] + reply_attachments_map[att.feedback_reply_id].append(att) + reply_user_ids = sorted({int(r.user_id) for r in replies}) reply_users = ( User.query.filter(User.id.in_(reply_user_ids)).all() if reply_user_ids else [] @@ -196,6 +297,8 @@ def feedback_detail(item_id: int): user_voted=bool(user_voted), replies=replies, reply_user_map=reply_user_map, + item_attachments=item_attachments, + reply_attachments_map=reply_attachments_map, ) @main_bp.route("/feedback//reply", methods=["POST"]) @@ -222,6 +325,31 @@ def feedback_reply(item_id: int): created_at=datetime.utcnow(), ) db.session.add(reply) + db.session.flush() # Get reply.id for attachments + + # Handle file uploads (multiple files allowed) + files = request.files.getlist('screenshots') + for file in files: + if file and file.filename: + is_valid, error_msg, mime_type = _validate_image_file(file) + if not is_valid: + db.session.rollback() + flash(f"Screenshot error: {error_msg}", "danger") + return redirect(url_for("main.feedback_detail", item_id=item.id)) + + filename = secure_filename(file.filename) + file_data = file.read() + + attachment = FeedbackAttachment( + feedback_item_id=item.id, + feedback_reply_id=reply.id, + filename=filename, + file_data=file_data, + mime_type=mime_type, + file_size=len(file_data), + ) + db.session.add(attachment) + db.session.commit() flash("Reply added.", "success") @@ -308,3 +436,27 @@ def feedback_delete(item_id: int): flash("Feedback item deleted.", "success") return redirect(url_for("main.feedback_page")) + + +@main_bp.route("/feedback/attachment/") +@login_required +@roles_required("admin", "operator", "reporter", "viewer") +def feedback_attachment(attachment_id: int): + """Serve a feedback attachment image.""" + attachment = FeedbackAttachment.query.get_or_404(attachment_id) + + # Check if the feedback item is deleted + item = FeedbackItem.query.get(attachment.feedback_item_id) + if not item or item.deleted_at is not None: + abort(404) + + # Serve the image + from flask import send_file + import io + + return send_file( + io.BytesIO(attachment.file_data), + mimetype=attachment.mime_type, + as_attachment=False, + download_name=attachment.filename, + ) diff --git a/containers/backupchecks/src/backend/app/migrations.py b/containers/backupchecks/src/backend/app/migrations.py index 323bcf6..fb8f4fa 100644 --- a/containers/backupchecks/src/backend/app/migrations.py +++ b/containers/backupchecks/src/backend/app/migrations.py @@ -1095,6 +1095,7 @@ def run_migrations() -> None: migrate_object_persistence_tables() migrate_feedback_tables() migrate_feedback_replies_table() + migrate_feedback_attachments_table() migrate_tickets_active_from_date() migrate_tickets_resolved_origin() migrate_remarks_active_from_date() @@ -1446,6 +1447,49 @@ def migrate_feedback_replies_table() -> None: print("[migrations] Feedback replies table ensured.") +def migrate_feedback_attachments_table() -> None: + """Ensure feedback attachments table exists. + + Table: + - feedback_attachments (screenshots/images for feedback items and replies) + """ + engine = db.get_engine() + with engine.begin() as conn: + conn.execute( + text( + """ + CREATE TABLE IF NOT EXISTS feedback_attachments ( + id SERIAL PRIMARY KEY, + feedback_item_id INTEGER NOT NULL REFERENCES feedback_items(id) ON DELETE CASCADE, + feedback_reply_id INTEGER REFERENCES feedback_replies(id) ON DELETE CASCADE, + filename VARCHAR(255) NOT NULL, + file_data BYTEA NOT NULL, + mime_type VARCHAR(64) NOT NULL, + file_size INTEGER NOT NULL, + created_at TIMESTAMP NOT NULL DEFAULT NOW() + ); + """ + ) + ) + conn.execute( + text( + """ + CREATE INDEX IF NOT EXISTS idx_feedback_attachments_item + ON feedback_attachments (feedback_item_id); + """ + ) + ) + conn.execute( + text( + """ + CREATE INDEX IF NOT EXISTS idx_feedback_attachments_reply + ON feedback_attachments (feedback_reply_id); + """ + ) + ) + print("[migrations] Feedback attachments table ensured.") + + def migrate_tickets_active_from_date() -> None: """Ensure tickets.active_from_date exists and is populated. diff --git a/containers/backupchecks/src/backend/app/models.py b/containers/backupchecks/src/backend/app/models.py index 2d14e97..c4a4703 100644 --- a/containers/backupchecks/src/backend/app/models.py +++ b/containers/backupchecks/src/backend/app/models.py @@ -567,6 +567,23 @@ class FeedbackReply(db.Model): created_at = db.Column(db.DateTime, default=datetime.utcnow, nullable=False) +class FeedbackAttachment(db.Model): + __tablename__ = "feedback_attachments" + + id = db.Column(db.Integer, primary_key=True) + feedback_item_id = db.Column( + db.Integer, db.ForeignKey("feedback_items.id", ondelete="CASCADE"), nullable=False + ) + feedback_reply_id = db.Column( + db.Integer, db.ForeignKey("feedback_replies.id", ondelete="CASCADE"), nullable=True + ) + filename = db.Column(db.String(255), nullable=False) + file_data = db.Column(db.LargeBinary, nullable=False) + mime_type = db.Column(db.String(64), nullable=False) + file_size = db.Column(db.Integer, nullable=False) + created_at = db.Column(db.DateTime, default=datetime.utcnow, nullable=False) + + class NewsItem(db.Model): __tablename__ = "news_items" diff --git a/containers/backupchecks/src/templates/main/feedback_detail.html b/containers/backupchecks/src/templates/main/feedback_detail.html index 4e80c45..0109af7 100644 --- a/containers/backupchecks/src/templates/main/feedback_detail.html +++ b/containers/backupchecks/src/templates/main/feedback_detail.html @@ -29,6 +29,23 @@
Component: {{ item.component }}
{% endif %}
{{ item.description }}
+ + {% if item_attachments %} +
+ Screenshots: +
+ {% for att in item_attachments %} + + {{ att.filename }} + + {% endfor %} +
+
+ {% endif %} {% endfor %} @@ -76,10 +109,15 @@
Add reply
{% if item.status == 'open' %} -
+
+
+ + +
You can attach multiple screenshots (PNG, JPG, GIF, WEBP, max 5MB each)
+
{% else %} diff --git a/containers/backupchecks/src/templates/main/feedback_new.html b/containers/backupchecks/src/templates/main/feedback_new.html index ebc6b05..57bc49f 100644 --- a/containers/backupchecks/src/templates/main/feedback_new.html +++ b/containers/backupchecks/src/templates/main/feedback_new.html @@ -6,7 +6,7 @@ Back
-
+
@@ -28,6 +28,11 @@
+
+ + +
You can attach multiple screenshots (PNG, JPG, GIF, WEBP, max 5MB each)
+