@Jonas Eberle
That construct is invalid:
$data = [
'pages' => [
'NEW_IT' => [
'pid' => 1,
'hidden' => false,
'title' => 'IT',
],
591 => [
'move' => 'NEW_IT',
],
]
];
That mixed $data and $cmd structure in on go. That is not supported. In that case, it tries to update 591 and set the database field 'move' to 'NEW_IT'.
So, what could be the use-case to came up with the try to move the record in a dataMap call:
As I read the stackoverflow / slack notes from Jonas, I remebered some huge migration stuff in the last years in minimum one project.
The we had to change a lot in the page structure. That was moving pages / records and updating further fields in the records. Even mixed with creating new ones.
Because it was not working, because of how datahandler works, it was a huge pane to manage and doing different cylies, managing/retrieving the realUids and so on.
And what is not clear, but a developer could thing (from a record based view) - if i can update fields of a database record, why not update the pid ( => move ) ?
Benni Mack wrote:
The example is wrong - The one should be a datamap (create page) the other should be a command map (move) I assume - right?
No - it is not wrong. The documentation is.
If you read the documentation and examples, it first came up with examples for new ones, where the 'pid' field is used ( with NEW<id> ) for creating new records (pages).
Then, there is a example for an record update (example without pid). There is nowhere noted, that pid is only evaluated/used for creation, not for updating. (and silently ommited there). (As other fields to).
So, it is not expressivly allowed for updating records using the dataMap datahandler part, but it is not stated that it is not allowed / working.
The further thing is - it is silently discarded. You get no "feedback". No errorlog entries, if datahandler errorlogging is enabled.
So no - no error
, so it should be allowed, beside it is not working.
Yes - the example is wrong - but the documentation also
If you are a new developer, or new/unexperienced with the datahandler, where would you now that this is not expressivly disallowed.
Changing pid is a move. Moving and updating in one go is not allowed.
If it is wrong to update the pid through the dataMap part, it should be considered as error and throw/nothing an error - or not ?
Basicly it could be the "better" way to leave it - adding a kind of "move & update" to the dataMap part could introduce complexity you want to ommit.
------------------
Also I had myself this usecase in the past, and now Jonas, I could live that there "is no need to allow a pid field record update move" behaviour.
But at least two things should be done:
- updating the documentation (see below for some thought what should be added)
- add/throw error if pid (or other suppressed fields) is tried to update in record-update-mode
Just to make other would not waste time in something which sound logical, but is not supported.
The other point - would it be so bad, to allow the moving in the dataMap with record field updates ? Or could we say that it is a duplicate, but a valid usecase - and worth to try to implement it ?
Updating the documentation:
- clearify the two modes for the dataMap part. Eventually name them in some ways ( NEW<id> => data-record-create, RealIntUid => data-record-update )
- in the "describe keywords in syntax" table add a note to the value field, that some fields are mainly dropped/ignorend in data-record-update mode, but have special meanings in data-record-create mode.
- split examples more prominent for the two modes.
- eventually add a note you need different cyles of datahandler calls/processes instead doing things in one run.