IMN-657 GET EService descriptor by ID #712

Viktor-K · 2024-07-08T09:33:58Z

Part of BFF Catalog Routes
Closes IMN-657

Description

Implements route /producers/eservices/:eserviceId/descriptors/:descriptorId

TEST

✅ HTTP Status 200 - BFF returns expected descriptor

✅ HTTP Status 404 - Descriptor not Found

✅ HTTP Status - 403 Invalid Eservice Requester

packages/bff/src/services/catalogService.ts

packages/bff/src/model/api/converters/catalogClientApiConverter.ts

ecamellini

Left some minor comments

ecamellini · 2024-07-17T09:07:43Z

packages/bff/src/services/catalogService.ts

+      const descriptorAttributes = {
+        certified: [
+          toBffCatalogApiDescriptorAttribute(
+            attributes,
+            descriptor.attributes.certified.flat()
+          ),
+        ],
+        declared: [
+          toBffCatalogApiDescriptorAttribute(
+            attributes,
+            descriptor.attributes.declared.flat()
+          ),
+        ],
+        verified: [
+          toBffCatalogApiDescriptorAttribute(
+            attributes,
+            descriptor.attributes.verified.flat()
+          ),
+        ],
+      };


This could also become a function toBffCatalogApiDescriptorAttributes(descriptorAttributes)

done in this commit

ecamellini · 2024-07-17T09:13:37Z

packages/bff/src/model/api/converters/catalogClientApiConverter.ts

+  producer: tenantApi.Tenant
+): bffApi.ProducerDescriptorEService {
+  const producerMail = producer.mails.find(
+    (m) => m.kind === tenantMailKind.ContactEmail


I know the values are the same and that TS probably helps with this, but here you are comparing enums of different types: tenantApi.Tenant and our internal TenantMailKind. To be safer I would refactor as follows:

Suggested change

(m) => m.kind === tenantMailKind.ContactEmail

(m) => m.kind === tenantApi.MailKind.Values.CONTACT_EMAIL

Furthermore this commit move it to separate file

ecamellini · 2024-07-17T09:20:14Z

packages/bff/src/services/catalogService.ts

+  headers: Headers,
+  descriptorAttributeIds: string[]
+) => {
+  // Fetched all attributes in a recursive way


Suggested change

// Fetched all attributes in a recursive way

// Fetch all attributes in a recursive way

ecamellini · 2024-07-17T09:25:55Z

packages/bff/src/model/domain/errors.ts

+  descriptorNotFound: "0006",
+  attributeNotExists: "0008",
+  invalidEserviceRequester: "0009",


Doubt: in the BFF do we have to use the same codes used in Scala, being exposed directly to the FE, or can we do like in the processes and redefine our codes from scratch just incrementing whenever we add a new error?
cc @galales

As discussed offline with @Carminepo2 and @beetlecrunch, this code could be different from the Scala codebase

Perfect, so let's do the increment by one since we don't need to adhere to Scala codes:

Suggested change

descriptorNotFound: "0006",

attributeNotExists: "0008",

invalidEserviceRequester: "0009",

descriptorNotFound: "0004",

attributeNotExists: "0005",

invalidEserviceRequester: "0006",

ecamellini · 2024-07-17T09:36:29Z

packages/bff/src/model/api/converters/catalogClientApiConverter.ts

+export function toBffCatalogApiEserviceRiskAnalysis(
+  riskAnalysis: catalogApi.EServiceRiskAnalysis
+): bffApi.EServiceRiskAnalysis {
+  const answers: { [key: string]: string[] } =


Could you reuse type from bffApi.RiskAnalysisForm?

Suggested change

const answers: { [key: string]: string[] } =

const answers: bffApi.RiskAnalysisForm["answers"] =

packages/bff/src/model/api/converters/catalogClientApiConverter.ts

AsterITA · 2024-07-18T12:24:26Z

packages/bff/src/services/catalogService.ts

@@ -129,6 +124,36 @@ const getBulkAttributes = async (
  return await attributesBulk(0, []);
 };

+const getEserviceDesciptor = (


Suggested change

const getEserviceDesciptor = (

const retrieveEserviceDescriptor = (

We can align this name with the other retrieves like this and we can fix the typo

Done with this commit

packages/bff/src/services/catalogService.ts

Carminepo2 · 2024-07-18T13:45:09Z

packages/bff/src/model/api/converters/catalogClientApiConverter.ts

+  const isUpgradable = (agreement: agreementApi.Agreement): boolean => {
+    const eserviceDescriptor = eservice.descriptors.find(
+      (e) => e.id === agreement.descriptorId
+    );
+
+    return (
+      eserviceDescriptor !== undefined &&
+      eservice.descriptors
+        .filter((d) => Number(d.version) > Number(eserviceDescriptor.version))
+        .find(
+          (d) =>
+            (d.state === descriptorApiState.PUBLISHED ||
+              d.state === descriptorApiState.SUSPENDED) &&
+            (agreement.state === agreementApiState.ACTIVE ||
+              agreement.state === agreementApiState.SUSPENDED)
+        ) !== undefined
+    );
+  };


For reference: an almost the same function is present here in another PR

@Carminepo2 in this commit have introduced a new file mappers.ts, my intent is to move here the commons function used to pick, transform or evaluate data.
Also in this next PR I'm using this approach, but in this case probably I prefer to place this method in mapper.

this commit move this logic in a specific file

Carminepo2 · 2024-07-18T13:54:26Z

packages/bff/src/model/api/converters/catalogClientApiConverter.ts

+export function toEserviceAttribute(
+  attributes: catalogApi.Attribute[]
+): EServiceAttribute[] {
+  return attributes.map((attribute) => ({
+    ...attribute,
+    id: unsafeBrandId(attribute.id),
+  }));
+}
+
+export function toDescriptorWithOnlyAttributes(
+  descriptor: catalogApi.EServiceDescriptor
+): DescriptorWithOnlyAttributes {
+  return {
+    ...descriptor,
+    attributes: {
+      certified: descriptor.attributes.certified.map(toEserviceAttribute),
+      declared: descriptor.attributes.declared.map(toEserviceAttribute),
+      verified: descriptor.attributes.verified.map(toEserviceAttribute),
+    },
+  };
+}


We are using runtime resources to change something at type level, I know the performance impact could be negligible but I saw this in a lot of part of the monorepo and I'm wondering if it is worth discussing.
What do you think?

@Carminepo2 good point 👍🏻
unfortunately branded type usages requires mapping like this, so we need to choose what we want encourage. Id as string are very error prone when they are function's parameter, branded type is a sort of guard to avoid bugs, meanwhile every time we convert models it's necessary this wrapping...
I suggest to discuss a possible policy offline.

ecamellini

Works for me! Just left some minor comments but consider it approved on my side :)

packages/bff/src/model/api/converters/catalogClientApiConverter.ts

ecamellini · 2024-07-23T13:13:09Z

packages/bff/src/model/domain/errors.ts

+  descriptorNotFound: "0006",
+  attributeNotExists: "0008",
+  invalidEserviceRequester: "0009",


Perfect, so let's do the increment by one since we don't need to adhere to Scala codes:

Suggested change

descriptorNotFound: "0006",

attributeNotExists: "0008",

invalidEserviceRequester: "0009",

descriptorNotFound: "0004",

attributeNotExists: "0005",

invalidEserviceRequester: "0006",

packages/bff/src/model/mappers.ts

packages/bff/src/model/validators.ts

packages/bff/src/model/modelMappingUtils.ts

Co-authored-by: Eric Camellini <eric.camellini@gmail.com>

packages/bff/src/routers/catalogRouter.ts

Viktor-K changed the base branch from main to IMN-604-catalog-router July 8, 2024 09:34

Base automatically changed from IMN-604-catalog-router to main July 9, 2024 10:03

Viktor-K force-pushed the IMN-657-getEserviceDescriptorById branch from 356946c to 2c04084 Compare July 9, 2024 12:56

Viktor-K added 3 commits July 10, 2024 14:59

Implements get Eservice's descriptor by id

a02dc84

Fix assertion on GetEserivcesDescriptorById

b6d1ce8

Fix types from API clients package

af35559

Viktor-K force-pushed the IMN-657-getEserviceDescriptorById branch 3 times, most recently from ec02c5e to 62f2849 Compare July 10, 2024 15:07

remove wrong changes

56a0277

Viktor-K force-pushed the IMN-657-getEserviceDescriptorById branch from 62f2849 to 56a0277 Compare July 10, 2024 15:10

Viktor-K added 2 commits July 10, 2024 17:22

Fix linting

b1ac408

Merge branch 'main' into IMN-657-getEserviceDescriptorById

8f82ffe

Viktor-K marked this pull request as ready for review July 10, 2024 15:35

Viktor-K requested review from beetlecrunch, galales, AsterITA, Carminepo2 and taglioni-r as code owners July 10, 2024 15:35

Viktor-K requested review from MalpenZibo and ecamellini July 10, 2024 15:35

Viktor-K added 2 commits July 10, 2024 17:47

Move converter to properly ApiConverter file

87effa8

Fix linting

6b080a5

Viktor-K requested a review from sandrotaje July 11, 2024 09:38

AsterITA reviewed Jul 16, 2024

View reviewed changes

packages/bff/src/services/catalogService.ts Outdated Show resolved Hide resolved

AsterITA reviewed Jul 16, 2024

View reviewed changes

packages/bff/src/model/api/converters/catalogClientApiConverter.ts Outdated Show resolved Hide resolved

AsterITA requested changes Jul 16, 2024

View reviewed changes

packages/bff/src/model/api/converters/catalogClientApiConverter.ts Outdated Show resolved Hide resolved

packages/bff/src/model/api/converters/catalogClientApiConverter.ts Outdated Show resolved Hide resolved

Viktor-K mentioned this pull request Jul 16, 2024

IMN-656 get producer eservice detail by ID #730

Merged

Viktor-K added 2 commits July 16, 2024 17:12

Create getter help methods in catalog service

107daaa

Rename converter catalog API converter method

17be40e

Merge branch 'main' into IMN-657-getEserviceDescriptorById

9cf1835

ecamellini approved these changes Jul 17, 2024

View reviewed changes

AsterITA reviewed Jul 18, 2024

View reviewed changes

packages/bff/src/model/api/converters/catalogClientApiConverter.ts Outdated Show resolved Hide resolved

AsterITA reviewed Jul 18, 2024

View reviewed changes

packages/bff/src/services/catalogService.ts Show resolved Hide resolved

Carminepo2 reviewed Jul 18, 2024

View reviewed changes

Viktor-K added 2 commits July 22, 2024 12:17

Refactorize validators and mapper on catalogService

7dc73a2

move agreement upgredeable veirification in mapper file

846effd

Viktor-K requested a review from Carminepo2 July 22, 2024 13:26

Viktor-K added 2 commits July 22, 2024 15:39

Move getTenant email to mappers

98a7bad

Add proper type during risk anlysis converter

748d8c6

Viktor-K requested review from AsterITA and ecamellini July 22, 2024 15:56

Viktor-K added 3 commits July 23, 2024 10:22

Merge branch 'main' into IMN-657-getEserviceDescriptorById

9fe126b

fix linting

9e9d46e

Rename BFF validator

3d974f2

ecamellini approved these changes Jul 23, 2024

View reviewed changes

Minor Fix : renaming function and vars

00e0562

ecamellini reviewed Jul 24, 2024

View reviewed changes

packages/bff/src/model/modelMappingUtils.ts Outdated Show resolved Hide resolved

Viktor-K and others added 5 commits July 24, 2024 17:44

Update packages/bff/src/model/modelMappingUtils.ts

1b30f5a

Co-authored-by: Eric Camellini <eric.camellini@gmail.com>

Fix typo

28954da

Merge branch 'main' into IMN-657-getEserviceDescriptorById

d3d4657

Rename const variable

5ba6040

IMN-656 get producer eservice detail by ID (#730)

31c5dcd

AsterITA approved these changes Jul 25, 2024

View reviewed changes

packages/bff/src/routers/catalogRouter.ts Outdated Show resolved Hide resolved

Viktor-K added 2 commits July 25, 2024 11:32

Add operative log

11026a3

Merge branch 'main' into IMN-657-getEserviceDescriptorById

c41d3ec

Viktor-K merged commit a9561da into main Jul 25, 2024
27 checks passed

Viktor-K deleted the IMN-657-getEserviceDescriptorById branch July 25, 2024 10:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IMN-657 GET EService descriptor by ID #712

IMN-657 GET EService descriptor by ID #712

Viktor-K commented Jul 8, 2024 •

edited

Loading

ecamellini left a comment

ecamellini Jul 17, 2024

Viktor-K Jul 22, 2024

ecamellini Jul 17, 2024

Viktor-K Jul 22, 2024 •

edited

Loading

ecamellini Jul 17, 2024

ecamellini Jul 17, 2024

Viktor-K Jul 22, 2024

ecamellini Jul 23, 2024

ecamellini Jul 17, 2024

AsterITA Jul 18, 2024 •

edited

Loading

Viktor-K Jul 22, 2024

Carminepo2 Jul 18, 2024

Viktor-K Jul 22, 2024 •

edited

Loading

Viktor-K Jul 22, 2024

Carminepo2 Jul 18, 2024

Viktor-K Jul 22, 2024

ecamellini left a comment

ecamellini Jul 23, 2024

	(m) => m.kind === tenantMailKind.ContactEmail
	(m) => m.kind === tenantApi.MailKind.Values.CONTACT_EMAIL

	// Fetched all attributes in a recursive way
	// Fetch all attributes in a recursive way

	const answers: { [key: string]: string[] } =
	const answers: bffApi.RiskAnalysisForm["answers"] =

	const getEserviceDesciptor = (
	const retrieveEserviceDescriptor = (

IMN-657 GET EService descriptor by ID #712

IMN-657 GET EService descriptor by ID #712

Conversation

Viktor-K commented Jul 8, 2024 • edited Loading

Description

TEST

ecamellini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Viktor-K Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AsterITA Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Viktor-K Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecamellini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Viktor-K commented Jul 8, 2024 •

edited

Loading

Viktor-K Jul 22, 2024 •

edited

Loading

AsterITA Jul 18, 2024 •

edited

Loading

Viktor-K Jul 22, 2024 •

edited

Loading