-
Notifications
You must be signed in to change notification settings - Fork 228
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
Remove html elements from ScubaResults.json #1384
base: main
Are you sure you want to change the base?
Conversation
9645dbc
to
98500b0
Compare
Some functional tests have failed, but didn't seem to be related to the changes I made |
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.
Reviewed the code and output produced against a test tenant from both the main branch and this branch. All conversions in the associated outputs conformed to expected transformations in the ScubaResults.JSON. See comment on additional unit testing.
Recommend updating PR title to use imperative style (i.e. Remove vs. Removed) and fix punctuation and case so it reads something like:
Remove HTML elements from ScubaResults.json
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.
The tests themselves are good, however, they do not include a test for the replaced unicode character \u0026. Recommend adding test(s) for this for completeness.
Removed html tags from ScubaResults.json
🗣 Description
I used the exisiting Format-PlainText method to strip out html tags in ScubaResults.json, specifically for links. This was already being done for ScubaResults.csv.
💭 Motivation and context
Closes #1290
🧪 Testing
To test it, run ScubaGear and verify that the "links" render as text without unicode tags.
✅ Pre-approval checklist
✅ Pre-merge checklist
PR passed smoke test check.
Feature branch has been rebased against changes from parent branch, as needed
Use
Rebase branch
button below or use this reference to rebase from the command line.Resolved all merge conflicts on branch
Notified merge coordinator that PR is ready for merge via comment mention
✅ Post-merge checklist