Skip to content

Conversation

wxiaoguang
Copy link
Contributor

https://developers.dingtalk.com/document/app/message-link-description

To open the link in browser, we should use this URL: "dingtalk://dingtalkclient/page/link?pc_slide=false&url=" + url.QueryEscape(singleURL)

Otherwise the page is displayed inside DingTalk client, it makes users very difficult to visit non-public URLs in DingTalk webhook messages.

The patch is tested and it works as expected.

@wxiaoguang wxiaoguang force-pushed the dingtalk-link-in-browser branch from 36a5aa7 to cc3d6a3 Compare September 18, 2021 09:10
@a1012112796
Copy link
Member

@wxiaoguang please update the test code.
image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 18, 2021
@codecov-commenter
Copy link

codecov-commenter commented Sep 18, 2021

Codecov Report

Merging #17084 (ca0e642) into main (ea207f6) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17084      +/-   ##
==========================================
- Coverage   45.27%   45.23%   -0.04%     
==========================================
  Files         771      771              
  Lines       86767    86767              
==========================================
- Hits        39285    39253      -32     
- Misses      41130    41165      +35     
+ Partials     6352     6349       -3     
Impacted Files Coverage Δ
services/webhook/dingtalk.go 76.34% <100.00%> (ø)
models/repo_mirror.go 58.69% <0.00%> (-10.87%) ⬇️
services/mirror/mirror.go 8.19% <0.00%> (-6.56%) ⬇️
modules/queue/queue_bytefifo.go 58.08% <0.00%> (-5.39%) ⬇️
modules/cron/tasks_basic.go 85.43% <0.00%> (-2.92%) ⬇️
modules/git/repo_base_nogogit.go 82.85% <0.00%> (-2.86%) ⬇️
modules/git/utils.go 68.05% <0.00%> (-2.78%) ⬇️
models/unit.go 41.09% <0.00%> (-2.74%) ⬇️
modules/queue/workerpool.go 48.09% <0.00%> (-0.77%) ⬇️
models/issue_comment.go 51.17% <0.00%> (-0.59%) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea207f6...ca0e642. Read the comment docs.

@wxiaoguang
Copy link
Contributor Author

Unit tests are fixed.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 18, 2021
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 18, 2021
@zeripath zeripath merged commit 6532aa2 into go-gitea:main Sep 18, 2021
@wxiaoguang wxiaoguang deleted the dingtalk-link-in-browser branch September 22, 2021 06:08
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/webhooks type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants