Skip to content
This repository was archived by the owner on Nov 5, 2025. It is now read-only.

Comments

[WIP] Redesign webhooks#158

Merged
d-gubert merged 12 commits intoRocketChat:redesign-webhooksfrom
shiqimei:redesign-webhooks
Oct 24, 2019
Merged

[WIP] Redesign webhooks#158
d-gubert merged 12 commits intoRocketChat:redesign-webhooksfrom
shiqimei:redesign-webhooks

Conversation

@shiqimei
Copy link
Contributor

@shiqimei shiqimei commented Oct 8, 2019

Closes #154

@codecov
Copy link

codecov bot commented Oct 8, 2019

Codecov Report

Merging #158 into alpha will increase coverage by 1.41%.
The diff coverage is 7.4%.

Impacted file tree graph

@@            Coverage Diff             @@
##            alpha     #158      +/-   ##
==========================================
+ Coverage   57.67%   59.09%   +1.41%     
==========================================
  Files          65       73       +8     
  Lines        2266     2403     +137     
  Branches      350      356       +6     
==========================================
+ Hits         1307     1420     +113     
- Misses        959      983      +24
Impacted Files Coverage Δ
src/server/managers/AppListenerManager.ts 4.1% <0%> (-0.41%) ⬇️
src/server/compiler/AppImplements.ts 100% <100%> (ø) ⬆️
src/server/errors/index.ts 100% <0%> (ø)
src/server/managers/index.ts 100% <0%> (ø)
src/server/accessors/index.ts 100% <0%> (ø)
src/server/marketplace/license/index.ts 100% <0%> (ø)
src/server/compiler/index.ts 100% <0%> (ø)
src/server/storage/index.ts 100% <0%> (ø)
src/server/logging/index.ts 100% <0%> (ø)
src/server/bridges/index.ts 100% <0%> (ø)

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 50a4c95...c2c225e. Read the comment docs.

@shiqimei
Copy link
Contributor Author

shiqimei commented Oct 8, 2019

@graywolf336 @d-gubert Should we implement PreEventModify and PreEventExtend handlers for both ExternalComponentOpened and ExternalComponentClosed events?
IMO, the PreEventPrevent and PostEvent handlers are enough. If we give app developers APIs to modify or extend external components in PreEventModify and PreEventExtend handlers, it may cause security concerns (He can easily change the external component's URL to an untrusted one).

Love to hear your opinions on this 😃

@shiqimei shiqimei added the ⏳ in progress Items which are currently a work in progress label Oct 9, 2019
* Remove IWebHooks
* Remove webhooks options from IExternalComponentOptions and redesign it

Introduce state for external components

Add IPreExternalComponentOpenedPrevent handler

Add IPostExternalComponentOpened handler
* add accessors for IPreExternalComponentOpenedPrevent handler
* Add IPostExternalComponentOpened handler

Add IPostExternalComponentClosed handler

Remove IPreExternalComponentOpenedPrevent
* Remove IWebHooks
* Remove webhooks options from IExternalComponentOptions and redesign it

Introduce state for external components

Add IPreExternalComponentOpenedPrevent handler

Add IPostExternalComponentOpened handler
* add accessors for IPreExternalComponentOpenedPrevent handler
* Add IPostExternalComponentOpened handler

Add IPostExternalComponentClosed handler

Remove IPreExternalComponentOpenedPrevent
@shiqimei
Copy link
Contributor Author

This PR depends on #165 and RocketChat/Rocket.Chat#15618.

@d-gubert d-gubert changed the base branch from alpha to redesign-webhooks October 24, 2019 20:41
@d-gubert d-gubert merged commit 24030cb into RocketChat:redesign-webhooks Oct 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

⏳ in progress Items which are currently a work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants