Initial Commit #2
Conversation
This project is a Kanban-style task board with a Go backend and a Svelte frontend. Tasks are persisted to a local JSON file and exposed via REST endpoints. The UI supports columns (Kanban statuses), drag-and-drop, inline editing, and a calendar view for deadlines.
Summary of ChangesHello @antongo10, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers the foundational implementation of a Kanban-style task board application. It establishes a robust Go backend for data management and a dynamic Svelte frontend for user interaction. The system supports full lifecycle management of tasks and columns, offering both a traditional board view and a calendar-based deadline view. Key improvements include a file-based data store, comprehensive API endpoints, and a refined frontend development experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the initial implementation of a Kanban board application, with a Go backend and a Svelte frontend. The changes are extensive and form the foundation of the project. My review focuses on improving correctness, robustness, and maintainability. Key feedback points include addressing potential race conditions and non-uniqueness in ID generation in the Go backend, improving error handling for file I/O and API responses, and making the frontend more resilient to API failures and edge cases. I've also included suggestions to improve code clarity and adhere to best practices in both Go and Svelte.
| _ = f.save(); return true | ||
| } | ||
|
|
||
| func randomID() string { return time.Now().UTC().Format("20060102T150405.000000000") } |
There was a problem hiding this comment.
Using time.Now() to generate IDs is not safe. Under high load, or if the system clock is adjusted, it's possible to generate duplicate IDs. This is a critical issue as it can lead to data corruption or incorrect behavior when trying to reference specific items.
A much safer approach is to use a library that generates universally unique identifiers (UUIDs).
import "github.com/google/uuid"
func randomID() string { return uuid.New().String() }| function cancelAdd() { addingDateISO = null; } | ||
| function saveAdd() { | ||
| if (!addForm.name?.trim()) return; | ||
| const defaultColId = columns?.[0]?.id; |
There was a problem hiding this comment.
If there are no columns, columns?.[0]?.id will be undefined. When taskboard.addTask is called, it will send a request to the backend with an undefined status_id. The Go backend does not validate the presence of StatusID on task creation, which will result in a task being created that doesn't belong to any column, making it invisible on the board.
You should add a check to ensure a column exists before attempting to create a task, or disable the add functionality if no columns are present.
| } | ||
| } | ||
|
|
||
| func writeJSON(w http.ResponseWriter, status int, v interface{}) { w.WriteHeader(status); _ = json.NewEncoder(w).Encode(v) } |
There was a problem hiding this comment.
The error returned by json.NewEncoder(w).Encode(v) is being ignored. This can hide important errors, such as if the value v cannot be marshaled to JSON, or if there's an issue writing the response to the client. This error should be logged to help with debugging.
func writeJSON(w http.ResponseWriter, status int, v interface{}) {
w.WriteHeader(status)
if err := json.NewEncoder(w).Encode(v); err != nil {
log.Printf("ERROR: failed to write JSON response: %v", err)
}
}| async addColumn(name) { | ||
| const trimmed = (name ?? '').trim(); | ||
| const res = await fetch(`${API_BASE}/api/kanban`, { method: 'POST', headers: { 'Content-Type':'application/json' }, body: JSON.stringify({ title: trimmed }) }); | ||
| const created = await res.json(); | ||
| update((s) => { s.columns = [...s.columns, { id: created.id, name: created.title }]; return s; }); | ||
| return created.id; | ||
| }, |
There was a problem hiding this comment.
The fetch calls in this store are not wrapped in try...catch blocks. If a network error occurs or the server is down, fetch will throw an error, resulting in an unhandled promise rejection which will crash the application or leave it in an inconsistent state. All asynchronous API calls should handle potential errors gracefully.
async addColumn(name) {
const trimmed = (name ?? '').trim();
try {
const res = await fetch(`${API_BASE}/api/kanban`, { method: 'POST', headers: { 'Content-Type':'application/json' }, body: JSON.stringify({ title: trimmed }) });
if (!res.ok) throw new Error(`API error: ${res.status}`);
const created = await res.json();
update((s) => { s.columns = [...s.columns, { id: created.id, name: created.title }]; return s; });
return created.id;
} catch (e) {
console.error('Failed to add column:', e);
// Optionally, you could show a notification to the user here.
return null;
}
},| f.mu.Lock(); defer f.mu.Unlock() | ||
| id := randomID() | ||
| c := Column{ID: id, Title: strings.TrimSpace(title)} | ||
| f.db.Kanban = append(f.db.Kanban, c); _ = f.save(); return c |
There was a problem hiding this comment.
Errors from f.save() are being ignored here and in many other database methods (UpdateColumn, DeleteColumn, UpdateTask, DeleteTask). If saving the database file fails (e.g., due to disk space or permissions issues), the operation will appear to succeed in memory, but the data will be lost when the application restarts. This leads to data inconsistency and a poor user experience.
All calls to f.save() should have their errors handled. The calling function should probably return the error to the HTTP handler, which can then respond with a 500 Internal Server Error status.
f.db.Kanban = append(f.db.Kanban, c)
if err := f.save(); err != nil {
log.Printf("ERROR: failed to save database: %v", err)
// Decide how to handle the failed save. Maybe revert the change?
}
return c| w.Header().Set("Access-Control-Allow-Origin", "*") | ||
| func withCORS(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Access-Control-Allow-Origin", "*") |
There was a problem hiding this comment.
Using Access-Control-Allow-Origin: * is a security risk in a production environment, as it allows any website to make requests to your API. For development, this is often fine, but for production, you should restrict this to the specific origin of your frontend application.
origin := r.Header.Get("Origin")
// In a real app, you'd have a list of allowed origins.
if origin == "http://localhost:5174" { // Or your frontend's production URL
w.Header().Set("Access-Control-Allow-Origin", origin)
} else {
// Optionally, deny the request if the origin is not allowed.
}| .pytest_cache/ | ||
| .cache/ |
There was a problem hiding this comment.
These patterns seem either unrelated to the project's tech stack or too broad, which could lead to unintentionally ignoring important files.
.pytest_cache/is a cache directory forpytest, a Python testing framework. Since this is a Go and JavaScript project, this line is unnecessary..cache/is a very generic pattern and could match cache directories from various tools that you might want to keep, or it could be too broad and catch something unexpected in the future. It's better to specify caches for tools you are actually using, e.g.,frontend/.vite/which is already present.
.idea/
| (async () => { | ||
| try { | ||
| const [colsRes, tasksRes] = await Promise.all([ | ||
| fetch(`${API_BASE}/api/kanban`), | ||
| fetch(`${API_BASE}/api/tasks`) | ||
| ]); | ||
| const cols = await colsRes.json(); | ||
| const tasks = await tasksRes.json(); | ||
| set({ columns: cols.map(c=>({ id: c.id, name: c.title })), tasks: tasks.map(t=>({ id: t.id, name: t.name, description: t.description, priority: t.priority, deadline: t.deadline, columnId: t.status_id, position: t.position })) }); | ||
| } catch (e) { | ||
| // Fallback to persisted/local if backend not available | ||
| try { const raw = localStorage.getItem('taskboard_v1'); if (raw) set(JSON.parse(raw)); } catch {} | ||
| } | ||
| })(); |
There was a problem hiding this comment.
The catch blocks for both the initial data fetch and localStorage access are either empty or don't properly inform the user or developer of the failure.
- If the initial
fetchfails, the app falls back tolocalStoragesilently. It would be better to log this error to the console so developers are aware of backend issues. - If
localStorageparsing fails, the error is swallowed. This could hide issues with corrupted local data.
Errors should be logged to aid in debugging.
(async () => {
try {
const [colsRes, tasksRes] = await Promise.all([
fetch(`${API_BASE}/api/kanban`),
fetch(`${API_BASE}/api/tasks`)
]);
const cols = await colsRes.json();
const tasks = await tasksRes.json();
set({ columns: cols.map(c=>({ id: c.id, name: c.title })), tasks: tasks.map(t=>({ id: t.id, name: t.name, description: t.description, priority: t.priority, deadline: t.deadline, columnId: t.status_id, position: t.position })) });
} catch (e) {
console.error("Failed to fetch data from backend, falling back to localStorage.", e);
// Fallback to persisted/local if backend not available
try {
const raw = localStorage.getItem('taskboard_v1');
if (raw) set(JSON.parse(raw));
} catch (lsError) {
console.error("Failed to load data from localStorage.", lsError);
}
}
})();| b, err := ioutil.ReadFile(f.dbPath) | ||
| if err != nil { return err } | ||
| var db Database | ||
| if err := json.Unmarshal(b, &db); err != nil { return err } | ||
| f.db = db | ||
| return nil | ||
| } | ||
|
|
||
| func (f *FileDB) save() error { | ||
| // ensure directory exists | ||
| if dir := filepath.Dir(f.dbPath); dir != "." && dir != "" { _ = os.MkdirAll(dir, 0755) } | ||
| b, err := json.MarshalIndent(f.db, "", " ") | ||
| if err != nil { return err } | ||
| return ioutil.WriteFile(f.dbPath, b, 0644) |
There was a problem hiding this comment.
The io/ioutil package has been deprecated since Go 1.16. It's recommended to use os.ReadFile and os.WriteFile instead. This improves code maintainability by using the current standard library APIs.
func (f *FileDB) load() error {
f.mu.Lock(); defer f.mu.Unlock()
b, err := os.ReadFile(f.dbPath)
if err != nil { return err }
var db Database
if err := json.Unmarshal(b, &db); err != nil { return err }
f.db = db
return nil
}
func (f *FileDB) save() error {
// ensure directory exists
if dir := filepath.Dir(f.dbPath); dir != "." && dir != "" { _ = os.MkdirAll(dir, 0755) }
b, err := json.MarshalIndent(f.db, "", " ")
if err != nil { return err }
return os.WriteFile(f.dbPath, b, 0644)
}| async updateTask(id, patch) { | ||
| await fetch(`${API_BASE}/api/tasks/${id}`, { method: 'PATCH', headers: { 'Content-Type':'application/json' }, body: JSON.stringify({ name: patch.name, description: patch.description, priority: patch.priority, deadline: patch.deadline }) }); | ||
| update((s) => { s.tasks = s.tasks.map((t) => (t.id === id ? { ...t, ...patch } : t)); return s; }); |
There was a problem hiding this comment.
This function uses an optimistic UI update, which is fine, but it doesn't use the response from the backend. The backend API for updating a task returns the fully updated task object. The frontend should use this response to update its state. This ensures that the frontend state is perfectly in sync with the backend, especially if the backend performs any transformations on the data (like trimming whitespace, which it does for the task name). This pattern should be applied to other update functions like renameColumn and moveTask as well.
async updateTask(id, patch) {
const res = await fetch(`${API_BASE}/api/tasks/${id}`, { method: 'PATCH', headers: { 'Content-Type':'application/json' }, body: JSON.stringify({ name: patch.name, description: patch.description, priority: patch.priority, deadline: patch.deadline, status_id: patch.columnId, position: patch.position }) });
const updatedTask = await res.json();
update((s) => {
s.tasks = s.tasks.map((t) => (t.id === id ? { ...t, ...patch, name: updatedTask.name /* and other fields from response */ } : t));
return s;
});
},1 participant
This project is a Kanban-style task board with a Go backend and a Svelte frontend. Tasks are persisted to a local JSON file and exposed via REST endpoints. The UI supports columns (Kanban statuses), drag-and-drop, inline editing, and a calendar view for deadlines.
Changes summary
Implemented Go backend emulating a production database with JSON.
Risks
JSON database is not scalable.
Manual testing done
Curl requests and UAT.