You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by GitBox <gi...@apache.org> on 2022/11/12 06:33:22 UTC
[GitHub] [incubator-streampark] 1996fanrui opened a new pull request, #2009: [Bug] Clear the teamId when delete the member
1996fanrui opened a new pull request, #2009:
URL: https://github.com/apache/incubator-streampark/pull/2009
## What changes were proposed in this pull request
Issue Number: close #2008
The lastTeamId may be wrong when delete the member
## Brief change log
Clear the teamId when delete the member
## Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
## Does this pull request potentially affect one of the following parts
- Dependencies (does it add or upgrade a dependency): no
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on a diff in pull request #2009: [Bug] Clear the teamId when delete the member
Posted by GitBox <gi...@apache.org>.
wolfboys commented on code in PR #2009:
URL: https://github.com/apache/incubator-streampark/pull/2009#discussion_r1020708075
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/UserService.java:
##########
@@ -120,6 +120,8 @@ public interface UserService extends IService<User> {
void setLatestTeam(Long teamId, Long userId);
+ void clearDeletedTeamId(Long userId, Long teamId);
Review Comment:
how about `unbindTeam(Long userId, Long teamId)` ?
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/MemberServiceImpl.java:
##########
@@ -136,14 +136,17 @@ public void createMember(Member member) {
}
@Override
- public void deleteMember(Member member) {
+ public void deleteMember(Member memberArg) {
+ Member member = Optional.ofNullable(this.getById(memberArg.getId()))
+ .orElseThrow(() -> new IllegalArgumentException(String.format("The member [id=%s] not found", memberArg.getId())));
this.removeById(member);
+ userService.clearDeletedTeamId(member.getUserId(), member.getTeamId());
}
@Override
public void updateMember(Member member) {
Member oldMember = Optional.ofNullable(this.getById(member.getId()))
- .orElseThrow(() -> new IllegalArgumentException(String.format("The mapping [id=%s] not found", member.getId())));
+ .orElseThrow(() -> new IllegalArgumentException(String.format("The member [id=%s] not found", member.getId())));
Review Comment:
Maybe throw `ApiAlertException` is better
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys merged pull request #2009: [Bug] Clear the teamId when delete the member
Posted by GitBox <gi...@apache.org>.
wolfboys merged PR #2009:
URL: https://github.com/apache/incubator-streampark/pull/2009
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on a diff in pull request #2009: [Bug] Clear the teamId when delete the member
Posted by GitBox <gi...@apache.org>.
macksonmu commented on code in PR #2009:
URL: https://github.com/apache/incubator-streampark/pull/2009#discussion_r1020755977
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/UserServiceImpl.java:
##########
@@ -188,6 +188,16 @@ public void setLatestTeam(Long teamId, Long userId) {
this.baseMapper.updateById(user);
}
+ @Override
+ public void unbindTeam(Long userId, Long teamId) {
+ User user = getById(userId);
+ AssertUtils.checkArgument(user != null);
Review Comment:
hi, fan rui, thank you very much for the hard work, when the user is deleted first, then an exception will be thrown when the member is deleted. Is it possible to add a judgment, that is, returns when user is null.
```
if (user == null || !teamId.equals(user.getTeamId())) {
return;
}
```
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/UserServiceImpl.java:
##########
@@ -188,6 +188,16 @@ public void setLatestTeam(Long teamId, Long userId) {
this.baseMapper.updateById(user);
}
+ @Override
+ public void unbindTeam(Long userId, Long teamId) {
+ User user = getById(userId);
+ AssertUtils.checkArgument(user != null);
Review Comment:
When deleting a user, the member dependency is not judged
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/UserServiceImpl.java:
##########
@@ -188,6 +188,16 @@ public void setLatestTeam(Long teamId, Long userId) {
this.baseMapper.updateById(user);
}
+ @Override
+ public void unbindTeam(Long userId, Long teamId) {
+ User user = getById(userId);
+ AssertUtils.checkArgument(user != null);
Review Comment:
Is it possible that when a person leaves, I may delete this user directly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #2009: [Bug] Clear the teamId when delete the member
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #2009:
URL: https://github.com/apache/incubator-streampark/pull/2009#discussion_r1020761464
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/UserServiceImpl.java:
##########
@@ -188,6 +188,16 @@ public void setLatestTeam(Long teamId, Long userId) {
this.baseMapper.updateById(user);
}
+ @Override
+ public void unbindTeam(Long userId, Long teamId) {
+ User user = getById(userId);
+ AssertUtils.checkArgument(user != null);
Review Comment:
Hi @macksonmu , thanks for your review.
Good catch, updated. The member mapping will be deleted when delete user.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on pull request #2009: [Bug] Clear the teamId when delete the member
Posted by GitBox <gi...@apache.org>.
wolfboys commented on PR #2009:
URL: https://github.com/apache/incubator-streampark/pull/2009#issuecomment-1312473741
cc @macksonmu please double check in your free time, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] macksonmu commented on pull request #2009: [Bug] Clear the teamId when delete the member
Posted by GitBox <gi...@apache.org>.
macksonmu commented on PR #2009:
URL: https://github.com/apache/incubator-streampark/pull/2009#issuecomment-1312482301
LGTM, thanks for your contribution
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on pull request #2009: [Bug] Clear the teamId when delete the member
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on PR #2009:
URL: https://github.com/apache/incubator-streampark/pull/2009#issuecomment-1312392925
Hi @macksonmu @wolfboys, please help take a look in your free time, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #2009: [Bug] Clear the teamId when delete the member
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #2009:
URL: https://github.com/apache/incubator-streampark/pull/2009#discussion_r1020744880
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/UserService.java:
##########
@@ -120,6 +120,8 @@ public interface UserService extends IService<User> {
void setLatestTeam(Long teamId, Long userId);
+ void clearDeletedTeamId(Long userId, Long teamId);
Review Comment:
Hi @wolfboys , thanks for your review. I addressed all comments~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org