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

Closed

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 from b5d9ce4 to 3103f12 Compare October 22, 2024 02:28
@GuySartorelli GuySartorelli force-pushed the pulls/3/generic-cmsmain branch 4 times, most recently from 555907c to c59d3ee Compare November 8, 2024 01:19
@GuySartorelli GuySartorelli force-pushed the pulls/3/generic-cmsmain branch 6 times, most recently from 8dc3d7f to c3ddc49 Compare November 28, 2024 23:07
@GuySartorelli
Copy link
Member Author

Replaced with #1867

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