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.

Sign up for GitHub

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

Jump to bottom

dalai2547/sucu-52-documents-update-doc-by-id #11

Merged
merged 5 commits into from
Oct 2, 2024
Merged

dalai2547/sucu-52-documents-update-doc-by-id #11

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9db3439
feat: document update and delete
D33102 Sep 29, 2024
5946111
fix: from comments
D33102 Sep 29, 2024
4a1ae8f
fix: no doc id found
D33102 Oct 1, 2024
78c8b73
fix: change to patch
D33102 Oct 1, 2024
4dd7968
fix: comment
D33102 Oct 1, 2024
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
12 changes: 6 additions & 6 deletions cmd/server/fiber_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,19 @@ func (s *FiberHttpServer) initUserRouter(router fiber.Router, httpHandler handle
userRouter := router.Group("/users")

userRouter.Get("/", httpHandler.User().GetAllUsers)
userRouter.Post("/", httpHandler.Middleware().IsLogin, httpHandler.Middleware().SuperAdmin, httpHandler.User().CreateUser)
userRouter.Patch("/", httpHandler.Middleware().IsLogin, httpHandler.User().UpdateProfile)
userRouter.Put("/:user_id", httpHandler.Middleware().IsLogin, httpHandler.Middleware().SuperAdmin, httpHandler.User().UpdateUserByID)
userRouter.Post("/", httpHandler.User().CreateUser)
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you updated with dev branch?

Copy link
Contributor

Choose a reason for hiding this comment

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

please update with latest dev branch kub

}

func (s *FiberHttpServer) initAttachmentRouter(router fiber.Router, httpHandler handlers.Handler) {
attachmentRouter := router.Group("/attachments")

attachmentRouter.Post("/:document_id", httpHandler.Attachment().CreateAttachments)
attachmentRouter.Get("/:document_id", httpHandler.Attachment().CreateAttachments)
Copy link
Contributor

Choose a reason for hiding this comment

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

please update with latest dev branch kub

}

func (s *FiberHttpServer) initDocumentRouter(router fiber.Router, httpHandler handlers.Handler) {
documentRouter := router.Group("/documents")
documentsRouter := router.Group("/documents")

documentsRouter.Post("/", httpHandler.Document().CreateDocument)
documentsRouter.Delete("/:document_id", httpHandler.Document().DeleteDocumentByID)
Copy link
Contributor

Choose a reason for hiding this comment

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

For post/delete routes, let's add some middlewares to e.g.

documentsRouter.Post("/", httpHandler.Middleware().IsLogin, httpHandler.Middleware().SuperAdmin, httpHandler.Document().CreateDocument)
documentsRouter.Delete("/:document_id", httpHandler.Middleware().IsLogin, httpHandler.Middleware().SuperAdmin, httpHandler.Document().DeleteDocumentByID)

If you have any problems trying to test routes that requires login, we can talk na

Copy link
Contributor

Choose a reason for hiding this comment

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

apply middleware here kub, both using .IsLogin() middleware kub


documentRouter.Post("/", httpHandler.Middleware().IsLogin, httpHandler.Document().CreateDocument)
}
34 changes: 34 additions & 0 deletions internal/domain/usecases/document_usecase.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package usecases

import (
"errors"
"fmt"

"github.com/isd-sgcu/sucu-backend-2024/internal/domain/entities"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/isd-sgcu/sucu-backend-2024/utils"
"github.com/isd-sgcu/sucu-backend-2024/utils/constant"
"go.uber.org/zap"
"gorm.io/gorm"
)

type documentUsecase struct {
Expand Down Expand Up @@ -75,9 +77,41 @@ func (u *documentUsecase) CreateDocument(document *dtos.CreateDocumentDTO) *appe
}

func (u *documentUsecase) UpdateDocumentByID(ID string, updateMap interface{}) *apperror.AppError {
existingDocument, err := u.documentRepository.FindDocumentByID(ID)
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
u.logger.Named("UpdateDocumentByID").Error(constant.ErrDocumentNotFound, zap.String("documentID", ID))
return apperror.InternalServerError(constant.ErrDocumentNotFound)
}
u.logger.Named("UpdateDocumentByID").Error(constant.ErrFindDocumentByID, zap.String("documentID", ID), zap.Error(err))
return apperror.InternalServerError(constant.ErrFindDocumentByID)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure about this, but I think u.documentRepository.UpdateDocumentByID will return gorm.ErrRecordNotFound if there's no documents with that id. So, you don't have to u.documentRepository.FindDocumentByID and maybe move if errors.Is(err, gorm.ErrRecordNotFound) into if err := u.documentRepository.UpdateDocumentByID(ID, updateMap)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure too bro @bookpanda, please check it kub @D33102

Copy link
Contributor Author

@D33102 D33102 Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think it will return ErrRecordNotFound since when I tested with non-recorded ID, it didn't give any error. @bookpanda @wiraphatys

Copy link
Contributor Author

@D33102 D33102 Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Screenshot 2567-09-30 at 15 03 48

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I do this instead?
Screenshot 2567-09-30 at 15 06 36

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks that looks good to me


if err := u.documentRepository.UpdateDocumentByID(ID, updateMap); err != nil {
u.logger.Named("UpdateDocumentByID").Error(constant.ErrUpdateDocumentFailed, zap.String("documentID", ID), zap.Error(err))
return apperror.InternalServerError(constant.ErrUpdateDocumentFailed)
}

u.logger.Named("UpdateDocumentByID").Info("Success: Document updated", zap.String("documentID", existingDocument.ID))
return nil
}

func (u *documentUsecase) DeleteDocumentByID(ID string) *apperror.AppError {
existingDocument, err := u.documentRepository.FindDocumentByID(ID)

if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
u.logger.Named("DeleteDocumentByID").Error(constant.ErrDocumentNotFound, zap.String("documentID", ID))
return apperror.InternalServerError(constant.ErrDocumentNotFound)
}
u.logger.Named("DeleteDocumentByID").Error(constant.ErrFindDocumentByID, zap.String("documentID", ID), zap.Error(err))
return apperror.InternalServerError(constant.ErrFindDocumentByID)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

also here

if err := u.documentRepository.DeleteDocumentByID(ID); err != nil {
u.logger.Named("DeleteDocumentByID").Error(constant.ErrDeleteDocumentFailed, zap.String("documentID", ID), zap.Error(err))
return apperror.InternalServerError(constant.ErrDeleteDocumentFailed)
}

u.logger.Named("DeleteDocumentByID").Info("Success: Document deleted", zap.String("documentID", existingDocument.ID))
return nil
}
47 changes: 45 additions & 2 deletions internal/interface/handlers/document_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/isd-sgcu/sucu-backend-2024/internal/interface/dtos"
"github.com/isd-sgcu/sucu-backend-2024/pkg/response"
"github.com/isd-sgcu/sucu-backend-2024/pkg/validator"
"github.com/isd-sgcu/sucu-backend-2024/utils/constant"
)

type DocumentHandler struct {
Expand Down Expand Up @@ -107,7 +108,32 @@ func (h *DocumentHandler) CreateDocument(c *fiber.Ctx) error {
// @Failure 500 {object} response.Response
// @Router /documents/{document_id} [put]
func (h *DocumentHandler) UpdateDocumentByID(c *fiber.Ctx) error {
return nil
documentID := c.Params("document_id")
if documentID == "" {
resp := response.NewResponseFactory(response.ERROR, "Document ID is required")
return resp.SendResponse(c, fiber.StatusBadRequest)
}

// Parse the request body into UpdateDocumentDTO
var updateDocumentDTO dtos.UpdateDocumentDTO
if err := c.BodyParser(&updateDocumentDTO); err != nil {
resp := response.NewResponseFactory(response.ERROR, "Invalid request body")
return resp.SendResponse(c, fiber.StatusBadRequest)
}

err := h.documentUsecase.UpdateDocumentByID(documentID, updateDocumentDTO)
if err != nil {
if err.Error() == "document not found" {
resp := response.NewResponseFactory(response.ERROR, "Document not found")
return resp.SendResponse(c, fiber.StatusNotFound)
}
resp := response.NewResponseFactory(response.ERROR, "Failed to update document")
return resp.SendResponse(c, fiber.StatusInternalServerError)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use this instead so that the logic for checking status code is all in usecase layer

apperr := h.documentUsecase.UpdateDocumentByID(documentID, updateDocumentDTO)
	if apperr != nil {
		resp := response.NewResponseFactory(response.ERROR, apperr.Error())
		return resp.SendResponse(c, apperr.HttpCode)
	}


// Return success response
resp := response.NewResponseFactory(response.SUCCESS, "Document updated successfully")
return resp.SendResponse(c, fiber.StatusOK)
}

// DeleteDocumentByID godoc
Expand All @@ -120,5 +146,22 @@ func (h *DocumentHandler) UpdateDocumentByID(c *fiber.Ctx) error {
// @Failure 500 {object} response.Response
// @Router /documents/{document_id} [delete]
func (h *DocumentHandler) DeleteDocumentByID(c *fiber.Ctx) error {
return nil
documentID := c.Params("document_id")
if documentID == "" {
resp := response.NewResponseFactory(response.ERROR, "Document ID is required")
return resp.SendResponse(c, fiber.StatusBadRequest)
}

err := h.documentUsecase.DeleteDocumentByID(documentID)
if err != nil {
if err.Error() == constant.ErrDocumentNotFound {
resp := response.NewResponseFactory(response.ERROR, constant.ErrDocumentNotFound)
return resp.SendResponse(c, fiber.StatusNotFound)
}
resp := response.NewResponseFactory(response.ERROR, constant.ErrDeleteDocumentFailed)
return resp.SendResponse(c, fiber.StatusInternalServerError)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

also here


resp := response.NewResponseFactory(response.SUCCESS, "Document deleted successfully")
return resp.SendResponse(c, fiber.StatusOK)
}
2 changes: 1 addition & 1 deletion internal/interface/repositories/document.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ type DocumentRepository interface {
FindDocumentsByRole(roles *[]string) (*[]entities.Document, error)
InsertDocument(document *entities.Document) error
UpdateDocumentByID(ID string, updateMap interface{}) error
DeleteUserByID(ID string) error
DeleteDocumentByID(ID string) error
}
6 changes: 3 additions & 3 deletions internal/interface/repositories/document_repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ func (r *documentRepository) InsertDocument(document *entities.Document) error {
}

func (r *documentRepository) UpdateDocumentByID(ID string, updateMap interface{}) error {
return nil
return r.db.Model(&entities.Document{}).Where("id = ?", ID).Updates(updateMap).Error
}

func (r *documentRepository) DeleteUserByID(ID string) error {
return nil
func (r *documentRepository) DeleteDocumentByID(ID string) error {
return r.db.Where("id = ?", ID).Delete(&entities.Document{}).Error
}
4 changes: 4 additions & 0 deletions utils/constant/error_constant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@ var (
// doc error
ErrInvalidDocType = "invalid document type"
ErrInsertDocumentFailed = "failed to insert document"
ErrDocumentNotFound = "document not found"
ErrFindDocumentByID = "failed to find document by ID"
ErrUpdateDocumentFailed = "failed to update document"
ErrDeleteDocumentFailed = "failed to delete document"
)