WEB-638 fix: improve core utility logic and migrate from deprecated … - #3062
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
String method modernization (substr → substring) src/app/core/utils/commons.ts, src/app/system/manage-data-tables/column-dialog/column-dialog.component.ts, src/app/system/manage-data-tables/edit-data-table/edit-data-table.component.ts |
Replaces substr() with substring() for extracting substrings; behavior preserved for typical inputs. |
Bitwise → logical OR for password defaults src/app/core/utils/passwords-utility.ts, src/app/shared/change-password-dialog/change-password-dialog.component.ts |
Replaces bitwise OR (` |
Date format/parse adjustments src/app/core/utils/dates.ts |
Transforms incoming format tokens (y→Y, d→D, a→A) and uses moment(value, adjustedFormat) for parsing and moment(date).format(adjustedFormat) for formatting. |
External service masking range tweak src/app/system/external-services/amazon-s3/amazon-s3.component.ts, src/app/system/external-services/notification/notification.component.ts |
Switches masking from substr to substring and adjusts the masked range endpoints (now using substring(1, value.length - 2)), changing which middle characters are replaced with *. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
- WEB-562:fix(auth) align password validation with backend regex requir… #2990 — Modifies
src/app/core/utils/passwords-utility.tsand addressesPASSWORD_REGEX/minPasswordLengthdefaulting logic.
Suggested reviewers
- gkbishnoi07
- 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 accurately reflects the main changes: fixing core utility logic and migrating from deprecated substr method across multiple utility files. |
| 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 docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/app/core/utils/dates.ts`:
- Around line 47-48: The convertToDate method currently passes Angular
DatePipe-style formats (e.g., SettingsService.businessDateFormat = 'yyyy-MM-dd')
directly to Moment, which uses different tokens; update convertToDate to first
normalize common Angular DatePipe tokens to Moment equivalents (e.g., yyyy→YYYY,
MM→MM, dd→DD), then call moment(value, normalizedFormat, true) to enable strict
parsing, and if strict parsing fails fall back to a safe alternative (e.g., try
ISO parsing via Date or a non-format moment parse) before returning a Date;
reference convertToDate and SettingsService.businessDateFormat when locating
where to add the normalization, strict flag, and fallback logic.
5c0122c to
6bcf417
Compare
…substr
This PR addresses several underlying logic errors and technical debt in the core utility functions and system components to improve application stability and security.
WEB-638
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
✏️ Tip: You can customize this high-level summary in your review settings.