Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: WorkspaceName with reference for unused contentStream #5167

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Jun 28, 2024

As written in #5034 (comment) we actually decided against it, but look how pretty it seems? :D

introduces the syntax WorkspaceName('cs:<ContentStreamId>') ... that way we'd now what to write inside the fetched workspace name of a "forbidden" node.

But we initially didnt want to go down this road:

Neos 9.0 weekly - 01-03-2024

  • Decide for WorkspaceName|DetachedWorkspaceName $workspaceName later if we really need it.
    • Changing this later would be a b/c disaster
    • Instead: WorkspaceName::fromString() stricter without colons (so that we can allow for extension, e.g. WorkspaceName::detached(ContentStreamid $csId) (with an internal representation like “cs:”)

Additionally we should restrict the workspace name to be stricter: #5125 and discuss if fromString should allow the cs: notation.


TODO

1.) discuss if we are okay with being able to reference the content stream id in a node address per uri:

http://localhost/neos/preview?node=%7B%22contentRepositoryId%22%3A%22default%22%2C%22workspaceName%22%3A%22cs%3Acs-identifier%22%2C%22dimensionSpacePoint%22%3A%5B%5D%2C%22aggregateId%22%3A%22non-existing%22%7D

2.) disallow the 'fake' workspace name to be used in commands like createWorkspace, or modify a node.

3.) check other usages where a fake workspace name can be passed and see if we should allow it there.

@mhsdesign mhsdesign force-pushed the feature/workspaceNameWithReferenceForUnusedContentStream branch from b741580 to 8052772 Compare June 28, 2024 19:36
@mhsdesign mhsdesign force-pushed the feature/workspaceNameWithReferenceForUnusedContentStream branch from 60aeaec to 10011fb Compare June 29, 2024 08:10
@kitsunet
Copy link
Member

kitsunet commented Jul 1, 2024

Ok but why, is there a usecase?

@mhsdesign
Copy link
Member Author

mhsdesign commented Jul 1, 2024

just for the tests a usecase.

the commit TASK: Remove deprecated usage $node->subgraphIdentity->contentStreamId solves this issue #5043

or see over here in this issue where we discussed how to solve this last riddle: #5034 (comment)

@kitsunet
Copy link
Member

kitsunet commented Jul 1, 2024

As I wrote over in the discussion I would say this is a low level (non API) testing usecase, where the tests want to check something that hte API "blackbox" shouldn't expose and I would suggest to handle it that way. As in by circumventing API on a loewr level (e.g. direct DB checks) and not adding API for that.

@mhsdesign
Copy link
Member Author

Closing in favour of #5169

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants