docs(ui): trim the Slack disconnect action and unit-test comments

- Keep only the non-obvious why, stated once at its load-bearing site
This commit is contained in:
Pablo F.G
2026-08-18 17:46:57 +02:00
parent e78c128de4
commit ba64cf35b8
2 changed files with 25 additions and 40 deletions
+3 -8
View File
@@ -352,7 +352,7 @@ const channelOptions = (count: number) =>
const RATE_LIMITED_MESSAGE =
"Slack is rate limiting Prowler right now. Try again in about 30 seconds.";
/** A dead grant, as the API reports one: the reason in `code`, prose in `detail`. */
/** A dead grant as the API reports it: reason in `code`, prose in `detail`. */
const TOKEN_EXPIRED_CODE = "token_expired";
const TOKEN_EXPIRED_DETAIL = "Slack refused the request: token_expired.";
const TOKEN_EXPIRED_MESSAGE =
@@ -452,8 +452,7 @@ describe("getSlackChannels", () => {
const result = await getSlackChannels(SLACK_INTEGRATION_ID);
// Slack named no reason for the wait, so the truncation carries none: a
// rate limit says nothing about the grant itself.
// A rate limit says nothing about the grant, so the truncation names none.
expect(result).toEqual({
channels: [channelOption(FIRST_CHANNEL)],
incomplete: RATE_LIMITED_MESSAGE,
@@ -462,9 +461,6 @@ describe("getSlackChannels", () => {
});
it("names the reason a later page was refused, not only the wording", async () => {
// A grant that has stopped working refuses the second cursor page exactly
// as it refuses the first (contract, Cross-cutting) — and the read is a
// success either way, so `code` is the only way the caller can hear it.
fetchMock
.mockResolvedValueOnce(channelPage(FIRST_CHANNEL, "?page[cursor]=2"))
.mockResolvedValueOnce(
@@ -662,7 +658,6 @@ describe("sendSlackTestMessage", () => {
expect(result).toEqual({
error: SLACK_ERROR_MESSAGES[SLACK_ERROR_CODE.NOT_IN_CHANNEL],
// The reason travels on as the class it is, not only as its sentence.
code: SLACK_ERROR_CODE.NOT_IN_CHANNEL,
});
});
@@ -765,7 +760,7 @@ describe.each(COPY_ONLY_ACTIONS)("$name", ({ call }) => {
expect(captureExceptionMock).not.toHaveBeenCalled();
expect(captureMessageMock).not.toHaveBeenCalled();
// `code` travels alongside the copy; none of these refusals names one.
// None of these refusals names a `code`.
expect(result).toEqual({ error: refusal.expected, code: null });
});
});
+22 -32
View File
@@ -44,12 +44,11 @@ interface SlackUnconfirmed {
interface SlackActionError {
error: string;
/**
* The refusal's `code`, when it named one, alongside the copy.
*
* A caller reads it to recognise a *class* of failure the wording cannot be
* pattern-matched for — a Slack grant that has stopped working, which the
* contract says can surface from any of these calls (Cross-cutting) and which
* is recovered from by reconnecting rather than by retrying.
* The refusal's `code`, when it named one, alongside the copy. A caller reads
* it to recognise a class of failure the wording cannot be pattern-matched
* for — a Slack grant that has stopped working, which the contract allows
* from any of these calls (Cross-cutting) and is recovered from by
* reconnecting rather than by retrying.
*/
code?: string | null;
}
@@ -186,11 +185,9 @@ const failureFrom = async (
/**
* `failureFrom` flattened to one refusal, for the calls whose only outcome is
* "it did not work" — with the `code` carried alongside, unworded, for the
* caller that has to act on the class rather than show the sentence. Rate
* limiting keeps its own wording: `conversations.list` is Slack tier 2, so a
* `429` shows up here (contract, Errors) and the wait it names is the useful
* part.
* "it did not work". Rate limiting keeps its own wording: `conversations.list`
* is Slack tier 2, so a `429` shows up here (contract, Errors) and the wait it
* names is the useful part.
*/
const refusalFrom = async (
response: Response,
@@ -518,8 +515,6 @@ export const sendSlackTestMessage = async (
const response = await fetch(url.toString(), { method: "POST", headers });
if (!response.ok) {
// Awaited inside the `try`: unawaited, a 5xx's rejection would skip
// this `catch`.
return await refusalFrom(
response,
`Unable to send the test message: ${response.statusText}`,
@@ -556,8 +551,8 @@ export const sendSlackTestMessage = async (
{ code: reason },
slackUnknownReasonMessage(reason),
),
// A dead grant can surface here as much as anywhere else, so the
// reason travels on as the class it is, not only as its sentence.
// A dead grant can surface here too, so the reason travels on as
// a `code`, not only as its sentence.
code: reason,
}
: { error: reason };
@@ -574,16 +569,15 @@ export const sendSlackTestMessage = async (
/**
* What the API reports about revoking Prowler's token at Slack: one boolean in
* `meta`, and nothing else. The API sends no reason for a revocation that did
* not happen, so there is none to report — and a UI that invented a place to
* put one would be promising the user an explanation it can never fill in.
* `meta`, and nothing else — it sends no reason for a revocation that did not
* happen, so there is no field here to hold one.
*/
export interface SlackRevocation {
/**
* Whether Slack confirmed the token no longer grants Prowler anything, or
* `null` when the response carried no outcome at all. The contract says the
* outcome is always reported, so `null` means the response is wrong rather
* than the revocation — and neither answer is claimed on the user's behalf.
* `null` when the response carried no outcome. The contract says the outcome
* is always reported, so `null` means the response is wrong, not the
* revocation.
*/
revoked: boolean | null;
}
@@ -600,16 +594,14 @@ export type SlackDisconnectResult = SlackDisconnectSuccess | SlackActionError;
* Disconnect the workspace: `DELETE /integrations/{id}`.
*
* The generic `deleteIntegration` cannot serve this: it discards the response
* body, and the whole point here is what the body carries. Revocation at Slack
* is best-effort — the row is removed either way and the outcome travels in
* JSON:API `meta` — so a caller has to be able to distinguish "gone and revoked"
* from "gone, but still installed in Slack".
* body, and the body is the whole point. Revocation at Slack is best-effort —
* the row is removed either way and the outcome travels in JSON:API `meta` — so
* a caller has to tell "gone and revoked" from "gone, but still installed in
* Slack".
*
* `revoked` is reported only as the API states it: a body without the field (an
* empty `204`, say) yields `null`, not `false`. An unreported outcome is not a
* failed revocation — it must not send the user off to clean up Slack — and it
* is not a confirmed one either, so it must not be reported as access having
* been revoked. The row is gone in all three cases, and that much is said.
* A body without the field (an empty `204`, say) yields `null`, not `false`: an
* unreported outcome must not send the user off to clean up Slack, nor be shown
* as access revoked.
*/
export const disconnectSlackIntegration = async (
integrationId: string,
@@ -624,8 +616,6 @@ export const disconnectSlackIntegration = async (
const response = await fetch(url.toString(), { method: "DELETE", headers });
if (!response.ok) {
// Awaited inside the `try`: unawaited, a 5xx's rejection would skip
// this `catch`.
return await refusalFrom(
response,
`Unable to disconnect the Slack workspace: ${response.statusText}`,