From f601e0aa96972685411f652b951ae6667a252057 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Mon, 8 Jul 2019 08:14:19 +0200 Subject: [PATCH] Improve record attachment upload capabilities Add missing upload-to-non-existing record featue by moving endpoint away from specific record and moving recordID, fieldName from PATH to POST parameters --- api/compose/spec.json | 10 ++++------ api/compose/spec/record.json | 10 ++++------ compose/internal/service/attachment.go | 23 +++++++++++++++++++---- compose/rest/handlers/record.go | 2 +- compose/rest/request/record.go | 20 +++++++++++++------- docs/compose/README.md | 10 +++++----- 6 files changed, 46 insertions(+), 29 deletions(-) diff --git a/api/compose/spec.json b/api/compose/spec.json index c3f5e2349..c803b52b0 100644 --- a/api/compose/spec.json +++ b/api/compose/spec.json @@ -729,15 +729,15 @@ }, { "name": "upload", - "path": "/{recordID}/{fieldName}/attachment", + "path": "/attachment", "method": "POST", "title": "Uploads attachment and validates it against record field requirements", "parameters": { - "path": [ + "post": [ { "name": "recordID", "type": "uint64", - "required": true, + "required": false, "title": "Record ID" }, { @@ -745,9 +745,7 @@ "type": "string", "required": true, "title": "Field name" - } - ], - "post": [ + }, { "name": "upload", "type": "*multipart.FileHeader", diff --git a/api/compose/spec/record.json b/api/compose/spec/record.json index dbac96acd..2f9cedeb5 100644 --- a/api/compose/spec/record.json +++ b/api/compose/spec/record.json @@ -167,12 +167,12 @@ "Name": "upload", "Method": "POST", "Title": "Uploads attachment and validates it against record field requirements", - "Path": "/{recordID}/{fieldName}/attachment", + "Path": "/attachment", "Parameters": { - "path": [ + "post": [ { "name": "recordID", - "required": true, + "required": false, "title": "Record ID", "type": "uint64" }, @@ -181,9 +181,7 @@ "required": true, "title": "Field name", "type": "string" - } - ], - "post": [ + }, { "name": "upload", "required": true, diff --git a/compose/internal/service/attachment.go b/compose/internal/service/attachment.go index e0718967f..867838fc9 100644 --- a/compose/internal/service/attachment.go +++ b/compose/internal/service/attachment.go @@ -49,6 +49,7 @@ type ( attachmentAccessController interface { CanUpdatePage(context.Context, *types.Page) bool CanUpdateRecord(context.Context, *types.Module) bool + CanCreateRecord(context.Context, *types.Module) bool } AttachmentService interface { @@ -187,10 +188,24 @@ func (svc attachment) CreateRecordAttachment(namespaceID uint64, name string, si if m, err := svc.moduleSvc.FindByID(namespaceID, moduleID); err != nil { return nil, err - } else if _, err := svc.recordSvc.FindByID(namespaceID, recordID); err != nil { - return nil, err - } else if !svc.ac.CanUpdateRecord(svc.ctx, m) { - return nil, ErrNoUpdatePermissions.withStack() + } else if recordID > 0 { + // Uploading to existing record + // + // To allow upload (attachment creation) user must have permissions to + // alter that record + if _, err := svc.recordSvc.FindByID(namespaceID, recordID); err != nil { + return nil, err + } else if !svc.ac.CanUpdateRecord(svc.ctx, m) { + return nil, ErrNoUpdatePermissions.withStack() + } + } else { + // Uploading to non-existing record + // + // To allow upload (attachment creation) user must have permissions to + // create records + if !svc.ac.CanCreateRecord(svc.ctx, m) { + return nil, ErrNoCreatePermissions.withStack() + } } att := &types.Attachment{ diff --git a/compose/rest/handlers/record.go b/compose/rest/handlers/record.go index 6491b9a50..be01c1417 100644 --- a/compose/rest/handlers/record.go +++ b/compose/rest/handlers/record.go @@ -203,6 +203,6 @@ func (h Record) MountRoutes(r chi.Router, middlewares ...func(http.Handler) http r.Get("/namespace/{namespaceID}/module/{moduleID}/record/{recordID}", h.Read) r.Post("/namespace/{namespaceID}/module/{moduleID}/record/{recordID}", h.Update) r.Delete("/namespace/{namespaceID}/module/{moduleID}/record/{recordID}", h.Delete) - r.Post("/namespace/{namespaceID}/module/{moduleID}/record/{recordID}/{fieldName}/attachment", h.Upload) + r.Post("/namespace/{namespaceID}/module/{moduleID}/record/attachment", h.Upload) }) } diff --git a/compose/rest/request/record.go b/compose/rest/request/record.go index 7eb07b738..0b7473672 100644 --- a/compose/rest/request/record.go +++ b/compose/rest/request/record.go @@ -408,9 +408,9 @@ var _ RequestFiller = NewRecordDelete() type RecordUpload struct { RecordID uint64 `json:",string"` FieldName string + Upload *multipart.FileHeader NamespaceID uint64 `json:",string"` ModuleID uint64 `json:",string"` - Upload *multipart.FileHeader } func NewRecordUpload() *RecordUpload { @@ -422,11 +422,12 @@ func (r RecordUpload) Auditable() map[string]interface{} { out["recordID"] = r.RecordID out["fieldName"] = r.FieldName - out["namespaceID"] = r.NamespaceID - out["moduleID"] = r.ModuleID out["upload.size"] = r.Upload.Size out["upload.filename"] = r.Upload.Filename + out["namespaceID"] = r.NamespaceID + out["moduleID"] = r.ModuleID + return out } @@ -457,14 +458,19 @@ func (r *RecordUpload) Fill(req *http.Request) (err error) { post[name] = string(param[0]) } - r.RecordID = parseUInt64(chi.URLParam(req, "recordID")) - r.FieldName = chi.URLParam(req, "fieldName") - r.NamespaceID = parseUInt64(chi.URLParam(req, "namespaceID")) - r.ModuleID = parseUInt64(chi.URLParam(req, "moduleID")) + if val, ok := post["recordID"]; ok { + r.RecordID = parseUInt64(val) + } + if val, ok := post["fieldName"]; ok { + r.FieldName = val + } if _, r.Upload, err = req.FormFile("upload"); err != nil { return errors.Wrap(err, "error procesing uploaded file") } + r.NamespaceID = parseUInt64(chi.URLParam(req, "namespaceID")) + r.ModuleID = parseUInt64(chi.URLParam(req, "moduleID")) + return err } diff --git a/docs/compose/README.md b/docs/compose/README.md index 57d4ba71a..f0f5e8d22 100644 --- a/docs/compose/README.md +++ b/docs/compose/README.md @@ -685,7 +685,7 @@ Compose records | `GET` | `/namespace/{namespaceID}/module/{moduleID}/record/{recordID}` | Read records by ID from module section | | `POST` | `/namespace/{namespaceID}/module/{moduleID}/record/{recordID}` | Update records in module section | | `DELETE` | `/namespace/{namespaceID}/module/{moduleID}/record/{recordID}` | Delete record row from module section | -| `POST` | `/namespace/{namespaceID}/module/{moduleID}/record/{recordID}/{fieldName}/attachment` | Uploads attachment and validates it against record field requirements | +| `POST` | `/namespace/{namespaceID}/module/{moduleID}/record/attachment` | Uploads attachment and validates it against record field requirements | ## Generates report from module records @@ -795,17 +795,17 @@ Compose records | URI | Protocol | Method | Authentication | | --- | -------- | ------ | -------------- | -| `/namespace/{namespaceID}/module/{moduleID}/record/{recordID}/{fieldName}/attachment` | HTTP/S | POST | | +| `/namespace/{namespaceID}/module/{moduleID}/record/attachment` | HTTP/S | POST | | #### Request parameters | Parameter | Type | Method | Description | Default | Required? | | --------- | ---- | ------ | ----------- | ------- | --------- | -| recordID | uint64 | PATH | Record ID | N/A | YES | -| fieldName | string | PATH | Field name | N/A | YES | +| recordID | uint64 | POST | Record ID | N/A | NO | +| fieldName | string | POST | Field name | N/A | YES | +| upload | *multipart.FileHeader | POST | File to upload | N/A | YES | | namespaceID | uint64 | PATH | Namespace ID | N/A | YES | | moduleID | uint64 | PATH | Module ID | N/A | YES | -| upload | *multipart.FileHeader | POST | File to upload | N/A | YES | ---