-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PIN 5558 - Get eservice descriptor #1218
base: feature/incaricato
Are you sure you want to change the base?
PIN 5558 - Get eservice descriptor #1218
Conversation
@@ -92,7 +92,7 @@ export function toBffCatalogDescriptorEService( | |||
descriptor: catalogApi.EServiceDescriptor, | |||
producerTenant: tenantApi.Tenant, | |||
agreement: agreementApi.Agreement | undefined, | |||
requesterTenant: tenantApi.Tenant | |||
requesterTenants: tenantApi.Tenant[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment below: since isMine should probably consider only the requester, while hasCertifiedAttributes should consider the requester and the consumer delegators, we can split the two params here
requesterTenants: tenantApi.Tenant[] | |
requesterTenant: tenantApi.Tenant, | |
consumerDelegators: tenantApi.Tenant[] |
isMine: requesterTenants.some((t) => | ||
isRequesterEserviceProducer(t.id, eservice) | ||
), | ||
hasCertifiedAttributes: requesterTenants.some((t) => | ||
hasCertifiedAttributes(descriptor, t) | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure isMine
shall consider the delegators, because it's about the requester being the producer of the eservice, the one that can manage it/edit it, while if I am the delegate of a consumer delegation I want to consume the eservice. Should this maybe be set to true in the case the requester is the delegate of a producer delegation? (to discuss with the "Capofila" team, probably, as a further task).
I would also add a comment to explain why hasCertifiedAttributes
checks the delegators, it's hard to understand because the name is misleading now. If you think it makes sense, let's also add the same comment as the API spec's description for the field.
isMine: requesterTenants.some((t) => | |
isRequesterEserviceProducer(t.id, eservice) | |
), | |
hasCertifiedAttributes: requesterTenants.some((t) => | |
hasCertifiedAttributes(descriptor, t) | |
), | |
isMine: isRequesterEserviceProducer(requesterTenant.id, eservice), | |
hasCertifiedAttributes: [requesterTenant, ...consumerDelegators].some((t) => | |
hasCertifiedAttributes(descriptor, t) | |
/* True in case: | |
- the requester has the certified attributes required to consume the eservice, or | |
- the requester is the delegated consumer for the eservice and the delegator has the certified attributes required to consume the eservice */ | |
), |
Closes PIN-5558
Manual test
Delegate without attribute, Delegator without attribute
Delegate without attribute, Delegator with attribute