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
Fix #3124: Activity Feed: Add support for deleting a post #3161
Conversation
Schema Change Detected. Needs ingestion-core version bump Please run |
catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java
Outdated
Show resolved
Hide resolved
catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/FeedRepository.java
Outdated
Show resolved
Hide resolved
}, | ||
"from": { | ||
"description": "Name of the User posting the message", | ||
"type": "string" |
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.
If all users are in OpenMetadata internal db, should we use user id?
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.
Actually not. Let's keep them as user names which are supposed to be unique and human readable. One reason why to use UUID is to identify an instance of an entity uniquely (entity with same name can be created and hence name might not be unique) and when permanent links are needed.
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.
Lets not merge the delete part before we make the permission checks to authorizer
@mithmatt @harshach Thank you for the reviews and feedback. I have delegated the authorization logic to the AuthZ. I will continue to work on converting the type of post.from and thread.createdBy to UUID. Then, the createPost will only require a message and we will populate post.from as id of the logged in user (securityContext.getUserPrincipal().getName()). |
[open-metadata-ingestion] Kudos, SonarCloud Quality Gate passed! |
[catalog] Kudos, SonarCloud Quality Gate passed! |
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.
Thanks for addressing the review feedback @vivekratnavel
Describe your changes :
See #3124
Type of change :
Checklist:
Reviewers