WEB-879 Handle API response where report category returns '(NULL)' string#3422
WEB-879 Handle API response where report category returns '(NULL)' string#3422Omar-Nabil2 wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Note
|
| Cohort / File(s) | Summary |
|---|---|
Data Handling & Display src/app/system/manage-reports/manage-reports.component.ts, src/app/system/manage-reports/manage-reports.component.html |
TypeScript now transforms '(NULL)' strings to null for reportCategory and description fields in setReports(). Template updated to conditionally apply translateKey pipe only when reportCategory exists, preventing translation attempts on null/undefined values. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~8 minutes
Suggested reviewers
- IOhacker
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and accurately summarizes the main change: handling API responses where report category returns '(NULL)' string instead of null. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Pull request overview
Updates the System → Manage Reports table rendering to avoid displaying the literal '(NULL)' string returned by the reports API, showing an empty cell instead.
Changes:
- Sanitizes reports table data by converting
'(NULL)'values tonullbefore creating theMatTableDataSource. - Updates the
reportCategorycell template to render blank output when the category is missing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/app/system/manage-reports/manage-reports.component.ts | Transforms incoming reports data (replacing '(NULL)') before binding to the table; adds debug logging/imports. |
| src/app/system/manage-reports/manage-reports.component.html | Renders reportCategory as empty when null/absent instead of translating/displaying null-like text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { FaIconComponent } from '@fortawesome/angular-fontawesome'; | ||
| import { MatTooltip } from '@angular/material/tooltip'; | ||
| import { STANDALONE_SHARED_IMPORTS } from 'app/standalone-shared.module'; | ||
| import { log } from 'console'; |
There was a problem hiding this comment.
import { log } from 'console'; pulls in Node's console module (not browser/Angular) and the imported symbol is unused. This will likely break the web build and/or fail linting—please remove this import (and avoid Node built-ins in Angular code).
| import { log } from 'console'; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/system/manage-reports/manage-reports.component.ts`:
- Line 108: Remove the debug console.log in ManageReportsComponent (the line
logging 'Reports Data:' in manage-reports.component.ts); locate the statement
inside the component method that sets or handles this.reportsData (e.g.,
ngOnInit or the method that loads reports) and delete that console.log call so
no debug logging remains in production.
- Line 38: Remove the invalid Node.js import statement "import { log } from
'console'" from manage-reports.component.ts and any references to the imported
symbol (if present); the browser environment should use the global console API
(e.g., console.log) already used elsewhere (e.g., in the code that calls
console.log), so simply delete the import line and ensure there are no usages of
the identifier log remaining.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 68461667-99a3-42cd-8328-d1621efe9142
📒 Files selected for processing (2)
src/app/system/manage-reports/manage-reports.component.htmlsrc/app/system/manage-reports/manage-reports.component.ts
| import { FaIconComponent } from '@fortawesome/angular-fontawesome'; | ||
| import { MatTooltip } from '@angular/material/tooltip'; | ||
| import { STANDALONE_SHARED_IMPORTS } from 'app/standalone-shared.module'; | ||
| import { log } from 'console'; |
There was a problem hiding this comment.
Remove invalid Node.js import.
import { log } from 'console' imports from the Node.js console module, which is not available in browser environments. This import is also unused since line 108 uses console.log directly (the global browser API). This will cause a build/runtime error.
🐛 Proposed fix
-import { log } from 'console';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { log } from 'console'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/system/manage-reports/manage-reports.component.ts` at line 38, Remove
the invalid Node.js import statement "import { log } from 'console'" from
manage-reports.component.ts and any references to the imported symbol (if
present); the browser environment should use the global console API (e.g.,
console.log) already used elsewhere (e.g., in the code that calls console.log),
so simply delete the import line and ensure there are no usages of the
identifier log remaining.
| constructor() { | ||
| this.route.data.subscribe((data: { reports: any }) => { | ||
| this.reportsData = data.reports; | ||
| console.log('Reports Data:', this.reportsData); |
There was a problem hiding this comment.
Remove debug logging before merging.
This console.log statement appears to be debug code that should not be included in production. Remove it before merging.
🐛 Proposed fix
this.route.data.subscribe((data: { reports: any }) => {
this.reportsData = data.reports;
- console.log('Reports Data:', this.reportsData);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('Reports Data:', this.reportsData); | |
| this.route.data.subscribe((data: { reports: any }) => { | |
| this.reportsData = data.reports; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/system/manage-reports/manage-reports.component.ts` at line 108,
Remove the debug console.log in ManageReportsComponent (the line logging
'Reports Data:' in manage-reports.component.ts); locate the statement inside the
component method that sets or handles this.reportsData (e.g., ngOnInit or the
method that loads reports) and delete that console.log call so no debug logging
remains in production.
Omar-Nabil2
force-pushed
the
fix/handle-null-report-category-display
branch
from
4f95191 to
ab6c4f4
Compare
alberto-art3ch
left a comment
alberto-art3ch
left a comment
There was a problem hiding this comment.
Please remove los console.lo y el import
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@IOhacker I have removed it. Could you check the changes again ? |
Description
Handle API response where report category returns '(NULL)' string, displaying empty cell instead of null text in table.
Screenshots, if any
before
after
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
Release Notes