docs(ui): trim the Slack disconnect test and fixture comments

- Drop assertion narration and keep contract facts stated once
This commit is contained in:
Pablo F.G
2026-08-18 17:46:57 +02:00
parent da166c04e6
commit 652c950a71
4 changed files with 46 additions and 82 deletions
+14 -21
View File
@@ -81,10 +81,10 @@ export interface SlackRefusalFixture {
/**
* What `DELETE /integrations/{id}` reports about revoking the token at Slack.
* Revocation is best-effort: the row goes either way, and the outcome travels in
* JSON:API `meta` so the UI can say when access still needs removing by hand.
* JSON:API `meta`.
*
* One boolean is the whole of it. The API sends no reason for a revocation that
* did not happen, so modelling one here would let a test prove copy the real
* One boolean is the whole of it: the API sends no reason for a revocation that
* did not happen, so modelling one would let a test prove copy the real
* deployment can never produce.
*/
export interface SlackRevocationFixture {
@@ -139,7 +139,6 @@ export interface SlackFixture {
*/
channelSaveRefusal: SlackRefusalFixture | null;
testMessage: SlackTestMessageFixture;
/** What disconnecting reports about revoking the token at Slack. */
revocation: SlackRevocationFixture;
}
@@ -204,9 +203,8 @@ export const INTEGRATIONS_SERVER_ERROR_DETAIL = "A server error occurred.";
export const SLACK_MISSING_SCOPE_DETAIL =
"Slack refused the request: missing_scope.";
/**
* The API wording a dead grant arrives with. It names the raw reason, the same
* way the missing-scope one does — which is what lets a test tell "the UI read
* `code` and used its own copy" apart from "the UI echoed `detail`".
* Names the raw reason, as the missing-scope wording does: what lets a test tell
* copy the UI mapped from `code` apart from an echoed `detail`.
*/
export const SLACK_TOKEN_EXPIRED_DETAIL =
"Slack refused the request: token_expired.";
@@ -248,9 +246,9 @@ export const SLACK_NOT_IN_CHANNEL_CODE = "not_in_channel";
*/
export const SLACK_UNMAPPED_REASON_CODE = "is_archived";
/**
* The two dead-grant codes the tests drive with, out of the four the contract
* lists. Whichever call surfaces one, the integration is disconnected and the
* only way out is connecting the workspace again (contract, Cross-cutting).
* Two of the four dead-grant codes the contract lists. Whichever call surfaces
* one, the integration is disconnected and the only way out is connecting the
* workspace again (contract, Cross-cutting).
*/
export const SLACK_TOKEN_REVOKED_CODE = "token_revoked";
export const SLACK_TOKEN_EXPIRED_CODE = "token_expired";
@@ -277,9 +275,9 @@ export const SLACK_RATE_LIMITED_REFUSAL: SlackRefusalFixture = {
};
/**
* The stored grant is no longer usable. A `400` like any other actionable
* refusal deliberately not a `401`, which would read as "your Prowler session
* expired" (contract, Errors) — and the integration is marked disconnected.
* The stored grant is no longer usable: a `400` like any other actionable
* refusal, deliberately not the `401` that would read as an expired Prowler
* session (contract, Errors).
*/
export const SLACK_TOKEN_EXPIRED_REFUSAL: SlackRefusalFixture = {
status: 400,
@@ -458,9 +456,7 @@ export const configuredSlackFixture = (
/**
* A connected tenant whose disconnect removes the row but cannot revoke at
* Slack — the outcome the user has to finish by hand in the workspace. The API
* reports it as `revoked: false` and says no more than that, which is exactly
* as much as the UI can honestly tell them.
* Slack — the outcome the user has to finish by hand in the workspace.
*/
export const revokeFailureSlackFixture = (
overrides: Partial<SlackFixture> = {},
@@ -472,8 +468,7 @@ export const revokeFailureSlackFixture = (
/**
* A connected tenant whose disconnect answers a plain `204` with no body: the
* row is gone and the revocation is unreported. Neither a confirmed revocation
* nor a failed one, so the UI can only say the workspace is no longer connected.
* row is gone and the revocation is unreported.
*/
export const unreportedRevocationSlackFixture = (
overrides: Partial<SlackFixture> = {},
@@ -485,9 +480,7 @@ export const unreportedRevocationSlackFixture = (
/**
* A connected tenant whose token has been revoked at Slack: the row still says
* connected until a check runs, and the check is what surfaces it. The check
* itself needs a destination channel on record — the API refuses to test one
* that has none — so this builds on the finished setup, not the bare install.
* connected until a check runs, and the check is what surfaces it.
*/
export const revokedTokenSlackFixture = (
overrides: Partial<SlackFixture> = {},
+2 -9
View File
@@ -368,15 +368,8 @@ export const handlersForSlack = (fx: SlackFixture) => {
),
// Disconnect. Revocation at Slack is best-effort: the row is removed either
// way and the outcome is reported in `meta`, so the UI can tell the user
// when access still has to be removed by hand in the workspace.
//
// `revoked` is the entire outcome. The API sends no reason for a revocation
// that failed, and a handler that invented one would let the page grow copy
// around a field the deployment never sends.
//
// A `null` outcome is the plain `204` a deployment that overrides nothing
// sends: no body, so no `meta` to read the outcome from.
// way and the outcome travels in `meta` — or nowhere at all, in the plain
// `204` a deployment with no `destroy` override sends.
http.delete(`${API}/integrations/:id`, () => {
install = null;
if (fx.revocation.revoked === null) {
@@ -40,7 +40,6 @@ export type TestMessageOutcome =
/** Sentinel: the page settled on "no channel recorded", rather than not yet. */
const NO_DEFAULT_CHANNEL = "<no channel recorded>";
/** Whether disconnecting also revoked Prowler's token at Slack. */
export const REVOCATION_OUTCOME = {
REVOKED: "revoked",
NOT_REVOKED: "not-revoked",
@@ -695,15 +694,15 @@ export class SlackIntegrationHarness extends BrowserHarness<SlackFixture> {
/**
* Disconnects the workspace, confirming the way a user has to, and reports
* what the page says about the revocation — the three outcomes are mutually
* exclusive, so asking for one is also a check that the others are absent.
* what the page says about the revocation. The outcomes are mutually
* exclusive, so asking for one also checks the others are absent.
*
* The two reported-by-toast outcomes share a title, so each is read from its
* own description: a title match alone would agree with either wording.
* The revoked and unreported outcomes share a toast title, so each is read
* from its own description: a title match would agree with either.
*/
async disconnect(): Promise<RevocationOutcome> {
// The card's action opens the confirmation; the dialog's own button carries
// the noun too, so the two never resolve to each other.
// The dialog's own button carries the noun too, hence the exact match on
// the card's action.
await this.clickButton(/^\s*Disconnect\s*$/);
await this.clickButton(/Disconnect workspace/);
@@ -654,35 +654,27 @@ describe("disconnecting a workspace", () => {
const harness = new SlackIntegrationHarness(connectedSlackFixture());
await harness.mount();
// When — the user disconnects and confirms.
// Then — Slack confirmed the revocation, so the user is told the access is
// gone and nothing warns them to finish the job by hand.
// When — the user disconnects and confirms; Slack confirms the revocation.
expect(await harness.disconnect()).toBe(REVOCATION_OUTCOME.REVOKED);
// And the integration is gone, with the page offering a fresh install.
expect(harness.disconnectCallCount).toBe(1);
expect(await harness.returnedToUnconnectedState()).toBe(true);
}, 30000);
it("still removes the integration when the revocation fails, and says access may need removing by hand", async () => {
// Given — Slack will not accept the revocation. Revocation is best-effort:
// the row goes either way and the outcome travels in `meta` as the single
// boolean the API sends — there is no reason alongside it.
// Given — Slack will not accept the revocation; the row goes either way.
const harness = new SlackIntegrationHarness(revokeFailureSlackFixture());
await harness.mount();
// When
expect(await harness.disconnect()).toBe(REVOCATION_OUTCOME.NOT_REVOKED);
// And — the disconnect revalidates the page, so the server data refreshes
// under the notice and comes back with no integration at all: the record
// the workspace was read from is gone by the time the user reads about it.
// And — the disconnect revalidates, so the copy below is read from props
// that no longer carry an integration at all.
await harness.refreshPageData();
// Then — the user reads what is true of both sides: nothing is left in
// Prowler to retry, and the app may still be installed at Slack. Saying
// "there is nothing to retry here" is the point — the one thing a user
// reaches for after a failure is the thing that cannot help.
// Then — what is true of both sides: nothing is left in Prowler to retry,
// and the app may still be installed at Slack.
const notice = await harness.revocationNotice();
expect(notice).toMatch(/gone from Prowler/);
expect(notice).toMatch(/nothing to retry here/);
@@ -690,15 +682,12 @@ describe("disconnecting a workspace", () => {
expect(notice).toMatch(
/remove it from that workspace's Slack app settings/,
);
// The row is removed regardless, so the page does not keep offering a
// workspace that no longer exists here.
expect(await harness.returnedToUnconnectedState()).toBe(true);
}, 30000);
it("says only that the workspace is no longer connected when nothing reports the revocation", async () => {
// Given — the disconnect answers a plain `204` with no body, so there is no
// `meta` to read the outcome from. A deployment that overrides nothing
// answers exactly this, which makes it the outcome users really meet.
// Given — the plain `204` a deployment that overrides nothing answers: no
// body, so no `meta` to read the outcome from. The case users really meet.
const harness = new SlackIntegrationHarness(
unreportedRevocationSlackFixture(),
);
@@ -707,9 +696,8 @@ describe("disconnecting a workspace", () => {
// When
expect(await harness.disconnect()).toBe(REVOCATION_OUTCOME.UNREPORTED);
// Then — an unreported outcome is neither a confirmed revocation nor a
// failed one, so the page claims neither: nothing sends the user to Slack
// to finish a job no answer said was unfinished.
// Then — nothing sends the user to Slack to finish a job no answer said
// was unfinished.
expect(harness.showsRevocationNotice()).toBe(false);
expect(await harness.returnedToUnconnectedState()).toBe(true);
}, 30000);
@@ -725,9 +713,8 @@ describe("a credential Slack no longer accepts", () => {
// When
expect(await harness.testConnection()).toBe(CONNECTION_OUTCOME.FAILURE);
// Then — what died, in Prowler's words, and a way forward rather than only
// an error: a revoked token is fixed by approving Prowler again, not by
// checking a second time.
// Then — a way forward rather than only an error: a revoked token is fixed
// by approving Prowler again, not by checking a second time.
const notice = await harness.revokedCredentialNotice();
expect(notice).toMatch(/no longer accepts Prowler's access to Prowler HQ/);
expect(notice).toMatch(/Prowler's access to Slack was revoked/);
@@ -745,8 +732,8 @@ describe("a credential Slack no longer accepts", () => {
it("offers the same recovery when the channel listing is what finds the credential dead", async () => {
// Given — a finished setup whose credential expired. The listing runs on
// arrival, so it, not the connection check, is what meets Slack first —
// and the contract says any call can be the one that surfaces this.
// arrival, so it meets Slack before any check does, and the contract says
// any call can be the one that surfaces this.
const harness = new SlackIntegrationHarness(
configuredSlackFixture({ channelsRefusal: SLACK_TOKEN_EXPIRED_REFUSAL }),
);
@@ -754,26 +741,23 @@ describe("a credential Slack no longer accepts", () => {
// When — nothing but opening the page.
await harness.mount();
// Then — the same answer as the connection check gives, worded for the way
// this credential died, and not left as a channel problem the user would
// go looking for a channel fix for.
// Then — the same answer the connection check gives, worded for how this
// credential died rather than left as a channel problem.
const notice = await harness.revokedCredentialNotice();
expect(notice).toMatch(/Prowler's Slack credential has expired/);
expect(notice).toMatch(/Connect the workspace again to restore access/);
expect(harness.offersReconnect()).toBe(true);
// And the picker says the same thing, in the same words: the API's own
// `detail` names the raw reason, and it is `code` the UI answered from.
// And the picker says the same, in the same words: `detail` names the raw
// reason, and it is `code` the UI answered from.
const message = await harness.channelPickerMessage();
expect(message).toMatch(/Prowler's Slack credential has expired/);
expect(message).not.toMatch(new RegExp(SLACK_TOKEN_EXPIRED_CODE));
}, 30000);
it("offers it too when only a later cursor page is what Slack refuses", async () => {
// Given — a two-page workspace whose first page reads fine and whose second
// is refused by a credential Slack no longer accepts. The read stopped
// short of the workspace rather than failing, and a grant that has stopped
// working refuses page two exactly as it refuses page one.
// Given — a two-page workspace whose second page is refused by a credential
// Slack no longer accepts: the read stops short rather than failing.
const harness = new SlackIntegrationHarness(
partiallyReadSlackFixture({
channelsRefusal: SLACK_TOKEN_EXPIRED_REFUSAL,
@@ -794,15 +778,12 @@ describe("a credential Slack no longer accepts", () => {
const notice = await harness.revokedCredentialNotice();
expect(notice).toMatch(/Prowler's Slack credential has expired/);
expect(harness.offersReconnect()).toBe(true);
// The row still reads connected until something says otherwise, and this
// is that something.
expect(await harness.connectionBadge()).toBe("Disconnected");
}, 60000);
it("keeps saying so when a later check fails without Slack naming a reason", async () => {
// Given — the listing found the credential dead on arrival, and a
// connection check that settles as failed naming nothing: the generic check
// reports its own wording for a failure Slack never answered.
// Given — the listing found the credential dead on arrival, and a later
// check that fails naming no reason at all.
const harness = new SlackIntegrationHarness(
configuredSlackFixture({
channelsRefusal: SLACK_TOKEN_EXPIRED_REFUSAL,
@@ -814,13 +795,11 @@ describe("a credential Slack no longer accepts", () => {
/Prowler's Slack credential has expired/,
);
// When — the user checks the connection, and the check fails saying nothing
// about the credential.
// When
expect(await harness.testConnection()).toBe(CONNECTION_OUTCOME.FAILURE);
// Then — a failure Slack never answered is no evidence the grant works
// again: the dead credential is still what the page reports, and the way
// out of it is still a click away.
// again, so the dead credential is still what the page reports.
expect(await harness.revokedCredentialNotice()).toMatch(
/Prowler's Slack credential has expired/,
);