ENG-2877: Update Slack integration for privacy assessments#7584
Merged
ENG-2877: Update Slack integration for privacy assessments#7584
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughAdds Slack questionnaire support to privacy assessments: polling incomplete assessments for Slack-sent questions, detecting new team-sourced answers and showing notifications, sending questionnaires to Slack via a mutation, updates answer-source labels, and sets a default Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Assessment Detail UI
participant Poll as Polling Hook
participant API as Privacy Assessment API
participant Notif as Notification System
rect rgba(100,150,200,0.5)
Note over UI,Poll: Polling for updates (e.g., every 15s)
UI->>Poll: subscribe if questionnaire sent & incomplete
Poll->>API: fetch assessment + answers
API-->>Poll: return assessment data
Poll-->>UI: deliver updated data
end
rect rgba(100,200,150,0.5)
Note over UI,Notif: Detect new team-input answers
UI->>UI: compare previous vs current team-input question IDs
alt New team-input answer detected
UI->>Notif: trigger "New answer from Slack" toast
Notif-->>UI: display notification
end
end
rect rgba(200,150,100,0.5)
Note over UI,API: Send questionnaire to Slack
UI->>API: useCreateQuestionnaireMutation (send NEEDS_INPUT questions)
API-->>UI: success / error
UI->>UI: show loading state and toast
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Contributor
Greptile SummaryThis PR replaces the modal-based Slack questionnaire flow in the privacy assessment detail page with a direct button action, adds 15-second polling for new answers, and shows toast notifications when Slack responses arrive. It also updates answer source labels and applies a temporary Key issues:
Confidence Score: 2/5
Last reviewed commit: 46d3c04 |
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx
Outdated
Show resolved
Hide resolved
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx
Outdated
Show resolved
Hide resolved
clients/admin-ui/src/features/privacy-assessments/privacy-assessments.slice.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx (1)
321-328: Consider removing commented-out code or creating a tracking issue.Commented-out code tends to become stale and confusing over time. If
RequestInputModalis no longer needed for the new flow, consider removing it entirely. If it may be needed later, create a tracking issue and remove the code—version control preserves it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx` around lines 321 - 328, Remove the commented-out RequestInputModal block in AssessmentDetail (the JSX referencing RequestInputModal, isRequestInputOpen, setIsRequestInputOpen, assessment.id, allQuestions, slackChannelName) or if it may be needed later open a tracking issue (with rationale and where it lives) and then remove the commented code so the component stays clean; ensure any necessary behavior is documented in the issue and rely on VCS to restore the snippet if required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx`:
- Around line 321-328: Remove the commented-out RequestInputModal block in
AssessmentDetail (the JSX referencing RequestInputModal, isRequestInputOpen,
setIsRequestInputOpen, assessment.id, allQuestions, slackChannelName) or if it
may be needed later open a tracking issue (with rationale and where it lives)
and then remove the commented code so the component stays clean; ensure any
necessary behavior is documented in the issue and rely on VCS to restore the
snippet if required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cea22ba3-fd92-42d9-ae61-c8ade4472d82
📒 Files selected for processing (4)
changelog/7584-slack-assessment-integration.yamlclients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsxclients/admin-ui/src/features/privacy-assessments/constants.tsclients/admin-ui/src/features/privacy-assessments/privacy-assessments.slice.ts
kruulik
commented
Mar 6, 2026
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx (1)
319-329: Consider removing or tracking the commented-out modal code.Commented-out code adds noise and can become stale. If the
RequestInputModalmight be restored later, consider tracking it in an issue rather than keeping it commented in the codebase. If it's truly obsolete, removing it would clean up the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx` around lines 319 - 329, Remove the commented-out RequestInputModal block to clean up the file or, if you intend to restore it later, delete the commented code and create a tracking issue referencing this UI piece (RequestInputModal and the related state variables isRequestInputOpen / setIsRequestInputOpen and props assessment.id, allQuestions, slackChannelName); alternatively move the modal behind a feature flag or feature branch if you want it preserved in code but inactive. Ensure any removed references (isRequestInputOpen, setIsRequestInputOpen) are also cleaned up if they become unused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx`:
- Around line 155-181: handleRequestInput can call createQuestionnaire with an
empty include_question_ids array when there are no NEEDS_INPUT questions; add a
defensive guard in handleRequestInput to compute needsInputIds (from
allQuestions.filter...map...) and return early (and show a user-friendly
warning) if needsInputIds.length === 0 so the API is never called with an empty
array, and also update the button enable/disable logic (currently using
isComplete/pendingAnswers) to base visibility/disabled state on the presence of
NEEDS_INPUT questions (i.e., the same filter used to compute needsInputIds) so
UI and action are consistent; reference symbols: handleRequestInput,
needsInputIds, allQuestions, AnswerStatus.NEEDS_INPUT, isComplete,
pendingAnswers, createQuestionnaire, include_question_ids.
---
Nitpick comments:
In `@clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx`:
- Around line 319-329: Remove the commented-out RequestInputModal block to clean
up the file or, if you intend to restore it later, delete the commented code and
create a tracking issue referencing this UI piece (RequestInputModal and the
related state variables isRequestInputOpen / setIsRequestInputOpen and props
assessment.id, allQuestions, slackChannelName); alternatively move the modal
behind a feature flag or feature branch if you want it preserved in code but
inactive. Ensure any removed references (isRequestInputOpen,
setIsRequestInputOpen) are also cleaned up if they become unused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d94db060-6f9f-4310-9729-d5c6b17f8fb0
📒 Files selected for processing (1)
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx
Comment on lines
+155
to
+181
| const handleRequestInput = async () => { | ||
| if (!slackChannelName) { | ||
| return; | ||
| } | ||
|
|
||
| const needsInputIds = allQuestions | ||
| .filter((q) => q.answer_status === AnswerStatus.NEEDS_INPUT) | ||
| .map((q) => q.question_id); | ||
|
|
||
| try { | ||
| await createQuestionnaire({ | ||
| id: assessment.id, | ||
| body: { | ||
| channel: slackChannelName, | ||
| include_question_ids: needsInputIds, | ||
| }, | ||
| }).unwrap(); | ||
| message.success(`Questions sent to ${slackChannelName} on Slack.`); | ||
| } catch (error) { | ||
| message.error( | ||
| getErrorMessage( | ||
| error as RTKErrorResult["error"], | ||
| "Failed to send questionnaire. Please try again.", | ||
| ), | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The empty questionnaire scenario is theoretically possible but requires specific conditions.
The current implementation has a logical issue: isComplete is true when there are no questions with NEEDS_INPUT status AND no answer text (pendingAnswers.length === 0 at line 75). However, handleRequestInput sends all questions with NEEDS_INPUT status regardless of whether they have answer text. If all NEEDS_INPUT questions happen to have answer text, the button would be disabled (isComplete = true) but needsInputIds could potentially be empty depending on the question lifecycle.
Adding the guard clause is good defensive programming, especially since there's no API-level validation visible in the codebase to prevent empty include_question_ids arrays. However, consider also reviewing whether the button visibility logic should be based on whether any NEEDS_INPUT questions exist, rather than on pendingAnswers (questions without both status AND text).
Proposed fix
const handleRequestInput = async () => {
if (!slackChannelName) {
return;
}
const needsInputIds = allQuestions
.filter((q) => q.answer_status === AnswerStatus.NEEDS_INPUT)
.map((q) => q.question_id);
+ if (needsInputIds.length === 0) {
+ message.info("No questions currently need input.");
+ return;
+ }
+
try {
await createQuestionnaire({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleRequestInput = async () => { | |
| if (!slackChannelName) { | |
| return; | |
| } | |
| const needsInputIds = allQuestions | |
| .filter((q) => q.answer_status === AnswerStatus.NEEDS_INPUT) | |
| .map((q) => q.question_id); | |
| try { | |
| await createQuestionnaire({ | |
| id: assessment.id, | |
| body: { | |
| channel: slackChannelName, | |
| include_question_ids: needsInputIds, | |
| }, | |
| }).unwrap(); | |
| message.success(`Questions sent to ${slackChannelName} on Slack.`); | |
| } catch (error) { | |
| message.error( | |
| getErrorMessage( | |
| error as RTKErrorResult["error"], | |
| "Failed to send questionnaire. Please try again.", | |
| ), | |
| ); | |
| } | |
| }; | |
| const handleRequestInput = async () => { | |
| if (!slackChannelName) { | |
| return; | |
| } | |
| const needsInputIds = allQuestions | |
| .filter((q) => q.answer_status === AnswerStatus.NEEDS_INPUT) | |
| .map((q) => q.question_id); | |
| if (needsInputIds.length === 0) { | |
| message.info("No questions currently need input."); | |
| return; | |
| } | |
| try { | |
| await createQuestionnaire({ | |
| id: assessment.id, | |
| body: { | |
| channel: slackChannelName, | |
| include_question_ids: needsInputIds, | |
| }, | |
| }).unwrap(); | |
| message.success(`Questions sent to ${slackChannelName} on Slack.`); | |
| } catch (error) { | |
| message.error( | |
| getErrorMessage( | |
| error as RTKErrorResult["error"], | |
| "Failed to send questionnaire. Please try again.", | |
| ), | |
| ); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx`
around lines 155 - 181, handleRequestInput can call createQuestionnaire with an
empty include_question_ids array when there are no NEEDS_INPUT questions; add a
defensive guard in handleRequestInput to compute needsInputIds (from
allQuestions.filter...map...) and return early (and show a user-friendly
warning) if needsInputIds.length === 0 so the API is never called with an empty
array, and also update the button enable/disable logic (currently using
isComplete/pendingAnswers) to base visibility/disabled state on the presence of
NEEDS_INPUT questions (i.e., the same filter used to compute needsInputIds) so
UI and action are consistent; reference symbols: handleRequestInput,
needsInputIds, allQuestions, AnswerStatus.NEEDS_INPUT, isComplete,
pendingAnswers, createQuestionnaire, include_question_ids.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx (1)
154-170:⚠️ Potential issue | 🟠 MajorGuard empty
include_question_idsand align button visibility withNEEDS_INPUT.Line 159-Line 169 can send an empty
include_question_idsarray, and Line 269 currently gates UI byisCompleteinstead of whether any question is actuallyNEEDS_INPUT. This can trigger avoidable API calls and inconsistent UX.Proposed fix
+ const needsInputIds = useMemo( + () => + allQuestions + .filter((q) => q.answer_status === AnswerStatus.NEEDS_INPUT) + .map((q) => q.question_id), + [allQuestions], + ); + const handleRequestInput = async () => { if (!slackChannelName) { return; } - - const needsInputIds = allQuestions - .filter((q) => q.answer_status === AnswerStatus.NEEDS_INPUT) - .map((q) => q.question_id); + if (needsInputIds.length === 0) { + message.info("No questions currently need input."); + return; + } try { await createQuestionnaire({ id: assessment.id, body: { channel: slackChannelName, include_question_ids: needsInputIds, }, }).unwrap(); message.success(`Questions sent to ${slackChannelName} on Slack.`); @@ - {!isComplete && ( + {!isComplete && needsInputIds.length > 0 && ( <TooltipAlso applies to: 269-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx` around lines 154 - 170, The request function handleRequestInput should not call createQuestionnaire when include_question_ids would be an empty array: compute needsInputIds from allQuestions.filter(q => q.answer_status === AnswerStatus.NEEDS_INPUT).map(...), and early-return if needsInputIds.length === 0 before calling createQuestionnaire (so do not send an empty include_question_ids). Also change the UI button visibility logic that currently uses isComplete to instead check whether there exists at least one question with answer_status === AnswerStatus.NEEDS_INPUT (e.g., use the same needsInputIds.length > 0 condition) so the button is shown only when input is actually needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx`:
- Around line 118-125: The teamInputIdsRef Set is only initialized once for the
component lifetime, causing stale baseline when switching assessments
(assessment.id) and leading to incorrect "New answer from Slack" toasts; update
initialization logic so teamInputIdsRef is reset whenever the current assessment
changes: move or add a useEffect that watches assessment.id (or allQuestions)
and reassigns teamInputIdsRef.current = new Set(...) using
allQuestions.filter(...).map(q => q.question_id), and apply the same reset
pattern for the other similar block referenced at lines 127-142 to ensure
baselines are recomputed per assessment.
---
Duplicate comments:
In `@clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx`:
- Around line 154-170: The request function handleRequestInput should not call
createQuestionnaire when include_question_ids would be an empty array: compute
needsInputIds from allQuestions.filter(q => q.answer_status ===
AnswerStatus.NEEDS_INPUT).map(...), and early-return if needsInputIds.length ===
0 before calling createQuestionnaire (so do not send an empty
include_question_ids). Also change the UI button visibility logic that currently
uses isComplete to instead check whether there exists at least one question with
answer_status === AnswerStatus.NEEDS_INPUT (e.g., use the same
needsInputIds.length > 0 condition) so the button is shown only when input is
actually needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ef093261-a92d-4365-b484-eb4eb8c6fd8b
📒 Files selected for processing (1)
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx
Comment on lines
+118
to
+125
| const teamInputIdsRef = useRef<Set<string> | null>(null); | ||
| if (teamInputIdsRef.current === null) { | ||
| teamInputIdsRef.current = new Set( | ||
| allQuestions | ||
| .filter((q) => q.answer_source === AnswerSource.TEAM_INPUT) | ||
| .map((q) => q.question_id), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Reset team-input baseline when switching assessments.
Line 118-Line 125 initializes teamInputIdsRef only once for the component lifetime. If assessment.id changes without unmounting, Line 136 can emit a false “New answer from Slack” toast on first render of the new assessment.
Proposed fix
- const teamInputIdsRef = useRef<Set<string> | null>(null);
- if (teamInputIdsRef.current === null) {
- teamInputIdsRef.current = new Set(
- allQuestions
- .filter((q) => q.answer_source === AnswerSource.TEAM_INPUT)
- .map((q) => q.question_id),
- );
- }
+ const teamInputIdsRef = useRef<Set<string>>(
+ new Set(
+ allQuestions
+ .filter((q) => q.answer_source === AnswerSource.TEAM_INPUT)
+ .map((q) => q.question_id),
+ ),
+ );
+
+ useEffect(() => {
+ teamInputIdsRef.current = new Set(
+ allQuestions
+ .filter((q) => q.answer_source === AnswerSource.TEAM_INPUT)
+ .map((q) => q.question_id),
+ );
+ }, [assessment.id]);
@@
- const prevIds = teamInputIdsRef.current!;
+ const prevIds = teamInputIdsRef.current;Also applies to: 127-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx`
around lines 118 - 125, The teamInputIdsRef Set is only initialized once for the
component lifetime, causing stale baseline when switching assessments
(assessment.id) and leading to incorrect "New answer from Slack" toasts; update
initialization logic so teamInputIdsRef is reset whenever the current assessment
changes: move or add a useEffect that watches assessment.id (or allQuestions)
and reassigns teamInputIdsRef.current = new Set(...) using
allQuestions.filter(...).map(q => q.question_id), and apply the same reset
pattern for the other similar block referenced at lines 127-142 to ensure
baselines are recomputed per assessment.
lucanovera
approved these changes
Mar 6, 2026
Contributor
lucanovera
left a comment
There was a problem hiding this comment.
New UI works great, the polling for new Slack answers is a nice improvement. Approved!
mfbrown
pushed a commit
that referenced
this pull request
Mar 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket ENG-2877
Description Of Changes
Enhance the privacy assessment detail page Slack integration by replacing the modal-based input request flow with a direct button action, adding real-time polling for new answers, and showing toast notifications when Slack responses arrive.
RequestInputModalwith direct "Request input from team" button that sends questions withNEEDS_INPUTstatus to the configured Slack channelCode Changes
clients/admin-ui/src/features/privacy-assessments/AssessmentDetail.tsx- Replace modal flow with direct Slack send, add polling and new-answer notificationsclients/admin-ui/src/features/privacy-assessments/constants.ts- Update answer source display labelsclients/admin-ui/src/features/privacy-assessments/privacy-assessments.slice.ts- Addhigh_risk_only: falsedefault to assessment creationSteps to Confirm
Pre-Merge Checklist
CHANGELOG.mdupdatedSummary by CodeRabbit
New Features
Changed