Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

亻尔女子 #4

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
49 changes: 49 additions & 0 deletions l1nk4i/api/answer/create.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package answer

import (
"github.com/gin-contrib/sessions"
"github.com/gin-gonic/gin"
"github.com/google/uuid"
"l1nk4i/db"
"net/http"
)

func Create(c *gin.Context) {
var answerInfo struct {
QuestionID string `json:"question_id"`
Content string `json:"content"`
}

if err := c.ShouldBind(&answerInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
return
}

session := sessions.Default(c)
userID, exists := session.Get("user_id").(string)
if !exists {
c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
return
}

// Verify that the question exists
_, err := db.GetQuestionByQuestionID(answerInfo.QuestionID)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid question_id"})
return
}

answer := db.Answer{
AnswerID: uuid.New().String(),
UserID: userID,
QuestionID: answerInfo.QuestionID,
Content: answerInfo.Content,
}
if err := db.CreateAnswer(&answer); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "create answer error"})
return
}

c.JSON(http.StatusOK, gin.H{"answer_id": answer.AnswerID})
return
}
47 changes: 47 additions & 0 deletions l1nk4i/api/answer/delete.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package answer

import (
"github.com/gin-contrib/sessions"
"github.com/gin-gonic/gin"
"l1nk4i/db"
"net/http"
)

func Delete(c *gin.Context) {
var answerInfo struct {
AnswerID string `json:"answer_id"`
}

if err := c.ShouldBindJSON(&answerInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
return
}

// Verify user identity
session := sessions.Default(c)
userid, exists := session.Get("user_id").(string)
if !exists {
c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
return
}

answer, err := db.GetAnswerByAnswerID(answerInfo.AnswerID)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid answer_id"})
return
}

if answer.UserID != userid {
c.JSON(http.StatusForbidden, gin.H{"error": "permission denied"})
return
}

// Delete answer
err = db.DeleteAnswer(answerInfo.AnswerID)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "delete answer error"})
return
}

c.JSON(http.StatusOK, gin.H{"msg": "delete answer successful!"})
}
27 changes: 27 additions & 0 deletions l1nk4i/api/answer/get.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package answer

import (
"github.com/gin-gonic/gin"
"l1nk4i/db"
"net/http"
)

// Get gets answers to the question by question_id
func Get(c *gin.Context) {
var QuestionInfo struct {
QuestionId string `json:"question_id"`
}

if err := c.ShouldBind(&QuestionInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
return
}

answers, err := db.GetAnswersByQuestionID(QuestionInfo.QuestionId)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid question_id"})
return
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve error handling and add logging for database errors.

While it's good that error handling is implemented, there are a few areas for improvement:

  1. A database error doesn't necessarily mean the question_id is invalid. It could be a connection issue, for example.
  2. Returning a 500 Internal Server Error might be more appropriate for database errors.
  3. Logging the actual error would be beneficial for debugging.

Consider applying these changes:

 	answers, err := db.GetAnswersByQuestionID(QuestionInfo.QuestionId)
 	if err != nil {
-		c.JSON(http.StatusBadRequest, gin.H{"error": "invalid question_id"})
+		log.Printf("Database error: %v", err)
+		c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to retrieve answers"})
 		return
 	}

Also, consider adding a check for empty answers and return an appropriate message:

	if len(answers) == 0 {
		c.JSON(http.StatusNotFound, gin.H{"message": "No answers found for this question"})
		return
	}


c.JSON(http.StatusOK, gin.H{"data": answers})
}
48 changes: 48 additions & 0 deletions l1nk4i/api/answer/update.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package answer

import (
"github.com/gin-contrib/sessions"
"github.com/gin-gonic/gin"
"l1nk4i/db"
"net/http"
)

func Update(c *gin.Context) {
var answerInfo struct {
AnswerID string `json:"answer_id"`
Content string `json:"content"`
}

if err := c.ShouldBindJSON(&answerInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
return
}

// Verify user identity
session := sessions.Default(c)
userid, exists := session.Get("user_id").(string)
if !exists {
c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
return
}
Comment on lines +22 to +28
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance session handling and error messaging.

The current implementation correctly checks for user authentication, but there are a couple of improvements that can be made:

  1. The error message for invalid sessions could be more specific.
  2. It's a good practice to use constants for session keys to avoid typos and improve maintainability.

Consider applying the following changes:

  1. Improve the error message:
 if !exists {
-    c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
+    c.JSON(http.StatusUnauthorized, gin.H{"error": "User not authenticated", "details": "Valid session required"})
     return
 }
  1. Define a constant for the session key:
const UserIDKey = "user_id"

Then use it in the code:

-userid, exists := session.Get("user_id").(string)
+userid, exists := session.Get(UserIDKey).(string)


answer, err := db.GetAnswerByAnswerID(answerInfo.AnswerID)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid answer_id"})
return
}

if answer.UserID != userid {
c.JSON(http.StatusForbidden, gin.H{"error": "permission denied"})
return
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance error handling and add input validation.

The current implementation correctly retrieves the answer and checks user authorization. However, there are some improvements that can be made:

  1. Error messages could be more specific.
  2. There's no check for empty AnswerID or Content.

Consider applying the following changes:

  1. Improve error messages:
 if err != nil {
-    c.JSON(http.StatusBadRequest, gin.H{"error": "invalid answer_id"})
+    c.JSON(http.StatusNotFound, gin.H{"error": "Answer not found", "details": "The specified answer_id does not exist"})
     return
 }

 if answer.UserID != userid {
-    c.JSON(http.StatusForbidden, gin.H{"error": "permission denied"})
+    c.JSON(http.StatusForbidden, gin.H{"error": "Permission denied", "details": "You are not authorized to update this answer"})
     return
 }
  1. Add input validation:
if answerInfo.AnswerID == "" || answerInfo.Content == "" {
    c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid input", "details": "AnswerID and Content cannot be empty"})
    return
}

Place this validation immediately after the JSON binding and before the database operations.


// Update answer
err = db.UpdateAnswer(answerInfo.AnswerID, answerInfo.Content)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid answer"})
return
}

c.JSON(http.StatusOK, gin.H{"msg": "update answer successful!"})
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve error handling and add check for no changes made.

The current implementation correctly updates the answer and returns a success message. However, there are some improvements that can be made:

  1. The error message for update failure could be more specific.
  2. There's no check for the case where the update succeeds but no rows are affected (i.e., no changes were made).

Consider applying the following changes:

  1. Improve error message for update failure:
 err = db.UpdateAnswer(answerInfo.AnswerID, answerInfo.Content)
 if err != nil {
-    c.JSON(http.StatusBadRequest, gin.H{"error": "invalid answer"})
+    c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update answer", "details": err.Error()})
     return
 }
  1. Add a check for no changes made:
rowsAffected, err := db.UpdateAnswer(answerInfo.AnswerID, answerInfo.Content)
if err != nil {
    c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update answer", "details": err.Error()})
    return
}
if rowsAffected == 0 {
    c.JSON(http.StatusOK, gin.H{"msg": "No changes made to the answer"})
    return
}

This assumes that db.UpdateAnswer returns the number of rows affected. If it doesn't, you may need to modify the database function to return this information.

42 changes: 42 additions & 0 deletions l1nk4i/api/question/create.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package question

import (
"github.com/gin-contrib/sessions"
"github.com/gin-gonic/gin"
"github.com/google/uuid"
"l1nk4i/db"
"net/http"
)

func Create(c *gin.Context) {
var questionInfo struct {
Title string `json:"title"`
Content string `json:"content"`
}

if err := c.ShouldBind(&questionInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
return
}

session := sessions.Default(c)
userID, exists := session.Get("user_id").(string)
if !exists {
c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
return
}
Comment on lines +22 to +27
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding validation for the user ID.

While the function correctly retrieves and checks for the existence of a user ID in the session, it doesn't validate the format or content of the user ID.

Consider adding a validation step for the user ID:

 	userID, exists := session.Get("user_id").(string)
 	if !exists {
 		c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
 		return
 	}
+	if !isValidUserID(userID) {
+		c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid user ID"})
+		return
+	}

 // Add this helper function
+func isValidUserID(id string) bool {
+	// Implement your validation logic here
+	// For example, check if it's a valid UUID
+	_, err := uuid.Parse(id)
+	return err == nil
+}

Committable suggestion was skipped due to low confidence.


question := db.Question{
QuestionID: uuid.New().String(),
UserID: userID,
Title: questionInfo.Title,
Content: questionInfo.Content,
}
if err := db.CreateQuestion(&question); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "create question error"})
return
}
Comment on lines +29 to +38
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation and improve error handling for question creation.

While the function correctly creates a new question and interacts with the database, there are a few areas for improvement:

  1. Input validation for the question title and content is missing.
  2. The error message for database interaction could be more specific.

Consider implementing these improvements:

  1. Add input validation:
+	if len(questionInfo.Title) == 0 || len(questionInfo.Content) == 0 {
+		c.JSON(http.StatusBadRequest, gin.H{"error": "Title and content cannot be empty"})
+		return
+	}
+	// Add more validation as needed (e.g., max length)
  1. Improve error handling:
 	if err := db.CreateQuestion(&question); err != nil {
-		c.JSON(http.StatusInternalServerError, gin.H{"error": "create question error"})
+		c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create question: " + err.Error()})
 		return
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
question := db.Question{
QuestionID: uuid.New().String(),
UserID: userID,
Title: questionInfo.Title,
Content: questionInfo.Content,
}
if err := db.CreateQuestion(&question); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "create question error"})
return
}
if len(questionInfo.Title) == 0 || len(questionInfo.Content) == 0 {
c.JSON(http.StatusBadRequest, gin.H{"error": "Title and content cannot be empty"})
return
}
// Add more validation as needed (e.g., max length)
question := db.Question{
QuestionID: uuid.New().String(),
UserID: userID,
Title: questionInfo.Title,
Content: questionInfo.Content,
}
if err := db.CreateQuestion(&question); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create question: " + err.Error()})
return
}


c.JSON(http.StatusOK, gin.H{"question_id": question.QuestionID})
return
}
Comment on lines +40 to +42
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using HTTP 201 Created status for successful question creation.

While the current implementation returns a 200 OK status, it's more RESTful to use 201 Created when a new resource is successfully created.

Consider modifying the response as follows:

-	c.JSON(http.StatusOK, gin.H{"question_id": question.QuestionID})
+	c.JSON(http.StatusCreated, gin.H{"question_id": question.QuestionID})
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
c.JSON(http.StatusOK, gin.H{"question_id": question.QuestionID})
return
}
c.JSON(http.StatusCreated, gin.H{"question_id": question.QuestionID})
return
}

70 changes: 70 additions & 0 deletions l1nk4i/api/question/delete.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package question

import (
"github.com/gin-contrib/sessions"
"github.com/gin-gonic/gin"
"l1nk4i/db"
"net/http"
)

func Delete(c *gin.Context) {
var questionInfo struct {
QuestionID string `json:"question_id"`
}

if err := c.ShouldBind(&questionInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
return
}

// Verify user identity
session := sessions.Default(c)
userid, exists := session.Get("user_id").(string)
if !exists {
c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
return
}
Comment on lines +15 to +19
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix the session user_id retrieval logic

The current code uses a type assertion to retrieve user_id from the session, but it incorrectly interprets the result. The type assertion .(string) returns two values: the asserted value and a boolean indicating success. The variable exists actually indicates whether the type assertion succeeded, not whether the user_id exists in the session.

To correctly check for the existence of user_id and ensure it's of type string, consider the following modification:

-userid, exists := session.Get("user_id").(string)
-if !exists {
+userIDInterface := session.Get("user_id")
+if userIDInterface == nil {
     c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
     return
 }
+userid, ok := userIDInterface.(string)
+if !ok {
+    c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
+    return
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
userid, exists := session.Get("user_id").(string)
if !exists {
c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
return
}
userIDInterface := session.Get("user_id")
if userIDInterface == nil {
c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
return
}
userid, ok := userIDInterface.(string)
if !ok {
c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid session"})
return
}


question, err := db.GetQuestionByQuestionID(questionInfo.QuestionID)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid question_id"})
return
}

if question.UserID != userid {
c.JSON(http.StatusForbidden, gin.H{"error": "permission denied"})
return
}

// Delete question
err = db.DeleteQuestion(questionInfo.QuestionID)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "delete question error"})
return
}

// Delete answers to the question
err = deleteAnswers(questionInfo.QuestionID)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "delete answers error"})
return
}

c.JSON(http.StatusOK, gin.H{"msg": "delete question successful!"})
}

func deleteAnswers(questionID string) error {
answers, err := db.GetAnswersByQuestionID(questionID)
if err != nil {
return err
}

for _, answer := range *answers {
err = db.DeleteAnswer(answer.AnswerID)
if err != nil {
return err
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Handle errors during answer deletion more comprehensively

Currently, if an error occurs while deleting an individual answer, the function returns immediately, potentially leaving some answers undeleted. This could lead to inconsistent data states. Consider modifying the function to attempt deletion of all answers, collecting any errors, and handling them after all deletions have been attempted.

You can refactor the code as follows:

 func deleteAnswers(questionID string) error {
     answers, err := db.GetAnswersByQuestionID(questionID)
     if err != nil {
         return err
     }

-    for _, answer := range *answers {
-        err = db.DeleteAnswer(answer.AnswerID)
-        if err != nil {
-            return err
-        }
+    var deleteErrors []error
+    for _, answer := range *answers {
+        if err := db.DeleteAnswer(answer.AnswerID); err != nil {
+            deleteErrors = append(deleteErrors, err)
+        }
     }

-    return nil
+    if len(deleteErrors) > 0 {
+        // Aggregate errors or handle accordingly
+        return fmt.Errorf("errors occurred during answer deletion: %v", deleteErrors)
+    }
+    return nil
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for _, answer := range *answers {
err = db.DeleteAnswer(answer.AnswerID)
if err != nil {
return err
}
}
var deleteErrors []error
for _, answer := range *answers {
if err := db.DeleteAnswer(answer.AnswerID); err != nil {
deleteErrors = append(deleteErrors, err)
}
}
if len(deleteErrors) > 0 {
// Aggregate errors or handle accordingly
return fmt.Errorf("errors occurred during answer deletion: %v", deleteErrors)
}
return nil


return nil
}
27 changes: 27 additions & 0 deletions l1nk4i/api/question/get.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package question

import (
"github.com/gin-gonic/gin"
"l1nk4i/db"
"net/http"
)

// Get gets question by question_id
func Get(c *gin.Context) {
var questionInfo struct {
QuestionID string `json:"question_id"`
}

if err := c.ShouldBind(&questionInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
return
}

question, err := db.GetQuestionByQuestionID(questionInfo.QuestionID)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid question_id"})
return
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve error handling and add logging for database interactions.

While error handling is implemented, there are a few areas for improvement:

  1. A 400 Bad Request status might not be appropriate for all database errors. Consider using 500 Internal Server Error for database-related issues.
  2. The error message "invalid question_id" might not accurately represent all possible database errors.
  3. Logging the actual error would be beneficial for debugging.

Consider applying these changes:

 question, err := db.GetQuestionByQuestionID(questionInfo.QuestionID)
 if err != nil {
-    c.JSON(http.StatusBadRequest, gin.H{"error": "invalid question_id"})
+    log.Printf("Error fetching question: %v", err)
+    if err == db.ErrQuestionNotFound {
+        c.JSON(http.StatusNotFound, gin.H{"error": "Question not found"})
+    } else {
+        c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to fetch question"})
+    }
     return
 }

This assumes the existence of a db.ErrQuestionNotFound error. If it doesn't exist, consider adding it to your database package.

Committable suggestion was skipped due to low confidence.


c.JSON(http.StatusOK, gin.H{"data": question})
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Overall, the implementation is solid with room for minor improvements.

The file successfully implements a GET endpoint for retrieving question details using the Gin framework. The structure is clean and the basic functionality is in place. However, consider the following general improvement:

Enhance input validation by checking if the QuestionID is empty before making the database call. This can prevent unnecessary database queries for obviously invalid inputs.

 if err := c.ShouldBind(&questionInfo); err != nil {
     c.JSON(http.StatusBadRequest, gin.H{"error": "Failed to parse request body"})
     return
 }

+if questionInfo.QuestionID == "" {
+    c.JSON(http.StatusBadRequest, gin.H{"error": "Question ID cannot be empty"})
+    return
+}

 question, err := db.GetQuestionByQuestionID(questionInfo.QuestionID)
 // ... rest of the function

This addition will improve the robustness of your API and potentially reduce unnecessary load on your database.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
package question
import (
"github.com/gin-gonic/gin"
"l1nk4i/db"
"net/http"
)
// Get gets question by question_id
func Get(c *gin.Context) {
var questionInfo struct {
QuestionID string `json:"question_id"`
}
if err := c.ShouldBind(&questionInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
return
}
question, err := db.GetQuestionByQuestionID(questionInfo.QuestionID)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid question_id"})
return
}
c.JSON(http.StatusOK, gin.H{"data": question})
}
package question
import (
"github.com/gin-gonic/gin"
"l1nk4i/db"
"net/http"
)
// Get gets question by question_id
func Get(c *gin.Context) {
var questionInfo struct {
QuestionID string `json:"question_id"`
}
if err := c.ShouldBind(&questionInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
return
}
if questionInfo.QuestionID == "" {
c.JSON(http.StatusBadRequest, gin.H{"error": "Question ID cannot be empty"})
return
}
question, err := db.GetQuestionByQuestionID(questionInfo.QuestionID)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid question_id"})
return
}
c.JSON(http.StatusOK, gin.H{"data": question})
}

31 changes: 31 additions & 0 deletions l1nk4i/api/question/list.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package question

import (
"github.com/gin-contrib/sessions"
"github.com/gin-gonic/gin"
"l1nk4i/db"
"net/http"
)

// List lists user's all question_id
func List(c *gin.Context) {
session := sessions.Default(c)
userID, exists := session.Get("user_id").(string)
if !exists {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid session"})
return
}

questions, err := db.GetQuestionByUserID(userID)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid session"})
return
}
Comment on lines +19 to +23
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve error handling for database operations.

The database interaction is straightforward, but the error handling can be improved in two ways:

  1. The error message is incorrect for a database error.
  2. The error handling doesn't distinguish between different types of errors.

Consider implementing the following changes:

  1. Update the error message:
- c.JSON(http.StatusBadRequest, gin.H{"error": "invalid session"})
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "database error"})
  1. Implement more granular error handling:
if err != nil {
    if err == db.ErrNotFound {
        c.JSON(http.StatusNotFound, gin.H{"error": "no questions found for user"})
    } else {
        c.JSON(http.StatusInternalServerError, gin.H{"error": "database error"})
    }
    return
}

This assumes that db.ErrNotFound is defined in your db package. If it's not, consider adding it to distinguish between "no results" and other database errors.


var questionIDs []string
for _, question := range *questions {
questionIDs = append(questionIDs, question.QuestionID)
}

c.JSON(http.StatusOK, gin.H{"question_id": questionIDs})
}
Comment on lines +25 to +31
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider handling the case of no questions found.

The extraction of question IDs and the response format look good. However, there's no specific handling for the case where no questions are found for the user.

Consider adding a check for empty results:

if len(questionIDs) == 0 {
    c.JSON(http.StatusOK, gin.H{"message": "No questions found for user", "question_id": []string{}})
} else {
    c.JSON(http.StatusOK, gin.H{"question_id": questionIDs})
}

This will provide a more informative response when a user has no questions, while still maintaining the expected response structure.

24 changes: 24 additions & 0 deletions l1nk4i/api/question/search.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package question

import (
"github.com/gin-gonic/gin"
"l1nk4i/db"
"net/http"
)

func Search(c *gin.Context) {
var searchInfo struct {
Content string `json:"content"`
}

if err := c.ShouldBind(&searchInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling in request binding.

The current implementation is good, but we can enhance the error handling to provide more detailed feedback to the client.

Consider updating the error handling as follows:

 func Search(c *gin.Context) {
 	var searchInfo struct {
 		Content string `json:"content" binding:"required"`
 	}

 	if err := c.ShouldBind(&searchInfo); err != nil {
-		c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
+		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+		return
 	}

+	if searchInfo.Content == "" {
+		c.JSON(http.StatusBadRequest, gin.H{"error": "search content cannot be empty"})
+		return
+	}

This change will:

  1. Provide more specific error messages from the binding process.
  2. Add a check for empty content after successful binding.
  3. Ensure the function returns after sending an error response.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func Search(c *gin.Context) {
var searchInfo struct {
Content string `json:"content"`
}
if err := c.ShouldBind(&searchInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON"})
}
func Search(c *gin.Context) {
var searchInfo struct {
Content string `json:"content" binding:"required"`
}
if err := c.ShouldBind(&searchInfo); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
if searchInfo.Content == "" {
c.JSON(http.StatusBadRequest, gin.H{"error": "search content cannot be empty"})
return
}


questions, err := db.SearchQuestions(searchInfo.Content)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid content"})
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Refine error handling for database operations.

The current implementation returns a 400 Bad Request for any database error, which might not always be appropriate. Consider differentiating between client errors and server errors.

Suggested improvement:

 	questions, err := db.SearchQuestions(searchInfo.Content)
 	if err != nil {
-		c.JSON(http.StatusBadRequest, gin.H{"error": "invalid content"})
+		if err == db.ErrNoResults {
+			c.JSON(http.StatusNotFound, gin.H{"error": "No questions found"})
+		} else {
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to search questions"})
+		}
+		return
 	}

This change assumes the existence of a db.ErrNoResults error. If it doesn't exist, you might need to implement it in the db package.

Committable suggestion was skipped due to low confidence.


c.JSON(http.StatusOK, gin.H{"data": questions})
}
Loading