From 1ffe17eb3acf3251a02af39abb620ac07255a8d3 Mon Sep 17 00:00:00 2001 From: Neethika Date: Tue, 28 Apr 2026 22:32:09 -0400 Subject: [PATCH 1/2] feat(decisions): replace confirm() with custom delete modal (ref Sentinent-AI/Sentinent#29) --- .../decision-list/decision-list.component.css | 136 ++++++++++++++++++ .../decision-list.component.html | 58 +++++++- .../decision-list.component.spec.ts | 70 ++++++++- .../decision-list/decision-list.component.ts | 56 +++++--- 4 files changed, 302 insertions(+), 18 deletions(-) diff --git a/src/app/components/decision-list/decision-list.component.css b/src/app/components/decision-list/decision-list.component.css index 59737e5..b0fd5e5 100644 --- a/src/app/components/decision-list/decision-list.component.css +++ b/src/app/components/decision-list/decision-list.component.css @@ -64,6 +64,16 @@ border-radius: 8px; cursor: pointer; font-size: 13px; + transition: opacity 0.15s; +} + +.btn-danger:hover:not(:disabled) { + opacity: 0.82; +} + +.btn-danger:disabled { + opacity: 0.55; + cursor: not-allowed; } .decision-card { @@ -171,3 +181,129 @@ @keyframes spin { to { transform: rotate(360deg); } } + +/* ── Delete confirmation modal ───────────────────────────────────────────── */ + +.modal-backdrop { + position: fixed; + inset: 0; + z-index: 50; + background: rgba(2, 6, 23, 0.55); + display: grid; + place-items: center; + padding: 18px; + animation: backdropIn 0.18s ease; +} + +@keyframes backdropIn { + from { opacity: 0; } + to { opacity: 1; } +} + +.delete-modal { + width: min(460px, 100%); + background: #fff; + border: 1px solid rgba(0, 0, 0, 0.12); + border-radius: 16px; + box-shadow: 0 28px 72px rgba(0, 0, 0, 0.24); + padding: 24px 24px 22px; + animation: modalIn 0.2s cubic-bezier(0.22, 1, 0.36, 1); +} + +@keyframes modalIn { + from { opacity: 0; transform: translateY(10px) scale(0.97); } + to { opacity: 1; transform: translateY(0) scale(1); } +} + +.delete-modal-icon { + display: inline-flex; + align-items: center; + justify-content: center; + width: 40px; + height: 40px; + border-radius: 10px; + background: #f3f4f6; + border: 1px solid #e5e7eb; + color: #374151; + margin-bottom: 14px; +} + +.delete-modal-eyebrow { + margin: 0 0 6px; + text-transform: uppercase; + letter-spacing: 0.08em; + font-size: 11px; + color: #6b7280; + font-weight: 700; +} + +.delete-modal-title { + margin: 0; + font-size: 20px; + line-height: 1.3; + font-weight: 700; + color: #0a0a0a; + word-break: break-word; +} + +.delete-modal-copy { + margin: 10px 0 0; + font-size: 14px; + color: #4b5563; + line-height: 1.55; +} + +.delete-modal-error { + margin: 14px 0 0; + background: #fdecec; + color: #8d1c1c; + border: 1px solid #f7c5c5; + border-radius: 10px; + padding: 9px 12px; + font-size: 13px; + font-weight: 600; +} + +.delete-modal-actions { + margin-top: 22px; + display: flex; + justify-content: flex-end; + gap: 10px; +} + +.modal-btn { + border-radius: 10px; + border: 1px solid transparent; + padding: 9px 16px; + font-size: 13px; + font-weight: 600; + cursor: pointer; + transition: opacity 0.15s, background 0.15s; + line-height: 1; +} + +.modal-btn.cancel { + background: #fff; + color: #111; + border-color: #d1d5db; +} + +.modal-btn.cancel:hover:not(:disabled) { + background: #f9fafb; +} + +.modal-btn.confirm { + background: #111; + color: #fff; + border-color: #111; + min-width: 100px; +} + +.modal-btn.confirm:hover:not(:disabled) { + background: #000; +} + +.modal-btn:disabled { + opacity: 0.55; + cursor: not-allowed; +} diff --git a/src/app/components/decision-list/decision-list.component.html b/src/app/components/decision-list/decision-list.component.html index 687fb9c..628d28e 100644 --- a/src/app/components/decision-list/decision-list.component.html +++ b/src/app/components/decision-list/decision-list.component.html @@ -34,8 +34,64 @@

{{ decision.title }}

Edit - +
+ + + diff --git a/src/app/components/decision-list/decision-list.component.spec.ts b/src/app/components/decision-list/decision-list.component.spec.ts index f4e20f3..02b2ad3 100644 --- a/src/app/components/decision-list/decision-list.component.spec.ts +++ b/src/app/components/decision-list/decision-list.component.spec.ts @@ -66,8 +66,76 @@ describe('DecisionListComponent', () => { it('should have correct edit link', () => { const editButton = fixture.debugElement.query(By.css('.btn-secondary')); expect(editButton).toBeTruthy(); - // Since it's a relative link [decision.id, 'edit'], we check if the attribute is present or just trust the binding const link = editButton.nativeElement as HTMLAnchorElement; expect(link.textContent).toContain('Edit'); }); + + // ── Delete modal flow ──────────────────────────────────────────────────── + + it('should not show delete modal by default', () => { + const backdrop = fixture.nativeElement.querySelector('.modal-backdrop'); + expect(backdrop).toBeNull(); + }); + + it('should open delete modal with decision title when requestDeleteDecision is called', () => { + component.requestDeleteDecision(mockDecisions[0]); + fixture.detectChanges(); + + const backdrop = fixture.nativeElement.querySelector('.modal-backdrop'); + expect(backdrop).toBeTruthy(); + + const title = fixture.nativeElement.querySelector('.delete-modal-title'); + expect(title.textContent).toContain('Test Decision'); + }); + + it('should close modal and clear state when cancelDeleteDecision is called', () => { + component.requestDeleteDecision(mockDecisions[0]); + fixture.detectChanges(); + + component.cancelDeleteDecision(); + fixture.detectChanges(); + + const backdrop = fixture.nativeElement.querySelector('.modal-backdrop'); + expect(backdrop).toBeNull(); + expect(component.pendingDeleteDecision).toBeNull(); + expect(component.deleteDecisionError).toBe(''); + }); + + it('should close modal when backdrop overlay is clicked', () => { + component.requestDeleteDecision(mockDecisions[0]); + fixture.detectChanges(); + + const backdrop = fixture.nativeElement.querySelector('.modal-backdrop') as HTMLElement; + backdrop.click(); + fixture.detectChanges(); + + expect(component.pendingDeleteDecision).toBeNull(); + }); + + it('should call deleteDecision service and close modal on confirmDeleteDecision', () => { + component.requestDeleteDecision(mockDecisions[0]); + fixture.detectChanges(); + + component.confirmDeleteDecision(); + fixture.detectChanges(); + + expect(mockDecisionService.deleteDecision).toHaveBeenCalledWith('10', '1'); + expect(component.pendingDeleteDecision).toBeNull(); + expect(component.isDeletingDecision).toBeFalse(); + }); + + it('should not open modal while a delete is already in progress', () => { + component.isDeletingDecision = true; + component.requestDeleteDecision(mockDecisions[0]); + + expect(component.pendingDeleteDecision).toBeNull(); + }); + + it('should not close modal while a delete is in progress', () => { + component.requestDeleteDecision(mockDecisions[0]); + component.isDeletingDecision = true; + component.cancelDeleteDecision(); + + expect(component.pendingDeleteDecision).toEqual(mockDecisions[0]); + }); }); diff --git a/src/app/components/decision-list/decision-list.component.ts b/src/app/components/decision-list/decision-list.component.ts index 432a775..efd612f 100644 --- a/src/app/components/decision-list/decision-list.component.ts +++ b/src/app/components/decision-list/decision-list.component.ts @@ -15,6 +15,12 @@ import { Observable, Subject, takeUntil, startWith, switchMap, catchError, of } export class DecisionListComponent implements OnInit, OnDestroy { decisions$: Observable | undefined; error: string | null = null; + + // Delete confirmation state + pendingDeleteDecision: Decision | null = null; + isDeletingDecision = false; + deleteDecisionError = ''; + private refresh$ = new Subject(); private destroy$ = new Subject(); private workspaceId: string | null = null; @@ -30,7 +36,7 @@ export class DecisionListComponent implements OnInit, OnDestroy { this.decisions$ = this.refresh$.pipe( startWith(undefined), switchMap(() => this.decisionService.getDecisions(this.workspaceId!).pipe( - catchError(err => { + catchError(() => { this.error = 'Failed to load decisions. Please try again.'; return of([]); }) @@ -44,21 +50,39 @@ export class DecisionListComponent implements OnInit, OnDestroy { this.destroy$.complete(); } - deleteDecision(id: string): void { - if (confirm('Are you sure you want to delete this decision?') && this.workspaceId) { - this.error = null; - this.decisionService.deleteDecision(this.workspaceId, id) - .pipe(takeUntil(this.destroy$)) - .subscribe({ - next: () => { - this.refresh$.next(); - }, - error: (err) => { - console.error('Delete failed', err); - this.error = 'Failed to delete decision. Please try again.'; - } - }); - } + requestDeleteDecision(decision: Decision): void { + if (this.isDeletingDecision) return; + this.pendingDeleteDecision = decision; + this.deleteDecisionError = ''; + } + + cancelDeleteDecision(): void { + if (this.isDeletingDecision) return; + this.pendingDeleteDecision = null; + this.deleteDecisionError = ''; + } + + confirmDeleteDecision(): void { + const decision = this.pendingDeleteDecision; + if (!decision || !this.workspaceId || this.isDeletingDecision) return; + + this.isDeletingDecision = true; + this.deleteDecisionError = ''; + + this.decisionService.deleteDecision(this.workspaceId, decision.id) + .pipe(takeUntil(this.destroy$)) + .subscribe({ + next: () => { + this.pendingDeleteDecision = null; + this.isDeletingDecision = false; + this.error = null; + this.refresh$.next(); + }, + error: () => { + this.isDeletingDecision = false; + this.deleteDecisionError = 'Unable to delete decision. Please try again.'; + } + }); } isOverdue(decision: Decision): boolean { From 7f4be0138310b24fe75729fb4c7354242f8ecb05 Mon Sep 17 00:00:00 2001 From: Sentinent Agent Date: Wed, 29 Apr 2026 03:21:27 +0000 Subject: [PATCH 2/2] fix(integrations): persist connection status on tab re-visit and guard GitHub close issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - getSlackChannels: build baseState from DB record first; catch inner API errors so Slack 'connected' flag no longer flips to false when channels endpoint is temporarily unreachable - getGitHubRepos: same pattern — baseState from DB record, inner catchError falls back to connected:true with empty repo list instead of re-throwing - Fix double-slash URL in replyToSlack (/api/integrations/integrations/slack/reply → /api/integrations/slack/reply) - signal-board: guard toggleGitHubState and addGitHubComment against missing workspaceId; surface actionable toast instead of silent 400 Fixes Sentinent-AI/Sentinent#35 --- .../components/signal-board/signal-board.ts | 13 +++++-- src/app/services/integration.service.ts | 38 +++++++++++++------ 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/app/components/signal-board/signal-board.ts b/src/app/components/signal-board/signal-board.ts index 2f52588..d0d0514 100644 --- a/src/app/components/signal-board/signal-board.ts +++ b/src/app/components/signal-board/signal-board.ts @@ -157,7 +157,10 @@ export class SignalBoardComponent { const number = signal.metadata.number; const comment = this.githubComments[signal.id]; - if (!comment || !comment.trim() || !repo || !number) return; + if (!workspaceId || !comment || !comment.trim() || !repo || !number) { + if (!workspaceId) this.showToast('Cannot comment: workspace context is missing'); + return; + } this.submittingGitHubComment[signal.id] = true; this.integrationService.addGitHubComment(workspaceId, repo, number, comment).subscribe({ @@ -181,6 +184,10 @@ export class SignalBoardComponent { const number = signal.metadata.number; const currentState = signal.metadata.state; + if (!workspaceId) { + this.showToast('Cannot update: workspace context is missing'); + return; + } if (!repo || !number) return; const newState = currentState === 'open' ? 'closed' : 'open'; @@ -193,8 +200,8 @@ export class SignalBoardComponent { } this.cdr.detectChanges(); }, - error: () => { - this.showToast('Failed to update GitHub status'); + error: (err: Error) => { + this.showToast(err?.message || 'Failed to update GitHub issue state'); } }); } diff --git a/src/app/services/integration.service.ts b/src/app/services/integration.service.ts index 10b6698..b50ec93 100644 --- a/src/app/services/integration.service.ts +++ b/src/app/services/integration.service.ts @@ -65,27 +65,32 @@ export class IntegrationService { switchMap((integrations) => { const slackIntegration = integrations.find((integration) => integration.provider === 'slack'); if (!slackIntegration) { - return of({ - connected: false, - channels: [], - }); + return of({ connected: false, channels: [] }); } + // Connection is confirmed by the DB record — build the base state first. const metadata = this.parseMetadata(slackIntegration.metadata); - const params = new HttpParams().set('integration_id', String(slackIntegration.id)); + const baseState: SlackConnectionState = { + connected: true, + workspaceName: this.readString(metadata['team_name']), + workspaceUrl: this.readString(metadata['team_domain']), + channels: [], + lastSyncAt: slackIntegration.updated_at ? new Date(slackIntegration.updated_at) : undefined, + }; + const params = new HttpParams().set('integration_id', String(slackIntegration.id)); return this.http.get(`${this.apiUrl}/slack/channels`, { params }).pipe( map((response) => ({ - connected: true, - workspaceName: this.readString(metadata['team_name']), - workspaceUrl: this.readString(metadata['team_domain']), + ...baseState, channels: response.channels.map((channel) => ({ id: channel.id, name: channel.name, isConnected: this.readStringArray(metadata['selected_channels']).includes(channel.id), })), - lastSyncAt: slackIntegration.updated_at ? new Date(slackIntegration.updated_at) : undefined, })), + // If the Slack API call fails, still return connected: true with empty channels + // so the UI doesn't flash "disconnected" when the channel list is temporarily unreachable. + catchError(() => of(baseState)), ); }), catchError((error) => throwError(() => toError(error, 'Unable to load Slack channels.'))), @@ -159,14 +164,23 @@ export class IntegrationService { const selectedRepoIds = this.readNumberArray(metadata['selected_repo_ids']); const params = new HttpParams().set('workspace_id', workspaceId); + // Connection is confirmed by the DB record — build the base state first. + const baseState: GitHubConnectionState = { + connected: true, + repos: [], + lastSyncAt: githubIntegration.updated_at ? new Date(githubIntegration.updated_at) : undefined, + }; + return this.http.get(`${this.apiUrl}/github/repos`, { params }).pipe( map((repos) => ({ - connected: true, + ...baseState, repos: repos.map((repo) => this.mapGitHubRepo(repo, selectedRepoIds)), accountName: repos[0]?.owner?.login ?? undefined, accountHandle: repos[0]?.owner?.login ? `@${repos[0].owner.login}` : undefined, - lastSyncAt: githubIntegration.updated_at ? new Date(githubIntegration.updated_at) : undefined, })), + // If the GitHub API call fails, still return connected: true with empty repos + // so the UI doesn't flash "disconnected" when the repo list is temporarily unreachable. + catchError(() => of(baseState)), ); }), catchError((error) => throwError(() => toError(error, 'Unable to load GitHub repositories.'))), @@ -382,7 +396,7 @@ export class IntegrationService { replyToSlack(workspaceId: string, channelId: string, threadTs: string, text: string): Observable { const params = new HttpParams().set('workspace_id', workspaceId); - return this.http.post(`${this.apiUrl}/integrations/slack/reply`, { + return this.http.post(`${this.apiUrl}/slack/reply`, { channel_id: channelId, thread_ts: threadTs, text: text