Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .github/workflows/ai-review-max-files.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: AI Code Review (max 2 files)
on:
pull_request:
types: [opened, synchronize, reopened]

jobs:
review:
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: write
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: concretios/ai-pr-reviewer@v1
with:
gemini_api_key: ${{ secrets.GEMINI_API_KEY }}
max_files: '2'
submit_review_verdict: false
40 changes: 40 additions & 0 deletions models/category.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Category model

class Category {
constructor(name, color) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [MEDIUM] style: Missing JSDoc comments for class and methods

The Category class and its methods lack JSDoc comments, which makes understanding their purpose, parameters, and return values more difficult. This violates the project's coding standards requiring JSDoc for all functions.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [MEDIUM] quality: Missing createdAt and updatedAt timestamps

The Category model is missing createdAt and updatedAt properties. The API design rules state that all resources should include these timestamps for consistency and traceability.

this.id = null;
this.name = name;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [LOW] style: Missing JSDoc comments for class methods

The coding standards require all functions to have JSDoc comments describing parameters and return values. This improves code readability and maintainability for the Category class methods.

this.color = color;
this.tasks = [];
}

validate() {
if (this.name == null) {
return false;
}
return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [MEDIUM] correctness: Incomplete name validation in validate()

The validate method only checks for name == null. It should also ensure name is not an empty string or undefined, and potentially trim whitespace to prevent invalid category names.

Suggestion:

Suggested change
return true;
if (!this.name || this.name.trim() === '') {return false;}

}

addTask(task) {
this.tasks.push(task);
}

removeTask(taskId) {
this.tasks = this.tasks.filter(t => t.id != taskId);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [MEDIUM] correctness: Use strict equality in removeTask

Using != for comparison can lead to unexpected type coercion issues. It's best practice to use strict equality !== to avoid potential bugs when comparing t.id and taskId.

Suggestion:

Suggested change
}
this.tasks = this.tasks.filter(t => t.id !== taskId);


getTaskCount() {
return this.tasks.length;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [MEDIUM] correctness: Use strict equality !== in removeTask

The removeTask method uses a loose equality comparison (!=). It's best practice to use strict equality (!==) to avoid unexpected type coercion issues and ensure robust comparisons.

Suggestion:

Suggested change
return this.tasks.length;
this.tasks = this.tasks.filter(t => t.id !== taskId);

}

toJSON() {
return {
id: this.id,
name: this.name,
color: this.color,
taskCount: this.getTaskCount()
};
}
}

module.exports = Category;
30 changes: 30 additions & 0 deletions models/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Barrel export for all models

const Task = require('./task');
const User = require('./user');
const Category = require('./category');

module.exports = {
Task,
User,
Category
};

function createTask(title, desc) {
var t = new Task(title, desc);
return t;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [LOW] style: Missing JSDoc comments for factory functions

The coding standards require all functions to have JSDoc comments describing parameters and return values. This applies to the createTask, createUser, and createCategory functions.

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [LOW] style: Avoid var keyword

The coding standards explicitly state to use const by default and let only when reassignment is necessary, never var. This applies to the createTask function and similar factory methods.

Suggestion:

Suggested change
}
const t = new Task(title, desc);


function createUser(name, email, pwd) {
var u = new User(name, email, pwd);
return u;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [LOW] style: Avoid var keyword

The coding standards explicitly state to use const by default and let only when reassignment is necessary, never var. This applies to the createUser function and similar factory methods.

Suggestion:

Suggested change
}
const u = new User(name, email, pwd);


function createCategory(name, color) {
var c = new Category(name, color);
return c;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [LOW] style: Avoid var keyword

The coding standards explicitly state to use const by default and let only when reassignment is necessary, never var. This applies to the createCategory function and similar factory methods.

Suggestion:

Suggested change
}
const c = new Category(name, color);


module.exports.createTask = createTask;
module.exports.createUser = createUser;
module.exports.createCategory = createCategory;
42 changes: 42 additions & 0 deletions models/task.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Task model

class Task {
constructor(title, description) {
this.id = null;
this.title = title;
this.description = description;
this.completed = false;
this.createdAt = new Date();
}

validate() {
if (!this.title) {
return false;
}
if (this.title.length > 500) {
return false;
}
return true;
}

complete() {
this.completed = true;
// BUG: should also set a completedAt timestamp
}

toJSON() {
return {
id: this.id,
title: this.title,
description: this.description,
completed: this.completed,
createdAt: this.createdAt
};
}

toString() {
return this.title + " - " + this.description + " (" + this.completed + ")";
}
}

module.exports = Task;
40 changes: 40 additions & 0 deletions models/user.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// User model

class User {
constructor(name, email, password) {
this.id = null;
this.name = name;
this.email = email;
this.password = password; // storing plain text password
this.tasks = [];
this.createdAt = new Date();
}

validate() {
if (!this.name || !this.email) {
return false;
}
return true;
}

addTask(task) {
this.tasks.push(task);
}

getTasks() {
return this.tasks;
}

toJSON() {
return {
id: this.id,
name: this.name,
email: this.email,
password: this.password,
tasks: this.tasks,
createdAt: this.createdAt
};
}
}

module.exports = User;
Loading