Edits to Getting Started and Basic Info docs #177
Conversation
|
can you link to the original PR with the comments? i have no idea where those are now lol. |
| @@ -5,36 +5,41 @@ order: 2 | |||
| --- | |||
|
|
|||
| <div class="section-content"> | |||
There was a problem hiding this comment.
this section is so so good!
docs/_advanced/authorization.md
Outdated
|
|
||
| const authorizeFn = async (source, body) => { | ||
| const authorizeFn = async ({ teamId, enterpriseId }) => { | ||
| // Fetch team info from database. You could also set userToken instead. | ||
| const fetchAuthorizedTeam = new Promise((resolve, reject) => { | ||
| teamInfo[source.enterpriseId][source.teamId] !== undefined ? |
There was a problem hiding this comment.
this no longer works. needs to be changed to iterate over the installations array and find any authorization that makes both the given teamId and enterpriseId. that's my bad. this is where i stopped my previous review and never was able to finish.
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #177 +/- ##
=======================================
Coverage 48.86% 48.86%
=======================================
Files 7 7
Lines 440 440
Branches 120 120
=======================================
Hits 215 215
Misses 213 213
Partials 12 12Continue to review full report at Codecov.
|
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
Co-Authored-By: shanedewael <[email protected]>
docs/_advanced/authorization.md
Outdated
| // Check for matching teamId and enterpriseId in the installations array | ||
| if ((team.teamId === teamId) && (team.enterpriseId === enterpriseId)) { | ||
| // This is a match. Use these installaton credentials. | ||
| Promise.resolve(team); |
There was a problem hiding this comment.
sorry, this is a nitpick. can we do something like return { botToken: team.botToken, botId: team.botId, botUserId: team.botUserId }; instead? first, wrapping in Promise.resolve() is unnecessary since we are in an async function. second, i think it might be important to "show our work" and that the naming of the properties is explicit and important (not just rely on the coincidence that the persistence model happens to also use a compatible shape).
docs/_advanced/authorization.md
Outdated
| } | ||
| } | ||
|
|
||
| Promise.reject(); |
There was a problem hiding this comment.
throw an error instead of using Promise.reject().
Summary
Went through and make edits based on @aoberoi suggestions. Also made a pass through the Getting Started guide.
Requirements (place an
xin each[ ])