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.

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

3.1.0 dev 11发布界面加载流程错误 #11902

Closed
3 tasks done
leolee9086 opened this issue Jul 7, 2024 · 15 comments
Closed
3 tasks done

3.1.0 dev 11发布界面加载流程错误 #11902

leolee9086 opened this issue Jul 7, 2024 · 15 comments

Comments

@leolee9086
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Can the issue be reproduced with the default theme (daylight/midnight)?

  • I was able to reproduce the issue with the default theme

Could the issue be due to extensions?

  • I've ruled out the possibility that the extension is causing the problem.

Describe the problem

经过排查,应该是在issue中#11733

image

这两个位置增加角色鉴权之后造成前端加载时收到403错误

这里可能需要对只读界面进行单独处理

Expected result

在发布界面中跳过同步和获取云端用户,类似这样?
image

Screenshot or screen recording presentation

No response

Version environment

- Version: 3.0.1 dev11
- Operating System: windows 11
- Browser (if used):edge

Log file

siyuan.log

More information

No response

@Zuoqiu-Yingyi
Copy link
Contributor

Zuoqiu-Yingyi commented Jul 7, 2024

@leolee9086
该问题在 #11733 中已修复, 但在 cd40ec5 里回滚了应用初始化相关的更改导致又出现了

@leolee9086
Copy link
Author

@leolee9086 该问题在 #11733 中已修复, 但在 cd40ec5 里移除了回调函数中的状态校验导致又出现了

我也不知道该咋弄 我本地先加上用着得了

@leolee9086 leolee9086 changed the title 3.0.1 dev 11发布界面加载流程错误 3.1.0 dev 11发布界面加载流程错误 Jul 7, 2024
@88250 88250 assigned Vanessa219 and unassigned Vanessa219 Jul 7, 2024
88250 added a commit that referenced this issue Jul 7, 2024
@88250
Copy link
Member

88250 commented Jul 7, 2024

这两个路由拦截删掉了,还是在 controller 里面判断权限。

@88250 88250 closed this as completed Jul 7, 2024
@Zuoqiu-Yingyi
Copy link
Contributor

Zuoqiu-Yingyi commented Jul 7, 2024

这两个路由拦截删掉了,还是在 controller 里面判断权限。

这是 API 调用的问题, 不是 API 实现的问题
通过移除权限中间件的方式会影响 API 语义与行为的一致性❗
不要图省事而增加未来的维护成本❗

@88250
Copy link
Member

88250 commented Jul 7, 2024

似乎问题不大吧。

@Zuoqiu-Yingyi
Copy link
Contributor

似乎问题不大吧。

会影响 API 的语义与行为的一致性,进而增加未来的维护成本

@88250
Copy link
Member

88250 commented Jul 7, 2024

我觉得这个问题不大,getWorkspaces 也是类似处理的。

@Zuoqiu-Yingyi
Copy link
Contributor

我觉得这个问题不大,getWorkspaces 也是类似处理的。

问题很大, getWorkspaces 也不应进行类似的处理

  1. 引入错误的语义: getCloudUsergetBootSync API 调用不需要鉴权
  2. 引入错误的行为: 无权限的请求调用了该 API 并获取了正常的响应
  3. 未来的维护成本: 需要意识到以上问题才能正确理解这两个 API 的行为逻辑

@Zuoqiu-Yingyi
Copy link
Contributor

Zuoqiu-Yingyi commented Jul 7, 2024

如果问题都是使用这种方案修复的话会导致项目的熵不断增加, 最终导致项目不可维护❗
这又不是需要临时紧急修复的、会产生恶劣影响的 bug

@88250
Copy link
Member

88250 commented Jul 7, 2024 via email

@Zuoqiu-Yingyi
Copy link
Contributor

@Vanessa219
能接受 #11733 Appconstructor() 中相关的优化嘛, 能接受的话我再开 issue 并提交相应的 PR

@88250
Copy link
Member

88250 commented Jul 8, 2024

这个问题稍微推后吧,我和 @Vanessa219 先记一下,多谢。

@Vanessa219
Copy link
Member

这个没问题就暂时不动了。

@Zuoqiu-Yingyi
Copy link
Contributor

这个没问题就暂时不动了。

有性能+上述问题

@88250
Copy link
Member

88250 commented Jul 9, 2024

先记录 issue 吧 #11918

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants