refactor: do not mix reactive and static programming paradigmas in configurator-attribute-price-change.service.ts

feature-libs/product-configurator/rulebased/components/attribute/price-change/configurator-attribute-price-change.service.spec.ts

@@ -68,15 +68,13 @@ describe('ConfiguratorAttributePriceChangeService', () => {
     expect(classUnderTest).toBeTruthy();
   });
 
-  describe('getPriceChangedEvents', () => {
+  describe('getChangedPrices', () => {
     it('should emit always true for the initial rendering/call', () => {
       let emitCounter = 0;
-      classUnderTest
-        .getPriceChangedEvents(undefined)
-        .subscribe((priceChanged) => {
-          expect(priceChanged).toBe(true);
-          emitCounter++;
-        });
+      classUnderTest.getChangedPrices(undefined).subscribe((priceChanged) => {
+        expect(priceChanged).toEqual({});
+        emitCounter++;
+      });
       mockConfig.priceSupplements = undefined;
       configSubject.next(mockConfig);
       expect(emitCounter).toBe(1);
@@ -86,16 +84,15 @@ describe('ConfiguratorAttributePriceChangeService', () => {
     it('should detect price changes during initial rendering/call if supplements are present', () => {
       let emitCounter = 0;
       classUnderTest
-        .getPriceChangedEvents('group1@attribute_1_1')
+        .getChangedPrices('group1@attribute_1_1')
         .subscribe((priceChanged) => {
-          expect(priceChanged).toBe(true);
+          expect(priceChanged['value_1_1']).toBeDefined();
+          expect(priceChanged['value_1_2']).toBeDefined();
+          expect(priceChanged['value_1_3']).toBeDefined();
           emitCounter++;
         });
       configSubject.next(mockConfig);
       expect(emitCounter).toBe(1);
-      expect(classUnderTest['valuePrices']['value_1_1']).toBeDefined();
-      expect(classUnderTest['valuePrices']['value_1_2']).toBeDefined();
-      expect(classUnderTest['valuePrices']['value_1_3']).toBeDefined();
     });
 
     describe('after initial rendering/call', () => {
@@ -108,7 +105,7 @@ describe('ConfiguratorAttributePriceChangeService', () => {
 
       it('should not emit, if config has no price supplements', () => {
         classUnderTest
-          .getPriceChangedEvents(undefined)
+          .getChangedPrices(undefined)
           .pipe(skip(1))
           .subscribe(() => {
             fail('priceChanged observable should not emit!');
@@ -120,7 +117,7 @@ describe('ConfiguratorAttributePriceChangeService', () => {
 
       it('should not emit, if config has no matching price supplements', () => {
         classUnderTest
-          .getPriceChangedEvents('otherAttrKey')
+          .getChangedPrices('otherAttrKey')
           .pipe(skip(1))
           .subscribe(() => {
             fail('priceChanged observable should not emit!');
@@ -132,25 +129,24 @@ describe('ConfiguratorAttributePriceChangeService', () => {
       it('should emit true and store matching value prices, if config has matching price supplements', () => {
         let emitCounter = 0;
         classUnderTest
-          .getPriceChangedEvents('group1@attribute_1_1')
+          .getChangedPrices('group1@attribute_1_1')
           .pipe(skip(1))
           .subscribe((priceChanged) => {
-            expect(priceChanged).toBe(true);
+            expect(priceChanged['value_1_1']).toBeDefined();
+            expect(priceChanged['value_1_2']).toBeDefined();
+            expect(priceChanged['value_1_3']).toBeDefined();
             emitCounter++;
           });
         simulateFirstCall();
         configSubject.next(mockConfig);
         expect(emitCounter).toBe(1);
-        expect(classUnderTest['valuePrices']['value_1_1']).toBeDefined();
-        expect(classUnderTest['valuePrices']['value_1_2']).toBeDefined();
-        expect(classUnderTest['valuePrices']['value_1_3']).toBeDefined();
       });
 
       it('should not emit again if prices are not changed', () => {
         classUnderTest['lastAttributeSupplement'] =
           mockConfig.priceSupplements?.[0];
         classUnderTest
-          .getPriceChangedEvents('group1@attribute_1_1')
+          .getChangedPrices('group1@attribute_1_1')
           .pipe(skip(1))
           .subscribe(() => {
             fail('priceChanged observable should not emit!');
@@ -160,35 +156,4 @@ describe('ConfiguratorAttributePriceChangeService', () => {
       });
     });
   });
-
-  describe('mergePriceIntoValue', () => {
-    it('should create a new object combining value and price if onPriceChanged was called for this value before', () => {
-      const valuePrice = { value: 100, currencyIso: 'USD' };
-      classUnderTest['storeValuePrice']('valueKey', valuePrice);
-      expect(
-        classUnderTest['mergePriceIntoValue']({
-          valueCode: '1223',
-          name: 'valueKey',
-        })
-      ).toEqual({
-        valueCode: '1223',
-        name: 'valueKey',
-        valuePrice: valuePrice,
-      });
-    });
-
-    it('should return just the value if onPriceChanged was NOT called for this value before', () => {
-      const valuePrice = { value: 100, currencyIso: 'USD' };
-      classUnderTest['storeValuePrice']('anotherValueKey', valuePrice);
-      expect(
-        classUnderTest['mergePriceIntoValue']({
-          valueCode: '1223',
-          name: 'valueKey',
-        })
-      ).toEqual({
-        valueCode: '1223',
-        name: 'valueKey',
-      });
-    });
-  });
 });

feature-libs/product-configurator/rulebased/components/attribute/price-change/configurator-attribute-price-change.service.ts

@@ -20,12 +20,12 @@ import { ConfiguratorCommonsService } from '../../../core/facade/configurator-co
 import { Configurator } from '../../../core/model/configurator.model';
 
 /**
- * Stateful service to react on price changes of the configuration. Hence components using this service must provide it within their declaration:
- *
+ * Stateful service to react on price changes of the configuration.
+ * Hence components using this service must provide it within their declaration, for instance:
  * @Component({ providers: [ConfiguratorAttributePriceChangeService] })
  *
- * getPriceChangedEvents will return an observable that emits whenever the price of a monitored attribute (identified by the provided attribute key) changes,
- * hence allowing the subscriber for example to trigger rerendering of the prices on the UI.
+ * getChangedPrices will return an observable that emits whenever the price of a monitored attribute (identified by the provided attribute key) changes.
+ * Hence allowing the subscriber to trigger rerendering of the prices on the UI.
  */
 @Injectable()
 export class ConfiguratorAttributePriceChangeService {
@@ -36,22 +36,20 @@ export class ConfiguratorAttributePriceChangeService {
   protected lastAttributeSupplement:
     | Configurator.AttributeSupplement
     | undefined;
-  protected valuePrices: { [valueName: string]: Configurator.PriceDetails } =
-    {};
 
   /**
-   * Returns an observable that shall be used by all components supporting delta rendering mode.
-   * It will monitor the price supplements of configuration observable and emit true if price supplements
-   * matching the given attribute key have changed.
-   * Additionally it returns always true for the first emission of the underlying configuration observable.
-   * This ensures that an enclosing UI component will initially render, even if the async pricing request ist still running,
+   * Retrieves an observable that emits whenever the price of a monitored attribute (identified by the provided attribute key) changes,
+   * hence allowing the subscriber to trigger rerendering of the prices on the UI.
+   * This ensures that an enclosing UI component will initially render, even if the async pricing request is still running,
    * so that the UI is not blocked. Afterwards a rerender shall only occur if prices change.
    * This all assumes that the enclosing UI component itself gets recreated or rerendered (triggered elsewhere) whenever the attribute itself changes content wise.
    *
    * @param attributeKey key of the attribute for which the prices should be checked for changes
-   * @returns observable that emits 'true' each time a price changes and hence there is the need to rerender the enclosing component
+   * @returns observable that emits price of monitored attribute changes and hence there is the need to rerender the enclosing component
    */
-  getPriceChangedEvents(attributeKey: string | undefined): Observable<boolean> {
+  getChangedPrices(
+    attributeKey: string | undefined
+  ): Observable<Record<string, Configurator.PriceDetails>> {
     let isInitialConfiguration: boolean = true;
     return this.configuratorRouterExtractorService.extractRouterData().pipe(
       switchMap((routerData) => {
@@ -65,13 +63,13 @@ export class ConfiguratorAttributePriceChangeService {
             ),
             switchMap((config) => {
               if (!config.priceSupplements) {
-                return of(true);
+                return of({});
               }
-              const pricesChanged = this.checkForValuePriceChanges(
+              const changedPrices = this.checkForValuePriceChanges(
                 config,
                 attributeKey
               );
-              return pricesChanged ? of(true) : EMPTY;
+              return changedPrices ? of(changedPrices) : EMPTY;
             }),
             tap(() => (isInitialConfiguration = false))
           );
@@ -80,34 +78,19 @@ export class ConfiguratorAttributePriceChangeService {
     );
   }
 
-  /**
-   * Merges the stored value price data into the given value, if available.
-   * As the value might be read-only a new object will be returned combining price and value.
-   *
-   * @param value the value
-   * @returns the new value with price
-   */
-  mergePriceIntoValue(value: Configurator.Value): Configurator.Value {
-    const valueName = value.name;
-    if (valueName && this.valuePrices[valueName]) {
-      value = { ...value, valuePrice: this.valuePrices[valueName] };
-    }
-    return value;
-  }
-
   /**
    * Extracts the relevant value prices from the price supplements
-   * and stores them within the component. Returns a boolean indicating
-   * whether there were any value price changes.
+   * and stores them within the component.
+   * Returns price of a monitored attribute changes.
    *
-   * @param config current config
+   * @param config current configuration
    * @param attributeKey key of the attribute for which the prices should be checked for changes
-   * @returns {true}, only if at least one value price changed
+   * @returns observable that emits price of a monitored attribute changes
    */
   protected checkForValuePriceChanges(
     config: Configurator.Configuration,
     attributeKey: string | undefined
-  ): boolean {
+  ): Record<string, Configurator.PriceDetails> | undefined {
     const attributeSupplement = config.priceSupplements?.find(
       (supplement) => supplement.attributeUiKey === attributeKey
     );
@@ -116,21 +99,15 @@ export class ConfiguratorAttributePriceChangeService {
       attributeSupplement ?? {}
     );
     if (changed) {
+      const changedPrices: Record<string, Configurator.PriceDetails> = {};
       this.lastAttributeSupplement = attributeSupplement;
-      attributeSupplement?.valueSupplements.forEach((valueSupplement) =>
-        this.storeValuePrice(
-          valueSupplement.attributeValueKey,
-          valueSupplement.priceValue
-        )
+      attributeSupplement?.valueSupplements.forEach(
+        (valueSupplement) =>
+          (changedPrices[valueSupplement.attributeValueKey] =
+            valueSupplement.priceValue)
       );
+      return changedPrices;
     }
-    return changed;
-  }
-
-  protected storeValuePrice(
-    valueName: string,
-    valuePrice: Configurator.PriceDetails
-  ) {
-    this.valuePrices[valueName] = valuePrice;
+    return undefined;
   }
 }