-
Notifications
You must be signed in to change notification settings - Fork 26
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
Feat:finish task #7
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package admin | ||
|
||
import ( | ||
"github.com/gin-contrib/sessions" | ||
"github.com/gin-gonic/gin" | ||
"hduhelp_text/api/user" | ||
"hduhelp_text/db" | ||
"net/http" | ||
) | ||
|
||
func Admin(c *gin.Context) { | ||
session := sessions.Default(c) | ||
if auth, ok := session.Get("authenticated").(bool); !ok || !auth { | ||
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"}) | ||
return | ||
} | ||
if session.Get("username") != "admin" { | ||
c.JSON(http.StatusUnauthorized, gin.H{"error": "管理员才可以访问!"}) | ||
return | ||
} | ||
var questions []user.Question | ||
if err := db.DB.Preload("Answers").Find(&questions).Error; err != nil { | ||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||
return | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Optimize database query and improve error handling. The current implementation fetches all questions and their answers, which may lead to performance issues with large datasets. Consider the following improvements:
Here's a suggested refactoring: func Admin(c *gin.Context) {
// Previous code...
page, _ := strconv.Atoi(c.DefaultQuery("page", "1"))
pageSize, _ := strconv.Atoi(c.DefaultQuery("pageSize", "10"))
questions, total, err := fetchQuestions(page, pageSize)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to fetch questions: " + err.Error()})
return
}
c.JSON(http.StatusOK, gin.H{
"questions": questions,
"total": total,
"page": page,
"pageSize": pageSize,
})
}
func fetchQuestions(page, pageSize int) ([]user.Question, int64, error) {
var questions []user.Question
var total int64
offset := (page - 1) * pageSize
if err := db.DB.Model(&user.Question{}).Count(&total).Error; err != nil {
return nil, 0, fmt.Errorf("error counting questions: %w", err)
}
if err := db.DB.Preload("Answers").Offset(offset).Limit(pageSize).Find(&questions).Error; err != nil {
return nil, 0, fmt.Errorf("error fetching questions: %w", err)
}
return questions, total, nil
} This refactoring introduces pagination and improves error handling, which should enhance performance and maintainability. |
||
c.JSON(http.StatusOK, questions) | ||
|
||
} | ||
Comment on lines
+25
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance response structure with metadata. The current response only includes the questions data. To improve the API, consider adding metadata to the response. This could include:
Here's a suggested improvement for the response structure: c.JSON(http.StatusOK, gin.H{
"data": questions,
"meta": gin.H{
"total": total,
"page": page,
"pageSize": pageSize,
"totalPages": int(math.Ceil(float64(total) / float64(pageSize))),
},
}) This structure provides more context to the client about the data being returned and allows for easier implementation of pagination on the frontend. |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,49 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
package admin | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"github.com/gin-contrib/sessions" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"github.com/gin-gonic/gin" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"gorm.io/gorm" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"hduhelp_text/db" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"net/http" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"strconv" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func CheckAnswer(c *gin.Context) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
session := sessions.Default(c) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if auth, ok := session.Get("authenticated").(bool); !ok || !auth { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if session.Get("username") != "admin" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "管理员才可以访问!"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+18
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Role-Based Access Control (RBAC) for Admin Verification Currently, the code checks if Apply this diff to improve admin verification: if auth, ok := session.Get("authenticated").(bool); !ok || !auth {
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"})
return
}
-if session.Get("username") != "admin" {
+if session.Get("role") != "admin" {
c.JSON(http.StatusUnauthorized, gin.H{"error": "管理员才可以访问!"})
return
} Ensure that the user's role is set during authentication: // After successful authentication
-session.Set("username", user.Username)
+session.Set("role", user.Role) // Assume 'Role' is a field in your user model
session.Save()
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
idStr := c.PostForm("id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
checkStatusStr := c.PostForm("check") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
checkStatus, err := strconv.Atoi(checkStatusStr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusBadRequest, gin.H{"error": "无效的审核状态"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+25
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate 'checkStatus' Value Before Updating Currently, If checkStatus, err := strconv.Atoi(checkStatusStr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "无效的审核状态"})
return
}
+if checkStatus != 0 && checkStatus != 1 {
+ c.JSON(http.StatusBadRequest, gin.H{"error": "审核状态只能是0或1"})
+ return
+} 📝 Committable suggestion
Suggested change
Comment on lines
+23
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate the 'id' Parameter Before Database Query The Apply this diff to validate and convert idStr := c.PostForm("id")
+id, err := strconv.Atoi(idStr)
+if err != nil {
+ c.JSON(http.StatusBadRequest, gin.H{"error": "无效的ID"})
+ return
+}
checkStatusStr := c.PostForm("check")
checkStatus, err := strconv.Atoi(checkStatusStr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "无效的审核状态"})
return
} Update the database query to use the integer var answer db.Answer
-if err := db.DB.First(&answer, idStr).Error; err != nil {
+if err := db.DB.First(&answer, id).Error; err != nil {
if err == gorm.ErrRecordNotFound {
c.JSON(http.StatusNotFound, gin.H{"error": "ID 不存在"})
} else { 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var answer db.Answer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err := db.DB.First(&answer, idStr).Error; err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err == gorm.ErrRecordNotFound { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusNotFound, gin.H{"error": "ID 不存在"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid Exposing Internal Error Messages to Users Returning Apply this diff: } else {
- c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ // Log the error details
+ // log.Errorf("Database error: %v", err)
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "服务器内部错误"})
} Ensure you have a logging mechanism in place to capture the error details for debugging purposes.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// 更新审核状态 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
answer.CheckStatus = checkStatus // 假设你在 Answer 结构体中添加了 CheckStatus 字段 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err := db.DB.Save(&answer).Error; err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusOK, gin.H{"message": "审核状态更新成功"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,47 @@ | ||||||||||||||||||||||||
package admin | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||
"github.com/gin-contrib/sessions" | ||||||||||||||||||||||||
"github.com/gin-gonic/gin" | ||||||||||||||||||||||||
"gorm.io/gorm" | ||||||||||||||||||||||||
"hduhelp_text/db" | ||||||||||||||||||||||||
"net/http" | ||||||||||||||||||||||||
"strconv" | ||||||||||||||||||||||||
) | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
func CheckQuestion(c *gin.Context) { | ||||||||||||||||||||||||
session := sessions.Default(c) | ||||||||||||||||||||||||
if auth, ok := session.Get("authenticated").(bool); !ok || !auth { | ||||||||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"}) | ||||||||||||||||||||||||
return | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
if session.Get("username") != "admin" { | ||||||||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "管理员才可以访问!"}) | ||||||||||||||||||||||||
return | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
checkStatusStr := c.PostForm("check") | ||||||||||||||||||||||||
checkStatus, err := strconv.Atoi(checkStatusStr) | ||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||
c.JSON(http.StatusBadRequest, gin.H{"error": "无效的审核状态"}) | ||||||||||||||||||||||||
return | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
Comment on lines
+23
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate After converting For example, if valid if checkStatus != 0 && checkStatus != 1 {
c.JSON(http.StatusBadRequest, gin.H{"error": "无效的审核状态"})
return
} |
||||||||||||||||||||||||
|
||||||||||||||||||||||||
idStr := c.PostForm("id") | ||||||||||||||||||||||||
var question db.Question | ||||||||||||||||||||||||
if err := db.DB.First(&question, idStr).Error; err != nil { | ||||||||||||||||||||||||
Comment on lines
+30
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parse and validate the Parsing Modify the code as follows: idStr := c.PostForm("id")
+id, err := strconv.Atoi(idStr)
+if err != nil {
+ c.JSON(http.StatusBadRequest, gin.H{"error": "无效的ID"})
+ return
+}
var question db.Question
-if err := db.DB.First(&question, idStr).Error; err != nil {
+if err := db.DB.First(&question, id).Error; err != nil { 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||
if err == gorm.ErrRecordNotFound { | ||||||||||||||||||||||||
c.JSON(http.StatusNotFound, gin.H{"error": "ID 不存在"}) | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid exposing internal error messages to clients Returning internal error details with Apply this diff to return a generic error message: if err == gorm.ErrRecordNotFound {
c.JSON(http.StatusNotFound, gin.H{"error": "ID 不存在"})
} else {
- c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "服务器内部错误"})
} Similarly, update the error handling in line 42: if err := db.DB.Save(&question).Error; err != nil {
- c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "服务器内部错误"})
return
} Also applies to: 42-42 |
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
return | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
question.CheckStatus = checkStatus | ||||||||||||||||||||||||
if err := db.DB.Save(&question).Error; err != nil { | ||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||
return | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
c.JSON(http.StatusOK, gin.H{"message": "审核状态更新成功"}) | ||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
package answer | ||
|
||
import ( | ||
"github.com/gin-contrib/sessions" | ||
"github.com/gin-gonic/gin" | ||
"hduhelp_text/db" | ||
"net/http" | ||
"strconv" | ||
) | ||
|
||
func Answer(c *gin.Context) { | ||
session := sessions.Default(c) | ||
if auth, ok := session.Get("authenticated").(bool); !ok || !auth { | ||
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"}) | ||
return | ||
} | ||
respondent := session.Get("username").(string) | ||
Comment on lines
+11
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider enhancing error handling and authentication check. The function signature and basic authentication check look good. However, consider the following improvements:
Example of using middleware: func AuthMiddleware() gin.HandlerFunc {
return func(c *gin.Context) {
session := sessions.Default(c)
if auth, ok := session.Get("authenticated").(bool); !ok || !auth {
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "Unauthorized", "code": "AUTH_REQUIRED"})
return
}
c.Next()
}
}
// In your router setup:
// r.POST("/answer", AuthMiddleware(), Answer) This approach would simplify your |
||
idStr := c.PostForm("id") // 获取问题 ID | ||
answerText := c.PostForm("Answer") | ||
|
||
// 将问题 ID 从 string 转换为 uint | ||
questionID, err := strconv.ParseUint(idStr, 10, 32) | ||
if err != nil { | ||
c.JSON(http.StatusBadRequest, gin.H{"error": "无效的问题 ID"}) | ||
return | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhance input validation for robustness. While the basic input retrieval and question ID conversion are implemented correctly, consider the following improvements:
Example implementation: const maxAnswerLength = 1000 // Adjust as needed
answerText := c.PostForm("Answer")
if answerText == "" {
c.JSON(http.StatusBadRequest, gin.H{"error": "Answer text is required"})
return
}
if len(answerText) > maxAnswerLength {
c.JSON(http.StatusBadRequest, gin.H{"error": "Answer text exceeds maximum length"})
return
}
questionID, err := strconv.ParseUint(idStr, 10, 32)
if err != nil || questionID == 0 || questionID > math.MaxUint32 {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid question ID"})
return
} These checks will help prevent invalid data from being processed or stored. |
||
|
||
answer := db.Answer{ | ||
QuestionID: uint(questionID), // 转换为 uint | ||
AnswerText: answerText, | ||
Respondent: respondent, | ||
} | ||
|
||
if err := db.DB.Create(&answer).Error; err != nil { | ||
c.JSON(http.StatusBadRequest, gin.H{"error": "回答失败"}) | ||
return | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve database interaction and error handling. The basic structure for creating and saving an answer is in place, but consider the following enhancements:
Example implementation: tx := db.DB.Begin()
defer func() {
if r := recover(); r != nil {
tx.Rollback()
}
}()
if err := tx.Error; err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to start transaction"})
return
}
// Check for existing answer
var existingAnswer db.Answer
if err := tx.Where("question_id = ? AND respondent = ?", questionID, respondent).First(&existingAnswer).Error; err == nil {
c.JSON(http.StatusConflict, gin.H{"error": "You have already answered this question"})
return
}
answer := db.Answer{
QuestionID: uint(questionID),
AnswerText: answerText,
Respondent: respondent,
}
if err := tx.Create(&answer).Error; err != nil {
tx.Rollback()
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to save answer"})
return
}
if err := tx.Commit().Error; err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to commit transaction"})
return
}
c.JSON(http.StatusCreated, gin.H{"message": "Answer submitted successfully", "id": answer.ID}) This approach provides better error handling, prevents duplicate answers, and ensures data consistency through the use of a transaction. |
||
c.JSON(http.StatusOK, gin.H{"message": "回答成功"}) | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,36 @@ | ||||||||||||||||||||||||||||||||||||
package answer | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||||||||
"github.com/gin-contrib/sessions" | ||||||||||||||||||||||||||||||||||||
"github.com/gin-gonic/gin" | ||||||||||||||||||||||||||||||||||||
"hduhelp_text/db" | ||||||||||||||||||||||||||||||||||||
"net/http" | ||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
func ChangeAnswer(c *gin.Context) { | ||||||||||||||||||||||||||||||||||||
session := sessions.Default(c) | ||||||||||||||||||||||||||||||||||||
if auth, ok := session.Get("authenticated").(bool); !ok || !auth { | ||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"}) | ||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
id := c.PostForm("id") | ||||||||||||||||||||||||||||||||||||
newAnswerText := c.PostForm("Answer") | ||||||||||||||||||||||||||||||||||||
Comment on lines
+16
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate input parameters to prevent empty or invalid data Currently, the Consider adding validation to ensure that id := c.PostForm("id")
newAnswerText := c.PostForm("Answer")
+if id == "" || newAnswerText == "" {
+ c.JSON(http.StatusBadRequest, gin.H{"error": "ID and Answer cannot be empty"})
+ return
+} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
var answer db.Answer | ||||||||||||||||||||||||||||||||||||
if err := db.DB.First(&answer, id).Error; err != nil { | ||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||
Comment on lines
+20
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid exposing internal error messages Returning internal error details ( Update the error response: - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "Internal server error"}) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+19
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return 404 Not Found when the answer does not exist When the specified answer is not found in the database, the current code returns a 500 Internal Server Error. It's more appropriate to return a 404 Not Found status to indicate that the resource doesn't exist. Modify the error handling to check for if err := db.DB.First(&answer, id).Error; err != nil {
- c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ if errors.Is(err, gorm.ErrRecordNotFound) {
+ c.JSON(http.StatusNotFound, gin.H{"error": "Answer not found"})
+ } else {
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "Internal server error"})
+ }
return
} Make sure to import the import (
"github.com/gin-contrib/sessions"
"github.com/gin-gonic/gin"
"hduhelp_text/db"
"net/http"
+ "errors"
) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if session.Get("username") != answer.Respondent { | ||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "只有本作者才可以修改!"}) | ||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+24
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use proper HTTP status code for authorization failure When the user is authenticated but not authorized to modify the answer, it's appropriate to return Adjust the status code: if session.Get("username") != answer.Respondent {
- c.JSON(http.StatusUnauthorized, gin.H{"error": "只有本作者才可以修改!"})
+ c.JSON(http.StatusForbidden, gin.H{"error": "只有本作者才可以修改!"})
return
} 📝 Committable suggestion
Suggested change
🛠️ Refactor suggestion Perform type assertion when retrieving session data When retrieving Modify the code to include a type assertion: -if session.Get("username") != answer.Respondent {
+username, ok := session.Get("username").(string)
+if !ok || username != answer.Respondent {
c.JSON(http.StatusForbidden, gin.H{"error": "只有本作者才可以修改!"})
return
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
answer.AnswerText = newAnswerText | ||||||||||||||||||||||||||||||||||||
if err := db.DB.Save(&answer).Error; err != nil { | ||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+30
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid exposing internal error messages on save failure Similar to the previous error handling, avoid sending internal error details when saving to the database fails. Update the error response: if err := db.DB.Save(&answer).Error; err != nil {
- c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update the answer"})
return
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusOK, gin.H{"message": "修改成功"}) | ||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,31 @@ | ||||||||||||||||||
package answer | ||||||||||||||||||
|
||||||||||||||||||
import ( | ||||||||||||||||||
"github.com/gin-contrib/sessions" | ||||||||||||||||||
"github.com/gin-gonic/gin" | ||||||||||||||||||
"hduhelp_text/db" | ||||||||||||||||||
"net/http" | ||||||||||||||||||
) | ||||||||||||||||||
|
||||||||||||||||||
func DeleteAnswer(c *gin.Context) { | ||||||||||||||||||
session := sessions.Default(c) | ||||||||||||||||||
if auth, ok := session.Get("authenticated").(bool); !ok || !auth { | ||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"}) | ||||||||||||||||||
return | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
id := c.PostForm("id") | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate the 'id' parameter to prevent unexpected behavior The 'id' parameter retrieved from the form data is not validated. If 'id' is empty or invalid, it could lead to unintended errors. Apply this diff to add input validation: id := c.PostForm("id")
+if id == "" {
+ c.JSON(http.StatusBadRequest, gin.H{"error": "缺少参数id"})
+ return
+} This ensures that the 'id' parameter is provided before proceeding. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
var question db.Question | ||||||||||||||||||
if err := db.DB.First(&question, id).Error; err != nil { | ||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||
return | ||||||||||||||||||
} | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid exposing internal error details to the client Exposing raw error messages can reveal sensitive internal information. It's better to return a generic error message to the client. The above diff addresses this concern by replacing Return 404 Not Found when the question does not exist Currently, when the question is not found, the code returns a 500 Internal Server Error. It's more appropriate to return a 404 Not Found status code to indicate that the requested resource does not exist. Apply this diff to adjust the response: if err := db.DB.First(&question, id).Error; err != nil {
- c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ c.JSON(http.StatusNotFound, gin.H{"error": "问题不存在"})
return
} This change ensures that clients receive the correct HTTP status code and a user-friendly error message. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
if session.Get("username") != question.Questioner { | ||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "只有本作者才可以删除!"}) | ||||||||||||||||||
return | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
db.DB.Delete(&db.Answer{}, "question_id = ?", id) | ||||||||||||||||||
db.DB.Delete(&question) | ||||||||||||||||||
c.JSON(http.StatusOK, gin.H{"message": "删除成功"}) | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider implementing internationalization for response messages To enhance user experience for a broader audience, consider supporting multiple languages by implementing internationalization for response messages. This will make the application more adaptable and user-friendly in different locales. |
||||||||||||||||||
} | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename function to accurately reflect its functionality The function Apply this diff to rename the function: -func DeleteAnswer(c *gin.Context) {
+func DeleteQuestion(c *gin.Context) { Additionally, update the file name and any references to this function accordingly.
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,58 @@ | ||||||||||||||||||
package question | ||||||||||||||||||
|
||||||||||||||||||
import ( | ||||||||||||||||||
"github.com/gin-contrib/sessions" | ||||||||||||||||||
"github.com/gin-gonic/gin" | ||||||||||||||||||
"hduhelp_text/db" | ||||||||||||||||||
"hduhelp_text/utils" | ||||||||||||||||||
"net/http" | ||||||||||||||||||
) | ||||||||||||||||||
|
||||||||||||||||||
func Ask(c *gin.Context) { | ||||||||||||||||||
session := sessions.Default(c) | ||||||||||||||||||
if auth, ok := session.Get("authenticated").(bool); !ok || !auth { | ||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"}) | ||||||||||||||||||
return | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
username, ok := session.Get("username").(string) | ||||||||||||||||||
if !ok || username == "" { | ||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "用户名不存在"}) | ||||||||||||||||||
return | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
questionText := c.PostForm("question") | ||||||||||||||||||
if questionText == "" { | ||||||||||||||||||
c.JSON(http.StatusBadRequest, gin.H{"error": "问题内容不能为空"}) | ||||||||||||||||||
return | ||||||||||||||||||
} | ||||||||||||||||||
Comment on lines
+24
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add input validation and sanitization for While checking if |
||||||||||||||||||
|
||||||||||||||||||
newQuestion := db.Question{ | ||||||||||||||||||
QuestionText: questionText, | ||||||||||||||||||
Questioner: username, | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
if err := db.DB.Create(&newQuestion).Error; err != nil { | ||||||||||||||||||
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) | ||||||||||||||||||
return | ||||||||||||||||||
} | ||||||||||||||||||
Comment on lines
+35
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid exposing internal error details and use appropriate HTTP status codes Returning Apply this diff to address the issues: if err := db.DB.Create(&newQuestion).Error; err != nil {
- c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "服务器内部错误"})
return
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
answerText, err := utils.GenerateAIAnswer(questionText) | ||||||||||||||||||
if err != nil { | ||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "无法生成答案"}) | ||||||||||||||||||
return | ||||||||||||||||||
} | ||||||||||||||||||
Comment on lines
+40
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider processing AI answer generation asynchronously to improve performance Generating the AI answer synchronously within the request handler can lead to increased response times and potential timeouts, especially if the AI service experiences delays. Consider offloading the AI answer generation to a background worker or message queue, and immediately respond to the user indicating that their question has been received and will be answered shortly. |
||||||||||||||||||
|
||||||||||||||||||
answer := db.Answer{ | ||||||||||||||||||
QuestionID: newQuestion.ID, // 确保这里是 uint 类型 | ||||||||||||||||||
AnswerText: answerText, | ||||||||||||||||||
Respondent: "ai", | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
if err := db.DB.Create(&answer).Error; err != nil { | ||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "答案保存失败"}) | ||||||||||||||||||
return | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
c.JSON(http.StatusOK, gin.H{"message": "提问成功"}) | ||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,38 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
package question | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"github.com/gin-contrib/sessions" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"github.com/gin-gonic/gin" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"hduhelp_text/db" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"net/http" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func ChangeQuestion(c *gin.Context) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
session := sessions.Default(c) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if auth, ok := session.Get("authenticated").(bool); !ok || !auth { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+10
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding logging for unauthorized access attempts. The authentication check is implemented correctly. However, to enhance security monitoring, consider adding logging for unauthorized access attempts. Here's a suggested modification: func ChangeQuestion(c *gin.Context) {
session := sessions.Default(c)
if auth, ok := session.Get("authenticated").(bool); !ok || !auth {
+ log.Printf("Unauthorized access attempt to ChangeQuestion")
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"})
return
} Don't forget to import the "log" package if it's not already imported. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id := c.PostForm("id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
questionText := c.PostForm("question") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var existingQuestion db.Question | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err := db.DB.First(&existingQuestion, "id = ?", id).Error; err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if session.Get("username") != existingQuestion.Questioner { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "只有本作者才可以修改!"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+17
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add input validation for question ID and text. While the question retrieval and authorization check are implemented correctly, there's no validation of the input parameters. This could potentially lead to security vulnerabilities. Consider adding input validation: id := c.PostForm("id")
questionText := c.PostForm("question")
+// Validate id
+if id == "" {
+ c.JSON(http.StatusBadRequest, gin.H{"error": "Question ID is required"})
+ return
+}
+
+// Validate questionText
+if len(questionText) < 10 || len(questionText) > 500 {
+ c.JSON(http.StatusBadRequest, gin.H{"error": "Question text must be between 10 and 500 characters"})
+ return
+}
var existingQuestion db.Question
if err := db.DB.First(&existingQuestion, "id = ?", id).Error; err != nil { Adjust the length constraints for 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
existingQuestion.QuestionText = questionText | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err := db.DB.Save(&existingQuestion).Error; err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+31
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Optimize to avoid unnecessary database writes. The question update process is correct, but it doesn't check if the new question text is different from the existing one. This could lead to unnecessary database writes. Consider adding a check before updating: +if existingQuestion.QuestionText != questionText {
existingQuestion.QuestionText = questionText
if err := db.DB.Save(&existingQuestion).Error; err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
return
}
+} else {
+ c.JSON(http.StatusOK, gin.H{"message": "No changes made"})
+ return
+} This will avoid unnecessary database operations if the question text hasn't changed. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusOK, gin.H{"message": "修改成功"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+37
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider returning updated question data with success message. While the success response is appropriate, it might be more informative to return the updated question data along with the success message. Consider modifying the response: -c.JSON(http.StatusOK, gin.H{"message": "修改成功"})
+c.JSON(http.StatusOK, gin.H{
+ "message": "修改成功",
+ "question": gin.H{
+ "id": existingQuestion.ID,
+ "questionText": existingQuestion.QuestionText,
+ "questioner": existingQuestion.Questioner,
+ // Add other relevant fields
+ },
+}) This provides more context to the client about the updated question.
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,37 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
package question | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"github.com/gin-contrib/sessions" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"github.com/gin-gonic/gin" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"hduhelp_text/db" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"net/http" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func DeleteQuestion(c *gin.Context) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
session := sessions.Default(c) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if auth, ok := session.Get("authenticated").(bool); !ok || !auth { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+12
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve session authentication check The current session authentication check may cause a panic if the type assertion fails. Ensure that the type assertion is handled properly. Update the authentication check: if auth, ok := session.Get("authenticated").(bool); !ok || !auth {
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"})
return
}
+ if !auth {
+ c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"})
+ return
+ } Alternatively, you can simplify the session check: auth := session.Get("authenticated")
if auth != true {
c.JSON(http.StatusUnauthorized, gin.H{"error": "未登录"})
return
}
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id := c.PostForm("id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var username string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+16
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate and convert The Apply this diff to validate and convert + import (
+ "strconv"
+ // other imports...
+ )
// inside the function
- id := c.PostForm("id")
+ idParam := c.PostForm("id")
+ id, err := strconv.Atoi(idParam)
+ if err != nil {
+ c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid question ID"})
+ return
+ }
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err := db.DB.Model(&db.Question{}).Select("questioner").Where("id = ?", id).Scan(&username).Error; err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+18
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle the case when the question does not exist When querying for the questioner, if the question with the given Modify the code to check if the question exists: var username string
- if err := db.DB.Model(&db.Question{}).Select("questioner").Where("id = ?", id).Scan(&username).Error; err != nil {
+ result := db.DB.Model(&db.Question{}).Select("questioner").Where("id = ?", id).Scan(&username)
+ if result.Error != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
return
+ }
+ if result.RowsAffected == 0 {
+ c.JSON(http.StatusNotFound, gin.H{"error": "Question not found"})
+ return
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if session.Get("username") != username { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "只有本作者才可以删除!"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+23
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure type assertion when retrieving the username from the session The comparison Update the code to safely retrieve and compare the username: - if session.Get("username") != username {
+ userSessionName, ok := session.Get("username").(string)
+ if !ok {
+ c.JSON(http.StatusInternalServerError, gin.H{"error": "Session error"})
+ return
+ }
+ if userSessionName != username {
c.JSON(http.StatusUnauthorized, gin.H{"error": "只有本作者才可以删除!"})
return
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err := db.DB.Delete(&db.Answer{}, "question_id = ?", id).Error; err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err := db.DB.Delete(&db.Question{}, id).Error; err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+31
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct the deletion query for the question When deleting the question, passing Modify the deletion statement to include the condition: - if err := db.DB.Delete(&db.Question{}, id).Error; err != nil {
+ if err := db.DB.Delete(&db.Question{}, "id = ?", id).Error; err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
return
} 📝 Committable suggestion
Suggested change
Comment on lines
+27
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use a database transaction to ensure atomicity of deletions Currently, the deletion of answers and the question are separate operations. If deleting the answers succeeds but deleting the question fails, the database will be left in an inconsistent state. Apply this refactor to use a transaction: + tx := db.DB.Begin()
- if err := db.DB.Delete(&db.Answer{}, "question_id = ?", id).Error; err != nil {
+ if err := tx.Delete(&db.Answer{}, "question_id = ?", id).Error; err != nil {
+ tx.Rollback()
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
return
}
- if err := db.DB.Delete(&db.Question{}, id).Error; err != nil {
+ if err := tx.Delete(&db.Question{}, "id = ?", id).Error; err != nil {
+ tx.Rollback()
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
return
}
+ if err := tx.Commit().Error; err != nil {
+ c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ return
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c.JSON(http.StatusOK, gin.H{"message": "删除成功"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance authentication and authorization checks.
While the current implementation is functional, consider the following improvements:
Here's a suggested refactoring:
This refactoring improves code organization and allows for easier expansion of the admin check logic in the future.