From 47bee94cdd7107cadc0b96e0f7a89caa1aef89c6 Mon Sep 17 00:00:00 2001 From: Lince Mathew Date: Thu, 29 Oct 2026 21:31:49 +0530 Subject: [PATCH 1/2] feat: redesign Auto Approve panel, move PR controls to top, show review time in local zone Auto Approve panel: aligned rows, status icons, Configure and Your level links, click a row to open its details. PR description: controls block at the top, decisions table at the bottom with a Time column in the review starter's saved time zone (new users.time_zone, saved once from the browser). LiveReview Pre-Commit Check: vouched (iter:1, coverage:0%) --- .../20261029120000_add_user_time_zone.sql | 5 + docs/pr-review-links-plan.md | 4 +- internal/api/server.go | 1 + internal/api/users/profile_handlers.go | 30 +++++ .../routes_guide/reviews/pr-review-link.md | 6 +- .../routes_guide/reviews/review-detail.md | 5 + internal/jobqueue/repo_sync_worker.go | 126 ++++++++++++------ ui/src/App.tsx | 3 + ui/src/api/profile.ts | 7 + .../components/reviews/AutoApprovePanel.tsx | 123 ++++++++++++++--- ui/src/hooks/useSyncTimeZone.ts | 24 ++++ .../pages/Settings/AutoApprovePolicyTab.tsx | 2 +- 12 files changed, 273 insertions(+), 63 deletions(-) create mode 100644 db/migrations/20261029120000_add_user_time_zone.sql create mode 100644 ui/src/hooks/useSyncTimeZone.ts diff --git a/db/migrations/20261029120000_add_user_time_zone.sql b/db/migrations/20261029120000_add_user_time_zone.sql new file mode 100644 index 00000000..6522d472 --- /dev/null +++ b/db/migrations/20261029120000_add_user_time_zone.sql @@ -0,0 +1,5 @@ +-- migrate:up +ALTER TABLE users ADD COLUMN IF NOT EXISTS time_zone TEXT; + +-- migrate:down +ALTER TABLE users DROP COLUMN IF EXISTS time_zone; diff --git a/docs/pr-review-links-plan.md b/docs/pr-review-links-plan.md index 8e19a08a..ea7ae73c 100644 --- a/docs/pr-review-links-plan.md +++ b/docs/pr-review-links-plan.md @@ -2,7 +2,7 @@ ## Goal -When a new PR/MR is opened, LiveReview adds a section with two links to the end of its description: +When a new PR/MR is opened, LiveReview adds a controls section with two links to the top of its description: 1. **Start a review** — opens LiveReview, triggers a review for that PR, and redirects to the review details page. @@ -21,7 +21,7 @@ built on CI/CD Gates jq rulesets) remain a later feature. | Providers | GitHub, GitLab, Bitbucket, Gitea, Azure DevOps | | Which repos | Every repo with the LiveReview webhook installed | | Which PRs | Only newly opened PRs (GitHub action `opened`, GitLab action `open`). Existing open PRs, reopen, push, and edit events are ignored. | -| Where links go | A section at the end of the PR description, between the markdown-comment markers `[//]: # (livereview:links)` and `[//]: # (livereview:end)` (HTML comments show as text on Bitbucket; the old `` markers are still recognised). Added at PR open; rewritten in place after each link-started review with an "Auto-approval decisions" table (one row per review, latest first: Review, Decision, Reason). The stage-by-stage policy breakdown is a separate PR comment. Replaced the earlier bot comment. | +| Where links go | A controls section at the top of the PR description, between `[//]: # (livereview:controls)` and `[//]: # (livereview:controls-end)`, followed by a horizontal line (HTML comments show as text on Bitbucket). Added at PR open. A separate results section at the bottom, between `livereview:results` and `livereview:results-end`, is rewritten in place after each link-started review with a "LiveReview auto-approval decisions" table. The old single section (`livereview:links` / `livereview:end`, or the legacy `` markers) is removed and replaced on the next review. The table (one row per review, latest first: Review, Time (starter's time zone, UTC fallback), Decision, Reason). The stage-by-stage policy breakdown is a separate PR comment. Replaced the earlier bot comment. | | Duplicate webhooks | Guarded by a "links added" flag (`links_comment_posted_at`) on the `pull_requests` row | | Auth | Both links require a LiveReview login. Logged-out users log in and are sent back to the link (existing `redirectAfterLogin` flow). | | Org | The user is switched to the PR's org automatically. Non-members get "not found". | diff --git a/internal/api/server.go b/internal/api/server.go index 429e20df..e2e237c1 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -1318,6 +1318,7 @@ func (s *Server) setupRoutes() { protected.GET("/users/profile", s.profileHandlers.GetProfile) protected.PUT("/users/profile", s.profileHandlers.UpdateProfile) protected.PUT("/users/default-org", s.orgHandlers.SetDefaultOrganization) + protected.PUT("/users/time-zone", s.profileHandlers.UpdateTimeZone) protected.PUT("/users/password", s.profileHandlers.ChangePassword) // Development mode test endpoints (only enabled when DEV_MODE=true) diff --git a/internal/api/users/profile_handlers.go b/internal/api/users/profile_handlers.go index ae77237c..e01f9c80 100644 --- a/internal/api/users/profile_handlers.go +++ b/internal/api/users/profile_handlers.go @@ -5,6 +5,7 @@ import ( "errors" "net/http" "strconv" + "time" "github.com/labstack/echo/v4" "github.com/livereview/internal/api/auth" @@ -219,3 +220,32 @@ func (ph *ProfileHandlers) ChangePassword(c echo.Context) error { "message": "Password changed successfully. Please log in again with your new password.", }) } + +// UpdateTimeZone saves the browser's IANA time zone so PR tables show the user's local time. +// It is a no-op when the zone is unchanged, so repeated calls write nothing. +func (ph *ProfileHandlers) UpdateTimeZone(c echo.Context) error { + if err := requireSession(c); err != nil { + return err + } + user := auth.GetUser(c) + if user == nil { + return echo.NewHTTPError(http.StatusUnauthorized, "User not found in context") + } + + var req struct { + TimeZone string `json:"time_zone"` + } + if err := c.Bind(&req); err != nil { + return echo.NewHTTPError(http.StatusBadRequest, "Invalid request body") + } + if _, err := time.LoadLocation(req.TimeZone); err != nil || req.TimeZone == "" || req.TimeZone == "Local" { + return echo.NewHTTPError(http.StatusBadRequest, "time_zone must be a valid IANA zone, e.g. Asia/Kolkata") + } + + if _, err := ph.db.ExecContext(c.Request().Context(), + `UPDATE users SET time_zone = $1 WHERE id = $2 AND time_zone IS DISTINCT FROM $1`, + req.TimeZone, user.ID); err != nil { + return echo.NewHTTPError(http.StatusInternalServerError, "Failed to save time zone") + } + return c.NoContent(http.StatusNoContent) +} diff --git a/internal/docindex/docs/routes_guide/reviews/pr-review-link.md b/internal/docindex/docs/routes_guide/reviews/pr-review-link.md index f5f812fe..45679918 100644 --- a/internal/docindex/docs/routes_guide/reviews/pr-review-link.md +++ b/internal/docindex/docs/routes_guide/reviews/pr-review-link.md @@ -53,8 +53,10 @@ organization with that PR or that account available see "pull request wasn't fou - When a review started from this link finishes, LiveReview runs the org's [auto-approve policy](../settings/auto-approve.md) on it. The result is shown in two places: - - **PR description:** under the two links, an "Auto-approval decisions" - table with one row per policy-checked review, latest first: Review (link), + - **PR description:** the controls (Start a review, Review history, policy + links) sit at the top, above a horizontal line. A "LiveReview auto-approval + decisions" table stays at the bottom, with one row per policy-checked review, latest first: Review (link), + Time (in the review starter's saved time zone, e.g. "29 Oct 2026, 19:35 IST"; UTC until they open the app once), Decision (✅ Approved / ❌ Not approved / ⚠️ Force-approved), and Reason (the failing stage and why, the repo mode override such as "mode override: always_approve", the force-approve reason, or "All policy diff --git a/internal/docindex/docs/routes_guide/reviews/review-detail.md b/internal/docindex/docs/routes_guide/reviews/review-detail.md index f85267a6..faeebf1c 100644 --- a/internal/docindex/docs/routes_guide/reviews/review-detail.md +++ b/internal/docindex/docs/routes_guide/reviews/review-detail.md @@ -51,6 +51,11 @@ assessment: Approved, Not approved, or Pending. For a blocked review, owners/adm `force_approve_min_level`) can **force approve** with a reason — recorded in the audit log and posted to the PR/MR. Any member can **mark a production outcome** (revert / incident / confirmed-safe) with a reason, which feeds the risk model. +Each stage row shows a pass/fail/skipped icon, its reason, and a **Details** +toggle; clicking anywhere on a stage row also opens or closes its details. At the end of the row, owners/admins get a **Configure** link on every +stage except the final decision; it opens that stage's sub-tab in Settings → +Auto Approve Policies. The eligibility row adds **Your level** before it, which +opens the viewer's own user settings (review level). See [Settings → Auto Approve](../settings/auto-approve.md). ## Blast-radius scoring and ordering diff --git a/internal/jobqueue/repo_sync_worker.go b/internal/jobqueue/repo_sync_worker.go index 67ef4a81..871e1c1d 100644 --- a/internal/jobqueue/repo_sync_worker.go +++ b/internal/jobqueue/repo_sync_worker.go @@ -338,11 +338,14 @@ func (w *PRStateSyncWorker) Work(ctx context.Context, job *river.Job[PRStateSync // The LiveReview section in a PR description sits between these markers so it can be replaced in place. // They are markdown comments, which every provider hides (Bitbucket shows HTML comments as text). const ( - prSectionStart = "[//]: # (livereview:links)" - prSectionEnd = "[//]: # (livereview:end)" - // Markers used before the switch, still matched so older sections get replaced. + controlsStart = "[//]: # (livereview:controls)" + controlsEnd = "[//]: # (livereview:controls-end)" + resultsStart = "[//]: # (livereview:results)" + resultsEnd = "[//]: # (livereview:results-end)" + // Old single section (controls and table together, at the bottom), removed when found. + prSectionStart = "[//]: # (livereview:links)" + prSectionEnd = "[//]: # (livereview:end)" legacySectionStart = "" - legacySectionEnd = "" ) // postLinksComment adds the LiveReview links (review, history, repo/org policy) to the end of a new PR/MR's description. @@ -366,7 +369,9 @@ func (w *PRStateSyncWorker) postLinksComment(ctx context.Context, repositoryID, info, err := loadActingConnectorInfo(w.db, repositoryID) if err == nil { - err = writePRSection(ctx, info, number, buildPRLinksSection(host, pullRequestID, repositoryID, "")) + err = writePRSection(ctx, info, number, func(body string) string { + return upsertControlsSection(body, buildControlsSection(host, pullRequestID, repositoryID)) + }) } if err != nil { // Release the claim so River's retry can add it. @@ -378,21 +383,25 @@ func (w *PRStateSyncWorker) postLinksComment(ctx context.Context, repositoryID, return nil } -// buildPRLinksSection renders the LiveReview section; history is the review history table, or "". -func buildPRLinksSection(host string, pullRequestID, repositoryID int64, history string) string { +// buildControlsSection renders the control panel (start review, history, policy links) shown at the top of the PR. +func buildControlsSection(host string, pullRequestID, repositoryID int64) string { host = strings.TrimSuffix(host, "/") return fmt.Sprintf(`%s - ---- ### 🔍 LiveReview - ▶️ **[Start a review](%s/#/pr/%d/review)**: reviews all changes in this PR against its target branch - 📜 **[Review history](%s/#/reviews?pull_request_id=%d)**: all past reviews of this PR -%s⚙️ Customize risk-based approval policy for [this repository](%s/#/explore/repositories/%d/settings#auto-approve) or [your organization](%s/#/settings#auto-approve). +⚙️ Customize risk-based approval policy for [this repository](%s/#/explore/repositories/%d/settings#auto-approve) or [your organization](%s/#/settings#auto-approve). +--- %s -`, prSectionStart, host, pullRequestID, host, pullRequestID, history, host, repositoryID, host, prSectionEnd) +`, controlsStart, host, pullRequestID, host, pullRequestID, host, repositoryID, host, controlsEnd) +} + +// buildResultsSection wraps the decisions table, which stays at the bottom of the PR. +func buildResultsSection(history string) string { + return fmt.Sprintf("%s\n\n---\n%s%s\n", resultsStart, history, resultsEnd) } // refreshPRSection rewrites the LiveReview section with one history row per policy-checked review of this PR. @@ -400,8 +409,9 @@ func buildPRLinksSection(host string, pullRequestID, repositoryID int64, history func refreshPRSection(ctx context.Context, db *sql.DB, info *repositoryConnectorInfo, host string, pullRequestID int64, number int) error { rows, err := db.QueryContext(ctx, // ponytail: only the last 10 rows, so the table stays under description size limits (Azure DevOps: 4000 chars). - `SELECT id, policy, forced_reason FROM ( - SELECT r.id, r.metadata->'policy_result' AS policy, + `SELECT id, created_at, policy, forced_reason, time_zone FROM ( + SELECT r.id, r.created_at, r.metadata->'policy_result' AS policy, + (SELECT u.time_zone FROM users u WHERE lower(u.email) = lower(r.user_email) LIMIT 1) AS time_zone, (SELECT o.reason FROM review_policy_overrides o WHERE o.review_id = r.id AND o.org_id = r.org_id ORDER BY o.created_at DESC LIMIT 1) AS forced_reason FROM reviews r @@ -414,12 +424,13 @@ func refreshPRSection(ctx context.Context, db *sql.DB, info *repositoryConnector host = strings.TrimSuffix(host, "/") var b strings.Builder - b.WriteString("#### Auto-approval decisions\n\n| Review | Decision | Reason |\n|---|---|---|\n") + b.WriteString("#### LiveReview auto-approval decisions\n\n| Review | Time | Decision | Reason |\n|---|---|---|---|\n") for rows.Next() { var reviewID int64 + var createdAt time.Time var raw []byte - var forcedReason sql.NullString - if err := rows.Scan(&reviewID, &raw, &forcedReason); err != nil { + var forcedReason, timeZone sql.NullString + if err := rows.Scan(&reviewID, &createdAt, &raw, &forcedReason, &timeZone); err != nil { return err } var res policyengine.FunnelResult @@ -432,13 +443,27 @@ func refreshPRSection(ctx context.Context, db *sql.DB, info *repositoryConnector } else if !res.Approved { decision = "❌ Not approved" } - fmt.Fprintf(&b, "| [#%d](%s/#/reviews/%d) | %s | %s |\n", reviewID, host, reviewID, decision, strings.ReplaceAll(reason, "|", "\\|")) + fmt.Fprintf(&b, "| [#%d](%s/#/reviews/%d) | %s | %s | %s |\n", reviewID, host, reviewID, formatReviewTime(createdAt, timeZone), decision, strings.ReplaceAll(reason, "|", "\\|")) } if err := rows.Err(); err != nil { return err } b.WriteString("\n") - return writePRSection(ctx, info, number, buildPRLinksSection(host, pullRequestID, info.RepositoryID, b.String())) + controls := buildControlsSection(host, pullRequestID, info.RepositoryID) + results := buildResultsSection(b.String()) + return writePRSection(ctx, info, number, func(body string) string { + return upsertResultsSection(upsertControlsSection(body, controls), results) + }) +} + +// formatReviewTime shows the time in the review starter's saved zone, or UTC if none is saved. +func formatReviewTime(t time.Time, timeZone sql.NullString) string { + if timeZone.Valid { + if loc, err := time.LoadLocation(timeZone.String); err == nil { + return t.In(loc).Format("2 Jan 2006, 15:04 MST") + } + } + return t.UTC().Format("2 Jan 2006, 15:04 UTC") } // decisionReason is the readable "why" for one decision row: each failing stage's reason, or that all passed. @@ -470,31 +495,52 @@ func decisionReason(res *policyengine.FunnelResult) string { return strings.Join(parts, "; ") } -// upsertPRSection swaps the old LiveReview section in body for section, or appends section if there is none. -// Sections from before the end marker existed were always last, so they are replaced up to the end of body. -func upsertPRSection(body, section string) string { - startMarker, endMarker := prSectionStart, prSectionEnd - start := strings.Index(body, startMarker) - if start < 0 { - startMarker, endMarker = legacySectionStart, legacySectionEnd - start = strings.Index(body, startMarker) - } - if start < 0 { - if strings.TrimSpace(body) == "" { - return section - } - return strings.TrimRight(body, "\n") + "\n\n" + section +// replaceBetween swaps the text from start through end markers for section; ok is false if start is absent. +func replaceBetween(body, start, end, section string) (string, bool) { + i := strings.Index(body, start) + if i < 0 { + return body, false } - rest := "" - if end := strings.Index(body[start:], endMarker); end >= 0 { - rest = body[start+end+len(endMarker):] - section = strings.TrimRight(section, "\n") + j := strings.Index(body[i:], end) + if j < 0 { + return body[:i] + strings.TrimRight(section, "\n"), true + } + return body[:i] + strings.TrimRight(section, "\n") + body[i+j+len(end):], true +} + +// removeOldSection drops the old single section; the legacy one was always last, so it goes to the end of body. +func removeOldSection(body string) string { + if next, ok := replaceBetween(body, prSectionStart, prSectionEnd, ""); ok { + return strings.TrimRight(next, "\n") + "\n" + } + if i := strings.Index(body, legacySectionStart); i >= 0 { + return strings.TrimRight(body[:i], "\n") + "\n" + } + return body +} + +// upsertControlsSection puts the control panel at the top of body, replacing it in place if already there. +func upsertControlsSection(body, section string) string { + body = removeOldSection(body) + if next, ok := replaceBetween(body, controlsStart, controlsEnd, section); ok { + return next + } + if strings.TrimSpace(body) == "" { + return section + } + return section + "\n" + strings.TrimLeft(body, "\n") +} + +// upsertResultsSection keeps the decisions table at the bottom of body, replacing it in place if already there. +func upsertResultsSection(body, section string) string { + if next, ok := replaceBetween(body, resultsStart, resultsEnd, section); ok { + return next } - return body[:start] + section + rest + return strings.TrimRight(body, "\n") + "\n\n" + section } -// writePRSection reads the PR/MR description and saves it with the LiveReview section updated. -func writePRSection(ctx context.Context, info *repositoryConnectorInfo, number int, section string) error { +// writePRSection reads the PR/MR description and saves it after update changes the LiveReview sections. +func writePRSection(ctx context.Context, info *repositoryConnectorInfo, number int, update func(body string) string) error { var get func() (string, error) var set func(string) error switch info.Provider { @@ -544,7 +590,7 @@ func writePRSection(ctx context.Context, info *repositoryConnectorInfo, number i if err != nil { return err } - if next := upsertPRSection(body, section); next != body { + if next := update(body); next != body { return set(next) } return nil diff --git a/ui/src/App.tsx b/ui/src/App.tsx index fe235e44..f6d4c8f5 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -2,6 +2,7 @@ import React, { Suspense, useEffect, useState } from 'react'; import { HashRouter as Router, Routes, Route, Navigate, useNavigate, useLocation, Link } from 'react-router-dom'; import { Navbar } from './components/Navbar/Navbar'; import { URLMismatchBanner } from './components/URLMismatchBanner'; +import { useSyncTimeZone } from './hooks/useSyncTimeZone'; import { useAppDispatch, useAppSelector } from './store/configureStore'; import { logout, checkSetupStatus, fetchUser } from './store/Auth/reducer'; import { fetchLicenseStatus, openModal as openLicenseModal, closeModal as closeLicenseModal } from './store/License/slice'; @@ -198,6 +199,8 @@ const AppContent: React.FC = () => { } }, [dispatch, accessToken]); + useSyncTimeZone(); + // Kick off initial license status load (non-blocking UI) useEffect(() => { // Only attempt after authentication established to avoid 401 noise diff --git a/ui/src/api/profile.ts b/ui/src/api/profile.ts index 74a4c9b1..49258c63 100644 --- a/ui/src/api/profile.ts +++ b/ui/src/api/profile.ts @@ -37,3 +37,10 @@ export const updateProfile = async (req: UpdateProfileRequest): Promise => { return apiClient.put<{ message: string }>('/users/password', req); }; + +/** + * Save the browser's time zone (IANA name) so PR tables show this user's local time. + */ +export const saveTimeZone = async (timeZone: string): Promise => { + await apiClient.put('/users/time-zone', { time_zone: timeZone }); +}; diff --git a/ui/src/components/reviews/AutoApprovePanel.tsx b/ui/src/components/reviews/AutoApprovePanel.tsx index 5d58e0a6..6fd8364f 100644 --- a/ui/src/components/reviews/AutoApprovePanel.tsx +++ b/ui/src/components/reviews/AutoApprovePanel.tsx @@ -1,4 +1,7 @@ import React, { useEffect, useState } from 'react'; +import { Link } from 'react-router-dom'; +import { useAppSelector } from '../../store/configureStore'; +import { useOrgContext } from '../../hooks/useOrgContext'; import { Alert, Button, Card, Spinner } from '../UIPrimitives'; import { getReviewPolicyLog, forceApproveReview, markReviewOutcome, PolicyLogResponse } from '../../api/riskPolicy'; import { InfoHint } from '../AutoApprove/InfoHint'; @@ -20,8 +23,63 @@ const STAGE_HINT: Record = { final: HELP.stageFinal, }; -const verdictIcon = (v: string) => - v === 'pass' || v === 'approved' ? '✅' : v === 'fail' || v === 'blocked' ? '❌' : '⏭️'; +const VerdictIcon: React.FC<{ verdict: string }> = ({ verdict }) => { + const pass = verdict === 'pass' || verdict === 'approved'; + const fail = verdict === 'fail' || verdict === 'blocked'; + const stroke = { fill: 'none', stroke: 'currentColor', strokeWidth: 2.2, strokeLinecap: 'round' as const, strokeLinejoin: 'round' as const }; + const base = 'w-5 h-5 rounded-full p-[3px] shrink-0'; + if (pass) { + return ( + + + + ); + } + if (fail) { + return ( + + + + ); + } + return ( + + + + ); +}; + +// Policy sub-tab where each stage is configured (the final decision has none). +const STAGE_SETTINGS_TAB: Record = { + eligibility: 'eligibility', + static: 'static-rules', + risk: 'risk-model', + llm: 'general', +}; + +const actionClass = + 'inline-flex items-center gap-1.5 rounded-md border border-slate-700 px-2 py-1 text-xs text-slate-300 ' + + 'hover:border-slate-500 hover:text-white focus-visible:outline focus-visible:outline-2 focus-visible:outline-blue-400 transition-colors'; + +const GearIcon = () => ( + +); + +const UserIcon = () => ( + +); + +const Chevron: React.FC<{ open: boolean }> = ({ open }) => ( + +); // Every stage's detail blob carries more than the one-line reason already // shown inline (risk's full feature-by-feature breakdown, llm's class + @@ -32,7 +90,7 @@ const StageDetail: React.FC<{ detail: Record }> = ({ detail }) const entries = Object.entries(detail).filter(([k]) => k !== 'reason' && k !== 'features'); if (!features && entries.length === 0) return null; return ( -
+
{entries.map(([k, v]) => (
{k.replace(/_/g, ' ')}:{' '} @@ -68,6 +126,8 @@ const AutoApprovePanel: React.FC<{ reviewId: number }> = ({ reviewId }) => { const [loading, setLoading] = useState(true); const [error, setError] = useState(''); const [expanded, setExpanded] = useState>(new Set()); + const { canManageCurrentOrg } = useOrgContext(); + const viewerId = useAppSelector((state) => state.Auth.user?.id); const [forceReason, setForceReason] = useState(''); const [forceBusy, setForceBusy] = useState(false); @@ -140,33 +200,60 @@ const AutoApprovePanel: React.FC<{ reviewId: number }> = ({ reviewId }) => { {error && {error}} -
+
{log.stages.map((st) => { const canExpand = hasExpandableDetail(st.detail || {}); const isOpen = expanded.has(st.stage); + const settingsTab = STAGE_SETTINGS_TAB[st.stage]; + const isFinal = st.stage === 'final'; + const toggle = () => + setExpanded((prev) => { + const next = new Set(prev); + if (next.has(st.stage)) next.delete(st.stage); + else next.add(st.stage); + return next; + }); return ( -
+
{ - if (!canExpand) return; - setExpanded((prev) => { - const next = new Set(prev); - if (next.has(st.stage)) next.delete(st.stage); - else next.add(st.stage); - return next; - }); + className={`grid grid-cols-[1.25rem_1fr] items-center gap-x-3 gap-y-2 text-sm md:grid-cols-[1.25rem_11rem_1fr_auto] ${canExpand ? 'cursor-pointer' : ''}`} + onClick={(e) => { + // Links and the Details button handle their own clicks. + if (canExpand && !(e.target as HTMLElement).closest('a,button')) toggle(); }} > - {verdictIcon(st.verdict)} - + + {STAGE_LABEL[st.stage] || st.stage} {STAGE_HINT[st.stage] && } - + {String((st.detail as any)?.reason ?? '')} - {canExpand && {isOpen ? '▾ hide details' : '▸ show details'}} +
+ {canExpand ? ( + + ) : ( +
{isOpen && }
diff --git a/ui/src/hooks/useSyncTimeZone.ts b/ui/src/hooks/useSyncTimeZone.ts new file mode 100644 index 00000000..7f9ed8ad --- /dev/null +++ b/ui/src/hooks/useSyncTimeZone.ts @@ -0,0 +1,24 @@ +import { useEffect } from 'react'; +import { useAppSelector } from '../store/configureStore'; +import { saveTimeZone } from '../api/profile'; + +// Sends the browser's time zone once per user, and again only if it changes. +export function useSyncTimeZone(): void { + const userId = useAppSelector((state) => state.Auth.user?.id); + useEffect(() => { + if (!userId) return; + const key = `lr_tz_synced:${userId}`; + let zone = ''; + try { + zone = Intl.DateTimeFormat().resolvedOptions().timeZone; + if (!zone || localStorage.getItem(key) === zone) return; + } catch { + return; + } + saveTimeZone(zone) + .then(() => { + try { localStorage.setItem(key, zone); } catch { /* storage blocked: retry next visit */ } + }) + .catch(() => { /* not critical: the PR table falls back to UTC */ }); + }, [userId]); +} diff --git a/ui/src/pages/Settings/AutoApprovePolicyTab.tsx b/ui/src/pages/Settings/AutoApprovePolicyTab.tsx index 16db13e9..bb042d76 100644 --- a/ui/src/pages/Settings/AutoApprovePolicyTab.tsx +++ b/ui/src/pages/Settings/AutoApprovePolicyTab.tsx @@ -70,7 +70,7 @@ const AutoApprovePolicyTab: React.FC = () => { const [overrideDraft, setOverrideDraft] = useState>({}); const saveTimers = useRef>>({}); const [outcomes, setOutcomes] = useState([]); - const [subtab, setSubtab] = useState('general'); + const [subtab, setSubtab] = useState(() => new URLSearchParams(window.location.search).get('subtab') || 'general'); const [simRows, setSimRows] = useState(null); const orgId = currentOrgId ?? 0; From adae38af7bf40c80cdf0abece61d07984c371c6d Mon Sep 17 00:00:00 2001 From: Lince Mathew Date: Fri, 30 Oct 2026 01:00:09 +0530 Subject: [PATCH 2/2] fix: keep PR text when a marker is missing, cap time zone length, ignore info icon clicks A missing end marker no longer deletes the text after the start marker. The time zone is capped at 64 characters. Clicking the info icon in a stage row no longer opens the row. LiveReview Pre-Commit Check: vouched (iter:1, coverage:0%) --- internal/api/users/profile_handlers.go | 5 ++++- internal/jobqueue/repo_sync_worker.go | 5 +++-- ui/src/components/reviews/AutoApprovePanel.tsx | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/api/users/profile_handlers.go b/internal/api/users/profile_handlers.go index e01f9c80..2d255689 100644 --- a/internal/api/users/profile_handlers.go +++ b/internal/api/users/profile_handlers.go @@ -238,7 +238,10 @@ func (ph *ProfileHandlers) UpdateTimeZone(c echo.Context) error { if err := c.Bind(&req); err != nil { return echo.NewHTTPError(http.StatusBadRequest, "Invalid request body") } - if _, err := time.LoadLocation(req.TimeZone); err != nil || req.TimeZone == "" || req.TimeZone == "Local" { + if len(req.TimeZone) > 64 || req.TimeZone == "" || req.TimeZone == "Local" { + return echo.NewHTTPError(http.StatusBadRequest, "time_zone must be a valid IANA zone, e.g. Asia/Kolkata") + } + if _, err := time.LoadLocation(req.TimeZone); err != nil { return echo.NewHTTPError(http.StatusBadRequest, "time_zone must be a valid IANA zone, e.g. Asia/Kolkata") } diff --git a/internal/jobqueue/repo_sync_worker.go b/internal/jobqueue/repo_sync_worker.go index 871e1c1d..d3e5af9a 100644 --- a/internal/jobqueue/repo_sync_worker.go +++ b/internal/jobqueue/repo_sync_worker.go @@ -495,7 +495,8 @@ func decisionReason(res *policyengine.FunnelResult) string { return strings.Join(parts, "; ") } -// replaceBetween swaps the text from start through end markers for section; ok is false if start is absent. +// replaceBetween swaps the text from start through end markers for section. +// ok is false if either marker is missing, so a lost end marker never deletes the author's text. func replaceBetween(body, start, end, section string) (string, bool) { i := strings.Index(body, start) if i < 0 { @@ -503,7 +504,7 @@ func replaceBetween(body, start, end, section string) (string, bool) { } j := strings.Index(body[i:], end) if j < 0 { - return body[:i] + strings.TrimRight(section, "\n"), true + return body, false } return body[:i] + strings.TrimRight(section, "\n") + body[i+j+len(end):], true } diff --git a/ui/src/components/reviews/AutoApprovePanel.tsx b/ui/src/components/reviews/AutoApprovePanel.tsx index 6fd8364f..268aa753 100644 --- a/ui/src/components/reviews/AutoApprovePanel.tsx +++ b/ui/src/components/reviews/AutoApprovePanel.tsx @@ -218,8 +218,8 @@ const AutoApprovePanel: React.FC<{ reviewId: number }> = ({ reviewId }) => {
{ - // Links and the Details button handle their own clicks. - if (canExpand && !(e.target as HTMLElement).closest('a,button')) toggle(); + // Links, the Details button and the info icon handle their own clicks. + if (canExpand && !(e.target as HTMLElement).closest('a,button,[aria-label="Help"]')) toggle(); }} >