-
Notifications
You must be signed in to change notification settings - Fork 47
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
[New Model]: Pi10 new model IdBasedComment #405
[New Model]: Pi10 new model IdBasedComment #405
Conversation
Validation Report for io.catenax.id_based_comment/1.0.0/IdBasedComment.ttlInput model is valid |
:IdBasedComment a samm:Aspect ; | ||
samm:preferredName "Id Based Comment"@en ; | ||
samm:description "Aspect model for an exchange of comment belongig to a entity."@en ; | ||
samm:properties ( :commentId :objectId [ samm:property :author; samm:optional true ] [ samm:property :postedAt; samm:optional true ] [ samm:property :changedAt; samm:optional true ] [ samm:property :commentText; samm:optional true ] [ samm:property :commentType; samm:optional true ] [ samm:property :requestDelete; samm:optional true ] [ samm:property :listOfReferenceDates; samm:optional true ] :objectType ) ; |
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.
This model is lacking customer
and supplier
. However, they are crucial for this model to be sound. Let me elaborate:
A customer-supplier relationship is a directed one: One party is customer, one is supplier. Comments, as well as the Material Demands and Capacity Groups they refer to, have IDs. These IDs are, in their description, explicitly defined to only be unique "within the business relationship between a customer and its supplier". Hence, to uniquely resolve a comment by its commentId
, as well as a Material Demand or Capacity Group by an
It is not possible to infer this information from the message header, as the message header only contains information about the sender and the receiver of the message. However, as both customer and supplier can send (create/update/delete) comments, it is not possible to know if the sender of the message is the customer or the supplier. Thus, it is not possible to derive the correct customer-supplier relationship. Hence, we can't uniquely address the comment, material demand or capacity group.
Example: As per our standard, it is valid that two companies 123
, and 123
. The standard, today, forces us to not throw an error but treat those as separate Material Demands (because the ID must be treated as unique only within the relationship, not globally). Now, A posts a comment on objectId
123
. Which Material Demand should receive the comment? A surely is the sender and B the receiver of the message, but from that you can't derive whether the business partner relationship
I know this is an edge case, but we allow (and must allow) sender-controlled IDs, and the other standards define them as not globally unique (allowing us to fail when we receive a second material demand (or comment/capacity group) with the same UUID but a different business partner relationship), but uniquely valid only within the business partner relationship (forcing us to accept them). Thus, the standards must also deal with this edge case.
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.
properties were added as discussed in the architecture meeting today
Validation Report for io.catenax.id_based_comment/1.0.0/IdBasedComment.ttlInput model is valid |
@@ -0,0 +1,1123 @@ | |||
|
|||
<!doctype html> |
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.
You can remove completely the 'gen' folder as this will be generated at run time automatically.
:objectId a samm:Property ; | ||
samm:preferredName "Object ID"@en ; | ||
samm:description "The ID of the object to which the comment belongs."@en ; | ||
samm:characteristic :UUIDv4IdTrait ; |
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.
you can make use of shared aspect model (io.catenax.shared.uuid:1.0.0) for UUID.
you can refer this https://github.com/eclipse-tractusx/sldt-semantic-models/blob/a07bdd5b1515767d4f8d07deb47cf2a4aa728bef/io.catenax.quality_message_content/1.0.0/QualityMessageContent.ttl#L35C9-L35C17
:customer a samm:Property ; | ||
samm:preferredName "Customer"@en ; | ||
samm:description "The Business Partner Number Legal Entity (BPNL) of the party requesting materials from a supplier."@en ; | ||
samm:characteristic :BPNLTrait ; |
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.
Here also please check if you can make use of shared aspect model https://github.com/eclipse-tractusx/sldt-semantic-models/blob/a07bdd5b1515767d4f8d07deb47cf2a4aa728bef/io.catenax.shared.business_partner_number/1.0.0/BusinessPartnerNumber.ttl
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 love it, this would make sense for the other models of DCM as well!
|
||
:CommentTextCharacteristic a samm:Characteristic ; | ||
samm:preferredName "Comment Text Characteristic"@en ; | ||
samm:description "A text."@en ; |
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.
would be great to provide a bit more elaborated description here.
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.
please check my comments
Validation Report for io.catenax.id_based_comment/1.0.0/IdBasedComment.ttlInput model is valid |
@agg3fe & @LukasHeimann: model uses shared BPN's + UUID aspect now, fixed description for the text. Ready for review |
|
||
:IdBasedComment a samm:Aspect ; | ||
samm:preferredName "Id Based Comment"@en ; | ||
samm:description "Aspect model for an exchange of comment belongig to a entity."@en ; |
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.
typo belongig
@nhaenis could you please correct a small typo mentioned in my PR comment. then we can do the MS2 today. |
Validation Report for io.catenax.id_based_comment/1.0.0/IdBasedComment.ttlInput model is valid |
@agg3fe the picture in the HTML in gen folder is not generated correctly. Is there already a bug in SAMM CLI opened for a fix of the visual representations of the aspects when another aspect is added as ext-ref? Locally i have the same issue and need to comment out the lines of the models UUID and BPN (lines 24 to 29 in both BPN aspect and UUID aspect) in order to generate it properly |
@nhaenis I think there are some issues going on with the SDK and they are working on it. I will have a look into it. |
Description
New model for an ID based data exchange of comments belonging to a UUID / entity.
Closes #308
MS2 Criteria
(to be filled out by PR reviewer)
DismantlerId
andDismantlerName
use an EntityDismantler
with the propertiesname
andid
or use a URN likeio.catenax.dismantler:0.0.1
)preferredName
anddescription
are not the samepreferredName
should be human readable and follow normal orthography (e.g., no camel case but normal word separation)MS3 Criteria
(to be filled out by semantic modeling team before merge to main-branch)