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

API Stop refering to "pages" when dealing with generic records #1837

Draft
wants to merge 2 commits into
base: 3
Choose a base branch
from

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Oct 16, 2024

Two commits here:

  1. Updates some i18n keys that should live in LeftAndMain. Found this because I had to review all i18n strings in CMSMain anyway.
  2. Updates some API that currently refers to "pages" when it's really dealing with any DataObject class, and adds a new getModelClass() method to LeftAndMain (standardises that method between ModelAdmin and CMSMain and will be used extensively in the follow-up refactor cards in this epic)

Issue

CMSMain::class . '.ACCESS',
LeftAndMain::class . '.ACCESS',
Copy link
Member Author

Choose a reason for hiding this comment

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

CMSMain doesn't even exist if you're installing silverstripe/admin without silverstripe/cms. LeftAndMain is the obvious choice here.

'category' => _t(Permission::class . '.CMS_ACCESS_CATEGORY', 'CMS Access'),
'category' => _t(LeftAndMain::class . '.CMS_ACCESS_CATEGORY', 'CMS Access'),
Copy link
Member Author

Choose a reason for hiding this comment

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

This belongs on LeftAndMain as it goes hand-in-hand with the one above.

$abstractClasses = [LeftAndMain::class, CMSMain::class];
$abstractClasses = [LeftAndMain::class];
Copy link
Member Author

Choose a reason for hiding this comment

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

This change is needed so that CMSMain can be included in the left menu and have its route built.
Without this, it was the CMSPagesController which was a very-nearly-useless subclass of CMSMain that was used.
That class has been merged into CMSMain so we need its menu item back

@GuySartorelli GuySartorelli force-pushed the pulls/3/generic-cmsmain branch 2 times, most recently from 60b2bda to 555907c Compare November 7, 2024 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant