refactor: eliminate legacy jQuery DOM manipulation with Angular-native form state#95
refactor: eliminate legacy jQuery DOM manipulation with Angular-native form state#95Anurag-elitx wants to merge 7 commits into
Conversation
Updating Master Branch
Merging develop to master
develop to master
develop to master
📝 WalkthroughWalkthroughThis PR removes jQuery dependencies across multiple Angular service components and replaces jQuery-triggered DOM form resets with Angular's native NgForm.resetForm() method. The co-feedback-services component also updates its form template binding pattern. HttpService language initialization is made explicit, and a new environment configuration file is introduced for backend endpoints and security settings. ChangesjQuery to Angular Form Migration
Service and Configuration Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/environments/environment.ts (3)
29-30: ⚡ Quick winAdd missing semicolons for consistency.
Lines 29-30 are missing trailing semicolons. While not a functional issue, adding them maintains consistency with the rest of the file.
📝 Proposed fix
-const siteKey = '' -const captchaChallengeURL = ''; +const siteKey = ''; +const captchaChallengeURL = '';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/environments/environment.ts` around lines 29 - 30, The two constant declarations siteKey and captchaChallengeURL are missing trailing semicolons; update the declarations for siteKey and captchaChallengeURL in environment.ts to add the missing semicolons so they match project style and the rest of the file's formatting.
33-44: Consider environment file management strategy.Since this is described as a "fallback environment file," consider these practices:
- Ensure this file is in
.gitignoreif it will contain developer-specific values- Consider creating an
environment.example.tstemplate that developers copy locally- Add runtime guards in the application to prevent accidental production deployment with development configuration
- Document the setup process for new developers in the README
This would prevent accidentally committing sensitive configuration and make onboarding clearer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/environments/environment.ts` around lines 33 - 44, The fallback environment.ts currently holds developer-specific config (see the exported environment object and keys like production, encKey, commonAPI, ip1097, enableCaptcha) which risks accidental commit; fix by (1) adding this file to .gitignore and creating an environment.example.ts template with placeholder values for encKey/commonAPI/siteKey/captchaChallengeURL, (2) updating docs/README with explicit setup steps to copy the example to environment.ts and populate secrets, and (3) adding a runtime guard in app bootstrap (check environment.production and required keys like encKey/commonAPI) to fail-fast or warn loudly if a non-production build uses missing/placeholder values to prevent accidental deployment.
41-42: 💤 Low valueFix spacing inconsistency around colons.
Lines 41-42 are missing spaces after colons (
siteKey:siteKeyinstead ofsiteKey: siteKey), which is inconsistent with the formatting on other lines.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/environments/environment.ts` around lines 41 - 42, The object properties siteKey and captchaChallengeURL in environment.ts have inconsistent spacing ("siteKey:siteKey" and "captchaChallengeURL:captchaChallengeURL"); update the object literal to use consistent spacing after the colon (e.g., "siteKey: siteKey" and "captchaChallengeURL: captchaChallengeURL") to match the rest of the file's formatting.src/app/services/http-services/http_services.service.ts (1)
47-48: ⚡ Quick winRename misspelled observable properties for consistency.
The variable names contain typos:
appCurrentLanguge(line 47) andcurrentLangugae$(line 48) should beappCurrentLanguageandcurrentLanguage$respectively. These properties are referenced in at least 3 component files, so renaming will require updates across the codebase. While optional, consistent naming improves code clarity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/services/http-services/http_services.service.ts` around lines 47 - 48, Rename the misspelled observable properties: change the BehaviorSubject property appCurrentLanguge to appCurrentLanguage and its observable currentLangugae$ to currentLanguage$ in the HttpServicesService (look for appCurrentLanguge and currentLangugae$ in the file), update all imports/usages across components/services that reference these symbols to the new names, and run/adjust any unit tests or type hints that reference the old identifiers to ensure consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/environments/environment.ts`:
- Around line 25-28: The environment config has mixed endpoints: commonAPI,
adminAPI, and API1097 point to localhost while telephoneServer points to the UAT
host; verify intent and make them consistent for the target environment. If this
should be a local/dev config, change telephoneServer to the local dev URL (or a
configurable dev variable); if it should target UAT, replace the other endpoints
with the UAT host or use environment-based variables. Update the variables
commonAPI, adminAPI, telephoneServer, and API1097 to come from a single
environment selection (or config file) so all endpoints align with the same
environment.
- Line 24: The sessionStorageEncKey constant is empty which allows accidental
insecure builds; replace the empty string in sessionStorageEncKey with a
non-secret placeholder value (e.g., "dev-placeholder-do-not-use-in-prod") and
add a clear inline comment next to sessionStorageEncKey warning that this is
only for local/dev use and must be overridden by a secure value from
environment/config in production (or load from process.env or a secure secrets
provider and validate non-empty at runtime).
---
Nitpick comments:
In `@src/app/services/http-services/http_services.service.ts`:
- Around line 47-48: Rename the misspelled observable properties: change the
BehaviorSubject property appCurrentLanguge to appCurrentLanguage and its
observable currentLangugae$ to currentLanguage$ in the HttpServicesService (look
for appCurrentLanguge and currentLangugae$ in the file), update all
imports/usages across components/services that reference these symbols to the
new names, and run/adjust any unit tests or type hints that reference the old
identifiers to ensure consistency.
In `@src/environments/environment.ts`:
- Around line 29-30: The two constant declarations siteKey and
captchaChallengeURL are missing trailing semicolons; update the declarations for
siteKey and captchaChallengeURL in environment.ts to add the missing semicolons
so they match project style and the rest of the file's formatting.
- Around line 33-44: The fallback environment.ts currently holds
developer-specific config (see the exported environment object and keys like
production, encKey, commonAPI, ip1097, enableCaptcha) which risks accidental
commit; fix by (1) adding this file to .gitignore and creating an
environment.example.ts template with placeholder values for
encKey/commonAPI/siteKey/captchaChallengeURL, (2) updating docs/README with
explicit setup steps to copy the example to environment.ts and populate secrets,
and (3) adding a runtime guard in app bootstrap (check environment.production
and required keys like encKey/commonAPI) to fail-fast or warn loudly if a
non-production build uses missing/placeholder values to prevent accidental
deployment.
- Around line 41-42: The object properties siteKey and captchaChallengeURL in
environment.ts have inconsistent spacing ("siteKey:siteKey" and
"captchaChallengeURL:captchaChallengeURL"); update the object literal to use
consistent spacing after the colon (e.g., "siteKey: siteKey" and
"captchaChallengeURL: captchaChallengeURL") to match the rest of the file's
formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 37471cb7-06be-4874-a930-98257940c568
📒 Files selected for processing (9)
src/app/beneficiary-registration/beneficiary-registration.component.tssrc/app/co-counselling-services/co-counselling-services.component.tssrc/app/co-feedback-services/co-feedback-services.component.htmlsrc/app/co-feedback-services/co-feedback-services.component.tssrc/app/co-information-services/co-information-services.component.tssrc/app/co-referral-services/co-referral-services.component.tssrc/app/co-services/co-services.component.tssrc/app/services/http-services/http_services.service.tssrc/environments/environment.ts
💤 Files with no reviewable changes (2)
- src/app/beneficiary-registration/beneficiary-registration.component.ts
- src/app/co-services/co-services.component.ts
| */ | ||
|
|
||
|
|
||
| const sessionStorageEncKey = ''; |
There was a problem hiding this comment.
Empty encryption key poses security risk.
The sessionStorageEncKey is set to an empty string, which would fail to provide any encryption if this configuration is accidentally used. Even for a fallback/development file, consider using a placeholder value with a clear comment warning developers not to use it in production.
🔒 Suggested approach
-const sessionStorageEncKey = '';
+// WARNING: Development-only placeholder key. DO NOT use in production!
+const sessionStorageEncKey = 'DEV_ONLY_PLACEHOLDER_KEY_DO_NOT_USE_IN_PROD';📝 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.
| const sessionStorageEncKey = ''; | |
| // WARNING: Development-only placeholder key. DO NOT use in production! | |
| const sessionStorageEncKey = 'DEV_ONLY_PLACEHOLDER_KEY_DO_NOT_USE_IN_PROD'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/environments/environment.ts` at line 24, The sessionStorageEncKey
constant is empty which allows accidental insecure builds; replace the empty
string in sessionStorageEncKey with a non-secret placeholder value (e.g.,
"dev-placeholder-do-not-use-in-prod") and add a clear inline comment next to
sessionStorageEncKey warning that this is only for local/dev use and must be
overridden by a secure value from environment/config in production (or load from
process.env or a secure secrets provider and validate non-empty at runtime).
| const commonAPI = 'http://localhost:8083/'; | ||
| const adminAPI = 'http://localhost:8082/'; | ||
| const telephoneServer = 'http://uatcz.piramalswasthya.org/'; | ||
| const API1097 = 'http://localhost:8090/'; |
There was a problem hiding this comment.
Inconsistent environment configuration detected.
Most API endpoints point to localhost (development), but telephoneServer points to an external UAT server (uatcz.piramalswasthya.org). This mixed configuration could cause unexpected behavior where some requests go to local services while others hit UAT. Verify this is intentional; if not, align all endpoints to the same environment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/environments/environment.ts` around lines 25 - 28, The environment config
has mixed endpoints: commonAPI, adminAPI, and API1097 point to localhost while
telephoneServer points to the UAT host; verify intent and make them consistent
for the target environment. If this should be a local/dev config, change
telephoneServer to the local dev URL (or a configurable dev variable); if it
should target UAT, replace the other endpoints with the UAT host or use
environment-based variables. Update the variables commonAPI, adminAPI,
telephoneServer, and API1097 to come from a single environment selection (or
config file) so all endpoints align with the same environment.
📋 Description
JIRA ID: #128
Summary of Change
This PR modernizes the
co-servicesdomain by replacing legacy, imperative jQuery DOM manipulation with standard, declarative Angular-native alternatives (using template reference variables,@ViewChild, and typedNgFormcontrols). It also resolves a property initialization compiler error inHttpServicesand establishes a default environment configuration to restore compiler stability.Motivation and Scope
In preparation for the migration from legacy Angular 4.4.4 to modern Angular 19 + Zard UI, we must eliminate all imperative DOM-touching jQuery dependencies.
jQuery('#feedbackForm').trigger("reset")and other tab-resets across the service components with nativeNgForm.resetForm()state lifecycle methods.form="ngForm") to compliant template-driven reference bindings (#feedbackForm="ngForm").jQuery("#md-tab-label-0-0").addClass(...)) fromCoServicesComponent.HttpServices(language: any = null;) to avoid TypeScript circular self-reference warnings.src/environments/environment.tspreset configuration to fix loader crashes.✅ Type of Change
ℹ️ Additional Information
Testing Details
npm run buildlocally to ensure the full codebase parses successfully with no syntax errors.Files Refactored
src/app/co-services/co-services.component.tssrc/app/co-referral-services/co-referral-services.component.tssrc/app/co-feedback-services/co-feedback-services.component.tssrc/app/co-feedback-services/co-feedback-services.component.htmlsrc/app/co-information-services/co-information-services.component.tssrc/app/co-counselling-services/co-counselling-services.component.tssrc/app/beneficiary-registration/beneficiary-registration.component.tssrc/app/services/http-services/http_services.service.tssrc/environments/environment.ts(created)Summary by CodeRabbit
Refactor
Chores