moved the Create and Manage API rule checks out of the OnRecordCreateRequest hook finalizer

CHANGELOG.md

@@ -1,14 +1,34 @@
 ## v0.27.0 (WIP)
 
+- ⚠️ Moved the Create and Manage API rule checks out of the `OnRecordCreateRequest` hook finalizer, **aka. now all API rules are checked BEFORE triggering their corresponding `*Request` hook.**.
+    This was done to minimize the confusion regarding the firing order of the request operations, making it more predictable and consistent with the other record List/View/Update/Delete request actions.
+    It could be a minor break change if you are relying on the old behavior or have a Go `tests.ApiScenario` that is testing a Create API rule failure and expect `OnRecordCreateRequest` to be fired. In that case for example you may have to update your test scenario like:
+    ```go
+    tests.ApiScenario{
+        Name:   "Example test that checks a Create API rule failure"
+        Method: http.MethodPost,
+        URL:    "/api/collections/example/records",
+        ...
+        // old:
+        ExpectedEvents:  map[string]int{
+            "*":                     0,
+            "OnRecordCreateRequest": 1,
+        },
+        // new:
+        ExpectedEvents:  map[string]int{"*": 0},
+    }
+    ```
+    If you are having difficulties adjusting your code, feel free to open a [Q&A discussion](https://github.com/pocketbase/pocketbase/discussions) with the failing/problematic code sample and I'll try to assist you migrating it.
+
+- Added new `geoPoint` field for storing `{lon:x,lat:y}` GPS coordinates (@todo docs and more info for the updated fexpr functions support, geoDistance, record.GetGeoPoint, etc.).
+
+- Updated the `select` field UI to accommodate better larger lists and RTL languages ([#4674](https://github.com/pocketbase/pocketbase/issues/4674)).
+
 - Updated the mail attachments auto MIME type detection to use `gabriel-vasile/mimetype` for consistency and broader sniffing signatures support.
 
-- Updated the `select` field UI to accomodate better larger lists and RTL languages ([#4674](https://github.com/pocketbase/pocketbase/issues/4674)).
-
-- Added new `geoPoint` field for storing `{lon:x,lat:y}` record value (@todo docs).
-
 - Forced `text/javascript` Content-Type when serving `.js`/`.mjs` collection uploaded files ([#6597](https://github.com/pocketbase/pocketbase/issues/6597)).
 
-- Soft-deprecated the `$http.send`'s `result.raw` field in favour of `result.body` that contains the response body as plain bytes slice to avoid the discrepencies between Go and the JSVM when casting binary data to string.
+- Soft-deprecated the `$http.send`'s `result.raw` field in favor of `result.body` that contains the response body as plain bytes slice to avoid the discrepancies between Go and the JSVM when casting binary data to string.
   (@todo update docs to use the new field)
 
 - Minor UI fixes (_removed the superuser fields from the auth record create/update body examples, etc._).

apis/record_crud.go

@@ -251,20 +251,9 @@ func recordCreate(optFinalizer func(data any) error) func(e *core.RequestEvent)
 			}
 		}
 
-		var isOptFinalizerCalled bool
+		// check the request and record data against the create and manage rules
-
+		if !hasSuperuserAuth && collection.CreateRule != nil {
-		event := new(core.RecordRequestEvent)
+			dummyRecord := record.Clone()
-		event.RequestEvent = e
-		event.Collection = collection
-		event.Record = record
-
-		hookErr := e.App.OnRecordCreateRequest().Trigger(event, func(e *core.RecordRequestEvent) error {
-			form.SetApp(e.App)
-			form.SetRecord(e.Record)
-
-			// temporary save the record and check it against the create and manage rules
-			if !hasSuperuserAuth && e.Collection.CreateRule != nil {
-				dummyRecord := e.Record.Clone()
 
 			dummyRandomPart := "__pb_create__" + security.PseudorandomString(6)
 
@@ -294,7 +283,7 @@ func recordCreate(optFinalizer func(data any) error) func(e *core.RequestEvent)
 			}
 
 			// shallow clone the current collection
-				dummyCollection := *e.Collection
+			dummyCollection := *collection
 			dummyCollection.Id += dummyRandomPart
 			dummyCollection.Name += inflector.Columnify(dummyRandomPart)
 
@@ -329,6 +318,17 @@ func recordCreate(optFinalizer func(data any) error) func(e *core.RequestEvent)
 			}
 		}
 
+		var isOptFinalizerCalled bool
+
+		event := new(core.RecordRequestEvent)
+		event.RequestEvent = e
+		event.Collection = collection
+		event.Record = record
+
+		hookErr := e.App.OnRecordCreateRequest().Trigger(event, func(e *core.RecordRequestEvent) error {
+			form.SetApp(e.App)
+			form.SetRecord(e.Record)
+
 			err := form.Submit()
 			if err != nil {
 				return firstApiError(err, e.BadRequestError("Failed to create record", err))
@@ -441,6 +441,14 @@ func recordUpdate(optFinalizer func(data any) error) func(e *core.RequestEvent)
 		}
 		form.Load(data)
 
+		manageRuleQuery := e.App.DB().Select("(1)").From(collection.Name).AndWhere(dbx.HashExp{
+			collection.Name + ".id": record.Id,
+		})
+		if !form.HasManageAccess() &&
+			hasAuthManageAccess(e.App, requestInfo, collection, manageRuleQuery) {
+			form.GrantManagerAccess()
+		}
+
 		var isOptFinalizerCalled bool
 
 		event := new(core.RecordRequestEvent)
@@ -452,15 +460,6 @@ func recordUpdate(optFinalizer func(data any) error) func(e *core.RequestEvent)
 			form.SetApp(e.App)
 			form.SetRecord(e.Record)
 
-			manageRuleQuery := e.App.DB().Select("(1)").From(e.Collection.Name).AndWhere(dbx.HashExp{
-				// note: use the original record id and not e.Record.Id because it may get overwritten
-				e.Collection.Name + ".id": e.Record.LastSavedPK(),
-			})
-			if !form.HasManageAccess() &&
-				hasAuthManageAccess(e.App, requestInfo, e.Collection, manageRuleQuery) {
-				form.GrantManagerAccess()
-			}
-
 			err := form.Submit()
 			if err != nil {
 				return firstApiError(err, e.BadRequestError("Failed to update record.", err))

apis/record_crud_test.go

@@ -1563,10 +1563,7 @@ func TestRecordCrudCreate(t *testing.T) {
 			Body:            strings.NewReader(`{"title":"test123"}`),
 			ExpectedStatus:  400,
 			ExpectedContent: []string{`"data":{}`},
-			ExpectedEvents: map[string]int{
+			ExpectedEvents:  map[string]int{"*": 0},
-				"*":                     0,
-				"OnRecordCreateRequest": 1,
-			},
 		},
 		{
 			Name:   "auth record submit in restricted collection (rule failure check)",
@@ -1579,10 +1576,7 @@ func TestRecordCrudCreate(t *testing.T) {
 			},
 			ExpectedStatus:  400,
 			ExpectedContent: []string{`"data":{}`},
-			ExpectedEvents: map[string]int{
+			ExpectedEvents:  map[string]int{"*": 0},
-				"*":                     0,
-				"OnRecordCreateRequest": 1,
-			},
 		},
 		{
 			Name:   "auth record submit in restricted collection (rule pass check) + expand relations",
@@ -1731,10 +1725,7 @@ func TestRecordCrudCreate(t *testing.T) {
 			},
 			ExpectedStatus:  400,
 			ExpectedContent: []string{`"data":{}`},
-			ExpectedEvents: map[string]int{
+			ExpectedEvents:  map[string]int{"*": 0},
-				"*":                     0,
-				"OnRecordCreateRequest": 1,
-			},
 		},
 		{
 			Name:   "submit via multipart form data with @jsonPayload key and satisfied @request.body rule",
@@ -1975,13 +1966,8 @@ func TestRecordCrudCreate(t *testing.T) {
 				"total-":2
 			}`),
 			ExpectedStatus:  400,
-			ExpectedContent: []string{
+			ExpectedContent: []string{`"data":{}`},
-				`"data":{}`,
+			ExpectedEvents:  map[string]int{"*": 0},
-			},
-			ExpectedEvents: map[string]int{
-				"*":                     0,
-				"OnRecordCreateRequest": 1,
-			},
 		},
 		{
 			Name:   "@request.body.field with compute modifers (rule pass check)",