-
Notifications
You must be signed in to change notification settings - Fork 22
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
Add Detailed report messages for Drive&Docs #300
Conversation
… detailed report for 3.1
…lling error from Complaint to Compliant
…e for 2.4, the failing condition is that download, print, and copy is turned on.
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.
I haven't finished my review yet (only done 1.1, 1,2, and 1.3 so far), but I'm releasing these comments now to get you feedback sooner.
It looks like the last edit requested by Alden was completed by Sheetal yesterday! Looks like it should be good to go now? |
Unfortunately, this PR won't make it in time for this release. I only reviewed group 1. We didn't have enough time to get to the remaining groups. |
Gotcha. All good. |
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.
Finished my review! Here are my remaining comments.
…github.com/cisagov/ScubaGoggles into 259-detailed-report-message-for-drive-docs
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.
2 small spacing requests.
1 question.
@adhilto fat fingered the review button and accidentally requested your rereview. |
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.
Looks good!
🗣 Description
Update Drive and Docs rego and unit tests to support detailed report messages and group override policy
Closes #259
Closes #282
Closes #196
💭 Motivation and context
This issue is part of the epic #190 and #90
🧪 Testing
Ensure all use cases for report and unit tests pass
✅ Pre-approval checklist
✅ Pre-merge Checklist
Squash and merge
button.✅ Post-merge Checklist