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
cortezaproject · Jul 8, 2019 · f601e0a · f601e0a
1 parent 60ad32e
commit f601e0a
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 29 deletions.
diff --git a/api/compose/spec.json b/api/compose/spec.json
@@ -729,25 +729,23 @@
             },
             {
                 "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"
                         },
                         {
                             "name": "fieldName",
                             "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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 |
 
 ---