demo: Collapsed Iterations UI Demo#16
Conversation
This sets up the base state for testing that files first modified in iteration 2 are correctly marked as Modified (not Added). See file-first-modified-later-iteration/README.md for details.
330cd19 to
67a8384
Compare
There was a problem hiding this comment.
Pull request overview
Adds a small set of demo utilities under demo-collapsed-iterations/ to support the “collapsed iterations UI” demo scenario in this E2E test-data repository.
Changes:
- Added a minimal
loggerwrapper aroundconsole.log/console.warn. - Added a stub
authenticate()function. - Added an
ApiClientwrapper forfetch()withget()/post()methods.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| demo-collapsed-iterations/logger.ts | Introduces a simple logging helper for the demo. |
| demo-collapsed-iterations/auth.ts | Adds a placeholder authentication function used by the demo. |
| demo-collapsed-iterations/api-client.ts | Adds a minimal fetch-based API client for demo requests. |
Comments suppressed due to low confidence (4)
demo-collapsed-iterations/auth.ts:2
- PR description is focused on the collapsed iterations UI demo, but this adds generic
logger,auth, andapi-clientmodules. Either update the PR description to explain why these modules are required for the demo, or move them to a separate PR to keep the demo change set aligned with S-4.2.2.
// Authentication module - token-based approach
interface AuthToken {
demo-collapsed-iterations/api-client.ts:10
- URL construction via string concatenation can produce incorrect URLs (e.g., double slashes, missing slashes, or a baseUrl with a path segment). Prefer building the URL with
new URL(path, baseUrl)(or normalize trailing/leading slashes) so callers can pass either/fooorfoosafely.
}
demo-collapsed-iterations/api-client.ts:17
post()JSON-stringifies the body but does not set aContent-Type: application/jsonheader. Many servers will not parse the request body as JSON without this header; set the appropriate header (and consider accepting/merging caller-provided headers).
const timeoutId = setTimeout(() => controller.abort(), this.timeout);
try {
const response = await fetch(`${this.baseUrl}${path}`, {
signal: controller.signal,
demo-collapsed-iterations/api-client.ts:11
get()returns the rawResponsewithout checkingresponse.ok(andpost()does the same). Consider standardizing behavior (e.g., throw on non-ok, or return a typed result) so callers don’t accidentally treat error responses as success.
}
async get<T>(path: string): Promise<T> {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| accessToken: string; | ||
| expiresAt: number; | ||
| } | ||
|
|
||
| export async function authenticate(credentials: { |
There was a problem hiding this comment.
Hardcoded credentials in the authentication check ('admin' / 'secret') are a security risk and can easily leak into non-demo usage. If this is meant to be a demo stub, move the values to a clearly named demo/test config (e.g., env var or fixture) and/or make the function name/exports explicitly demo-only to avoid accidental reuse.
Demo PR for S-4.2.2 collapsed iterations UI. This PR will have force-push history to demonstrate discarded iteration groups.
🔍 Review in CodjiFlo