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

Add teaser selection resolver #83

Merged

Conversation

luca-rath
Copy link
Contributor

@luca-rath luca-rath commented Apr 7, 2021

This TeaserSelectionResolver resolves teasers to something like

{
  "id": "74a36ca1-4805-48a0-b37d-3ffb3a6be9b1",
  "type": "pages",
  "locale": "en",
  "title": "My page",
  "description": "<p>hello world.</p>",
  "moreText": "foo",
  "url": "/my-page",
  "attributes": {
    "structureType": "default",
    "webspaceKey": "example"
  },
  "media": {
    "id": 1,
    "formatUri": "/uploads/media/{format}/01/1-wallpaper.jpg?v=1-0"
  }
}

@luca-rath
Copy link
Contributor Author

@wachterjohannes Is it enough, that the mediaId is returned, or should this resolver also resolve the media?

@alexander-schranz
Copy link
Member

@luca-rath the teaser should be returned with the resolved media.

@alexander-schranz alexander-schranz mentioned this pull request Apr 7, 2021
50 tasks
@wachterjohannes
Copy link
Member

@luca-rath as alex mentioned we should resolve the media and write into the reference store

@luca-rath luca-rath marked this pull request as ready for review April 7, 2021 12:08
Copy link
Member

@alexander-schranz alexander-schranz left a comment

Choose a reason for hiding this comment

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

The content resolver I think can be simplified a little bit.

Content/ContentTypeResolver/TeaserSelectionResolver.php Outdated Show resolved Hide resolved
Content/ContentTypeResolver/TeaserSelectionResolver.php Outdated Show resolved Hide resolved
Content/ContentTypeResolver/TeaserSelectionResolver.php Outdated Show resolved Hide resolved
Content/ContentTypeResolver/TeaserSelectionResolver.php Outdated Show resolved Hide resolved
$items = [
[
'id' => '74a36ca1-4805-48a0-b37d-3ffb3a6be9b1',
'type' => 'pages',
Copy link
Member

@alexander-schranz alexander-schranz Apr 7, 2021

Choose a reason for hiding this comment

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

its a little bit strange from outside that we return here pluralized types and in the structure itself it are the singular template key but will at current state create an issue for it. #84 /cc @nnatter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I totally agree

@wachterjohannes
Copy link
Member

@alexander-schranz can you recheck and merge if all of your thoughts are implemented

@alexander-schranz alexander-schranz added the Feature New functionality not yet included in the Bundle label Apr 7, 2021
@alexander-schranz alexander-schranz merged commit 40215f2 into sulu:0.x Apr 7, 2021
@luca-rath luca-rath deleted the feature/teaser-selection-resolver branch April 7, 2021 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New functionality not yet included in the Bundle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants