Port Slack Events Bot into Laravel#570
Port Slack Events Bot into Laravel#570oliviasculley wants to merge 19 commits intohackgvl:developfrom
Conversation
oliviasculley
force-pushed
the
slack-events-bot
branch
from
cd36c16 to
0a93a1e
Compare
|
After chatting with @irby, we should probably hash+salt the access tokens like user passwords, and then I think we still need to implement the check_api action to happen automatically every hour, I'll probably do more of that later. However, if someone else has a test setup they're welcome to message me and push to my branch! |
|
I had a brain fart, we're probably not going to want to hash the access token, I'll probably just remove that table later since I don't think we're going to need it for anything |
|
So turns out the current implementation working in our workspace might've been a bit misleading, since we were relying on our bot token rather than the installation-specific tokens to access the Slack API. Even though it was working in my testing, I think we need to actually use the installation-specific access tokens for each installation otherwise it would break for other installations, so I need to test this with my current implementation as well. I'm currently trying to get the tests to pass and this should be ready after that I think |
oliviasculley
force-pushed
the
slack-events-bot
branch
2 times, most recently
from
0be5cf7 to
0f7f19e
Compare
app-modules/slack-events-bot/tests/Services/MessageBuilderServiceTest.php
Show resolved
Hide resolved
|
All the unresolved comments are items that I still need to get to with this PR (or that someone else can get to), but I'll hopefully get to these soon |
irby
left a comment
irby
left a comment
There was a problem hiding this comment.
Code-wise I'm fine with everything here.
oliviasculley
force-pushed
the
slack-events-bot
branch
2 times, most recently
from
c011f44 to
f28602a
Compare
oliviasculley
force-pushed
the
slack-events-bot
branch
from
f28602a to
df68940
Compare
There was a problem hiding this comment.
Our production is still on older versions of PHP and Node.
Once we move things off to Railway, we shouldn't have an issue with versions.
Is changing this likely to mess with anything on the npm or composer packages?
There was a problem hiding this comment.
With this and the api.php routes files, do we need to put /slack/ in the route, or is that somehow implied?
There was a problem hiding this comment.
What's the deal with this /vendor directory and all the autoloading being in version control? It seems /vendor is typically ignored?
There was a problem hiding this comment.
I see a /docs directory was created, but it looks like there's still a README.md in the Slack module as well, so that one is probably a duplicate.
We'll also want to link to this in the main README.md and the links in the main README.md will need to be updated for the Orgs API and Events API links in that file. https://github.com/hackgvl/hackgreenville-com
4 participants
This is heavily based on #540 but should take us all the way to a fully working slack bot implementation. Thank you @bogdankharchenko!!!
Also updates to PHP 8.3.6 as part of this PR, it will need the CI scripts changed to 8.3 before this PR will pass
In addition, a new worker docker instance will need to be configured to run post refreshes