-
Notifications
You must be signed in to change notification settings - Fork 800
WEB-490 :- use date pipeline to enhance the displaying of date #2870
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
base: dev
Are you sure you want to change the base?
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Closing Entries Date Formatting src/app/accounting/closing-entries/closing-entries.component.html, src/app/accounting/closing-entries/view-closure/view-closure.component.html |
Applied date pipe with 'medium' format to closingDate and createdDate fields for consistent date presentation. |
Provisioning Entries Date Formatting src/app/accounting/provisioning-entries/provisioning-entries.component.html, src/app/accounting/provisioning-entries/view-provisioning-entry/view-provisioning-entry.component.html, src/app/accounting/provisioning-entries/view-provisioning-journal-entries/view-provisioning-journal-entries.component.html |
Applied date pipe with 'medium' format to createdDate and transactionDate fields for consistent date rendering. |
Search Journal Entry Date Formatting & Pipe Removal src/app/accounting/search-journal-entry/search-journal-entry.component.html, src/app/accounting/search-journal-entry/search-journal-entry.component.ts |
Replaced custom datetimeFormat pipe with Angular date pipe (medium format) in template; removed DatetimeFormatPipe import from component declarations. |
Notifications Date Formatting src/app/notifications/notifications-page/notifications-page.component.html, src/app/shared/notifications-tray/notifications-tray.component.html |
Applied date pipe with 'medium' format to createdAt fields in notification display entries. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
- Highly homogeneous changes—consistent date formatting pattern applied across multiple files with minimal variation
- No logic changes, control flow modifications, or structural alterations
- Single supporting TypeScript change (import removal) is straightforward
- Verification can focus on confirming the 'medium' format produces acceptable output and that the DatetimeFormatPipe removal doesn't create regressions elsewhere
Suggested reviewers
- steinwinde
- gkbishnoi07
Pre-merge checks and finishing touches
✅ 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 accurately describes the main change: replacing custom date formatting with Angular's built-in date pipe across multiple components. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/app/accounting/closing-entries/closing-entries.component.html(1 hunks)src/app/accounting/closing-entries/view-closure/view-closure.component.html(2 hunks)src/app/accounting/provisioning-entries/provisioning-entries.component.html(1 hunks)src/app/accounting/provisioning-entries/view-provisioning-entry/view-provisioning-entry.component.html(1 hunks)src/app/accounting/provisioning-entries/view-provisioning-journal-entries/view-provisioning-journal-entries.component.html(1 hunks)src/app/accounting/search-journal-entry/search-journal-entry.component.html(1 hunks)src/app/accounting/search-journal-entry/search-journal-entry.component.ts(0 hunks)src/app/notifications/notifications-page/notifications-page.component.html(1 hunks)src/app/shared/notifications-tray/notifications-tray.component.html(1 hunks)
💤 Files with no reviewable changes (1)
- src/app/accounting/search-journal-entry/search-journal-entry.component.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/accounting/provisioning-entries/view-provisioning-entry/view-provisioning-entry.component.htmlsrc/app/shared/notifications-tray/notifications-tray.component.htmlsrc/app/accounting/closing-entries/closing-entries.component.htmlsrc/app/accounting/provisioning-entries/view-provisioning-journal-entries/view-provisioning-journal-entries.component.htmlsrc/app/accounting/provisioning-entries/provisioning-entries.component.htmlsrc/app/notifications/notifications-page/notifications-page.component.htmlsrc/app/accounting/closing-entries/view-closure/view-closure.component.htmlsrc/app/accounting/search-journal-entry/search-journal-entry.component.html
🔇 Additional comments (8)
src/app/accounting/provisioning-entries/provisioning-entries.component.html (1)
30-30: Date formatting update looks good.The switch from raw date value to Angular's built-in date pipe with 'medium' format is correct and improves presentation consistency across the application.
src/app/notifications/notifications-page/notifications-page.component.html (1)
17-17: Date formatting update is correct.The change to display
createdAtwith the date pipe 'medium' format is properly implemented and aligns with other components in the PR.src/app/accounting/provisioning-entries/view-provisioning-entry/view-provisioning-entry.component.html (1)
28-28: Date formatting change is correct.The
createdDateis properly formatted using the date pipe. Additionally, all@forloops in this template properly includetrackproperties, complying with Angular best practices.src/app/accounting/closing-entries/closing-entries.component.html (1)
36-36: Date formatting update is properly implemented.The
closingDatechange to use the date pipe with 'medium' format is correct. The@forloop on line 19 properly includes thetrackparameter.src/app/accounting/closing-entries/view-closure/view-closure.component.html (1)
27-27: Both date fields correctly formatted.The
closingDate(line 27) andcreatedDate(line 51) are both properly formatted using the date pipe with 'medium' format. Changes maintain consistency across the view component.Also applies to: 51-51
src/app/accounting/provisioning-entries/view-provisioning-journal-entries/view-provisioning-journal-entries.component.html (1)
23-25: Date formatting is properly implemented.The
transactionDateis correctly formatted using the date pipe with 'medium' format. The multi-line formatting (lines 23-25) maintains readability.src/app/shared/notifications-tray/notifications-tray.component.html (1)
28-28: Date formatting is consistent and correct across both notification loops.Both
createdAtfields (lines 28 and 35) use the date pipe with 'medium' format. Both@forloops properly includetrackparameters as per Angular best practices.Also applies to: 35-35
src/app/accounting/search-journal-entry/search-journal-entry.component.html (1)
110-110: Inconsistent date formatting in table cells—verify complete refactoring.Line 110 uses a custom
dateFormatpipe fortransactionDate, while line 125 uses the built-indate: 'medium'pipe forsubmittedOnDate. This creates inconsistent presentation in the same table.The AI summary mentions that
DatetimeFormatPipewas removed from the component, but it's unclear whether:
dateFormat(line 110) is the same as the removedDatetimeFormatPipe- The refactoring was intentionally incomplete
transactionDateshould also be updated to usedate: 'medium'Please verify the TypeScript file changes to confirm the imports and clarify the intended formatting approach for both date fields.
Also applies to: 125-125
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.
alberto-art3ch
left a comment
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.
There are already a dateFormat and dateTimeFormat pipes, obviously based in the date pipe, but those use the date and datetime format setting
But as you can see in the SearchJournalEntryComponent, the datetimeFormat pipe isn’t working correctly. I think the date pipe is much better than datetimeFormat — or did you mean I should fix datetimeFormat instead of using the date pipe? @alberto-art3ch
|

Description
use date pipeline to enhance the displaying of date.
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
✏️ Tip: You can customize this high-level summary in your review settings.