Skip to content
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.

Sign up for GitHub

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

Jump to bottom

开放体育打卡 #501

Merged
merged 5 commits into from
Nov 17, 2024
Merged

开放体育打卡 #501

merged 5 commits into from
Nov 17, 2024

Conversation

lytMoon
Copy link
Contributor

@lytMoon lytMoon commented Nov 16, 2024

No description provided.

Gentlemuqiu
Gentlemuqiu previously approved these changes Nov 16, 2024
View reviewed changes
lytMoon
lytMoon commented Nov 17, 2024
View reviewed changes
Copy link
Contributor Author

@lytMoon lytMoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已drop

lytMoon
lytMoon commented Nov 17, 2024
View reviewed changes
Copy link
Contributor Author

@lytMoon lytMoon left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop

@lytMoon lytMoon requested a review from 985892345 November 17, 2024 05:06
985892345
985892345 requested changes Nov 17, 2024
View reviewed changes
module_sport/src/main/java/com/mredrock/cyxbs/sport/ui/activity/SportDetailActivity.kt Outdated
// "请先绑定教务在线才能继续使用哦~".toast()
// ARouter.getInstance().build(LOGIN_BIND_IDS).navigation()
// finish()
// }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

注释掉的代码能删就删，而且这个sSpIdsIsBind的逻辑可以一起删掉，如果后续不再需要端上绑定jwzx后

@lytMoon lytMoon dismissed 985892345’s stale review November 17, 2024 05:07

设置为resolve和review之后还是显示need request change，重新review

lytMoon
lytMoon commented Nov 17, 2024
View reviewed changes
Copy link
Contributor Author

@lytMoon lytMoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@lytMoon lytMoon changed the title 调整搜索学生课表部分ui、开放体育打卡 开放体育打卡 Nov 17, 2024
985892345
985892345 requested changes Nov 17, 2024
View reviewed changes
.../src/main/java/com/mredrock/cyxbs/course/page/find/ui/find/adapter/ShowStuResultRvAdapter.kt Outdated
post {
val parentWidth: Int = parentView.width
val maxWidth = (parentWidth - dpToPx(160))
setMaxWidth(maxWidth)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你这里也没drop掉啊

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

刚回退，现在ok了

@lytMoon lytMoon requested a review from 985892345 November 17, 2024 05:14
@985892345
Copy link
Member

DiscoverSportFeedViewModel#isBind还涉及了业务状态，还有一个IBindService、BindActivity，如果目前不再需要绑定统一认证码，理应一起删掉的

@985892345 985892345 requested a review from RedSun-V November 17, 2024 05:18
@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

DiscoverSportFeedViewModel#isBind还涉及了业务状态，还有一个IBindService、BindActivity，如果目前不再需要绑定统一认证码，理应一起删掉的

后续不确定要用，我这边先把相关都下吧

@985892345
Copy link
Member

DiscoverSportFeedViewModel#isBind还涉及了业务状态，还有一个IBindService、BindActivity，如果目前不再需要绑定统一认证码，理应一起删掉的

辛苦@RedSun-V 前来确认，这块之前是你在负责

@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

DiscoverSportFeedViewModel#isBind还涉及了业务状态，还有一个IBindService、BindActivity，如果目前不再需要绑定统一认证码，理应一起删掉的

why代码，辛苦王哥review下

@RedrockMobile RedrockMobile deleted a comment from 985892345 Nov 17, 2024
@RedSun-V
Copy link
Contributor

如果不需要 ids 认证，可以去掉，之前这里主要是爬数据要用的

@RedSun-V
Copy link
Contributor

假刷新删了为什么不把真刷新加上？那刷新不是直接没了吗

@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

DiscoverHomeFragment

真刷新是？这块我不是很熟，要不辛苦王哥新开个mr

@RedSun-V
Copy link
Contributor

ids 绑定界面相关代码直接删除，是确定后续不可能会再需要绑定 ids 了吗

@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

ids 绑定界面相关代码直接删除，是确定后续不可能会再需要绑定 ids 了吗

之前确定是换新接口

@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

主页有个体育打卡卡片也需要添加哈：DiscoverHomeFragment 271行

指的是？现在体育打卡不是可以进入吗？271行代码是
// 发现首页横排按钮点击埋点
functions[it].clickEvent?.let { clickEvent ->
processLifecycleScope.launch {
TrackingUtils.trackClickEvent(clickEvent)
}
}

@985892345
Copy link
Member

ids 绑定界面相关代码直接删除，是确定后续不可能会再需要绑定 ids 了吗

之前确定是换新接口

那用户怎么登陆ids呢？还是说server那边根据我们的学号去拿的数据？

@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

ids 绑定界面相关代码直接删除，是确定后续不可能会再需要绑定 ids 了吗

之前确定是换新接口

那用户怎么登陆ids呢？还是说server那边根据我们的学号去拿的数据？

现在后端改成了只需要get调用接口，带token访问

@985892345
Copy link
Member

主页有个体育打卡卡片也需要添加哈：DiscoverHomeFragment 271行

指的是？现在体育打卡不是可以进入吗？271行代码是 // 发现首页横排按钮点击埋点 functions[it].clickEvent?.let { clickEvent -> processLifecycleScope.launch { TrackingUtils.trackClickEvent(clickEvent) } }

就主页有todo和电费的卡片，体育打卡也是有的，在 DiscoverHomeFragment#initFeeds 里面添加，目前是注释了的

@RedSun-V
Copy link
Contributor

后续其他模块不会再有绑定 ids 的需求了吗，都能闭环在我们 server 的服务内？客户端无感知么

@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

DiscoverHomeFragment

主页有个体育打卡卡片也需要添加哈：DiscoverHomeFragment 271行

指的是？现在体育打卡不是可以进入吗？271行代码是 // 发现首页横排按钮点击埋点 functions[it].clickEvent?.let { clickEvent -> processLifecycleScope.launch { TrackingUtils.trackClickEvent(clickEvent) } }

就主页有todo和电费的卡片，体育打卡也是有的，在 DiscoverHomeFragment#initFeeds 里面添加，目前是注释了的

已经恢复

@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

后续其他模块不会再有绑定 ids 的需求了吗，都能闭环在我们 server 的服务内？客户端无感知么

不知道呀，目前需求是只恢复体育打卡，ids后续其他模块也说不准，得问server，所以是下掉还是暂存 cc @985892345 @RedSun-V

@985892345
Copy link
Member

后续其他模块不会再有绑定 ids 的需求了吗，都能闭环在我们 server 的服务内？客户端无感知么

不知道呀，目前需求是只恢复体育打卡，ids后续其他模块也说不准，得问server，所以是下掉还是暂存 cc @985892345 @RedSun-V

建议下掉吧，后续也不可能上单独的ids登录了，ids登录应该会集成到掌邮登陆里面

@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

后续其他模块不会再有绑定 ids 的需求了吗，都能闭环在我们 server 的服务内？客户端无感知么

不知道呀，目前需求是只恢复体育打卡，ids后续其他模块也说不准，得问server，所以是下掉还是暂存 cc @985892345 @RedSun-V

建议下掉吧，后续也不可能上单独的ids登录了，ids登录应该会集成到掌邮登陆里面

辛苦王哥cr下，我看是王哥代码，没啥问题这边就合了，后端那边催包测试，昨天发了个测试包，搜索课表的学生界面适配问题也辛苦郭神尽快合入线上 cc @985892345 @RedSun-V

@985892345
Copy link
Member

后续其他模块不会再有绑定 ids 的需求了吗，都能闭环在我们 server 的服务内？客户端无感知么

不知道呀，目前需求是只恢复体育打卡，ids后续其他模块也说不准，得问server，所以是下掉还是暂存 cc @985892345 @RedSun-V

建议下掉吧，后续也不可能上单独的ids登录了，ids登录应该会集成到掌邮登陆里面

辛苦王哥cr下，我看是王哥代码，没啥问题这边就合了，后端那边催包测试，昨天发了个测试包，搜索课表的学生界面适配问题也辛苦郭神尽快合入线上 cc @985892345 @RedSun-V

我觉得问题还很多呢，很多东西你都没删完的

@985892345
Copy link
Member

你先push上去，我来改点代码

@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

后续其他模块不会再有绑定 ids 的需求了吗，都能闭环在我们 server 的服务内？客户端无感知么

不知道呀，目前需求是只恢复体育打卡，ids后续其他模块也说不准，得问server，所以是下掉还是暂存 cc @985892345 @RedSun-V

建议下掉吧，后续也不可能上单独的ids登录了，ids登录应该会集成到掌邮登陆里面

辛苦王哥cr下，我看是王哥代码，没啥问题这边就合了，后端那边催包测试，昨天发了个测试包，搜索课表的学生界面适配问题也辛苦郭神尽快合入线上 cc @985892345 @RedSun-V

我觉得问题还很多呢，很多东西你都没删完的

辛苦郭神再看看 这个分支是lyt/improve/new 🫡 还是可以考虑单独开个分支来下掉ids

@985892345
Copy link
Member

后续其他模块不会再有绑定 ids 的需求了吗，都能闭环在我们 server 的服务内？客户端无感知么

不知道呀，目前需求是只恢复体育打卡，ids后续其他模块也说不准，得问server，所以是下掉还是暂存 cc @985892345 @RedSun-V

建议下掉吧，后续也不可能上单独的ids登录了，ids登录应该会集成到掌邮登陆里面

辛苦王哥cr下，我看是王哥代码，没啥问题这边就合了，后端那边催包测试，昨天发了个测试包，搜索课表的学生界面适配问题也辛苦郭神尽快合入线上 cc @985892345 @RedSun-V

我觉得问题还很多呢，很多东西你都没删完的

辛苦郭神再看看 这个分支是lyt/improve/new 🫡 还是可以考虑单独开个分支来下掉ids

就一个分支操作就行了，我提新的commit

@lytMoon
Copy link
Contributor Author

lytMoon commented Nov 17, 2024

后续其他模块不会再有绑定 ids 的需求了吗，都能闭环在我们 server 的服务内？客户端无感知么

不知道呀，目前需求是只恢复体育打卡，ids后续其他模块也说不准，得问server，所以是下掉还是暂存 cc @985892345 @RedSun-V

建议下掉吧，后续也不可能上单独的ids登录了，ids登录应该会集成到掌邮登陆里面

辛苦王哥cr下，我看是王哥代码，没啥问题这边就合了，后端那边催包测试，昨天发了个测试包，搜索课表的学生界面适配问题也辛苦郭神尽快合入线上 cc @985892345 @RedSun-V

我觉得问题还很多呢，很多东西你都没删完的

辛苦郭神再看看 这个分支是lyt/improve/new 🫡 还是可以考虑单独开个分支来下掉ids

就一个分支操作就行了，我提新的commit

收到🫡

@985892345
Copy link
Member

@lytMoon @RedSun-V 辛苦各位大佬再cr一下哈

@RedSun-V
Copy link
Contributor

考试也一起改了吗？

RedSun-V
RedSun-V previously approved these changes Nov 17, 2024
View reviewed changes
@985892345
Copy link
Member

考试也一起改了吗？

一起杨了ids相关的代码，但server那边应该是没数据的

985892345
985892345 previously approved these changes Nov 17, 2024
View reviewed changes
@985892345 985892345 dismissed stale reviews from RedSun-V and themself via 2ea3787 November 17, 2024 09:01
@985892345 985892345 merged commit a120c97 into develop Nov 17, 2024
3 checks passed
@985892345 985892345 deleted the lyt/improve/new branch November 17, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@985892345 985892345 985892345 left review comments

@RedSun-V RedSun-V RedSun-V left review comments

@Gentlemuqiu Gentlemuqiu Gentlemuqiu left review comments

@Black-Skyline Black-Skyline Awaiting requested review from Black-Skyline

@flyflowsmallblank flyflowsmallblank Awaiting requested review from flyflowsmallblank

@Faller665 Faller665 Awaiting requested review from Faller665

@False-Mask False-Mask Awaiting requested review from False-Mask

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lytMoon @985892345 @RedSun-V @Gentlemuqiu