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

修复工单审核不通过依然待审核展示的问题 #2712

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

woshiyanghai
Copy link
Contributor

@woshiyanghai woshiyanghai commented Jul 2, 2024

fix #2699

Copy link

codecov bot commented Jul 2, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 77.12%. Comparing base (4ac9fad) to head (8560e7e).
Report is 5 commits behind head on master.

Files Patch % Lines
sql/utils/workflow_audit.py 20.00% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2712   +/-   ##
=======================================
  Coverage   77.11%   77.12%           
=======================================
  Files         117      117           
  Lines       16170    16179    +9     
=======================================
+ Hits        12470    12478    +8     
- Misses       3700     3701    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LeoQuote
Copy link
Collaborator

LeoQuote commented Jul 3, 2024

pr 来迟了, 先 close 了, 有问题可以在这里讨论, 欢迎提出你的宝贵意见 #2692

@LeoQuote LeoQuote closed this Jul 3, 2024
@LeoQuote LeoQuote reopened this Jul 3, 2024
Copy link
Collaborator

@LeoQuote LeoQuote left a comment

Choose a reason for hiding this comment

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

如果工单是自动审核的,会在上方的 is auto review 逻辑中提前返回,所以你的逻辑需要提前到最前方才可行

可以考虑写成 is autoreview error return

@woshiyanghai
Copy link
Contributor Author

确实有这个问题,修改一下。

@woshiyanghai
Copy link
Contributor Author

自动审核不通过的时候,不再走自动审核逻辑
这里描述有点不清楚了,应该是sql检测不通过时,不再走自动审核逻辑

Copy link
Collaborator

@LeoQuote LeoQuote left a comment

Choose a reason for hiding this comment

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

这个逻辑写得复杂了, 你就写 if status == workflow_autoreviewwrong, 就自动驳回然后 提前return, 后面的 自动通过逻辑不变即可

@woshiyanghai
Copy link
Contributor Author

这个逻辑写得复杂了, 你就写 if status == workflow_autoreviewwrong, 就自动驳回然后 提前return, 后面的 自动通过逻辑不变即可

直接return的话,如果不记录sql_audit的话,查看工单detail的时候有点问题,而且工单没有日志记录,所以没有直接return,还是记录了audit 和audit_log 。

@woshiyanghai
Copy link
Contributor Author

这个逻辑写得复杂了, 你就写 if status == workflow_autoreviewwrong, 就自动驳回然后 提前return, 后面的 自动通过逻辑不变即可

我理解大佬你的意思是直接return 就不用在记录audit 和audit_log 了是么?可能我的理解不对,感谢大佬的指导

@LeoQuote
Copy link
Collaborator

LeoQuote commented Jul 4, 2024

还是正常记录 audit 的, 只是提前 return, 这样下面的逻辑就不会走了

if status == workflow_autoreviewwrong:
    # 记录 audit
    # 记录 audit log
    return "工单已自动驳回"

这样你即使后面写 if audit_setting.auto_pass: 也能正常驳回

你看看这么写是否符合预期

@LeoQuote
Copy link
Collaborator

LeoQuote commented Jul 4, 2024

辛苦再写一点单元测试, 测试一下自动驳回的工单表现是否正常, 就可以了

@woshiyanghai
Copy link
Contributor Author

辛苦再写一点单元测试, 测试一下自动驳回的工单表现是否正常, 就可以了

单元测试不会写,我在我项目部署测试了一下,功能正常

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

Successfully merging this pull request may close these issues.

自动审核不通过,但是在待审核工单列表中依然可以看到工单
2 participants