8000 Add Matrix webhook by S7evinK · Pull Request #10831 · go-gitea/gitea · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add Matrix webhook #10831

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
Merged
merged 14 commits into from
Mar 28, 2020
Merged

Add Matrix webhook #10831

merged 14 commits into from
Mar 28, 2020

Conversation

S7evinK
Copy link
Contributor
@S7evinK S7evinK commented Mar 26, 2020

This PR adds the possibility to add Matrix webhooks as requested in #6590.
Since the the URL displayed is a little bit long (spanning multiple lines), we might need to change

gitea/templates/repo/settings/webhook/list.tmpl

Line 50 in b1c331c

<a class="dont-break-out" href="{{$.BaseLink}}/{{.ID}}">{{.URL}}</a>
to contain a trimmed version of the URL to make it look better, or is there a better way to only display up to x chars of the URL?

S7evinK added 4 commits March 26, 2020 10:15
@S7evinK
Add Matrix webhook
9308796 
Signed-off-by: Till Faelligen <tfaelligen@gmail.com>
@S7evinK
Add template and related translations for Matrix hook
f40aabb 
Signed-off-by: Till Faelligen <tfaelligen@gmail.com>
@S7evinK
Add actual webhook routes and form
30fecbd 
Signed-off-by: Till Faelligen <tfaelligen@gmail.com>
@S7evinK
Add missing file 8000
36dab56 
Signed-off-by: Till Faelligen <tfaelligen@gmail.com>
@lafriks lafriks added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Mar 26, 2020
@lafriks lafriks added this to the 1.12.0 milestone Mar 26, 2020
lafriks
lafriks approved these changes Mar 26, 2020
View reviewed changes
Copy link
Member
@lafriks lafriks left a comment

Choose a reason for hiding this comment

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

Did not test but looks good ;)

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 26, 2020
@S7evinK
Copy link
Contributor Author
S7evinK commented Mar 26, 2020

I did some tests myself:
image

And like mentioned, the Webhooks overview doesn't look that nice with very long URLs:
image

guillep2k
guillep2k reviewed Mar 27, 2020
View reviewed changes
S7evinK added 4 commits March 27, 2020 07:19
@S7evinK
Use stricter regex to replace URLs
bd3496c 
Signed-off-by: Till Faelligen <tfaelligen@gmail.com>
@S7evinK
Escape url and text
dbb761a 
Signed-off-by: Till Faelligen <tfaelligen@gmail.com>
@S7evinK
Remove unnecessary whitespace
bedad37
@S7evinK
Merge branch 'add-matrix-webhook' of github.com:S7evinK/gitea into ad…
c5ec5a9 
…d-matrix-webhook
@codecov-io
Copy link
codecov-io commented Mar 27, 2020 •
edited
Loading

Codecov Report

Merging #10831 into master will decrease coverage by 0.09%.
The diff coverage is 20.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10831      +/-   ##
==========================================
- Coverage   43.49%   43.40%   -0.10%     
==========================================
  Files         592      593       +1     
  Lines       82979    83265     +286     
==========================================
+ Hits        36094    36141      +47     
- Misses      42399    42632     +233     
- Partials     4486     4492       +6
Impacted Files Coverage Δ
models/webhook.go 65.97% <0.00%> (-0.35%) ⬇️
modules/auth/repo_form.go 43.69% <0.00%> (-0.75%) ⬇️
modules/webhook/deliver.go 45.16% <0.00%> (-1.25%) ⬇️
modules/webhook/webhook.go 42.36% <0.00%> (-1.53%) ⬇️
routers/repo/webhook.go 1.11% <0.00%> (-0.12%) ⬇️
modules/webhook/matrix.go 28.88% <28.88%> (ø)
modules/setting/webhook.go 56.25% <100.00%> (ø)
routers/routes/routes.go 86.47% <100.00%> (+0.09%) ⬆️
modules/indexer/stats/db.go 40.62% <0.00%> (-18.75%) ⬇️
modules/indexer/stats/queue.go 62.50% <0.00%> (-18.75%) ⬇️
... and 11 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 7cd4704...b0c473e. Read the comment docs.

@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 Mar 27, 2020
tulir
tulir reviewed Mar 27, 2020
View reviewed changes
Copy link
Contributor
@tulir tulir 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.

Matrix events accept arbitrary JSON, so it might be nice to send some raw webhook data too. That way it could be used as a full alternative for traditional webhooks instead of just a chat thing. The only limit is that events can't be more than ~64 KiB, not sure how likely webhook data is to hit that.

Something like this:

{
  "msgtype": "m.text",
  "body": "...",
  "io.gitea.webhook": {
    "commits": [...]
  }
}

lafriks
lafriks requested changes Mar 27, 2020
View reviewed changes
Copy link
Member
@lafriks lafriks left a comment

Choose a reason for hiding this comment

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

As per comments above

@S7evinK
Copy link
Contributor Author
S7evinK commented Mar 28, 2020

Using the Authorization header feels like a dirty workaround. Couldn't figure out a better solution.

@lafriks lafriks merged commit 828a27f into go-gitea:master Mar 28, 2020
@OahzEgroeg
Copy link

Can't add matrix as default webhooks or system webooks from site administration, when select matrix, it doesn't return any form.

@techknowlogick
Copy link
Member

@EgroegOahz please create a ticket for this.

@go-gitea go-gitea locked and limited conversation to collaborators Jun 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tulir tulir tulir left review comments

@lafriks lafriks lafriks approved these changes

@guillep2k guillep2k guillep2k approved these changes

5BCB
Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.
8 participants
@S7evinK @codecov-io @OahzEgroeg @techknowlogick @lafriks @tulir @guillep2k @GiteaBot
0