fix(products): show only genuinely-changed fields in the product-edit diff
Three separate bugs in the checker's "Proposed changes" dialog, found while adding the Renewal Grace Period field above: - The "current" side resolved an enum field's value against the wrong payload key (preClosurePenalInterestOnTypeId, minDepositTermTypeId, etc. are exposed on the stored product as an object under a differently-named field, e.g. preClosurePenalInterestOnType.id) so it always read as blank -- these fields showed as changed on every single edit, never just the ones a maker actually touched. - Enum/option fields resolved their label against a naive `<field>Options` key that doesn't match Fineract's actual (inconsistent) naming, so a genuinely changed field showed its raw numeric id instead of the option a maker picked. - Renewal Grace Period Days and other newer fields never appeared at all, because the maker-checker pending-edit mirror table didn't carry them (see the backend commit for that). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+44
-12
@@ -69,7 +69,14 @@ export class ProductEditDiffDialogComponent implements OnInit {
|
||||
currencyCode: 'currency.code',
|
||||
digitsAfterDecimal: 'currency.decimalPlaces',
|
||||
inMultiplesOf: 'currency.inMultiplesOf',
|
||||
taxGroupId: 'taxGroup.id'
|
||||
taxGroupId: 'taxGroup.id',
|
||||
// Enum fields the stored product exposes as "<name>Type": {id, value}", not "<name>TypeId" --
|
||||
// without these the "current" side always read as blank, so these showed as changed on every
|
||||
// edit even when the maker never touched them.
|
||||
preClosurePenalInterestOnTypeId: 'preClosurePenalInterestOnType.id',
|
||||
minDepositTermTypeId: 'minDepositTermType.id',
|
||||
maxDepositTermTypeId: 'maxDepositTermType.id',
|
||||
inMultiplesOfDepositTermTypeId: 'inMultiplesOfDepositTermType.id'
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -78,6 +85,17 @@ export class ProductEditDiffDialogComponent implements OnInit {
|
||||
* `accountingMappingOptions` rather than a per-field `<field>Options` key, so both sides of the
|
||||
* diff need this table to turn an account id into the account's name.
|
||||
*/
|
||||
/**
|
||||
* Fields whose option list isn't named after the field at all -- min/max/in-multiples-of deposit
|
||||
* term type all resolve against one shared frequency list (Days/Weeks/Months/Years), not a list of
|
||||
* their own, so the `<field>Options` convention below never finds them.
|
||||
*/
|
||||
private static readonly OPTIONS_KEY_ALIASES: Record<string, string> = {
|
||||
minDepositTermTypeId: 'periodFrequencyTypeOptions',
|
||||
maxDepositTermTypeId: 'periodFrequencyTypeOptions',
|
||||
inMultiplesOfDepositTermTypeId: 'periodFrequencyTypeOptions'
|
||||
};
|
||||
|
||||
private static readonly GL_FIELD_TO_MAPPING: Record<string, string> = {
|
||||
savingsReferenceAccountId: 'savingsReferenceAccount',
|
||||
savingsControlAccountId: 'savingsControlAccount',
|
||||
@@ -149,23 +167,37 @@ export class ProductEditDiffDialogComponent implements OnInit {
|
||||
|
||||
/**
|
||||
* Enum/option fields arrive from the edit as a bare id, which reads as a meaningless number in the
|
||||
* table. The product fetched with template=true carries the matching option list under a predictable
|
||||
* `<field>Options` key (accountingRule -> accountingRuleOptions), so the id is resolved back to the
|
||||
* label the maker actually picked. Returns null when there is no such list to resolve against.
|
||||
* table. The product fetched with template=true usually carries the matching option list under a
|
||||
* predictable `<field>Options` key (accountingRule -> accountingRuleOptions), so the id is resolved
|
||||
* back to the label the maker actually picked. Two other shapes this also has to cover:
|
||||
* - a field ending in "...TypeId" whose list drops the "Id" (preClosurePenalInterestOnTypeId ->
|
||||
* preClosurePenalInterestOnTypeOptions), and
|
||||
* - a field resolving against a shared list that isn't named after the field at all
|
||||
* (OPTIONS_KEY_ALIASES).
|
||||
* All three are tried in turn; returns null only when none of them find a match, so the id is
|
||||
* shown as-is rather than silently wrong.
|
||||
*/
|
||||
private resolveOptionLabel(fieldKey: string, id: any, template: any): string | null {
|
||||
if (ProductEditDiffDialogComponent.GL_FIELD_TO_MAPPING[fieldKey]) {
|
||||
return this.glAccountNamesById.get(String(id)) ?? null;
|
||||
}
|
||||
const options = template?.[`${fieldKey}Options`];
|
||||
if (!Array.isArray(options)) {
|
||||
return null;
|
||||
const candidateKeys = [
|
||||
ProductEditDiffDialogComponent.OPTIONS_KEY_ALIASES[fieldKey],
|
||||
`${fieldKey}Options`,
|
||||
fieldKey.endsWith('Id') ? `${fieldKey.slice(0, -2)}Options` : null
|
||||
].filter((key): key is string => !!key);
|
||||
|
||||
for (const optionsKey of candidateKeys) {
|
||||
const options = template?.[optionsKey];
|
||||
if (!Array.isArray(options)) {
|
||||
continue;
|
||||
}
|
||||
const match = options.find((option: any) => String(option?.id) === String(id));
|
||||
if (match) {
|
||||
return match.value ?? match.name ?? match.code ?? null;
|
||||
}
|
||||
}
|
||||
const match = options.find((option: any) => String(option?.id) === String(id));
|
||||
if (!match) {
|
||||
return null;
|
||||
}
|
||||
return match.value ?? match.name ?? match.code ?? null;
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user