From 8dc5d0c703d2f63884c50112cdba5cdb00347896 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Fri, 1 Mar 2019 08:42:24 +0100 Subject: [PATCH] Fix regression (record endpoints overriding module) --- api/crm/spec.json | 14 +++---- api/crm/spec/record.json | 14 +++---- crm/repository/attachment.go | 8 +++- crm/rest/attachment.go | 2 +- crm/rest/handlers/record.go | 14 +++---- crm/rest/pageattachment.go_ | 67 ---------------------------------- crm/rest/recordattachment.go_ | 69 ----------------------------------- docs/crm/README.md | 6 +-- 8 files changed, 31 insertions(+), 163 deletions(-) delete mode 100644 crm/rest/pageattachment.go_ delete mode 100644 crm/rest/recordattachment.go_ diff --git a/api/crm/spec.json b/api/crm/spec.json index 498f36c87..c1ea80e71 100644 --- a/api/crm/spec.json +++ b/api/crm/spec.json @@ -351,7 +351,7 @@ "title": "Records", "description": "CRM records ", "entrypoint": "record", - "path": "/module/{moduleID}", + "path": "/module/{moduleID}/record", "authentication": [], "struct": [ { @@ -403,7 +403,7 @@ "name": "list", "method": "GET", "title": "List/read records from module section", - "path": "/record", + "path": "/", "parameters": { "get": [ { @@ -437,7 +437,7 @@ "name": "create", "method": "POST", "title": "Create record in module section", - "path": "/record", + "path": "/", "parameters": { "post": [ { @@ -453,7 +453,7 @@ "name": "read", "method": "GET", "title": "Read records by ID from module section", - "path": "/record/{recordID}", + "path": "/{recordID}", "parameters": { "path": [ { @@ -469,7 +469,7 @@ "name": "update", "method": "POST", "title": "Update records in module section", - "path": "/record/{recordID}", + "path": "/{recordID}", "parameters": { "path": [ { @@ -493,7 +493,7 @@ "name": "delete", "method": "DELETE", "title": "Delete record row from module section", - "path": "/record/{recordID}", + "path": "/{recordID}", "parameters": { "path": [ { @@ -507,7 +507,7 @@ }, { "name": "upload", - "path": "/record/{recordID}/{fieldName}/attachment", + "path": "/{recordID}/{fieldName}/attachment", "method": "POST", "title": "Uploads attachment and validates it against record field requirements", "parameters": { diff --git a/api/crm/spec/record.json b/api/crm/spec/record.json index d36c64d34..9a3f244ec 100644 --- a/api/crm/spec/record.json +++ b/api/crm/spec/record.json @@ -21,7 +21,7 @@ }, "Protocol": "", "Authentication": [], - "Path": "/module/{moduleID}", + "Path": "/module/{moduleID}/record", "APIs": [ { "Name": "report", @@ -55,7 +55,7 @@ "Name": "list", "Method": "GET", "Title": "List/read records from module section", - "Path": "/record", + "Path": "/", "Parameters": { "get": [ { @@ -89,7 +89,7 @@ "Name": "create", "Method": "POST", "Title": "Create record in module section", - "Path": "/record", + "Path": "/", "Parameters": { "post": [ { @@ -105,7 +105,7 @@ "Name": "read", "Method": "GET", "Title": "Read records by ID from module section", - "Path": "/record/{recordID}", + "Path": "/{recordID}", "Parameters": { "path": [ { @@ -121,7 +121,7 @@ "Name": "update", "Method": "POST", "Title": "Update records in module section", - "Path": "/record/{recordID}", + "Path": "/{recordID}", "Parameters": { "path": [ { @@ -145,7 +145,7 @@ "Name": "delete", "Method": "DELETE", "Title": "Delete record row from module section", - "Path": "/record/{recordID}", + "Path": "/{recordID}", "Parameters": { "path": [ { @@ -161,7 +161,7 @@ "Name": "upload", "Method": "POST", "Title": "Uploads attachment and validates it against record field requirements", - "Path": "/record/{recordID}/{fieldName}/attachment", + "Path": "/{recordID}/{fieldName}/attachment", "Parameters": { "path": [ { diff --git a/crm/repository/attachment.go b/crm/repository/attachment.go index 2522587cc..1443ef500 100644 --- a/crm/repository/attachment.go +++ b/crm/repository/attachment.go @@ -90,8 +90,10 @@ func (r *attachment) Find(filter types.AttachmentFilter) (set types.AttachmentSe switch f.Kind { case types.PageAttachment: // @todo implement filtering by page - err = errors.New("filtering by page not implemented") - return + if f.PageID > 0 { + err = errors.New("filtering by pageID not implemented") + return + } case types.RecordAttachment: query = query. Join("crm_record_value AS v ON (v.ref = a.id)") @@ -109,6 +111,8 @@ func (r *attachment) Find(filter types.AttachmentFilter) (set types.AttachmentSe if f.FieldName != "" { query = query.Where(sq.Eq{"v.name": f.FieldName}) } + default: + err = errors.New("unsupported kind value") } if f.Filter != "" { diff --git a/crm/rest/attachment.go b/crm/rest/attachment.go index 2700f37fa..4138847ac 100644 --- a/crm/rest/attachment.go +++ b/crm/rest/attachment.go @@ -23,7 +23,7 @@ func (Attachment) New() *Attachment { // Attachments returns list of all files attached to records func (ctrl *Attachment) List(ctx context.Context, r *request.AttachmentList) (interface{}, error) { f := types.AttachmentFilter{ - Kind: types.RecordAttachment, + Kind: r.Kind, ModuleID: r.ModuleID, RecordID: r.RecordID, FieldName: r.FieldName, diff --git a/crm/rest/handlers/record.go b/crm/rest/handlers/record.go index d03315e80..103c240d8 100644 --- a/crm/rest/handlers/record.go +++ b/crm/rest/handlers/record.go @@ -104,14 +104,14 @@ func NewRecord(rh RecordAPI) *Record { func (rh *Record) MountRoutes(r chi.Router, middlewares ...func(http.Handler) http.Handler) { r.Group(func(r chi.Router) { r.Use(middlewares...) - r.Route("/module/{moduleID}", func(r chi.Router) { + r.Route("/module/{moduleID}/record", func(r chi.Router) { r.Get("/report", rh.Report) - r.Get("/record", rh.List) - r.Post("/record", rh.Create) - r.Get("/record/{recordID}", rh.Read) - r.Post("/record/{recordID}", rh.Update) - r.Delete("/record/{recordID}", rh.Delete) - r.Post("/record/{recordID}/{fieldName}/attachment", rh.Upload) + r.Get("/", rh.List) + r.Post("/", rh.Create) + r.Get("/{recordID}", rh.Read) + r.Post("/{recordID}", rh.Update) + r.Delete("/{recordID}", rh.Delete) + r.Post("/{recordID}/{fieldName}/attachment", rh.Upload) }) }) } diff --git a/crm/rest/pageattachment.go_ b/crm/rest/pageattachment.go_ deleted file mode 100644 index bd5789bfd..000000000 --- a/crm/rest/pageattachment.go_ +++ /dev/null @@ -1,67 +0,0 @@ -package rest - -import ( - "context" - "fmt" - - "github.com/pkg/errors" - - "github.com/crusttech/crust/crm/rest/request" - "github.com/crusttech/crust/crm/service" -) - -var _ = errors.Wrap - -type PageAttachment struct { - att service.AttachmentService -} - -func (PageAttachment) New() *PageAttachment { - return &PageAttachment{att: service.DefaultAttachment} -} - -func (ctrl *PageAttachment) Upload(ctx context.Context, r *request.PageAttachmentUpload) (interface{}, error) { - // @todo [SECURITY] check if attachments can be added to this page - file, err := r.Upload.Open() - if err != nil { - return nil, err - } - - defer file.Close() - - a, err := ctrl.att.With(ctx).CreatePageAttachment( - r.Upload.Filename, - r.Upload.Size, - file, - r.PageID, - ) - - if err != nil { - return nil, err - } - - baseURL := fmt.Sprintf("/page/%d/attachment", r.PageID) - return makeAttachmentPayload(baseURL, a), nil -} - -func (ctrl *PageAttachment) Details(ctx context.Context, r *request.PageAttachmentDetails) (interface{}, error) { - // @todo [SECURITY] check if page can be accessed - // @todo [SECURITY] test any of the page blocks contain this attachment, return 404 if not - if a, err := ctrl.att.FindByID(r.AttachmentID); err != nil { - return nil, err - } else { - return makePageAttachmentPayload(r.PageID, a), nil - } -} - -func (ctrl *PageAttachment) Original(ctx context.Context, r *request.PageAttachmentOriginal) (interface{}, error) { - // @todo [SECURITY] check if page can be accessed - // @todo [SECURITY] test any of the page blocks contain this attachment, return 404 if not - return loadAttachedFile(ctrl.att, r.AttachmentID, false, r.Download) -} - -func (ctrl *PageAttachment) Preview(ctx context.Context, r *request.PageAttachmentPreview) (interface{}, error) { - // @todo [SECURITY] check if page can be accessed - // @todo [SECURITY] test any of the page blocks contain this attachment, return 404 if not - return loadAttachedFile(ctrl.att, r.AttachmentID, true, false) -} diff --git a/crm/rest/recordattachment.go_ b/crm/rest/recordattachment.go_ deleted file mode 100644 index 3db18a3e6..000000000 --- a/crm/rest/recordattachment.go_ +++ /dev/null @@ -1,69 +0,0 @@ -package rest - -import ( - "context" - "fmt" - - "github.com/pkg/errors" - - "github.com/crusttech/crust/crm/rest/request" - "github.com/crusttech/crust/crm/service" -) - -var _ = errors.Wrap - -type RecordAttachment struct { - att service.AttachmentService -} - -func (RecordAttachment) New() *RecordAttachment { - return &RecordAttachment{att: service.DefaultAttachment} -} - -func (ctrl *RecordAttachment) Upload(ctx context.Context, r *request.RecordAttachmentUpload) (interface{}, error) { - // @todo [SECURITY] check if attachments can be added to this page - file, err := r.Upload.Open() - if err != nil { - return nil, err - } - - defer file.Close() - - a, err := ctrl.att.With(ctx).CreateRecordAttachment( - r.Upload.Filename, - r.Upload.Size, - file, - r.ModuleID, - r.RecordID, - r.FieldName, - ) - - if err != nil { - return nil, err - } - - baseURL := fmt.Sprintf("/module/%d/record/%d/attachment/%s/", r.ModuleID, r.RecordID, r.FieldName) - return makeAttachmentPayload(baseURL, a), nil -} - -func (ctrl *RecordAttachment) Details(ctx context.Context, r *request.RecordAttachmentDetails) (interface{}, error) { - // @todo [security] check if record can be accessed - // @todo [SECURITY] test if module/record/field has this attachment, return 404 if not - if a, err := ctrl.att.FindByID(r.AttachmentID); err != nil { - return nil, err - } else { - return makeRecordAttachmentPayload(r.ModuleID, r.RecordID, r.FieldName, a), nil - } -} - -func (ctrl *RecordAttachment) Original(ctx context.Context, r *request.RecordAttachmentOriginal) (interface{}, error) { - // @todo [security] check if record can be accessed - // @todo [SECURITY] test if module/record/field has this attachment, return 404 if not - return loadAttachedFile(ctrl.att, r.AttachmentID, false, r.Download) -} - -func (ctrl *RecordAttachment) Preview(ctx context.Context, r *request.RecordAttachmentPreview) (interface{}, error) { - // @todo [security] check if record can be accessed - // @todo [SECURITY] test if module/record/field has this attachment, return 404 if not - return loadAttachedFile(ctrl.att, r.AttachmentID, true, false) -} diff --git a/docs/crm/README.md b/docs/crm/README.md index 88c746e4b..9e05db124 100644 --- a/docs/crm/README.md +++ b/docs/crm/README.md @@ -396,7 +396,7 @@ CRM records | URI | Protocol | Method | Authentication | | --- | -------- | ------ | -------------- | -| `/module/{moduleID}/report` | HTTP/S | GET | | +| `/module/{moduleID}/record/report` | HTTP/S | GET | | #### Request parameters @@ -413,7 +413,7 @@ CRM records | URI | Protocol | Method | Authentication | | --- | -------- | ------ | -------------- | -| `/module/{moduleID}/record` | HTTP/S | GET | | +| `/module/{moduleID}/record/` | HTTP/S | GET | | #### Request parameters @@ -431,7 +431,7 @@ CRM records | URI | Protocol | Method | Authentication | | --- | -------- | ------ | -------------- | -| `/module/{moduleID}/record` | HTTP/S | POST | | +| `/module/{moduleID}/record/` | HTTP/S | POST | | #### Request parameters