-
Notifications
You must be signed in to change notification settings - Fork 9
Fixing copyAttachments api for Projection entities #343
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
Conversation
…-name & List of objectIds to copy
pom.xml
Outdated
|
|
||
| <properties> | ||
| <revision>1.5.1-SNAPSHOT</revision> | ||
| <revision>1.0.0-RC1</revision> |
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.
pls change this
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.
Done
| // Find the parent entity | ||
| Optional<CdsEntity> optionalParentEntity = model.findEntity(parentEntity); | ||
| if (optionalParentEntity.isEmpty()) { | ||
| throw new ServiceException("Unable to find parent entity: " + parentEntity); |
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.
can we put this in constants file
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.
Done, added it in constants file.
| // Find the target attachment entity | ||
| Optional<CdsEntity> attachmentEntity = model.findEntity(targetEntityName); | ||
| if (attachmentEntity.isEmpty()) { | ||
| throw new ServiceException("Unable to find target attachment entity: " + targetEntityName); |
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.
add constant in SDMConstants file
| optionalParentEntity.get().findElement(compositionName); | ||
| if (compositionElement.isEmpty() || !compositionElement.get().getType().isAssociation()) { | ||
| throw new ServiceException( | ||
| "Unable to find composition '" + compositionName + "' in entity: " + parentEntity); |
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.
same 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.
Done, added it in constants file.
| String[] facetParts = input.facet().split("\\."); | ||
| if (facetParts.length < 3) { | ||
| throw new IllegalArgumentException( | ||
| "Invalid facet format. Expected: Service.Entity.Composition, got: " + input.facet()); |
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.
add to constants
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.
Done, added it in constants file.
|
|
||
| CopyAttachmentsResult copyResult = | ||
| copyAttachmentsToSDM( | ||
| context, objectIds, folderId, repositoryId, sdmCredentials, isSystemUser, folderExists); |
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.
can we add all these method parameters to a model class because its not good practice to have more than 4 parameters
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.
Done
|
|
||
| String upIdKey = resolveUpIdKey(context, parentEntity, compositionName); | ||
| createDraftEntries( | ||
| attachmentsMetadata, |
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.
can we add all these method parameters to a model class because its not good practice to have more than 4 parameters
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.
Done
pom.xml
Outdated
|
|
||
| <properties> | ||
| <revision>1.5.1-SNAPSHOT</revision> | ||
| <revision>1.0.0-RC1</revision> |
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 revert once testing is over
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.
Done
Describe your changes
This PR fixes copyAttachments API for Projection Entities. Earlier copyAttachments() was working only for non-projection entities and was failing for projection entities with 401 "Not authorized to send event 'DRAFT_NEW' to ". I raised this issue with CAP team and as per the suggestion I have incorporated this fix.
Any documentation
Type of change
Please delete options that are not relevant.
Checklist before requesting a review
Upload Screenshots/lists of the scenarios tested
Single tenant Integration Test: https://github.com/cap-java/sdm/actions/runs/18746444994
Multi tenant Integration Test: https://github.com/cap-java/sdm/actions/runs/18748340988