-
Notifications
You must be signed in to change notification settings - Fork 313
New extension: Webhooks (TurboHook replacement) #1873
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
base: master
Are you sure you want to change the base?
Conversation
SharkPool-SP
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems good now
yuri-kiss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
last change then im gonna review the docs
GarboMuffin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to await prompt() so that it works in the desktop app
ill read the rest of it later today
OHHHH SO THAT'S WHY IT BROKE IN THE DESKTOP APP DURING MY TESTING I'll get to fixing it |
…ense since it wont be null when restoring storage that already exists
|
|
dont like how empty try/catch is being used everywhere to handle errors as it is right now a project can make a block that contains something like |
|
TurboHook still has functionality that this doesn't have, specifically the ability to send to arbitrary webhooks not known until runtime |
@GarboMuffin I think I mentioned this in passing a year ago during TurboHook, but I had an idea where we could have our own webhook system as well to allow users to make their own Webhooks and receive data. We could incorporate this with this extension. |
Yes, I'm aware it was already possible, I'm just remarking at how much better this extension is than TurboHook I think webhooks in both directions would be fantastic |
This comment was marked as abuse.
This comment was marked as abuse.
|
Using this pull request to test something with the bot !format |
|
The formatting bot didn't find any formatting issues. It currently only checks the extensions folder. To format all files, the pull request's author can manually run the 'Format pull request' workflow from the 'Actions' tab on the fork. |
Dismissing old approvals so that moving forward everything follows #2327
|
What's the status of this PR as of right now? |
|
You need to restart the review process cuz if the new rules |
|
|
!format |
|
I'll have to take a look tomorrow |
Brackets-Coder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I verified that the extension follows all acceptance criteria and guidelines in CONTRIBUTING.md.
-
I believe the block list is well-designed and intuitive. - see note on hidden blocks, otherwise it's fine
-
I tested all functionality and found no bugs, security flaws, or unintuitive behavior. - haven't tested this yet, this is a skim review
-
I reviewed the code and believe it is well-written and easy to maintain or extend in the future, even without help from the original author.
-
I have no additional concerns.
If you could clarify some of my confusion I'd appreciate it, thanks so much!
|
@Brackets-Coder Have you even tried loading the extension into TurboWarp, lol?
|
|
If anything, the video attached to this PR shows exactly how this works. |
|
I have in the past and I did just now, the review a few minutes ago was a quick skim Sorry for the confusion about the hidden blocks My concern about the security of sending user data still applies. Am I simply misunderstanding something again? Can the JSON parsing be exploited or is basically everything sanitized before posting? (Of course, most URLs would also have validation on their end...) |
|
This extension only sends data to the defined URLs. There is no security needing to be done on our part, because we're assuming the URLs that get data pushed through accept only the data that it needs (like usual) |
Hello all!
The time has finally come, the long awaited successor to the horrendous extension that is "TurboHook". Under code name "TurboHook V2", this extension took time to develop as I waited for certain APIs to happen (extension storage my beloved), and also just forgetting about the extension in the first place 😅. I deemed it smart to make this a whole new extension entirely, and TurboHook will now be hidden.
Video Example:
2025-01-16.23-49-59.mp4
Honestly, I'm going to consider this extension my Magnum Opus at the moment, since this represents my growth since I first contributed here. The difference between TurboHook and this extension is insane.