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

Correctly resolve internal and external links #98

Merged
merged 4 commits into from Jul 7, 2021

Conversation

luca-rath
Copy link
Contributor

@luca-rath luca-rath commented Jun 23, 2021

fixes #62
fixes #96
fixes #97

$originalDocument = $originalStructure->getDocument();

/** @var string|null $structureType */
$structureType = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

would call this templateKey to be consistent with the SulucontentBundle

$type = 'page';
}
}
$type = $this->getTemplateType($structure, $document);
Copy link
Contributor

Choose a reason for hiding this comment

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

would call this templateType

@@ -65,7 +69,10 @@ public function resolve(
string $locale,
bool $includeExtension = true
): array {
$data = $this->getStructureData($structure);
$originalStructure = $structure;
Copy link
Contributor

Choose a reason for hiding this comment

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

would call this requestedStructure

@@ -65,7 +69,10 @@ public function resolve(
string $locale,
bool $includeExtension = true
): array {
$data = $this->getStructureData($structure);
$originalStructure = $structure;
$structure = $this->getMostSpecificStructure($originalStructure);
Copy link
Contributor

Choose a reason for hiding this comment

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

would call this targetStructure

@@ -87,6 +94,18 @@ public function resolve(
$data['view'][$property->getName()] = $contentView->getView();
}

$titleProperty = $originalStructure->getProperty('title');
Copy link
Contributor

Choose a reason for hiding this comment

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

think i would implement a private getProperties method with a $requestedStructure and $targetStructure argument. then we can loop over the properties of that method instead of $structure->getProperties(true) and the special logic for the title is handled at a single place

@@ -133,14 +156,19 @@ public function resolveProperties(
);
}
} else {
if (!$structure->hasProperty($sourceProperty)) {
$propertyStructure = $structure;
if ('title' === $sourceProperty) {
Copy link
Contributor

Choose a reason for hiding this comment

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

would use the getProperties proposed above here too, then we dont need to handle the title differently


$contentView = $this->resolveProperty(
$structure,
$propertyStructure,
Copy link
Contributor

Choose a reason for hiding this comment

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

think we can access the structure of the property via the getStructure() method

*
* @return StructureBridge
*/
private function getMostSpecificStructure(StructureInterface $structure): StructureInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

getTargetStructure

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