-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HTML Report, limit records displayed #1433
Conversation
This reverts commit c89c716.
efebe96
to
0b071ff
Compare
@isabelle-dr @davidgamez This was added onto the #1389, the metadata PR, so is marked as draft until that PR is merged. Here is an example of the message added when # of affected records > 100: |
Great :) |
@briandonahue now that #1389 is merged, this one is ready for review right? |
2422f8c
to
7ffb08f
Compare
@isabelle-dr Had to fix some conflicts, it should be good now, though! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
✅ Rule acceptance tests passed. |
Merging then :) Thank you @briandonahue for this contribution! |
Summary:
Closes #1306. The HTML report is not functioning well when large amounts of records are flagged for one or more notices.
We will limit the number of displayed records to a sample of 100, which does not cause performance errors. The full list is still available in the JSON report.
Expected behavior:
Accordion sections should still open/close and display sample records.
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle test
to make sure you didn't break anything