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/<id> 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 <noreply@anthropic.com>
This commit is contained in:
parent
588f788e31
commit
451ce1ab22
@ -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,6 +247,15 @@ 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)
|
||||
@ -181,6 +263,25 @@ def feedback_detail(item_id: int):
|
||||
.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/<int:item_id>/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/<int:attachment_id>")
|
||||
@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,
|
||||
)
|
||||
|
||||
@ -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.
|
||||
|
||||
|
||||
@ -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"
|
||||
|
||||
|
||||
@ -29,6 +29,23 @@
|
||||
<div class="mb-2"><strong>Component:</strong> {{ item.component }}</div>
|
||||
{% endif %}
|
||||
<div style="white-space: pre-wrap;">{{ item.description }}</div>
|
||||
|
||||
{% if item_attachments %}
|
||||
<div class="mt-3">
|
||||
<strong>Screenshots:</strong>
|
||||
<div class="d-flex flex-wrap gap-2 mt-2">
|
||||
{% for att in item_attachments %}
|
||||
<a href="{{ url_for('main.feedback_attachment', attachment_id=att.id) }}" target="_blank">
|
||||
<img src="{{ url_for('main.feedback_attachment', attachment_id=att.id) }}"
|
||||
alt="{{ att.filename }}"
|
||||
class="img-thumbnail"
|
||||
style="max-height: 200px; max-width: 300px; cursor: pointer;"
|
||||
title="Click to view full size" />
|
||||
</a>
|
||||
{% endfor %}
|
||||
</div>
|
||||
</div>
|
||||
{% endif %}
|
||||
</div>
|
||||
<div class="card-footer d-flex justify-content-between align-items-center">
|
||||
<div class="text-muted" style="font-size: 0.9rem;">
|
||||
@ -63,6 +80,22 @@
|
||||
</span>
|
||||
</div>
|
||||
<div style="white-space: pre-wrap;">{{ r.message }}</div>
|
||||
|
||||
{% if r.id in reply_attachments_map %}
|
||||
<div class="mt-2">
|
||||
<div class="d-flex flex-wrap gap-2">
|
||||
{% for att in reply_attachments_map[r.id] %}
|
||||
<a href="{{ url_for('main.feedback_attachment', attachment_id=att.id) }}" target="_blank">
|
||||
<img src="{{ url_for('main.feedback_attachment', attachment_id=att.id) }}"
|
||||
alt="{{ att.filename }}"
|
||||
class="img-thumbnail"
|
||||
style="max-height: 150px; max-width: 200px; cursor: pointer;"
|
||||
title="Click to view full size" />
|
||||
</a>
|
||||
{% endfor %}
|
||||
</div>
|
||||
</div>
|
||||
{% endif %}
|
||||
</div>
|
||||
{% endfor %}
|
||||
</div>
|
||||
@ -76,10 +109,15 @@
|
||||
<div class="card-body">
|
||||
<h5 class="card-title mb-3">Add reply</h5>
|
||||
{% if item.status == 'open' %}
|
||||
<form method="post" action="{{ url_for('main.feedback_reply', item_id=item.id) }}">
|
||||
<form method="post" action="{{ url_for('main.feedback_reply', item_id=item.id) }}" enctype="multipart/form-data">
|
||||
<div class="mb-2">
|
||||
<textarea class="form-control" name="message" rows="4" required></textarea>
|
||||
</div>
|
||||
<div class="mb-2">
|
||||
<label class="form-label">Screenshots (optional)</label>
|
||||
<input type="file" name="screenshots" class="form-control" multiple accept="image/png,image/jpeg,image/jpg,image/gif,image/webp" />
|
||||
<div class="form-text">You can attach multiple screenshots (PNG, JPG, GIF, WEBP, max 5MB each)</div>
|
||||
</div>
|
||||
<button type="submit" class="btn btn-primary">Post reply</button>
|
||||
</form>
|
||||
{% else %}
|
||||
|
||||
@ -6,7 +6,7 @@
|
||||
<a class="btn btn-outline-secondary" href="{{ url_for('main.feedback_page') }}">Back</a>
|
||||
</div>
|
||||
|
||||
<form method="post" class="card">
|
||||
<form method="post" enctype="multipart/form-data" class="card">
|
||||
<div class="card-body">
|
||||
<div class="row g-3">
|
||||
<div class="col-12 col-md-3">
|
||||
@ -28,6 +28,11 @@
|
||||
<label class="form-label">Component (optional)</label>
|
||||
<input type="text" name="component" class="form-control" />
|
||||
</div>
|
||||
<div class="col-12">
|
||||
<label class="form-label">Screenshots (optional)</label>
|
||||
<input type="file" name="screenshots" class="form-control" multiple accept="image/png,image/jpeg,image/jpg,image/gif,image/webp" />
|
||||
<div class="form-text">You can attach multiple screenshots (PNG, JPG, GIF, WEBP, max 5MB each)</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<div class="card-footer d-flex justify-content-end">
|
||||
|
||||
@ -4,6 +4,17 @@ This file documents all changes made to this project via Claude Code.
|
||||
|
||||
## [2026-02-10]
|
||||
|
||||
### Added
|
||||
- Added screenshot attachment support to Feedback/Bug system (user request: allow screenshots for bugs/features)
|
||||
- New database model: `FeedbackAttachment` with file_data (BYTEA), filename, mime_type, file_size
|
||||
- Upload support on feedback creation form (multiple files, PNG/JPG/GIF/WEBP, max 5MB each)
|
||||
- Upload support on reply forms (attach screenshots when replying)
|
||||
- Inline image display on feedback detail page (thumbnails with click-to-view-full-size)
|
||||
- Screenshot display for both main feedback items and replies
|
||||
- File validation: image type verification, size limits, secure filename handling
|
||||
- New route: `/feedback/attachment/<id>` to serve images (access-controlled)
|
||||
- Database migration: auto-creates `feedback_attachments` table with indexes on startup
|
||||
|
||||
### Fixed
|
||||
- Fixed Autotask ticket not being automatically linked to new runs when internal ticket is resolved by implementing independent Autotask propagation strategy (now checks for most recent non-deleted and non-resolved Autotask ticket on job regardless of internal ticket status, ensuring PSA ticket reference persists across runs until explicitly resolved or deleted)
|
||||
- Fixed internal and Autotask tickets being linked to new runs even after being resolved by removing date-based "open" logic from ticket query (tickets now only link to new runs if they are genuinely unresolved, not based on run date comparisons)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user