-
Notifications
You must be signed in to change notification settings - Fork 14
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
fix: fixed parsing after blizzard change of main div #216
Conversation
Reviewer's Guide by SourceryThis PR fixes an issue with HTML parsing after Blizzard changed their main div structure. The changes include updating the CSS selector to handle both old and new div structures, bumping the version number, and making some test improvements. Updated class diagram for the HTML parserclassDiagram
class HTMLParser {
- root_tag
+ create_parser_tag(html_content: str)
}
note for HTMLParser "Updated to handle both 'div.main-content' and 'main' as root tags"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Quality Gate passedIssues Measures |
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.
Hey @TeKrop - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Found a Google Tag Manager ID hardcoded in the JavaScript code (link)
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🔴 Security: 1 blocking issue
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0], | ||
j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src= | ||
'https://www.googletagmanager.com/gtm.js?id='+i+dl;f.parentNode.insertBefore(j,f); | ||
})(window,document,'script','dataLayer','GTM-TVHPB9J'); |
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.
🚨 issue (security): Found a Google Tag Manager ID hardcoded in the JavaScript code
While GTM IDs are not strictly secrets, they are often treated as such since they can be used to inject malicious code if compromised. Consider moving this to a configuration file or environment variable.
Summary by Sourcery
Fix parsing logic to handle Blizzard's change in the main content div structure by updating the HTML parser to support both 'div.main-content' and 'main' elements.
Bug Fixes:
Enhancements: