feat: adding opt-out support via script param in rum-standalone.js#317
feat: adding opt-out support via script param in rum-standalone.js#317
Conversation
| Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
| Claude finished @vdua's task —— View job PR Review Complete ✅I've conducted a thorough review of this PR that adds opt-out support via script parameter in rum-standalone.js. Todo List:
Review Feedback✅ Strengths
🚨 Issues FoundCritical: Logic Bug in Rate Calculationsrc/index.js:22-23 const rate = params.get('rum') || window.SAMPLE_PAGEVIEWS_AT_RATE || params.get('optel') || currentScript?.dataset?.rate;Problem: This logic is incorrect. When Expected behavior: According to tests, Fix needed: The precedence logic should check each source explicitly: const rate = params.get('rum') || window.SAMPLE_PAGEVIEWS_AT_RATE || params.get('optel') || currentScript?.dataset?.rate;Should be: const rumParam = params.get('rum'); const optelParam = params.get('optel'); const rate = rumParam || window.SAMPLE_PAGEVIEWS_AT_RATE || optelParam || currentScript?.dataset?.rate;Performance: Unnecessary Boolean Constructorsrc/index.js:33-34 const isSelected = Boolean((window.hlx.rum && window.hlx.rum.isSelected) || (weight && Math.random() * weight < 1));The 📝 Suggestions
🛡️ Security & Performance
🧪 Test CoverageExcellent test coverage including:
🔧 Minor Issues
Recommendation🔴 Request Changes - The precedence logic bug needs to be fixed before merging. Once addressed, this is a solid feature addition with excellent test coverage. Priority fix: Correct the rate calculation precedence logic in |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| This PR will trigger a minor release when merged. |
| Claude finished @vdua's task —— View job PR Review Complete ✅I've conducted a thorough review of this PR that adds opt-out support via script parameter in rum-standalone.js. Todo List:
Review Feedback✅ Strengths
✅ Code QualityThe implementation in const rate = params.get('rum') || window.SAMPLE_PAGEVIEWS_AT_RATE || params.get('optel') || (currentScript && currentScript.dataset.rate);This logic properly ensures 🧪 Test CoverageExcellent test coverage including:
🛡️ Security & Performance
📝 Minor Suggestions
🔧 Configuration Changes
Recommendation✅ Approve - This is a solid feature implementation with excellent test coverage. The code correctly implements the required precedence logic, handles edge cases appropriately, and maintains backward compatibility. The feature provides a valuable addition to the RUM collection system by offering an alternative parameter name for opt-out functionality, which can be useful in different deployment contexts. |
| Claude finished @vdua's task —— View job PR Review Complete ✅I've conducted a thorough review of this PR that adds opt-out support via script parameter in rum-standalone.js. Todo List:
Review Feedback✅ Strengths
✅ Code QualityThe implementation in const rate = params.get('rum') || window.SAMPLE_PAGEVIEWS_AT_RATE || params.get('optel') || (currentScript && currentScript.dataset.rate);This logic properly ensures 🧪 Test CoverageExcellent test coverage including:
🛡️ Security & Performance
📝 Minor Suggestions
🔧 Configuration Changes
Recommendation✅ Approve - This is a solid feature implementation with excellent test coverage. The code correctly implements the required precedence logic, handles edge cases appropriately, and maintains backward compatibility. The feature provides a valuable addition to the RUM collection system by offering an alternative parameter name for opt-out functionality, which can be useful in different deployment contexts. |
| Claude encountered an error —— View job I'll analyze this and get back to you. |
| Claude finished @vdua's task —— View job I'll conduct a thorough review of this PR that adds opt-out support via script parameter in rum-standalone.js. Todo List:
Review Feedback✅ Strengths
✅ Code QualityThe implementation in const rate = params.get('rum') || window.SAMPLE_PAGEVIEWS_AT_RATE || params.get('optel') || (currentScript && currentScript.dataset.rate);This logic properly ensures 🧪 Test CoverageExcellent test coverage including:
🛡️ Security & Performance
📝 Minor Suggestions
🔧 Configuration Changes
Recommendation✅ Approve - This is a solid feature implementation with excellent test coverage. The code correctly implements the required precedence logic, handles edge cases appropriately, and maintains backward compatibility. The feature provides a valuable addition to the RUM collection system by offering an alternative parameter name for opt-out functionality, which can be useful in different deployment contexts. |
# [2.14.0](v2.13.7...v2.14.0) (2025-11-15) ### Features * adding opt-out support via script param in rum-standalone.js ([#317](#317)) ([121c330](121c330))
| 🎉 This PR is included in version 2.14.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Please ensure your pull request adheres to the following guidelines:
Related Issues
Thanks for contributing!
Test URL
https://feat-opt-out-standalone--helix-rum-js--adobe.aem.live/test/static.html