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 2023/01/19 11:31:23 UTC
[GitHub] [incubator-streampark] RocMarshal opened a new pull request, #2262: [Feature][Issue-2214] Team add number, can direct select.
RocMarshal opened a new pull request, #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262
<!--
Thank you for contributing to StreamPark! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
## Contribution Checklist
- If this is your first time, please read our contributor guidelines: [Submit Code](https://streampark.apache.org/community/submit_guide/submit_code).
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/incubator-streampark/issues).
- Name the pull request in the form "[Feature] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- If the PR is unfinished, add `[WIP]` in your PR title, e.g., `[WIP][Feature] Title of the pull request`.
-->
## What changes were proposed in this pull request
Issue Number: close #2214
<!--(For example: This pull request proposed to add checkstyle plugin).-->
## Brief change log
[Feature][Issue-2214] Team add number, can direct select.
## Verifying this change
<!--*(Please pick either of the following options)*-->
- *Manually verified the change by testing locally.*
## Does this pull request potentially affect one of the following parts
- Dependencies (does it add or upgrade a dependency): (yes / **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] RocMarshal commented on pull request #2262: [Feature][Issue-2214] Team add number, can direct select.
Posted by GitBox <gi...@apache.org>.
RocMarshal commented on PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#issuecomment-1396841587
Hi, @wangsizhu0504 .
The new interface `candidateUsers` is healthy.
The data queried from backend can't be rendered in FE, Could you help to check it ? Thank you in advance.
CC @1996fanrui @wangsizhu0504 @wolfboys
--
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] RocMarshal commented on a diff in pull request #2262: [Feature][Issue-2214] Support to select user when binding member.
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#discussion_r1084763712
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/MemberController.java:
##########
@@ -53,6 +54,12 @@ public RestResponse memberList(RestRequest restRequest, Member member) {
return RestResponse.success(userList);
}
+ @PostMapping("candidateUsers")
+ public RestResponse candidateUsers(RestRequest restRequest, Long teamId) {
+ IPage<User> userList = memberService.findCandidateUsers(teamId, restRequest);
Review Comment:
@wolfboys thanks for your review & suggestions.
The truth what you described is right.
Before reaching a consensus, I want to express the original intention of this design:
The `interface implemented with Pagination` is a superset of `interface implemented without Pagination` , In other words, if there are other calls that need paging, the interface can still be reused. It will lead to some redundancy in the current front-end calling.
Looking forward to the final conclusion from you.
--
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] RocMarshal commented on pull request #2262: [Feature][Issue-2214] Team add number, can direct select.
Posted by GitBox <gi...@apache.org>.
RocMarshal commented on PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#issuecomment-1397811681
Thanks for the help from @1996fanrui @wangsizhu0504 offline.
I fixed the little case in https://github.com/apache/incubator-streampark/pull/2262/commits/c42061c83a38a4738fdcc001c6fef5db50252c7b
Could you @wolfboys help to review it ? thx
--
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] RocMarshal commented on a diff in pull request #2262: [Feature][Issue-2214] Support to select user when binding member.
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#discussion_r1084763973
##########
streampark-console/streampark-console-webapp/src/views/system/member/MemberDrawer.vue:
##########
@@ -41,7 +43,7 @@
import { useUserStoreWithOut } from '/@/store/modules/user';
import { RuleObject } from 'ant-design-vue/lib/form';
import { StoreValue } from 'ant-design-vue/lib/form/interface';
- import { fetchAddMember, fetchCheckUserName, fetchUpdateMember } from '/@/api/system/member';
+ import { fetchAddMember, fetchCandidateUsers, fetchCheckUserName, fetchUpdateMember } from "/@/api/system/member";
Review Comment:
keep the same code-style. `'`
##########
streampark-console/streampark-console-webapp/src/views/system/member/MemberDrawer.vue:
##########
@@ -53,7 +55,7 @@
roleOptions: {
type: Array as PropType<Array<Partial<RoleListItem>>>,
default: () => [],
- },
+ }
Review Comment:
```suggestion
},
```
--
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] RocMarshal commented on a diff in pull request #2262: [Feature][Issue-2214] Support to select user when binding member.
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#discussion_r1084832275
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/MemberController.java:
##########
@@ -53,6 +54,12 @@ public RestResponse memberList(RestRequest restRequest, Member member) {
return RestResponse.success(userList);
}
+ @PostMapping("candidateUsers")
+ public RestResponse candidateUsers(RestRequest restRequest, Long teamId) {
+ IPage<User> userList = memberService.findCandidateUsers(teamId, restRequest);
Review Comment:
> > @wolfboys thanks for your review & suggestions. The truth what you described is right. Before reaching a consensus, I want to express the original intention of this design: The `interface implemented with Pagination` is a superset of `interface implemented without Pagination` , In other words, if there are other calls that need paging, the interface can still be reused. It will lead to some redundancy in the current front-end calling.
> > Looking forward to the final conclusion from you.
>
> Happy Chinese new year, thanks for your feedback, My point : This method does not seem to be reusable. That is, it seems to be customized for specific requests, not a generic method (find user not in team),with explicit query conditions, what you think?
Got it & updated it.
Happy Chinese new year~ & Thanks for the clarification.
--
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 #2262: [Feature][Issue-2214] Support to select user when binding member.
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on code in PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#discussion_r1084787515
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/MemberController.java:
##########
@@ -53,6 +54,12 @@ public RestResponse memberList(RestRequest restRequest, Member member) {
return RestResponse.success(userList);
}
+ @PostMapping("candidateUsers")
+ public RestResponse candidateUsers(RestRequest restRequest, Long teamId) {
+ IPage<User> userList = memberService.findCandidateUsers(teamId, restRequest);
Review Comment:
> @wolfboys thanks for your review & suggestions. The truth what you described is right. Before reaching a consensus, I want to express the original intention of this design: The `interface implemented with Pagination` is a superset of `interface implemented without Pagination` , In other words, if there are other calls that need paging, the interface can still be reused. It will lead to some redundancy in the current front-end calling.
>
> Looking forward to the final conclusion from you.
Happy Chinese new year, thanks for your feedback, My point : This method does not seem to be reusable. That is, it seems to be customized for specific requests, not a generic method (find user not in team),with explicit query conditions, what you think?
--
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] RocMarshal commented on pull request #2262: [Feature][Issue-2214] Support to select user when binding member.
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#issuecomment-1399152100
>
The title `Support to select user when binding member.` SGTM.
--
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 merged pull request #2262: [Feature][Issue-2214] Support to select user when binding member.
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui merged PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262
--
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 #2262: [Feature][Issue-2214] Team add number, can direct select.
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on code in PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#discussion_r1082857197
##########
streampark-console/streampark-console-service/src/main/resources/mapper/system/MemberMapper.xml:
##########
@@ -68,4 +68,15 @@
</if>
</select>
+ <select id="findUsersNotInTeam" resultType="org.apache.streampark.console.system.entity.User">
+ select tu.* from t_user tu
+ where (tu.user_id, tu.username)
+ not in (
+ select u.user_id, u.username
+ from t_user u join t_member m
+ on m.team_id = #{teamId}
+ and m.user_id = u.user_id
+ )
+ </select>
Review Comment:
Can the `username` be removed?
--
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] RocMarshal commented on pull request #2262: [Feature][Issue-2214] Team add number, can direct select.
Posted by GitBox <gi...@apache.org>.
RocMarshal commented on PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#issuecomment-1396867846
<img width="1413" alt="截屏2023-01-19 19 57 52" src="https://user-images.githubusercontent.com/64569824/213438130-7084c38f-5fb9-4435-b5fb-8c512b610033.png">
<img width="1517" alt="截屏2023-01-19 19 59 14" src="https://user-images.githubusercontent.com/64569824/213438205-592e58cc-1902-434e-b36c-ad6ba0acd0a3.png">
--
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 #2262: [Feature][Issue-2214] Support to select user when binding member.
Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on code in PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#discussion_r1084716267
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/controller/MemberController.java:
##########
@@ -53,6 +54,12 @@ public RestResponse memberList(RestRequest restRequest, Member member) {
return RestResponse.success(userList);
}
+ @PostMapping("candidateUsers")
+ public RestResponse candidateUsers(RestRequest restRequest, Long teamId) {
+ IPage<User> userList = memberService.findCandidateUsers(teamId, restRequest);
Review Comment:
I suggest return `List<User>` instead of `IPage<User>`, The fact is that this does not use the paging function
##########
streampark-console/streampark-console-webapp/src/views/system/member/MemberDrawer.vue:
##########
@@ -99,8 +101,17 @@
{
field: 'userName',
label: t('system.member.table.userName'),
- component: 'Input',
- componentProps: { disabled: unref(isUpdate) },
+ component: 'ApiSelect',
+ componentProps: {
+ disabled: unref(isUpdate),
+ api: fetchCandidateUsers,
+ params: { page: 1, pageSize: 9999, teamId: userStore.getTeamId},
Review Comment:
Pagination will add unnecessary trouble, right?
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/MemberServiceImpl.java:
##########
@@ -82,6 +82,14 @@ public IPage<Member> findUsers(Member member, RestRequest request) {
return baseMapper.findUsers(page, member);
}
+ @Override
+ public IPage<User> findCandidateUsers(Long teamId, RestRequest request) {
+ Page<User> page = new Page<>();
+ page.setCurrent(request.getPageNum());
+ page.setSize(request.getPageSize());
+ return baseMapper.findUsersNotInTeam(page, teamId);
Review Comment:
Pagination will add unnecessary trouble, right?
--
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 #2262: [Feature][Issue-2214] Team add number, can direct select.
Posted by GitBox <gi...@apache.org>.
wolfboys commented on PR #2262:
URL: https://github.com/apache/incubator-streampark/pull/2262#issuecomment-1398457367
> Thanks for the help from @1996fanrui @wangsizhu0504 offline. I fixed the little case in [c42061c](https://github.com/apache/incubator-streampark/commit/c42061c83a38a4738fdcc001c6fef5db50252c7b) Could you @wolfboys help to review it ? thx
Thanks for your contribution, happy chinese new year👏, i'll review it later
--
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