RDKB-66820 : Parental control report & troubleshooting logs always timing out - #161
pavankumar464 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
It introduces an implicit-global ajaxrequest assignment (can break under strict mode) and includes a new touched typo in an accessibility header/id string that should be corrected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses log/report operations timing out by normalizing time-frame identifiers across the UI, download endpoints, and AJAX handlers, while simplifying client-side show/hide logic and making log parsing/writing more resilient.
Changes:
- Standardize time-frame values to stable keys (
today|yesterday|week|month|last) and pass localized labels separately for display. - Refactor UI logic for switching report/log views to reduce duplicated branching.
- Harden AJAX handlers’ file parsing/writing (guarding failed reads, avoiding partial records, and using consistent temp file naming).
File summaries
| File | Description |
|---|---|
| source/Styles/xb3/jst/troubleshooting_logs.jst | Switch UI to key-based timeframes, simplify view toggling, and pass a label for accessible table summaries. |
| source/Styles/xb3/jst/troubleshooting_logs_download.jst | Validate key-based timeframes and align downloaded temp-file naming with the generators. |
| source/Styles/xb3/jst/parental_reports.jst | Switch UI to key-based timeframes, simplify view toggling, and use selected label in summaries. |
| source/Styles/xb3/jst/parental_reports_download.jst | Validate key-based timeframes and align downloaded temp-file naming with the generators. |
| source/Styles/xb3/jst/actionHandler/ajax_troubleshooting_logs.jst | Update timeframe parsing to key-based values and harden log-file reading/writing. |
| source/Styles/xb3/jst/actionHandler/ajax_parental_reports.jst | Update timeframe parsing to key-based values and harden parental log processing/writing. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| var timef2=timef; | ||
| jProgress($.i18n('This may take several seconds.'),120); | ||
| ajaxrequest=$.ajax({ |
| var timef2=timef; | ||
| jProgress($.i18n('This may take several seconds.'),180); | ||
| $.ajax({ | ||
| ajaxrequest=$.ajax({ |
| } | ||
| //adjust current data table | ||
| adjust_acs_tb("This is "+mode+" logs, for "+timef, Array("Discription", "Time", "Level")); | ||
| adjust_acs_tb("This is "+mode+" logs, for "+timefLabel, Array("Discription", "Time", "Level")); |
pavankumar464
changed the title
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
There was a problem hiding this comment.
🟡 Changes recommended
Two unresolved findings remain in pagination behavior and parental-report time normalization.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
source/Styles/xb3/jst/troubleshooting_logs.jst:43
- Removing the paginator reset makes Print omit data whenever the selected table has more than 20 rows.
ajaxDostill callscomcastPaginator()for those tables, which hides every row outside the current page, sowindow.print()prints only that page. Restore the temporary all-rows view before printing and reapply pagination afterward.
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
| if (!$CountLine || !$SourceIPLine || !$UserLine || !$TargetIPLine || !$TypeLine || !$timeLine || !$DesLine) break; | ||
| $Count=trim(substr($CountLine,$pos)); $SourceIP=trim(substr($SourceIPLine,$pos)); $User=trim(substr($UserLine,$pos)); $TargetIP=trim(substr($TargetIPLine,$pos)); $Type=trim(substr($TypeLine,$pos)); $time=trim(substr($timeLine,$pos)); $Des=trim(substr($DesLine,$pos)); | ||
| $timeArr = str2time($time); | ||
| if ($timeArr['timeU'] > $maxtime || $timeArr['timeU'] < $mintime) continue; |
Reason for change: Parental control report & troubleshooting logs are always timing out
Test Procedure: Test for Parental control reports & troubleshooting logs
Risks: Low
Priority: P1