[Feature:Forum] PDF Attachment Support#12213
Closed
samarthnaikk wants to merge 6 commits into
Closed
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12213 +/- ##
============================================
+ Coverage 21.71% 21.73% +0.02%
- Complexity 9612 9620 +8
============================================
Files 268 268
Lines 36073 36083 +10
Branches 475 475
============================================
+ Hits 7832 7844 +12
+ Misses 27770 27768 -2
Partials 471 471
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
samarthnaikk
changed the title
RyanStyron
requested changes
Nov 18, 2025
Contributor
There was a problem hiding this comment.
The functionality is aligned with the description. I would suggest migrating from the use of popups to the use of new tabs, to be consistent with #12135, which is handled by loadInlineImages in forum.js. Additionally, the very same loadInlineImages function is used to render both images and PDFs. Perhaps there should be a separate method (or the same method with altered logic), to improve in-line rendering of PDFs (<iframe> tag?).
github-project-automation
Bot
moved this from Seeking Reviewer
to Work in Progress
in Submitty Development
Contributor
Also, I would recommend adding a test that creates a |
Contributor
Author
|
@RyanStyron I’ve made the changes. Please let me know if you'd like me to adjust or improve anything further. |
RyanStyron
requested changes
Dec 1, 2025
RyanStyron
left a comment
RyanStyron
left a comment
Contributor
There was a problem hiding this comment.
The feature works well. Thank you. The test, however, is slightly misrepresentative of what it claims. I would suggest replacing time.sleep with a short WebDriverWait to wait for an upload preview or for errors. Also, I would recommend strengthening the assertions. The issue with the current test is that it does not explicitly confirm that only the valid file was uploaded or that the invalid file was rejected. It assumes filtering happened but does not assert it. Similarly, it does not check if the image was actually attached to the created post. Please resolve the lint failures as well.
bmcutler
added a commit
that referenced
this pull request
Apr 10, 2026
## What is the current behavior? Discussion forum attachments currently allow valid image uploads, but reject PDF attachments. Closes #12202 ## What is the new behavior? This PR allows valid PDF attachments in discussion forum posts in addition to valid images. ## What changed? - allow forum attachments that are valid images or PDFs - validation is content-based and not extension-only - preserved existing image validation behavior - kept existing forum attachment limits unchanged - updated the forum upload UI to accept PDFs (`accept="image/*,.pdf"`) - added a separate PDF rendering path for forum attachments - kept existing inline image rendering behavior unchanged - updated edit/history views to correctly display PDF attachments - aligned attachment opening behavior with current forum behavior by using new-tab opening without popup-style window features - added unit tests for: - valid PDF accepted - invalid file rejected - existing image behavior still works - added focused e2e coverage for: - valid PDF upload - invalid file rejection - attachment visible on created post - PDF preview rendering ## Notes This implementation was informed by feedback from #12213: - attachment opening behavior was simplified to stay consistent with the current forum attachment UX - PDF handling was separated from image handling instead of forcing both through the same rendering path - existing attachment data flow was preserved to avoid breaking forum edit/history views - tests were strengthened to verify actual outcomes rather than timing-based assumptions ## Manual Testing - created a thread with a PDF attachment and verified the thread was created successfully - verified the PDF appears in the created post attachment section - verified PDF preview renders correctly - verified PDF open behavior uses a new tab - verified PDF appears correctly in the edit attachment table - verified remove/delete still works for PDF attachments - verified existing image upload, preview, and open behavior still works ## Screenshots ### Upload UI (PDF selectable)  ### PDF Preview on created post  ### Edit view showing PDF tile  --------- Co-authored-by: dagemcn <[email protected]> Co-authored-by: Barb Cutler <[email protected]>
GarvitKhandelwal31
pushed a commit
to GarvitKhandelwal31/Submitty
that referenced
this pull request
Apr 13, 2026
## What is the current behavior? Discussion forum attachments currently allow valid image uploads, but reject PDF attachments. Closes Submitty#12202 ## What is the new behavior? This PR allows valid PDF attachments in discussion forum posts in addition to valid images. ## What changed? - allow forum attachments that are valid images or PDFs - validation is content-based and not extension-only - preserved existing image validation behavior - kept existing forum attachment limits unchanged - updated the forum upload UI to accept PDFs (`accept="image/*,.pdf"`) - added a separate PDF rendering path for forum attachments - kept existing inline image rendering behavior unchanged - updated edit/history views to correctly display PDF attachments - aligned attachment opening behavior with current forum behavior by using new-tab opening without popup-style window features - added unit tests for: - valid PDF accepted - invalid file rejected - existing image behavior still works - added focused e2e coverage for: - valid PDF upload - invalid file rejection - attachment visible on created post - PDF preview rendering ## Notes This implementation was informed by feedback from Submitty#12213: - attachment opening behavior was simplified to stay consistent with the current forum attachment UX - PDF handling was separated from image handling instead of forcing both through the same rendering path - existing attachment data flow was preserved to avoid breaking forum edit/history views - tests were strengthened to verify actual outcomes rather than timing-based assumptions ## Manual Testing - created a thread with a PDF attachment and verified the thread was created successfully - verified the PDF appears in the created post attachment section - verified PDF preview renders correctly - verified PDF open behavior uses a new tab - verified PDF appears correctly in the edit attachment table - verified remove/delete still works for PDF attachments - verified existing image upload, preview, and open behavior still works ## Screenshots ### Upload UI (PDF selectable)  ### PDF Preview on created post  ### Edit view showing PDF tile  --------- Co-authored-by: dagemcn <[email protected]> Co-authored-by: Barb Cutler <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this Change Important & Necessary?
The forum currently only supports image attachments, but instructors often need to upload PDF documents such as rubrics or worksheets. Allowing PDFs improves usability while keeping the same file size and security restrictions already in place for images.
What is the New Behavior?
Users can attach both images and PDFs to forum posts. The UI labels now refer to “files” instead of only images, and the file input accepts PDFs. All existing limits (file size, max number of attachments, security checks) remain unchanged.
What steps should a reviewer take to reproduce or test the new feature?
Automated Testing & Documentation
Unit tests have been added to cover PDF detection, MIME validation, and compatibility with existing image validation. Documentation updates may be needed to mention PDF support in forum attachments.
Other information
This is not a breaking change and does not require database migrations. Existing security checks (file validation, MIME type checks, size limits) remain fully enforced.
This PR should solve #12202