Fix issue report screenshot size validation#8757
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the "reserved bytes" attachment budget that pre-counted hidden diagnostic/log file sizes against the screenshot attachment limit. After this change, the maxTotalBytes cap applies only to user-selected screenshots, eliminating false "attachment size exceeded" errors when logs are present but no screenshot is attached.
Changes:
- Deletes
attachment_budget.dartand its test, and removesreservedBytesparameter fromvalidateAttachments/addAttachmentsand the submitter validation path. - Updates
report_issue.dartto drop the budget lookup before adding attachments. - Updates tests to remove the fake budget and adjust the oversize test to use
maxTotalBytes + 1.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/features/report_issue/models/report_issue_attachment_rules.dart | Drops reservedBytes from validateAttachments size check. |
| lib/features/report_issue/provider/attachment_budget.dart | Deletes the entire budget abstraction (also removes the ReportIssueLogFileResolver typedef and default resolver). |
| lib/features/report_issue/provider/submitter.dart | Removes budget dependency; switches import to a (currently non-existent) log_file_resolver.dart. |
| lib/features/report_issue/provider/report_issue_draft_notifier.dart | Removes reservedBytes parameter from addAttachments. |
| lib/features/report_issue/report_issue.dart | Stops reading the budget before invoking addAttachments. |
| test/features/report_issue/report_issue_test.dart | Removes the fake budget and adjusts oversize-screenshot expectation. |
| test/features/report_issue/report_issue_submitter_test.dart | Removes fake budget overrides; oversize test now uses maxTotalBytes + 1. |
| test/features/report_issue/report_issue_attachment_budget_test.dart | Deletes the now-obsolete budget test file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
See copilot comment |
Contributor
Author
I added the missing file |
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
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.
Resolves https://github.com/getlantern/engineering/issues/3466
Updates report-issue attachment validation so the screenshot limit only counts selected screenshots, not hidden diagnostic/log bytes. This prevents false attachment size exceeded errors when users submit logs with no visible attachment