neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
260 stars 221 forks source link

!!! FEATURE: Add `workspaceName` to relevant events #5002

Closed bwaidelich closed 1 month ago

bwaidelich commented 2 months ago

Adds the workspaceName to the data of all content stream related events.

Breaking change

This is a breaking change because it changes the event payload. Exports need to be replaced and events can be migrated with the provided event migration:

Event migration

This PR comes with a event migration that allows for fixing already published events:

./flow migrateevents:migratePayloadToWorkspaceName

Resolves: #4996

bwaidelich commented 2 months ago

@mhsdesign thanks for the feedback, I'll take care of a follow-up.

We also have to adjust our 3 event export

Not sure if I got that.. Is that something that can be done in this PR directly or is that Neos UI?

mhsdesign commented 2 months ago

Okay, so the migration will def take some time like a good minute for 19k events. but what surprises me that it says Migration applied to 13320 events, while i do have 19.433?

mhsdesign commented 2 months ago

nevermind. I oversaw the continue;

bwaidelich commented 2 months ago

There is an initiating user id set? So not cli + node migration? If that is true and you also have preservations we should maybe reconsider.

I read that three times but still don't understand it ;)

mhsdesign commented 2 months ago

what i meant @bwaidelich, what do you think lead to this event:

payload:

{
  "contentStreamId": "9cc426e4-3520-4e48-8272-609267850177",
  "nodeAggregateId": "6b6e1251-4346-494f-ac56-526a30a5741d",
  "originDimensionSpacePoint": {
    "language": "pl"
  },
  "affectedDimensionSpacePoints": [
    {
      "language": "pl"
    }
  ],
  "propertyValues": {
    "uriPathSegment": {
      "value": "laszelechow",
      "type": "string"
    }
  },
  "propertiesToUnset": [],
  "workspaceName": "missing"
}

metadata

{
  "commandClass": "Neos\\ContentRepository\\Core\\Feature\\NodeModification\\Command\\SetSerializedNodeProperties",
  "commandPayload": {
    "nodeAggregateId": "6b6e1251-4346-494f-ac56-526a30a5741d",
    "originDimensionSpacePoint": {
      "language": "pl"
    },
    "propertyValues": {
      "uriPathSegment": {
        "value": "laszelechow",
        "type": "string"
      }
    },
    "propertiesToUnset": [],
    "workspaceName": "missing:9cc426e4-3520-4e48-8272-609267850177"
  },
  "initiatingUserId": "3e5ed99c-16e4-4a12-a23b-4489563e85e7",
  "initiatingTimestamp": "2023-11-07T23:16:15+00:00"
}
bwaidelich commented 2 months ago

what do you think lead to this event

@mhsdesign Uh, no idea. But if no corresponding workspace was created before, it should have never happened to begin with. I guess that's no longer possible with the current version. And it's not directly related to this PR, is it?

mhsdesign commented 2 months ago

And it's not directly related to this PR, is it?

only kindof, i feared we did something wrong that i have so many "workspaceName": "missing" were in my routes.

But as said i think there is no bug on our side. I just kindof fear that there might be the need to be some $workspaceName === 'missing' in php? But we cant do anything against it.

Every code must assume that the workspace name is no longer valid either way so +1.

kitsunet commented 2 months ago

For the shown event were does the workspaceName come from there? As in, where is this event created code wise?

mhsdesign commented 2 months ago

For the shown event were does the workspaceName come from there? As in, where is this event created code wise?

that is the problem due to missing metadata we dont surely now but i suspect from previous structure adjustments or more likely node migrations (@dlubitz mentioned they operate currently on cs instead of ws and that is what he is about to change: https://github.com/neos/neos-development-collection/pull/4685)

kitsunet commented 2 months ago

Neither of those would have a user ID though? I rather meant what event is it and which command handler creates it...

mhsdesign commented 2 months ago

Neither of those would have a user ID though? I rather meant what event is it and which command handler creates it...

yeah thats the thing that makes me rethink this as well. Its a properties was set event: (but you should have the same db dump as well)

17,Workspace:user-jon,2,WorkspaceWasRebased
18,ContentStream:33edf226-c93d-4243-8769-619977fb0c39,1,NodePropertiesWereSet
19,ContentStream:9cc426e4-3520-4e48-8272-609267850177,0,ContentStreamWasForked
20,ContentStream:9cc426e4-3520-4e48-8272-609267850177,1,NodePropertiesWereSet
21,ContentStream:90bcfbf8-c444-48f8-9911-ba0954ac795a,0,ContentStreamWasForked
22,ContentStream:63d38bd9-453e-4af1-8625-848716678038,10,NodePropertiesWereSet
23,ContentStream:9cc426e4-3520-4e48-8272-609267850177,2,ContentStreamWasRemoved

(the above is event 20)

mhsdesign commented 1 month ago

We also have to adjust our 3 event export

Not sure if I got that.. Is that something that can be done in this PR directly or is that Neos UI?

I meant these three json l:

all adjusted ;)