feat: UI polish batch 2 — log search, teamless SSO block, multi-select mappings, backup fixes#68
Merged
TerrifiedBug merged 4 commits intomainfrom Mar 8, 2026
Merged
Conversation
Contributor
Greptile SummaryThis PR delivers five UI/UX improvements across the dashboard: client-side log search with highlighting, a full-page gate for teamless SSO users, multi-select team mapping for IdP group sync, a reduced teams-pill threshold, and backup reliability fixes including a super-admin Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Browser requests backup download] --> B{Session valid?}
B -- No --> C[401 Unauthorized]
B -- Yes --> D{User is super-admin?}
D -- No --> E[403 Forbidden]
D -- Yes --> F{Filename passes sanitizeFilename?}
F -- No --> G[400 Bad Request]
F -- Yes --> H{Filename ends with .dump?}
H -- No --> I[400 Bad Request]
H -- Yes --> J[fs.access check]
J -- File missing --> K[404 Not Found]
J -- File exists --> L[fs.stat + createReadStream]
L --> M[Stream file as octet-stream response]
style L fill:#f97316,color:#fff
Last reviewed commit: 5d1d7de |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+56
to
+58
| const stat = await fs.stat(filePath); | ||
| const stream = createReadStream(filePath); | ||
| const webStream = Readable.toWeb(stream) as ReadableStream; |
Contributor
There was a problem hiding this comment.
fs.stat and createReadStream unprotected after fs.access check
The fs.access check at line 51 is wrapped in a try/catch that returns a clean 404, but fs.stat and createReadStream (lines 56–58) are outside any error handler. If the .dump file is removed between the fs.access check and the subsequent calls (e.g., during a concurrent backup cleanup), fs.stat will throw an unhandled exception and Next.js will return a 500 error instead of a graceful 404.
The simpler fix is to drop the redundant fs.access check and use fs.stat directly inside the try/catch:
Suggested change
| const stat = await fs.stat(filePath); | |
| const stream = createReadStream(filePath); | |
| const webStream = Readable.toWeb(stream) as ReadableStream; | |
| let stat: Awaited<ReturnType<typeof fs.stat>>; | |
| try { | |
| stat = await fs.stat(filePath); | |
| } catch { | |
| return new Response("Backup not found", { status: 404 }); | |
| } | |
| const stream = createReadStream(filePath); | |
| const webStream = Readable.toWeb(stream) as ReadableStream; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/backups/[filename]/download/route.ts
Line: 56-58
Comment:
**`fs.stat` and `createReadStream` unprotected after `fs.access` check**
The `fs.access` check at line 51 is wrapped in a try/catch that returns a clean 404, but `fs.stat` and `createReadStream` (lines 56–58) are outside any error handler. If the `.dump` file is removed between the `fs.access` check and the subsequent calls (e.g., during a concurrent backup cleanup), `fs.stat` will throw an unhandled exception and Next.js will return a 500 error instead of a graceful 404.
The simpler fix is to drop the redundant `fs.access` check and use `fs.stat` directly inside the try/catch:
```suggestion
let stat: Awaited<ReturnType<typeof fs.stat>>;
try {
stat = await fs.stat(filePath);
} catch {
return new Response("Backup not found", { status: 404 });
}
const stream = createReadStream(filePath);
const webStream = Readable.toWeb(stream) as ReadableStream;
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+3
to
+13
| export function highlightMatch(text: string, search: string): ReactNode { | ||
| if (!search) return text; | ||
| const idx = text.toLowerCase().indexOf(search.toLowerCase()); | ||
| if (idx === -1) return text; | ||
| return ( | ||
| <> | ||
| {text.slice(0, idx)} | ||
| <mark className="bg-yellow-500/30 text-yellow-200 rounded-sm px-0.5">{text.slice(idx, idx + search.length)}</mark> | ||
| {text.slice(idx + search.length)} | ||
| </> | ||
| ); |
Contributor
There was a problem hiding this comment.
highlightMatch only highlights the first occurrence
The current implementation uses indexOf and stops after the first match. A log message like "ERROR: failed connection, error code 500" searched for "error" would highlight only the first occurrence, leaving subsequent matches unhighlighted while all of them appeared in the filtered results.
Consider iterating over all occurrences by replacing the single-indexOf approach with a loop that accumulates all match segments before returning them as a React fragment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/log-search-utils.tsx
Line: 3-13
Comment:
**`highlightMatch` only highlights the first occurrence**
The current implementation uses `indexOf` and stops after the first match. A log message like `"ERROR: failed connection, error code 500"` searched for `"error"` would highlight only the first occurrence, leaving subsequent matches unhighlighted while all of them appeared in the filtered results.
Consider iterating over all occurrences by replacing the single-`indexOf` approach with a loop that accumulates all match segments before returning them as a React fragment.
How can I resolve this? If you propose a fix, please make it concise.
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
1 participant
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.
Summary
listBackups()now validates.dumpfile exists before showing a backup; added failed-backup error banner in settings.dumpfiles directly from the available backups tableTest plan