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.

Sign up for GitHub

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

ENH Add domain support for the main site. #473

Closed
wants to merge 2 commits into from
Closed

ENH Add domain support for the main site. #473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f52b857
ENH Add domain support for the main site.
GuySartorelli Dec 6, 2021
0b7a63e
ENH: Use sanitiseClassName for consistency.
GuySartorelli Dec 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions src/Admin/SubsiteAdmin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use SilverStripe\Forms\GridField\GridFieldPaginator;
use SilverStripe\Subsites\Forms\GridFieldSubsiteDetailForm;
use SilverStripe\Subsites\Model\Subsite;
use SilverStripe\Subsites\Model\SubsiteDomain;

/**
* Admin interface to manage and create {@link Subsite} instances.
Expand All @@ -15,7 +16,14 @@
*/
class SubsiteAdmin extends ModelAdmin
{
private static $managed_models = [Subsite::class];
private static $managed_models = [
Subsite::class => [
'title' => 'Subsites'
],
SubsiteDomain::class => [
'title' => 'Main site domains'
],
];

private static $url_segment = 'subsites';

Expand All @@ -31,11 +39,18 @@ public function getEditForm($id = null, $fields = null)
{
$form = parent::getEditForm($id, $fields);

$grid = $form->Fields()->dataFieldByName(str_replace('\\', '-', Subsite::class));
if ($grid) {
$grid->getConfig()->getComponentByType(GridFieldPaginator::class)->setItemsPerPage(100);
$grid->getConfig()->removeComponentsByType(GridFieldDetailForm::class);
$grid->getConfig()->addComponent(new GridFieldSubsiteDetailForm());
if ($this->modelClass === Subsite::class) {
$grid = $form->Fields()->dataFieldByName($this->sanitiseClassName(Subsite::class));
if ($grid) {
$grid->getConfig()->getComponentByType(GridFieldPaginator::class)->setItemsPerPage(100);
$grid->getConfig()->removeComponentsByType(GridFieldDetailForm::class);
$grid->getConfig()->addComponent(new GridFieldSubsiteDetailForm());
}
}

if ($this->modelClass === SubsiteDomain::class) {
$grid = $form->Fields()->dataFieldByName($this->sanitiseClassName(SubsiteDomain::class));
michalkleiner marked this conversation as resolved.
Show resolved Hide resolved
$grid->setList($grid->getList()->filter('SubsiteID', 0));
}

return $form;
Expand Down
4 changes: 1 addition & 3 deletions src/Extensions/SiteTreeSubsites.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,9 +412,7 @@ public function alternateAbsoluteLink($action = null)
// Generate the existing absolute URL and replace the domain with the subsite domain.
// This helps deal with Link() returning an absolute URL.
$url = Director::absoluteURL($this->owner->Link($action));
if ($this->owner->SubsiteID) {
$url = preg_replace('/\/\/[^\/]+\//', '//' . $this->owner->Subsite()->domain() . '/', $url);
}
$url = preg_replace('/\/\/[^\/]+\//', '//' . $this->owner->Subsite()->domain() . '/', $url);
Copy link
Member

Choose a reason for hiding this comment

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

Why are we removing this check? If $this->owner->SubsiteID == 0 then this will cause an exception i.e. (null)->domain()

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm about 60% sure $this->owner->Subsite() would give a new Subsite object with no ID - but that would need to be checked if someone picks this PR up again. If that's not the case, then another mechanism will need to be added here to get the main site Domain where no subsite is set.

Copy link
Contributor

@NightJar NightJar Feb 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'm 100% sure.
https://github.com/silverstripe/silverstripe-framework/blob/6d90eec0c883a69a87e0ad31d0e8ba617b809057/src/ORM/DataObject.php#L1890

return $url;
}

Expand Down
10 changes: 10 additions & 0 deletions src/Model/Subsite.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,16 @@ public function domain()
*/
public function getPrimarySubsiteDomain()
{
// Main site
if (!$this->isInDB()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now seeing the method this is in and the not-in-db check... is there a better/different place this could go to? Alternatively, can we get more explanation why it's here and why we need to check it's not in the db?

Copy link
Member Author

Choose a reason for hiding this comment

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

An alternative would be if (!$this->ID) {.
This is for situations like SiteTreeSubsites::alternateAbsoluteLink (inline below for convenience) where we call domain (which calls getPrimarySubsiteDomain) on the relationship directly from the has_one relation object.
If the page has no SubiteID (i.e. it belongs to the main site), the relation object will still be an instantiated Subsite object which has no ID and which returns false from isInDB - so we end up getting the main site domain.

I can't think of somewhere else to put the call, since the intention is to distinguish between a main site or subsite Subsite object (where a main site Subsite object is any Subsite object without an ID, effectively) when fetching the domain.

This allows project-specific code or code from other modules to remain the same (if they don't have the "if there is a subsite ID" check in place - in that case there's still only a very minor change required) and now get the added benefit of automagically fetching the correct main site domain as well.

silverstripe-subsites/src/Extensions/SiteTreeSubsites.php

Lines 410 to 417 in 0b7a63e

public function alternateAbsoluteLink($action = null)
{
// Generate the existing absolute URL and replace the domain with the subsite domain.
// This helps deal with Link() returning an absolute URL.
$url = Director::absoluteURL($this->owner->Link($action));
$url = preg_replace('/\/\/[^\/]+\//', '//' . $this->owner->Subsite()->domain() . '/', $url);
return $url;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Just realised I didn't explicitly say why we need to check for the main site vs an actual subsite there - the main site domains aren't stored on the Domains relation of any subsite object, so we can't just use $this->Domains() for the main site.

Subsite::disable_subsite_filter(true);
$domain = SubsiteDomain::get()->filter('SubsiteID', 0)
->sort('"IsPrimary" DESC')
->first();
Subsite::disable_subsite_filter(false);
return $domain;
}
// Subsites
return $this
->Domains()
->sort('"IsPrimary" DESC')
Expand Down