diff --git a/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.html b/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.html index 6c0cb5670..2e893ca41 100644 --- a/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.html +++ b/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.html @@ -187,6 +187,124 @@ }
+ + + +
+ badge + {{ 'labels.inputs.ID Proof Document' | translate }} +
+ + @if (existing.ID_PROOF.present) { +
+
+ +
+ Already available + from {{ existing.ID_PROOF.source }} + @if (existing.ID_PROOF.previewUnavailable) { + stored file unavailable + } +
+
+ @if (existing.ID_PROOF.previewUrl && isImage('ID_PROOF')) { + ID proof + } + @if (existing.ID_PROOF.previewUrl) { + + } +
+ + {{ existing.ID_PROOF.name }} + @if (existing.ID_PROOF.documentKey) { + {{ existing.ID_PROOF.documentKey }} + } + @if (existing.ID_PROOF.fileCount && existing.ID_PROOF.fileCount > 0) { + + {{ existing.ID_PROOF.fileCount }} + {{ existing.ID_PROOF.fileCount === 1 ? 'file' : 'files' }} + + } +
+ +
+ } + + @if (!existing.ID_PROOF.present || existing.ID_PROOF.expanded) { +

{{ 'labels.inputs.Accepted formats' | translate }}: PNG, JPEG, JPG, PDF

+
+ + + + + +
+ + @if (uploadErrors['ID_PROOF']) { +
+ error_outline + {{ uploadErrors['ID_PROOF'] }} +
+ } + @if (uploadResponses['ID_PROOF']) { +
+

+ {{ 'labels.inputs.Status' | translate }}: {{ uploadResponses['ID_PROOF'].status }} +

+

+ {{ 'labels.inputs.Request ID' | translate }}: {{ uploadResponses['ID_PROOF'].requestId }} +

+

+ {{ 'labels.inputs.Acknowledgement Number' | translate }}: + {{ uploadResponses['ID_PROOF'].acknowledgementNumber }} +

+

+ {{ 'labels.inputs.Processing Status' | translate }}: + {{ uploadResponses['ID_PROOF'].processingStatus }} +

+
+ } + } + + + @@ -202,8 +320,25 @@
Already available from {{ existing.ADDRESS_PROOF.source }} + @if (existing.ADDRESS_PROOF.previewUnavailable) { + stored file unavailable + }
+ @if (existing.ADDRESS_PROOF.previewUrl && isImage('ADDRESS_PROOF')) { + Address proof + } + @if (existing.ADDRESS_PROOF.previewUrl) { + + }
{{ existing.ADDRESS_PROOF.name }} @@ -308,8 +443,25 @@
Already available from {{ existing.PHOTO_PROOF.source }} + @if (existing.PHOTO_PROOF.previewUnavailable) { + stored file unavailable + }
+ @if (existing.PHOTO_PROOF.previewUrl && isImage('PHOTO_PROOF')) { + Photo proof + } + @if (existing.PHOTO_PROOF.previewUrl) { + + }
{{ existing.PHOTO_PROOF.name }} @@ -413,10 +565,22 @@
Already available from {{ existing.CLIENT_IMAGE.source }} + @if (existing.CLIENT_IMAGE.previewUnavailable) { + stored file unavailable + }
@if (existing.CLIENT_IMAGE.previewUrl) { Client photo + } + }
{{ existing.CLIENT_SIGNATURE.name }} diff --git a/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.scss b/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.scss index 66d527c48..51f57dc01 100644 --- a/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.scss +++ b/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.scss @@ -638,6 +638,35 @@ } } +/* Opens the uploaded artefact full size in a new tab. Sits directly under the thumbnail, and is + the only way to inspect a PDF - which cannot render as an inline . */ +.ckyc-existing__view { + align-self: flex-start; + color: #1d6fd0; + border-color: #bcd8f5; + background: #fff; + font-size: 13px; + font-weight: 600; + + &:hover { + background: #eef6ff; + } +} + +/* Shown when a document row exists but its file could not be fetched from the content store, + so "Already available" is not silently misleading. */ +.ckyc-existing__warn { + display: inline-block; + margin-left: 8px; + padding: 1px 8px; + border-radius: 10px; + background: #fdecec; + border: 1px solid #f3c2c2; + color: #a12b2b; + font-size: 11px; + font-weight: 600; +} + .ckyc-existing { display: flex; flex-direction: column; diff --git a/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.ts b/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.ts index adcd3d486..c5e72c1f0 100644 --- a/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.ts +++ b/src/app/clients/clients-view/ckyc-tab/ckyc-tab.component.ts @@ -9,7 +9,7 @@ /** Angular Imports */ import { Component, OnInit, OnDestroy, inject } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; -import { Subject, takeUntil, catchError, of, forkJoin } from 'rxjs'; +import { Subject, takeUntil, catchError, of } from 'rxjs'; /** Custom Services */ import { ClientsService } from '../../clients.service'; @@ -28,8 +28,60 @@ interface ExistingArtefact { documentKey?: string; fileCount?: number; expanded?: boolean; + /** Set when the stored file could not be fetched - the row exists but its content is gone. */ + previewUnavailable?: boolean; + /** Stored MIME type, so the card knows whether it can render an inline thumbnail. */ + mimeType?: string; } +/** + * Multipart `name` / `description` sent for each CKYC artefact type. + * + * The platform has no CKYC-specific document API; these are stored through the generic + * DocumentApiResource, whose contract carries no document-type field. The artefact type therefore + * travels in `name`, which is the same mechanism the client signature already uses - so + * `CLIENT_SIGNATURE` deliberately reuses the existing `clientSignature` value that + * loadExistingArtefacts() (and uploadClientSignatureImage()) match on. Changing that string would + * orphan every previously uploaded signature. + */ +export type CkycDocumentType = 'ID_PROOF' | 'ADDRESS_PROOF' | 'PHOTO_PROOF' | 'CLIENT_IMAGE' | 'CLIENT_SIGNATURE'; + +const CKYC_DOCUMENT_META: Record = { + ID_PROOF: { name: 'ckycIdProof', description: 'CKYC ID proof' }, + ADDRESS_PROOF: { name: 'ckycAddressProof', description: 'CKYC address proof' }, + PHOTO_PROOF: { name: 'ckycPhotoProof', description: 'CKYC photo proof' }, + CLIENT_IMAGE: { name: 'ckycClientImage', description: 'CKYC client photograph' }, + CLIENT_SIGNATURE: { name: 'clientSignature', description: 'Client signature' } +}; + +const CKYC_DOCUMENT_TYPES = Object.keys(CKYC_DOCUMENT_META) as CkycDocumentType[]; + +/** + * Client-side upload guards, kept deliberately in step with the backend so a file that passes here + * is not rejected server-side: + * - 5 MB matches `spring.servlet.multipart.max-file-size` + * - the extension list is the intersection of the backend `fineract.content.regex-whitelist` + * with what these cards can actually render. + * `accept=` on the input is only a file-picker hint and is trivially bypassed, so the real check + * lives in onFileSelect(). + */ +const MAX_UPLOAD_BYTES = 5 * 1024 * 1024; +const IMAGE_EXTENSIONS = [ + 'png', + 'jpeg', + 'jpg' +]; +const DOCUMENT_EXTENSIONS = [ + ...IMAGE_EXTENSIONS, + 'pdf' +]; + +/** Types whose card renders an inline preview, so a PDF would not display. */ +const IMAGE_ONLY_TYPES: CkycDocumentType[] = [ + 'CLIENT_IMAGE', + 'CLIENT_SIGNATURE' +]; + @Component({ selector: 'mifosx-ckyc-tab', templateUrl: './ckyc-tab.component.html', @@ -79,8 +131,9 @@ export class CkycTabComponent implements OnInit, OnDestroy { return this.panVerified && this.aadhaarVerified; } - /** Pre-existing artefacts detected from client data */ - existing: Record<'ADDRESS_PROOF' | 'PHOTO_PROOF' | 'CLIENT_IMAGE' | 'CLIENT_SIGNATURE', ExistingArtefact> = { + /** CKYC artefacts already uploaded for this client, keyed by artefact type. */ + existing: Record = { + ID_PROOF: { present: false }, ADDRESS_PROOF: { present: false }, PHOTO_PROOF: { present: false }, CLIENT_IMAGE: { present: false }, @@ -88,6 +141,7 @@ export class CkycTabComponent implements OnInit, OnDestroy { }; /** File references */ + idProofFile: File | null = null; addressProofFile: File | null = null; photoProofFile: File | null = null; clientImageFile: File | null = null; @@ -95,6 +149,7 @@ export class CkycTabComponent implements OnInit, OnDestroy { /** Per-document upload state */ uploading: { [key: string]: boolean } = { + ID_PROOF: false, ADDRESS_PROOF: false, PHOTO_PROOF: false, CLIENT_IMAGE: false, @@ -106,6 +161,7 @@ export class CkycTabComponent implements OnInit, OnDestroy { /** Per-document error messages */ uploadErrors: { [key: string]: string | null } = { + ID_PROOF: null, ADDRESS_PROOF: null, PHOTO_PROOF: null, CLIENT_IMAGE: null, @@ -207,98 +263,110 @@ export class CkycTabComponent implements OnInit, OnDestroy { } /** - * Fetch photo (profile image), signature (client document), - * ID proof and address proof (client identifiers) so already-uploaded - * artefacts are shown here instead of forcing re-upload. + * Loads every CKYC artefact from the client's documents (`m_document`), which is the single + * source of truth for this screen: each card matches the `name` its own upload writes, so what + * the checker sees is exactly what CKYC stored. + * + * Deliberately does NOT read the profile image or client identifiers any more. Those belong to + * client registration and the Identities section; reading them here showed artefacts this screen + * had never uploaded, and hid the ones it had. Those other screens are untouched. + * + * A client can accumulate several rows for one artefact (older uploads pre-date replacement, and + * the signature is also written by client registration), so the newest row - highest `id` - wins. */ private loadExistingArtefacts(): void { this.checkingExisting = true; - forkJoin({ - photo: this.clientsService.getClientProfileImage(this.clientId).pipe(catchError(() => of(null))), - documents: this.clientsService.getClientDocuments(this.clientId).pipe(catchError(() => of([]))), - identifiers: this.clientsService.getClientIdentifiers(this.clientId).pipe(catchError(() => of([]))), - proofTypes: this.clientsService.getKycProofTypes().pipe(catchError(() => of({ idProofs: [], addressProofs: [] }))) - }) - .pipe(takeUntil(this.destroy$)) - .subscribe(({ photo, documents, identifiers, proofTypes }) => { + this.clientsService + .getClientDocuments(this.clientId) + .pipe( + takeUntil(this.destroy$), + catchError(() => of([])) + ) + .subscribe((documents: any) => { this.checkingExisting = false; - // 1. Photo / image of client — from m_client.image_id - if (photo && typeof photo === 'string' && photo.length > 0) { - this.existing.CLIENT_IMAGE = { - present: true, - source: 'Client registration', - previewUrl: photo, - name: 'Profile photo' - }; - } + const list: any[] = Array.isArray(documents) ? documents : (documents?.pageItems ?? []); - // 2. Signature — client document with name='clientSignature'. Multiple rows can exist - // for clients who re-cropped/replaced their signature before uploads became single-slot, - // so pick the highest id (latest) rather than the first unordered match. - const signatureDoc = (documents as any[]) - ?.filter((d: any) => d?.name === 'clientSignature') - ?.sort((a: any, b: any) => b.id - a.id)?.[0]; - if (signatureDoc) { - this.existing.CLIENT_SIGNATURE = { - present: true, - source: 'Client registration', - name: signatureDoc.fileName || 'Client signature', - documentId: signatureDoc.id - }; - // Fetch blob for preview - this.clientsService - .getClientSignatureImage(this.clientId, signatureDoc.id) - .pipe(takeUntil(this.destroy$)) - .subscribe({ - next: (blob: any) => { - if (blob) { - this.existing.CLIENT_SIGNATURE.previewUrl = URL.createObjectURL(blob); - } - }, - error: () => undefined - }); - } + for (const type of CKYC_DOCUMENT_TYPES) { + const expectedName = CKYC_DOCUMENT_META[type].name; - // 3 & 4. ID proof + Address proof — client identifiers classified via proof-types - const idProofIds = new Set((proofTypes?.idProofs || []).map((p: any) => String(p.id))); - const addressProofIds = new Set((proofTypes?.addressProofs || []).map((p: any) => String(p.id))); + const doc = list + .filter((d: any) => d?.name === expectedName) + .sort((a: any, b: any) => (b?.id ?? 0) - (a?.id ?? 0))[0]; - const idList = Array.isArray(identifiers) ? identifiers : (identifiers as any)?.pageItems || []; - - for (const identity of idList) { - const docTypeId = String(identity?.documentType?.id ?? ''); - const isActive = - !identity?.status || - identity?.status?.code === 'clientIdentifierStatusType.active' || - identity?.status === 'clientIdentifierStatusType.active' || - identity?.status === 'Active'; - if (!isActive) continue; - - const proofName = identity?.documentType?.name || 'Identifier'; - // Address proof documentKey is auto-generated as " - " - // in identities-tab; hide it when it's just that synthetic key. - const rawKey = identity?.documentKey || ''; - const isSyntheticKey = rawKey.startsWith(`${proofName} - `); - const common = { - present: true, - source: 'Identities section', - name: proofName, - documentKey: isSyntheticKey ? '' : rawKey, - documentId: identity?.id, - fileCount: (identity?.documents || []).length - }; - - if (idProofIds.has(docTypeId) && !this.existing.PHOTO_PROOF.present) { - this.existing.PHOTO_PROOF = common; - } else if (addressProofIds.has(docTypeId) && !this.existing.ADDRESS_PROOF.present) { - this.existing.ADDRESS_PROOF = common; + if (!doc) { + // No upload yet for this artefact - leave the card in its empty state. + this.existing[type] = { present: false }; + continue; } + + this.existing[type] = { + present: true, + source: 'CKYC upload', + name: doc.fileName || expectedName, + documentId: doc.id, + mimeType: doc.type + }; + + this.loadPreview(type, doc.id); } }); } + /** + * Fetches the stored file so the card can show a thumbnail. A failure here is reported on the + * card rather than swallowed: the usual cause is a `m_document` row whose file is no longer in + * the content store, and silently showing "Already available" with no image hides that. + */ + private loadPreview(type: CkycDocumentType, documentId: string | number): void { + this.clientsService + // silent: a missing stored file returns 500, and the card below already reports that. Without + // this the global error interceptor pops an "Internal Server Error" modal on every page load + // and after every upload, for a failure the user cannot act on. + .downloadClientDocument(this.clientId, String(documentId), true) + .pipe(takeUntil(this.destroy$)) + .subscribe({ + next: (blob: any) => { + if (blob && blob.size > 0) { + this.existing[type].previewUrl = URL.createObjectURL(blob); + } else { + // A 200 with an empty body still means there is nothing to show. Without this the card + // would sit there claiming "Already available" with no thumbnail and no explanation. + this.existing[type].previewUnavailable = true; + } + }, + error: () => { + this.existing[type].previewUnavailable = true; + } + }); + } + + /** + * True when the stored file can be shown as an inline thumbnail. A PDF cannot, so its card falls + * back to the "View document" button alone rather than rendering a broken image. + */ + isImage(documentType: CkycDocumentType): boolean { + const entry = this.existing[documentType]; + if (entry?.mimeType) { + return entry.mimeType.startsWith('image/'); + } + // Older rows can have no stored MIME type - fall back to the file extension. + const extension = (entry?.name?.split('.').pop() || '').toLowerCase(); + return IMAGE_EXTENSIONS.includes(extension); + } + + /** + * Opens the uploaded document full size in a new tab. Uses the blob already fetched for the + * thumbnail, so it works for PDFs too and costs no extra request. + */ + openDocument(documentType: CkycDocumentType): void { + const url = this.existing[documentType]?.previewUrl; + if (url) { + window.open(url, '_blank', 'noopener'); + } + } + toggleReplace(documentType: string): void { const entry = (this.existing as any)[documentType]; if (entry) entry.expanded = !entry.expanded; @@ -309,11 +377,38 @@ export class CkycTabComponent implements OnInit, OnDestroy { this.selectedAccountType = this.selectedAccount?.accountType || ''; } - onFileSelect(event: any, documentType: string): void { - const file = event.target?.files?.[0]; - if (!file) return; + /** Extensions a given card can both store and render. */ + private allowedExtensions(documentType: string): string[] { + return IMAGE_ONLY_TYPES.includes(documentType as CkycDocumentType) ? IMAGE_EXTENSIONS : DOCUMENT_EXTENSIONS; + } + /** + * Rejects a file the backend would refuse (or the card could not render) before it is uploaded, + * so the user gets a clear message instead of a raw server error. Returns null when the file is + * acceptable. + */ + private validationError(file: File, documentType: string): string | null { + const allowed = this.allowedExtensions(documentType); + const extension = (file.name.split('.').pop() || '').toLowerCase(); + + if (!allowed.includes(extension)) { + return `Unsupported file type ".${extension}". Allowed: ${allowed.map((e) => e.toUpperCase()).join(', ')}.`; + } + if (file.size > MAX_UPLOAD_BYTES) { + const mb = (file.size / (1024 * 1024)).toFixed(1); + return `File is ${mb} MB. The maximum allowed size is 5 MB.`; + } + if (file.size === 0) { + return 'The selected file is empty.'; + } + return null; + } + + private setFile(documentType: string, file: File | null): void { switch (documentType) { + case 'ID_PROOF': + this.idProofFile = file; + break; case 'ADDRESS_PROOF': this.addressProofFile = file; break; @@ -327,11 +422,28 @@ export class CkycTabComponent implements OnInit, OnDestroy { this.clientSignatureFile = file; break; } + } + + onFileSelect(event: any, documentType: string): void { + const file = event.target?.files?.[0]; + if (!file) return; + + const error = this.validationError(file, documentType); + if (error) { + this.setFile(documentType, null); + this.uploadErrors[documentType] = error; + event.target.value = ''; + return; + } + + this.setFile(documentType, file); this.uploadErrors[documentType] = null; } getFile(documentType: string): File | null { switch (documentType) { + case 'ID_PROOF': + return this.idProofFile; case 'ADDRESS_PROOF': return this.addressProofFile; case 'PHOTO_PROOF': @@ -345,38 +457,64 @@ export class CkycTabComponent implements OnInit, OnDestroy { } } - uploadDocument(documentType: string): void { + uploadDocument(documentType: CkycDocumentType): void { const file = this.getFile(documentType); if (!file) return; this.uploading[documentType] = true; this.uploadErrors[documentType] = null; + // DocumentApiResource consumes multipart/form-data with body parts `file`, `name` and + // `description` (file + description mandatory). There is no document-type field in that + // contract, so the artefact type is carried in `name` - which is also the key + // loadExistingArtefacts() matches on when reading the document back. + const meta = CKYC_DOCUMENT_META[documentType]; + const formData = new FormData(); formData.append('file', file); - formData.append('documentType', documentType); + formData.append('name', meta.name); + formData.append('description', meta.description); - this.clientsService - .uploadCkycDocument(this.clientId, formData) - .pipe(takeUntil(this.destroy$)) - .subscribe({ - next: (response: any) => { - this.uploading[documentType] = false; - this.uploadResponses[documentType] = response; - this.alertService.alert({ - type: 'success', - message: response?.message || 'Document uploaded successfully.' - }); - }, - error: (err: any) => { - this.uploading[documentType] = false; - this.uploadErrors[documentType] = - err?.error?.developerMessage || - err?.error?.defaultUserMessage || - err?.message || - 'Failed to upload document.'; - } - }); + // Replace in place when this artefact already has a usable document, so a re-upload updates the + // existing row (same document id, same content-store location) instead of stacking up another + // copy. Falls back to create for the first upload of an artefact. + // + // Deliberately create instead of replace when the stored file is missing: the backend's update + // path writes the new content relative to the row's existing `location` and never calls + // setLocation(), so replacing a row whose file is already gone writes the upload to a fresh + // folder while the row keeps pointing at the missing one - the artefact would stay broken and + // the user would get a success message. Creating a new row repairs the card instead. + const entry = this.existing[documentType]; + const replacing = !!entry?.documentId && !entry?.previewUnavailable; + const existingId = entry?.documentId; + + const request$ = replacing + ? this.clientsService.replaceClientDocument(this.clientId, String(existingId), formData) + : this.clientsService.uploadCkycDocument(this.clientId, formData); + + request$.pipe(takeUntil(this.destroy$)).subscribe({ + next: (response: any) => { + this.uploading[documentType] = false; + this.uploadResponses[documentType] = response; + this.setFile(documentType, null); + this.alertService.alert({ + type: 'success', + message: + response?.message || (replacing ? 'Document replaced successfully.' : 'Document uploaded successfully.') + }); + // Re-read from m_document so the card reflects what was actually stored, rather than + // assuming the write succeeded exactly as sent. + this.loadExistingArtefacts(); + }, + error: (err: any) => { + this.uploading[documentType] = false; + this.uploadErrors[documentType] = + err?.error?.developerMessage || + err?.error?.defaultUserMessage || + err?.message || + 'Failed to upload document.'; + } + }); } ngOnDestroy(): void { diff --git a/src/app/clients/clients.service.ts b/src/app/clients/clients.service.ts index 7220fbe47..cd08181ce 100644 --- a/src/app/clients/clients.service.ts +++ b/src/app/clients/clients.service.ts @@ -569,8 +569,19 @@ export class ClientsService { return this.http.get(`/clients/${clientId}/documents`); } - downloadClientDocument(parentEntityId: string, documentId: string) { - return this.http.get(`/clients/${parentEntityId}/documents/${documentId}/attachment`, { responseType: 'blob' }); + /** + * Fetches a client document's stored file. + * + * `silent` opts the request out of ErrorHandlerInterceptor's alert. Pass it when the caller + * renders its own failure state - notably thumbnail fetches, where a `m_document` row whose file + * is no longer in the content store returns 500 and would otherwise pop a modal the user can do + * nothing about. The interceptor still rethrows, so the caller's error callback continues to run. + */ + downloadClientDocument(parentEntityId: string, documentId: string, silent = false) { + return this.http.get(`/clients/${parentEntityId}/documents/${documentId}/attachment`, { + responseType: 'blob', + ...(silent ? { context: new HttpContext().set(SILENT_HTTP_ERROR, true) } : {}) + }); } uploadClientDocument(clientId: string, documentData: any) { @@ -992,12 +1003,28 @@ export class ClientsService { return this.http.post(`/clients/${clientId}/entityclassification/${category}`, data); } - getClientCkyc(clientId: string): Observable { - return this.http.get(`/clients/${clientId}/ckyc`); + /** + * Uploads a single CKYC artefact against the client, using the platform's generic document + * endpoint (DocumentApiResource, POST /v1/{entityType}/{entityId}/documents). There is no + * CKYC-specific document API on the backend - the earlier `/clients/{id}/ckyc/documents` path + * was never implemented and returned 404. + * + * The multipart body must carry `file`, `name` and `description`; the caller builds those so the + * stored document stays identifiable by CKYC artefact type. See ckyc-tab.component.ts. + */ + uploadCkycDocument(clientId: string, formData: FormData): Observable { + return this.http.post(`/clients/${clientId}/documents`, formData); } - uploadCkycDocument(clientId: string, formData: FormData): Observable { - return this.http.post(`/clients/${clientId}/ckyc/documents`, formData); + /** + * Replaces the content of an existing client document in place (DocumentApiResource PUT + * /v1/{entityType}/{entityId}/documents/{documentId}), keeping the same document id. + * + * Used by the CKYC screen's "Upload new version" so a re-upload updates the existing artefact + * rather than adding another row. Takes the same multipart body as the create call. + */ + replaceClientDocument(clientId: string, documentId: string, formData: FormData): Observable { + return this.http.put(`/clients/${clientId}/documents/${documentId}`, formData); } /** diff --git a/src/app/tasks/checker-inbox-and-tasks-tabs/pending-savings-products/product-edit-diff-dialog/product-edit-diff-dialog.component.ts b/src/app/tasks/checker-inbox-and-tasks-tabs/pending-savings-products/product-edit-diff-dialog/product-edit-diff-dialog.component.ts index 43e1e7dea..07c51f9f8 100644 --- a/src/app/tasks/checker-inbox-and-tasks-tabs/pending-savings-products/product-edit-diff-dialog/product-edit-diff-dialog.component.ts +++ b/src/app/tasks/checker-inbox-and-tasks-tabs/pending-savings-products/product-edit-diff-dialog/product-edit-diff-dialog.component.ts @@ -18,6 +18,7 @@ import { import { MatButton } from '@angular/material/button'; import { MatIcon } from '@angular/material/icon'; import { STANDALONE_SHARED_IMPORTS } from 'app/standalone-shared.module'; +import { humanizeKey, sameValue, toIsoDate } from 'app/tasks/command-diff.util'; interface DiffRow { label: string; @@ -106,9 +107,9 @@ export class ProductEditDiffDialogComponent implements OnInit { this.rows = Object.keys(proposed) .filter((key) => !ProductEditDiffDialogComponent.IGNORED_KEYS.has(key)) .map((key) => ({ key, originalValue: this.originalValueFor(key, original) })) - .filter(({ key, originalValue }) => !this.sameValue(originalValue, proposed[key])) + .filter(({ key, originalValue }) => !sameValue(originalValue, proposed[key])) .map(({ key, originalValue }) => ({ - label: this.humanizeKey(key), + label: humanizeKey(key), original: this.displayValue(originalValue, key, original), proposed: this.displayValue(proposed[key], key, original) })) @@ -173,58 +174,11 @@ export class ProductEditDiffDialogComponent implements OnInit { * carries just the id, so a raw comparison reports every such field as changed. Comparing on the id * (and coercing numeric strings) keeps the list to fields whose value genuinely moved. */ - private sameValue(a: any, b: any): boolean { - return JSON.stringify(this.normalize(a)) === JSON.stringify(this.normalize(b)); - } - - private normalize(value: any): any { - if (value === null || value === undefined || value === '') { - return null; - } - // Dates come back from the product as [yyyy, m, d] but are submitted as "yyyy-MM-dd"; without - // folding both to one form every date field reads as changed on every edit. - const asDate = this.toIsoDate(value); - if (asDate !== null) { - return asDate; - } - if (Array.isArray(value)) { - // An empty array and an absent value mean the same thing here. - return value.length === 0 ? null : value.map((item) => this.normalize(item)); - } - if (typeof value === 'object') { - if (value.id !== undefined) { - return this.normalize(value.id); - } - return Object.keys(value).length === 0 ? null : JSON.stringify(value); - } - if (typeof value === 'boolean') { - return value; - } - const asNumber = Number(value); - return Number.isNaN(asNumber) ? value : asNumber; - } - - /** Recognises both date shapes the API uses and returns "yyyy-MM-dd", or null if not a date. */ - private toIsoDate(value: any): string | null { - if (Array.isArray(value) && value.length === 3 && value.every((part) => typeof part === 'number')) { - const [ - year, - month, - day - ] = value; - return `${year}-${String(month).padStart(2, '0')}-${String(day).padStart(2, '0')}`; - } - if (typeof value === 'string' && /^\d{4}-\d{2}-\d{2}$/.test(value)) { - return value; - } - return null; - } - private displayValue(value: any, fieldKey: string, template: any): string { if (value === null || value === undefined || value === '') { return '—'; } - const asDate = this.toIsoDate(value); + const asDate = toIsoDate(value); if (asDate !== null) { return asDate; } @@ -248,14 +202,4 @@ export class ProductEditDiffDialogComponent implements OnInit { // A bare id that names an option -- show the option's label, not the number. return this.resolveOptionLabel(fieldKey, value, template) ?? String(value); } - - /** camelCase / PascalCase field name to a spaced, capitalised label. */ - private humanizeKey(key: string): string { - const spaced = key - .replace(/([a-z0-9])([A-Z])/g, '$1 $2') - .replace(/([A-Z]+)([A-Z][a-z])/g, '$1 $2') - .replace(/[_-]+/g, ' ') - .trim(); - return spaced.charAt(0).toUpperCase() + spaced.slice(1); - } } diff --git a/src/app/tasks/command-diff.util.ts b/src/app/tasks/command-diff.util.ts new file mode 100644 index 000000000..f7a19f941 --- /dev/null +++ b/src/app/tasks/command-diff.util.ts @@ -0,0 +1,332 @@ +/** + * Copyright since 2025 Mifos Initiative + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +/** + * Shared comparison helpers for maker-checker review screens. + * + * Extracted verbatim from ProductEditDiffDialogComponent, which had already solved the awkward part: + * a naive comparison of a submitted command against the stored entity reports almost every field as + * changed, because the two sides encode the same value differently. The normalisation below is what + * keeps a diff down to the fields that genuinely moved: + * + * - dates arrive as `[yyyy, m, d]` from the API but as `"yyyy-MM-dd"` in the payload + * - option/enum fields come back as `{ id, value }` but are submitted as a bare id + * - `null`, `undefined`, `''`, `[]` and `{}` all mean "not set" + * - numeric strings and numbers are the same value + * + * Product-specific concerns (GL account mappings, `Options` lookups, currency aliases) stay in + * the product dialog - they have no meaning for a role or a client. + */ + +/** + * Keys that are never a setting a checker reviews, in two groups. + * + * The first is request plumbing - how the command was encoded, plus the identifiers naming the record + * it applies to, both of which the screen already shows. + * + * The second is values derived from another field rather than entered. An age is computed from + * `dateOfBirth` at the moment it is read, so a command submitted last month always disagrees with the + * entity read today - a difference caused by the calendar, not by the maker. The field the maker can + * actually change (`dateOfBirth`) is still compared, so a real edit is never hidden by this. + */ +export const COMMAND_NOISE_KEYS: ReadonlySet = new Set([ + // request plumbing + 'locale', + 'dateFormat', + 'monthDayFormat', + 'id', + 'productId', + 'savingsProductId', + // derived from dateOfBirth + 'ageYears', + 'ageMonths', + 'ageDays', + 'isMinor', + 'isSeniorCitizen' +]); + +const MONTH_NAMES = [ + 'january', + 'february', + 'march', + 'april', + 'may', + 'june', + 'july', + 'august', + 'september', + 'october', + 'november', + 'december' +]; + +/** + * Recognises every date shape these two sides use and returns "yyyy-MM-dd", or null if not a date. + * + * Three encodings are in play for the same day: + * - `[yyyy, m, d]` what the read APIs return + * - `"yyyy-MM-dd"` what some forms submit + * - `"29 June 2026"` what Fineract commands carry, per the payload's own `dateFormat` + * + * Missing the third one makes every dated field read as changed on every request, which is exactly + * the noise this whole comparison exists to remove. + */ +export function toIsoDate(value: any): string | null { + if (Array.isArray(value) && value.length === 3 && value.every((part) => typeof part === 'number')) { + const [ + year, + month, + day + ] = value; + return `${year}-${String(month).padStart(2, '0')}-${String(day).padStart(2, '0')}`; + } + if (typeof value !== 'string') { + return null; + } + if (/^\d{4}-\d{2}-\d{2}$/.test(value)) { + return value; + } + const spelled = /^(\d{1,2})\s+([A-Za-z]+)\s+(\d{4})$/.exec(value.trim()); + if (spelled) { + const day = Number(spelled[1]); + const monthName = spelled[2].toLowerCase(); + const monthIndex = MONTH_NAMES.findIndex((name) => name === monthName || name.slice(0, 3) === monthName); + if (monthIndex !== -1) { + return `${spelled[3]}-${String(monthIndex + 1).padStart(2, '0')}-${String(day).padStart(2, '0')}`; + } + } + return null; +} + +/** + * Folds a value into the one form both sides of a comparison can be measured in. See the class + * comment above for why each case exists. + */ +export function normalizeValue(value: any): any { + if (value === null || value === undefined || value === '') { + return null; + } + const asDate = toIsoDate(value); + if (asDate !== null) { + return asDate; + } + if (Array.isArray(value)) { + return value.length === 0 ? null : value.map((item) => normalizeValue(item)); + } + if (typeof value === 'object') { + if (value.id !== undefined) { + return normalizeValue(value.id); + } + return Object.keys(value).length === 0 ? null : JSON.stringify(value); + } + if (typeof value === 'boolean') { + return value; + } + const asNumber = Number(value); + return Number.isNaN(asNumber) ? value : asNumber; +} + +/** + * True when two values mean the same thing. + * + * The structural comparison handles most cases, but the two sides of a maker-checker review come + * from different APIs that expand the same option differently: `/audits/{id}` returns an enum as its + * label ("Daily") while the entity's own read API returns `{ id, code, value }`. Those normalise to + * different things yet are the same setting, so a second pass asks the question that actually + * matters to a checker - would these two render identically on screen? If they would, there is + * nothing to report. + */ +export function sameValue(a: any, b: any): boolean { + if (JSON.stringify(normalizeValue(a)) === JSON.stringify(normalizeValue(b))) { + return true; + } + return displayScalar(a) === displayScalar(b); +} + +/** camelCase / PascalCase / snake_case field name to a spaced, capitalised label. */ +export function humanizeKey(key: string): string { + const spaced = String(key ?? '') + .replace(/([a-z0-9])([A-Z])/g, '$1 $2') + .replace(/([A-Z]+)([A-Z][a-z])/g, '$1 $2') + .replace(/[_-]+/g, ' ') + .trim(); + return spaced.charAt(0).toUpperCase() + spaced.slice(1); +} + +/** + * Renders one value for display, without any entity-specific option lookup. Arrays and objects are + * summarised rather than dumped - a review row states *that* a collection changed; the detail belongs + * in the entity's own screen. + */ +export function displayScalar(value: any): string { + if (value === null || value === undefined || value === '') { + return '—'; + } + const asDate = toIsoDate(value); + if (asDate !== null) { + return asDate; + } + if (Array.isArray(value)) { + return value.length === 0 ? '—' : `${value.length} item${value.length === 1 ? '' : 's'}`; + } + if (typeof value === 'object') { + if (Object.keys(value).length === 0) { + return '—'; + } + if (value.name && value.glCode) { + return `${value.name} (${value.glCode})`; + } + return value.value ?? value.name ?? value.code ?? (value.id !== undefined ? String(value.id) : '—'); + } + if (typeof value === 'boolean') { + return value ? 'Yes' : 'No'; + } + return String(value); +} + +/** + * Sections a CREATE payload is bucketed into, so a checker reads a submitted product the way it was + * entered rather than as one flat run of fields. Matching is by key name, tried in order, so the + * grouping works for any entity - a role or a GL account simply lands everything in "Details". + */ +const CREATE_SECTIONS: { title: string; matches: RegExp }[] = [ + { title: 'Accounting', matches: /account(ing)?|glaccount|writeOff|receivable|payable|suspense|escheat|fund/i }, + { title: 'Charges & Fees', matches: /charge|fee|penalt|tax/i }, + { + title: 'Interest & Terms', + matches: /interest|term|period|lockin|compound|annualis|maturity|deposit(Amount|Min|Max)/i + }, + { title: 'Balances & Limits', matches: /balance|overdraft|limit|amount|minimum|maximum|multiplesOf|decimal/i }, + { title: 'Facilities & Features', matches: /facility|facilities|nominee|nomination|alert|banking|card|withdrawal/i } +]; + +const CREATE_DEFAULT_SECTION = 'General'; + +/** One named group of payload keys for a CREATE review. */ +export interface CreateSection { + title: string; + keys: string[]; +} + +/** + * Buckets a CREATE payload's keys into readable sections. Keys the sections do not recognise fall + * into "Details", which is also where everything lands for entities that have no product-like shape. + * Empty sections are dropped, and section order is stable so the same entity always reads the same. + */ +export function groupCreateFields(payload: Record, ignoredKeys?: ReadonlySet): CreateSection[] { + const ignored = ignoredKeys ?? COMMAND_NOISE_KEYS; + const buckets = new Map(); + buckets.set(CREATE_DEFAULT_SECTION, []); + CREATE_SECTIONS.forEach((section) => buckets.set(section.title, [])); + + Object.keys(payload ?? {}) + .filter((key) => !ignored.has(key)) + .forEach((key) => { + const section = CREATE_SECTIONS.find((candidate) => candidate.matches.test(key)); + buckets.get(section ? section.title : CREATE_DEFAULT_SECTION).push(key); + }); + + const ordered = [ + CREATE_DEFAULT_SECTION, + ...CREATE_SECTIONS.map((section) => section.title) + ]; + + return ordered + .map((title) => ({ title, keys: buckets.get(title) ?? [] })) + .filter((section) => section.keys.length > 0); +} + +/** + * Reads the current value for a payload key, allowing for the two naming conventions the read APIs + * use that the command payload does not: + * + * - lifecycle dates live under `timeline` (`timeline.submittedOnDate`) + * - a payload `Id` is usually the expanded `` object on the entity + * + * Returns `undefined` when the key cannot be located at all, which the caller reports honestly + * rather than claiming the field was newly added. + */ +export function currentValueFor(current: any, key: string): any { + if (current && key in current) { + return current[key]; + } + if (current?.timeline && key in current.timeline) { + return current.timeline[key]; + } + if (key.endsWith('Id')) { + const base = key.slice(0, -2); + if (current && base in current) { + return current[base]; + } + } + return undefined; +} + +/** + * How a single field moved between the stored entity and the submitted command. + * + * `SUBMITTED` is not a change: it marks a field the command carries but which could not be located + * on the entity, so no before-value exists to compare against. Calling those ADDED would tell the + * checker something untrue. + */ +export type ChangeKind = 'ADDED' | 'REMOVED' | 'MODIFIED' | 'SUBMITTED'; + +export interface CommandChange { + label: string; + kind: ChangeKind; + oldValue: string; + newValue: string; +} + +/** + * Compares a submitted command payload against the entity as it stands today, returning only the + * fields that actually moved. + * + * Only keys present in the payload are considered: a command says what the maker submitted, so a + * field absent from it was not part of the request and must not be reported as removed. + */ +export function diffCommandAgainstEntity( + submitted: Record, + current: Record, + options?: { ignoredKeys?: ReadonlySet; currentValueFor?: (key: string) => any } +): CommandChange[] { + const ignored = options?.ignoredKeys ?? COMMAND_NOISE_KEYS; + const readCurrent = options?.currentValueFor ?? ((key: string) => currentValueFor(current, key)); + const knownOnEntity = (key: string) => + !!current && (key in current || (!!current.timeline && key in current.timeline) || `${key}Id` in current); + + const rank: Record = { MODIFIED: 0, ADDED: 1, REMOVED: 2, SUBMITTED: 3 }; + + return ( + Object.keys(submitted ?? {}) + .filter((key) => !ignored.has(key)) + .map((key) => ({ key, before: readCurrent(key), after: submitted[key] })) + .filter(({ before, after }) => !sameValue(before, after)) + .map(({ key, before, after }) => { + const hadBefore = normalizeValue(before) !== null; + const hasAfter = normalizeValue(after) !== null; + // No before-value *and* no such field on the entity means there is nothing to compare, which is + // different from a field that exists and was empty. + const kind: ChangeKind = !hadBefore + ? knownOnEntity(key) || !!options?.currentValueFor + ? 'ADDED' + : 'SUBMITTED' + : !hasAfter + ? 'REMOVED' + : 'MODIFIED'; + return { + label: humanizeKey(key), + kind, + oldValue: displayScalar(before), + newValue: displayScalar(after) + }; + }) + // Real changes first, then the fields that could only be reported as submitted. + .sort((a, b) => rank[a.kind] - rank[b.kind] || a.label.localeCompare(b.label)) + ); +} diff --git a/src/app/tasks/maker-checker-entity-fetch.ts b/src/app/tasks/maker-checker-entity-fetch.ts new file mode 100644 index 000000000..d09f20563 --- /dev/null +++ b/src/app/tasks/maker-checker-entity-fetch.ts @@ -0,0 +1,205 @@ +/** + * Copyright since 2025 Mifos Initiative + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +import { Observable } from 'rxjs'; + +import { ProductsService } from 'app/products/products.service'; +import { ClientsService } from 'app/clients/clients.service'; +import { SavingsService } from 'app/savings/savings.service'; +import { FixedDepositsService } from 'app/deposits/fixed-deposits/fixed-deposits.service'; +import { RecurringDepositsService } from 'app/deposits/recurring-deposits/recurring-deposits.service'; +import { SystemService } from 'app/system/system.service'; +import { AccountingService } from 'app/accounting/accounting.service'; +import { UsersService } from 'app/users/users.service'; + +/** + * The services a maker-checker review screen needs in order to read an entity's current state. + * Passed in rather than injected here so this stays a plain lookup table with no Angular wiring. + */ +export interface EntityFetchServices { + products: ProductsService; + clients: ClientsService; + savings: SavingsService; + fixedDeposits: FixedDepositsService; + recurringDeposits: RecurringDepositsService; + system: SystemService; + accounting: AccountingService; + users: UsersService; +} + +/** + * Maps a maker-checker `entityName` to the existing API that returns that entity as it stands today, + * so an UPDATE can be shown as OLD -> NEW and a DELETE can show what is about to be removed. + * + * Every entry reuses an API the application already calls elsewhere - nothing new was added on the + * backend. Products are fetched with `template=true` because that response also carries the option + * lists (`accountingRuleOptions`, `interestCompoundingPeriodTypeOptions`, ...) the payload's bare ids + * refer to. + * + * An entity absent from this table simply has no current-state lookup; callers fall back to rendering + * the submitted payload, which is what every entity did before this table existed. + */ +const ENTITY_FETCHERS: Record Observable> = { + // ---- Products ---------------------------------------------------------------- + SAVINGSPRODUCT: (id, s) => s.products.getSavingProduct(id, true), + CURRENTPRODUCT: (id, s) => s.products.getCurrentProduct(id, true), + SHAREPRODUCT: (id, s) => s.products.getShareProduct(id, true), + LOANPRODUCT: (id, s) => s.products.getLoanProduct(id, true), + FIXEDDEPOSITPRODUCT: (id, s) => s.products.getFixedDepositProductAndTemplate(id), + RECURRINGDEPOSITPRODUCT: (id, s) => s.products.getRecurringDepositProduct(id, true), + CHARGE: (id, s) => s.products.getCharge(id, true), + + // ---- Accounts ---------------------------------------------------------------- + SAVINGSACCOUNT: (id, s) => s.savings.getSavingsAccountData(id), + // Deposit accounts must be read through their own endpoints: /savingsaccounts omits the deposit + // terms (depositAmount, depositPeriod, charts, ...), so comparing against it reports every one of + // those as newly added. + FIXEDDEPOSITACCOUNT: (id, s) => s.fixedDeposits.getFixedDepositsAccountData(id), + RECURRINGDEPOSITACCOUNT: (id, s) => s.recurringDeposits.getRecurringDepositsAccountAndTemplate(id), + + // ---- People and administration ---------------------------------------------- + CLIENT: (id, s) => s.clients.getClientData(id, true), + ROLE: (id, s) => s.system.getRole(id), + USER: (id, s) => s.users.getUser(id), + GLACCOUNT: (id, s) => s.accounting.getGlAccount(id, true) +}; + +/** Whether this screen can read the entity's current state, and therefore produce a real diff. */ +export function canFetchEntity(entityName: string | undefined | null): boolean { + return !!ENTITY_FETCHERS[(entityName ?? '').toUpperCase()]; +} + +/** + * Returns the entity as it stands today, or null when there is no mapping for it or no resource to + * read - in which case the caller keeps showing the submitted payload rather than failing. + */ +export function fetchCurrentEntity( + entityName: string | undefined | null, + resourceId: string | number | undefined | null, + services: EntityFetchServices +): Observable | null { + const fetcher = ENTITY_FETCHERS[(entityName ?? '').toUpperCase()]; + if (!fetcher || resourceId === null || resourceId === undefined || resourceId === '') { + return null; + } + return fetcher(String(resourceId), services); +} + +/** + * The handful of fields worth showing for a DELETE, whose payload is empty - the checker needs to + * recognise *which* record is about to go, not read its full configuration. Keys are tried in order + * and the first present one wins; anything not listed falls back to a generic set. + */ +const DELETE_SUMMARY_KEYS: Record = { + SAVINGSPRODUCT: [ + 'name', + 'shortName', + 'description', + 'currency' + ], + CURRENTPRODUCT: [ + 'name', + 'shortName', + 'description', + 'currency' + ], + SHAREPRODUCT: [ + 'name', + 'shortName', + 'description', + 'currency' + ], + LOANPRODUCT: [ + 'name', + 'shortName', + 'description', + 'currency' + ], + FIXEDDEPOSITPRODUCT: [ + 'name', + 'shortName', + 'description', + 'currency' + ], + RECURRINGDEPOSITPRODUCT: [ + 'name', + 'shortName', + 'description', + 'currency' + ], + CHARGE: [ + 'name', + 'currency', + 'amount', + 'chargeAppliesTo', + 'chargeTimeType' + ], + CLIENT: [ + 'displayName', + 'accountNo', + 'officeName', + 'status', + 'mobileNo', + 'emailAddress' + ], + SAVINGSACCOUNT: [ + 'accountNo', + 'clientName', + 'savingsProductName', + 'status', + 'currency' + ], + FIXEDDEPOSITACCOUNT: [ + 'accountNo', + 'clientName', + 'savingsProductName', + 'status', + 'currency' + ], + RECURRINGDEPOSITACCOUNT: [ + 'accountNo', + 'clientName', + 'savingsProductName', + 'status', + 'currency' + ], + ROLE: [ + 'name', + 'description', + 'disabled' + ], + USER: [ + 'username', + 'firstname', + 'lastname', + 'email', + 'officeName' + ], + GLACCOUNT: [ + 'name', + 'glCode', + 'type', + 'usage', + 'description' + ] +}; + +const GENERIC_SUMMARY_KEYS = [ + 'name', + 'displayName', + 'accountNo', + 'shortName', + 'username', + 'description', + 'status' +]; + +/** Field names worth surfacing for the entity about to be deleted. */ +export function deleteSummaryKeys(entityName: string | undefined | null): string[] { + return DELETE_SUMMARY_KEYS[(entityName ?? '').toUpperCase()] ?? GENERIC_SUMMARY_KEYS; +} diff --git a/src/app/tasks/view-checker-inbox/view-checker-inbox.component.html b/src/app/tasks/view-checker-inbox/view-checker-inbox.component.html index 7c382fb18..cf694305e 100644 --- a/src/app/tasks/view-checker-inbox/view-checker-inbox.component.html +++ b/src/app/tasks/view-checker-inbox/view-checker-inbox.component.html @@ -39,9 +39,6 @@ - @@ -74,7 +71,9 @@
-
{{ 'labels.inputs.Entity' | translate }}
+ +
{{ 'labels.inputs.Entity Type' | translate }}
{{ checkerInboxDetail.entityName }}
@if (checkerInboxDetail.resourceId) { @@ -108,9 +107,61 @@ - @if (displayJSONData) { + @if (comparing) { +
+ + Comparing against current values… +
+ } + + + @if (showChanges) {
-
{{ 'labels.heading.Command' | translate }}
+
Changes Requested ({{ changes.length }})
+ @if (changes.length === 0) { +

This request does not change any value.

+ } @else { +
+ @for (change of changes; track $index) { +
+
+ {{ change.label }} + {{ + change.kind + }} +
+
+ {{ change.oldValue }} + + {{ change.newValue }} +
+
+ } +
+ } +
+ } + + + @if (!showChanges && createSections.length > 0) { + @for (section of createSections; track section.title) { +
+
{{ section.title }}
+
+ @for (field of section.fields; track $index) { + + } +
+
+ } + } + + + @if (!showChanges && createSections.length === 0 && displayJSONData) { +
+
+ {{ deletingEntity ? 'Record To Be Deleted' : ('labels.heading.Command' | translate) }} +
@for (field of commandFields; track $index) { diff --git a/src/app/tasks/view-checker-inbox/view-checker-inbox.component.scss b/src/app/tasks/view-checker-inbox/view-checker-inbox.component.scss index e1d13e084..9bb961c1c 100644 --- a/src/app/tasks/view-checker-inbox/view-checker-inbox.component.scss +++ b/src/app/tasks/view-checker-inbox/view-checker-inbox.component.scss @@ -304,6 +304,97 @@ $bg-page: #f8fafc; color: $text-soft; } +/* ── UPDATE review: one row per field that actually moved ──────────────────── */ +.vci-changes { + display: flex; + flex-direction: column; + gap: 1px; + background: $border; +} + +.vci-change { + background: #fff; + padding: 11px 16px; + + &__head { + display: flex; + align-items: center; + gap: 10px; + margin-bottom: 5px; + } + + &__label { + font-size: 11px; + font-weight: 700; + text-transform: uppercase; + letter-spacing: 0.06em; + color: $text-soft; + } + + &__kind { + font-size: 9.5px; + font-weight: 700; + letter-spacing: 0.07em; + padding: 1px 7px; + border-radius: 2px; + + &--modified { + background: #e0f2fe; + color: #0369a1; + } + + &--added { + background: #dcfce7; + color: #15803d; + } + + &--removed { + background: #fee2e2; + color: #991b1b; + } + + /* Not a change: the entity has no such field, so there was no before-value to compare. */ + &--submitted { + background: #eef1f4; + color: #5b6673; + } + } + + &__values { + display: flex; + align-items: center; + gap: 10px; + flex-wrap: wrap; + font-size: 14px; + min-width: 0; + } + + &__old { + color: $text-soft; + text-decoration: line-through; + overflow-wrap: anywhere; + } + + &__arrow { + color: $text-soft; + font-size: 11px; + flex: none; + } + + &__new { + color: $text-dark; + font-weight: 600; + overflow-wrap: anywhere; + } +} + +.vci-nochange { + margin: 0; + padding: 16px; + font-size: 13.5px; + color: $text-soft; +} + .vci-tag { display: inline-flex; align-items: center; diff --git a/src/app/tasks/view-checker-inbox/view-checker-inbox.component.ts b/src/app/tasks/view-checker-inbox/view-checker-inbox.component.ts index cfc3cb336..c7ee498e1 100644 --- a/src/app/tasks/view-checker-inbox/view-checker-inbox.component.ts +++ b/src/app/tasks/view-checker-inbox/view-checker-inbox.component.ts @@ -14,6 +14,7 @@ import { MatDialog, MatDialogRef, MAT_DIALOG_DATA } from '@angular/material/dial /** Custom Services */ import { TasksService } from '../tasks.service'; +import { SystemService } from 'app/system/system.service'; /** Dialog Components */ import { ConfirmationDialogComponent } from 'app/shared/confirmation-dialog/confirmation-dialog.component'; @@ -23,6 +24,23 @@ import { NgTemplateOutlet } from '@angular/common'; import { DateFormatPipe } from '../../pipes/date-format.pipe'; import { STANDALONE_SHARED_IMPORTS } from 'app/standalone-shared.module'; import { isTempTableProductEdit } from '../product-entities'; +import { + CommandChange, + diffCommandAgainstEntity, + displayScalar, + groupCreateFields, + humanizeKey +} from '../command-diff.util'; +import { deleteSummaryKeys, EntityFetchServices, fetchCurrentEntity } from '../maker-checker-entity-fetch'; + +/** Services the entity lookup table needs; assembled once and handed to the shared fetcher. */ +import { ProductsService } from 'app/products/products.service'; +import { ClientsService } from 'app/clients/clients.service'; +import { SavingsService } from 'app/savings/savings.service'; +import { AccountingService } from 'app/accounting/accounting.service'; +import { UsersService } from 'app/users/users.service'; +import { FixedDepositsService } from 'app/deposits/fixed-deposits/fixed-deposits.service'; +import { RecurringDepositsService } from 'app/deposits/recurring-deposits/recurring-deposits.service'; /** * One row of the rendered Command payload. The dialog turns the parsed command JSON into a tree of @@ -35,6 +53,17 @@ export type CommandField = | { label: string; kind: 'object'; fields: CommandField[] } | { label: string; kind: 'list'; items: CommandField[] }; +/** + * Transport-level fields every Fineract command carries. They describe how the request was encoded, + * not what the maker actually changed, so they are dropped before the payload is shown - present in + * roughly three quarters of all stored commands, and pure noise to a checker. + */ +const NOISE_KEYS = new Set([ + 'locale', + 'dateFormat', + 'monthDayFormat' +]); + /** * Segments that read as initialisms in this domain, so `upiFacility` becomes "UPI Facility" rather * than "Upi Facility". Anything not listed just gets title-cased, so an unknown key still renders @@ -76,6 +105,24 @@ const LABEL_ACRONYMS = new Set([ 'nef' ]); +/** + * Fields that identify one element of an array, tried in order. Keeping this as a list of names rather + * than per-entity rules means an unfamiliar collection still gets a readable heading if it happens to + * carry any of them. + */ +const ENTRY_LABEL_KEYS = [ + 'limitType', + 'name', + 'displayName', + 'chargeName', + 'title', + 'label', + 'type', + 'description', + 'code', + 'value' +]; + @Component({ selector: 'mifosx-view-checker-inbox', templateUrl: './view-checker-inbox.component.html', @@ -93,6 +140,28 @@ export class ViewCheckerInboxComponent { private router = inject(Router); private translateService = inject(TranslateService); private tasksService = inject(TasksService); + private systemService = inject(SystemService); + private productsService = inject(ProductsService); + private clientsService = inject(ClientsService); + private savingsService = inject(SavingsService); + private accountingService = inject(AccountingService); + private usersService = inject(UsersService); + private fixedDepositsService = inject(FixedDepositsService); + private recurringDepositsService = inject(RecurringDepositsService); + + /** Bundle handed to the shared entity lookup, so that table stays free of Angular injection. */ + private get entityFetchServices(): EntityFetchServices { + return { + products: this.productsService, + clients: this.clientsService, + savings: this.savingsService, + system: this.systemService, + accounting: this.accountingService, + users: this.usersService, + fixedDeposits: this.fixedDepositsService, + recurringDeposits: this.recurringDepositsService + }; + } /** Present only when opened as a popup from the Checker Inbox list; absent when reached by direct * route (a bookmarked or shared link). Both entry points stay supported — this only changes where * the detail comes from and how "done" is signalled: closing the dialog vs. navigating back. */ @@ -122,6 +191,17 @@ export class ViewCheckerInboxComponent { */ commandFields: CommandField[] = []; + /** UPDATE review: the fields this command actually moves, as OLD -> NEW. */ + changes: CommandChange[] = []; + /** True once the current entity was read and a real comparison is on screen. */ + showChanges = false; + /** True while the current entity is being fetched for comparison. */ + comparing = false; + /** True when the screen is describing a record that is about to be deleted. */ + deletingEntity = false; + /** CREATE review: submitted data bucketed into readable sections. */ + createSections: { title: string; fields: CommandField[] }[] = []; + /** camelCase / snake_case key -> readable label, e.g. `upiFacility` -> "UPI Facility". */ private labelFor(key: string): string { const spaced = String(key ?? '') @@ -156,6 +236,27 @@ export class ViewCheckerInboxComponent { * Recursively turns one key/value pair into a render node. Nothing is dropped or summarised - every * key in the payload produces a node, so the checker still sees the complete request. */ + /** + * Names one element of an array of objects. + * + * Numbering them "Account Facility Limits 1 ... 10" tells a checker nothing - the position in a JSON + * array is not a fact about the record. Most entries carry a field that identifies them (`limitType`, + * a charge's `name`, a chart's `description`), so that value is used as the heading and the ordinal + * is only a fallback for entries that have nothing identifying at all. + */ + private entryLabel(item: Record, listLabel: string, index: number): string { + for (const key of ENTRY_LABEL_KEYS) { + const candidate = item?.[key]; + if (typeof candidate === 'string' && candidate.trim() !== '') { + return candidate.trim(); + } + if (typeof candidate === 'number') { + return String(candidate); + } + } + return `${listLabel} ${index + 1}`; + } + private toField(label: string, value: any): CommandField { if (Array.isArray(value)) { return { @@ -163,7 +264,7 @@ export class ViewCheckerInboxComponent { kind: 'list', items: value.map((item, index) => item !== null && typeof item === 'object' - ? this.toField(`${label} ${index + 1}`, item) + ? this.toField(this.entryLabel(item, label, index), item) : { label: '', kind: 'scalar' as const, text: this.scalarText(item) } ) }; @@ -172,7 +273,9 @@ export class ViewCheckerInboxComponent { return { label, kind: 'object', - fields: Object.keys(value).map((key) => this.toField(this.labelFor(key), value[key])) + fields: Object.keys(value) + .filter((key) => !NOISE_KEYS.has(key)) + .map((key) => this.toField(this.labelFor(key), value[key])) }; } return { label, kind: 'scalar', text: this.scalarText(value) }; @@ -182,7 +285,9 @@ export class ViewCheckerInboxComponent { if (!payload || typeof payload !== 'object' || Array.isArray(payload)) { return []; } - return Object.keys(payload).map((key) => this.toField(this.labelFor(key), payload[key])); + return Object.keys(payload) + .filter((key) => !NOISE_KEYS.has(key)) + .map((key) => this.toField(this.labelFor(key), payload[key])); } /** Checker Inbox Details Data */ @@ -238,6 +343,14 @@ export class ViewCheckerInboxComponent { private applyDetail(detail: any): void { this.checkerInboxDetail = detail; + // Angular reuses this component when only the route's id changes, so the previous request's review + // state has to be cleared explicitly — otherwise a DELETE opened after an UPDATE keeps showing the + // UPDATE's changes. + this.changes = []; + this.showChanges = false; + this.comparing = false; + this.deletingEntity = false; + this.createSections = []; // commandAsJson is absent on some entries (a DELETE carries no payload), and JSON.parse(undefined) // throws — which as a route only broke the page, but in a dialog would leave an empty shell open. try { @@ -247,6 +360,186 @@ export class ViewCheckerInboxComponent { } this.displayJSONData = !_.isEmpty(this.jsondata); this.commandFields = this.buildCommandFields(this.jsondata); + + // Role permissions get their own treatment: the payload is the role's whole permission map, so a + // field-by-field diff would be 1,100 rows rather than the handful that moved. + if (this.isRolePermissionsCommand(detail)) { + this.loadRolePermissionChanges(detail.resourceId, this.jsondata.permissions); + return; + } + + const action = (detail?.actionName ?? '').toUpperCase(); + if (action === 'UPDATE') { + this.loadUpdateChanges(detail); + } else if (action === 'DELETE') { + this.loadDeleteSummary(detail); + } else if (action === 'CREATE') { + this.createSections = this.buildCreateSections(this.jsondata); + } + } + + /** + * UPDATE: read the entity as it stands today and show only what the command actually moves. + * + * Several screens submit their entire form regardless of what the maker touched - a fixed deposit + * account update posts 28 fields every time - so without this the checker cannot tell which one + * changed. If the entity cannot be read the submitted payload stays on screen, which is what every + * entity showed before. + */ + private loadUpdateChanges(detail: any): void { + const request$ = fetchCurrentEntity(detail?.entityName, detail?.resourceId, this.entityFetchServices); + if (!request$) { + return; + } + this.comparing = true; + request$.subscribe({ + next: (current: any) => { + // A slow response for a record the user has already navigated away from must not overwrite the + // record now on screen. + if (!this.isCurrent(detail)) { + return; + } + this.comparing = false; + this.changes = diffCommandAgainstEntity(this.jsondata ?? {}, current ?? {}); + this.showChanges = true; + }, + error: () => { + this.comparing = false; + } + }); + } + + /** + * DELETE: the payload is empty for every entity, so the command alone tells the checker nothing. + * Reads the record instead and shows the few fields that identify it. + */ + private loadDeleteSummary(detail: any): void { + const request$ = fetchCurrentEntity(detail?.entityName, detail?.resourceId, this.entityFetchServices); + if (!request$) { + return; + } + this.comparing = true; + request$.subscribe({ + next: (current: any) => { + if (!this.isCurrent(detail)) { + return; + } + this.comparing = false; + const keys = deleteSummaryKeys(detail?.entityName); + const fields = keys + .filter((key) => current?.[key] !== undefined && current?.[key] !== null && current?.[key] !== '') + .map((key) => ({ + label: humanizeKey(key), + kind: 'scalar' as const, + text: displayScalar(current[key]) + })); + if (fields.length > 0) { + this.commandFields = fields; + this.displayJSONData = true; + this.deletingEntity = true; + } + }, + error: () => { + this.comparing = false; + } + }); + } + + /** Whether `detail` is still the request on screen, i.e. an in-flight lookup is not stale. */ + private isCurrent(detail: any): boolean { + return this.checkerInboxDetail?.id === detail?.id; + } + + /** CREATE: nothing to compare against, so the submitted data is grouped rather than diffed. */ + private buildCreateSections(payload: any): { title: string; fields: CommandField[] }[] { + if (!payload || typeof payload !== 'object' || Array.isArray(payload)) { + return []; + } + return groupCreateFields(payload).map((section) => ({ + title: section.title, + fields: section.keys.map((key) => this.toField(this.labelFor(key), payload[key])) + })); + } + + /** + * A role permission update posts the role's *entire* permission map - over 1,100 entries, of which + * a handful are actually being changed. Rendering that as-is buries the change completely, so this + * screen fetches the role's current permissions and shows only the difference. + */ + private isRolePermissionsCommand(detail: any): boolean { + return ( + (detail?.entityName ?? '').toUpperCase() === 'ROLE' && + (detail?.actionName ?? '').toUpperCase() === 'PERMISSIONS' && + detail?.resourceId != null && + !!this.jsondata?.permissions + ); + } + + /** + * Replaces the flat permission dump with Granted / Revoked lists, by comparing the submitted map + * against what the role holds today. Anything the maker left untouched is summarised as a count + * rather than listed. + * + * If the role lookup fails the original payload stays on screen - a degraded but complete view is + * better than showing the checker nothing. + */ + private loadRolePermissionChanges(roleId: string | number, submitted: Record): void { + this.systemService.getRole(roleId).subscribe({ + next: (role: any) => { + const current = new Map( + (role?.permissionUsageData ?? []).map((p: any): [string, boolean] => [ + p.code, + !!p.selected + ]) + ); + + const granted: string[] = []; + const revoked: string[] = []; + let unchanged = 0; + + Object.keys(submitted).forEach((code) => { + const now = !!submitted[code]; + const before = current.get(code); + // A code missing from the role lookup has no "before" to compare against, so treat it as + // changed only when it is being switched on. + if (before === undefined) { + if (now) { + granted.push(code); + } + return; + } + if (now === before) { + unchanged++; + } else if (now) { + granted.push(code); + } else { + revoked.push(code); + } + }); + + this.commandFields = this.buildPermissionChangeFields(granted, revoked, unchanged); + }, + error: () => undefined + }); + } + + private buildPermissionChangeFields(granted: string[], revoked: string[], unchanged: number): CommandField[] { + const fields: CommandField[] = []; + const asList = (label: string, codes: string[]): CommandField => ({ + label, + kind: 'list', + items: codes.sort().map((code) => ({ label: '', kind: 'scalar' as const, text: this.labelFor(code) })) + }); + + fields.push(asList(`Permissions Granted (${granted.length})`, granted)); + fields.push(asList(`Permissions Revoked (${revoked.length})`, revoked)); + + if (granted.length === 0 && revoked.length === 0) { + fields.push({ label: 'Result', kind: 'scalar', text: 'No permission changes in this request.' }); + } + fields.push({ label: 'Unchanged Permissions', kind: 'scalar', text: `${unchanged} left as they were` }); + + return fields; } /** "Done" for this screen — closes the popup (telling the list to refresh) when opened as a @@ -304,23 +597,4 @@ export class ViewCheckerInboxComponent { } }); } - - /** - * Delete Checker - */ - deleteChecker() { - const deleteCheckerDialogRef = this.dialog.open(ConfirmationDialogComponent, { - data: { - heading: this.translateService.instant('labels.heading.Delete Checker'), - dialogContext: this.translateService.instant('labels.dialogContext.Are you sure you want to delete checker') - } - }); - deleteCheckerDialogRef.afterClosed().subscribe((response: { confirm: any }) => { - if (response.confirm) { - this.tasksService.deleteMakerChecker(this.checkerInboxDetail.id).subscribe((res: any) => { - this.finish(); - }); - } - }); - } }