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: Neos.Neos:Site NodeType #4563

Merged
merged 17 commits into from
Oct 16, 2023
Merged

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Sep 26, 2023

resolves: #4531

Upgrade instructions

Your homepage must extend Neos.Neos:Site

For more upgrade information see neos/rector#66

Review instructions

Todo actually use the Neos.Neos:Site in the php code when querying.

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@kitsunet
Copy link
Member

Makes sense I guess. I am sure though there will be many sites out there that do not have an explicit homepage nodetype right now and need structure adjustments for this. Do we / can we prepare one for this? Or a command ala turnsitenodeintonodetype <your.node:type> ? Could probably also happen during converting from old to new.

@mhsdesign
Copy link
Member Author

Neos.ContentRepository.LegacyNodeMigration/Tests/Behavior/behat.yml.dist
AfterScenario # FeatureContext::failIfLastMigrationHasErrors()
RuntimeException: The last migration run led to an error: The site node "site-node-id" (type: "Some.Package:SomeNodeType") must be of type "Neos.Neos:Site". in Bootstrap/FeatureContext.php:90

@mhsdesign
Copy link
Member Author

About an upgrade guide for people without dedicated page node i have created this task: neos/rector#66

Copy link
Contributor

@dlubitz dlubitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the mix up of public constant and static method usage. Would be more clear if there were just one way of getting the nodeTypeName values. Even if it's more complex to get it in cases where you simply need the string value. But this is the price of type safetiness, isn't it?

Co-authored-by: Denny Lubitz <lubitz@vivomedia.de>
@mhsdesign
Copy link
Member Author

I don't like the mix up of public constant and static method usage.

i thought so too at first, but after seeing NodeTypeName::forSites()->value i was thankful for NodeTypeNameFactory::NAME_SITES ^^

But either way lets discuss this in a dedicated issue to not block this pr :D

dlubitz
dlubitz previously approved these changes Sep 29, 2023
@mhsdesign
Copy link
Member Author

I resolved merge conflicts and tested this locally with the neos demo (adjustment neos/Neos.Demo#182) and the neos ui (adjustment neos/neos-ui#3629)

the neos ui e2e test succeed locally.

ahaeslich
ahaeslich previously approved these changes Oct 11, 2023
@mhsdesign mhsdesign dismissed dlubitz’s stale review October 12, 2023 22:07

much has changed since the start ;)

@ahaeslich ahaeslich self-requested a review October 13, 2023 06:25
@nezaniel
Copy link
Member

Looks nice, I'll add the document uri path projection adjustments in a later pr

@mhsdesign mhsdesign merged commit 4d4bb55 into 9.0 Oct 16, 2023
8 checks passed
@mhsdesign mhsdesign deleted the feature/NeosNeosSiteNodeType branch October 16, 2023 21:39
ahaeslich added a commit to ahaeslich/neos-development-collection that referenced this pull request Jun 10, 2024
Relates: neos#4563
Relates: neos/neos-development-collection#4567
neos-bot pushed a commit to neos/neos-kickstarter that referenced this pull request Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

!!! FEATURE: Add Neos.Neos:Site (singular) NodeType for site nodes
6 participants