Uploaded image for project: 'eZ Publish / Platform'
  1. eZ Publish / Platform
  2. EZP-22068

Add loadRelatedContent() and loadEmbedContent() to the API

    Details

      Description

      These methods should check content/view_embed permissions and filter out those not accessible, which would remove the need to check for the same using Repository::sudo() in:

      • eZ/Publish/Core/FieldType/XmlText/Converter/EmbedToHtml5
      • eZ/Publish/Core/MVC/Symfony/Controller/Content/ViewController

      Ref:

      https://github.com/ezsystems/ezpublish-kernel/pull/490
      https://github.com/ezsystems/ezpublish-kernel/pull/644

      Both methods should check for the same permissions, but if for example content/view_relation is going to be introduced in the future it might make sense to implement them both.

        Issue Links

          Activity

          Petar Spanja (Inactive) created issue -
          Petar Spanja (Inactive) made changes -
          Field Original Value New Value
          Description These methods should check content/view_embed permissions, which would remove the need to check for the same in:

          * eZ/Publish/Core/FieldType/XmlText/Converter/EmbedToHtml5
          * eZ/Publish/Core/MVC/Symfony/Controller/Content/ViewController

          Ref:

          https://github.com/ezsystems/ezpublish-kernel/pull/490
          https://github.com/ezsystems/ezpublish-kernel/pull/644

          Both methods should check for the same permissions, but if for example content/view_relation is going to be introduced in the future it might make sense to implement them both.
          These methods should check content/view_embed permissions, which would remove the need to check for the same using Repository::sudo() in:

          * eZ/Publish/Core/FieldType/XmlText/Converter/EmbedToHtml5
          * eZ/Publish/Core/MVC/Symfony/Controller/Content/ViewController

          Ref:

          https://github.com/ezsystems/ezpublish-kernel/pull/490
          https://github.com/ezsystems/ezpublish-kernel/pull/644

          Both methods should check for the same permissions, but if for example content/view_relation is going to be introduced in the future it might make sense to implement them both.
          Petar Spanja (Inactive) made changes -
          Link This issue discovered while testing EZP-21438 [ EZP-21438 ]
          André Rømcke made changes -
          Status Open [ 1 ] Confirmed [ 10037 ]
          André Rømcke made changes -
          Status Confirmed [ 10037 ] Backlog [ 10000 ]
          André Rømcke made changes -
          Rank Ranked higher
          André Rømcke made changes -
          Remaining Estimate 5 hours [ 18000 ]
          André Rømcke made changes -
          Original Estimate 5 hours [ 18000 ]
          André Rømcke made changes -
          Sprint Ventoux Sprint 4 [ 25 ]
          André Rømcke made changes -
          Rank Ranked lower
          André Rømcke made changes -
          Description These methods should check content/view_embed permissions, which would remove the need to check for the same using Repository::sudo() in:

          * eZ/Publish/Core/FieldType/XmlText/Converter/EmbedToHtml5
          * eZ/Publish/Core/MVC/Symfony/Controller/Content/ViewController

          Ref:

          https://github.com/ezsystems/ezpublish-kernel/pull/490
          https://github.com/ezsystems/ezpublish-kernel/pull/644

          Both methods should check for the same permissions, but if for example content/view_relation is going to be introduced in the future it might make sense to implement them both.
          These methods should check content/view_embed permissions and filter out those not accessible, which would remove the need to check for the same using Repository::sudo() in:

          * eZ/Publish/Core/FieldType/XmlText/Converter/EmbedToHtml5
          * eZ/Publish/Core/MVC/Symfony/Controller/Content/ViewController

          Ref:

          https://github.com/ezsystems/ezpublish-kernel/pull/490
          https://github.com/ezsystems/ezpublish-kernel/pull/644

          Both methods should check for the same permissions, but if for example content/view_relation is going to be introduced in the future it might make sense to implement them both.
          Hide
          André Rømcke added a comment - - edited

          Christian Bacher / Petar Spanja As we already have loadRelations(), should we consider changing it?

          So basically change it to filter instead of throw if user does not have access to one of the related content? And then do the exact same thing in loadEmbedRealtions but using view_embed permission?

          While at it, this function is missing paging, but because of the permission stuff it will be pointless to do this in SPI as it will most likely not match what API returns.

          Show
          André Rømcke added a comment - - edited Christian Bacher / Petar Spanja As we already have loadRelations(), should we consider changing it? So basically change it to filter instead of throw if user does not have access to one of the related content? And then do the exact same thing in loadEmbedRealtions but using view_embed permission? While at it, this function is missing paging, but because of the permission stuff it will be pointless to do this in SPI as it will most likely not match what API returns.
          Hide
          Christian Bacher (Inactive) added a comment -

          loadRelatedContent is different from loadRelations(). First it loads content - not relations. This solves the problem that if you would return a relation in loadRelations where the user only has view_embed permission the loadContent method will throw an UnauthorizedException because the user has no content/view permission.
          By now i don't see any usecase for the method loadEmbedContent (when introduced, it should be named loadEmbeddedContent)

          Show
          Christian Bacher (Inactive) added a comment - loadRelatedContent is different from loadRelations(). First it loads content - not relations. This solves the problem that if you would return a relation in loadRelations where the user only has view_embed permission the loadContent method will throw an UnauthorizedException because the user has no content/view permission. By now i don't see any usecase for the method loadEmbedContent (when introduced, it should be named loadEmbeddedContent)
          André Rømcke made changes -
          Rank Ranked higher
          André Rømcke made changes -
          Summary Add ContentService::loadRelatedContent() and ContentService::loadEmbedContent() to the API Add loadRelatedContent() and loadEmbedContent() to the API
          Hide
          André Rømcke added a comment - - edited

          > This solves the problem that if you would return a relation in loadRelations where the user only has view_embed permission the loadContent method will throw an UnauthorizedException because the user has no content/view permission.
          > By now i don't see any usecase for the method loadEmbedContent (when introduced, it should be named loadEmbeddedContent)

          loadRelations() check content/read permissions on all relations, this is why we have this issue open.

          However, yes, there should perhaps be a spec on this, this issue jumps to conclusion that we need new API's, while the use case needs to be defined instead. (Also we are about to change how RichText field type deals with relations so it's done in view layer, this will affect the requirements of such a functionality).

          Show
          André Rømcke added a comment - - edited > This solves the problem that if you would return a relation in loadRelations where the user only has view_embed permission the loadContent method will throw an UnauthorizedException because the user has no content/view permission. > By now i don't see any usecase for the method loadEmbedContent (when introduced, it should be named loadEmbeddedContent) loadRelations() check content/read permissions on all relations, this is why we have this issue open. However, yes, there should perhaps be a spec on this, this issue jumps to conclusion that we need new API's, while the use case needs to be defined instead. (Also we are about to change how RichText field type deals with relations so it's done in view layer, this will affect the requirements of such a functionality).
          André Rømcke made changes -
          Rank Ranked lower
          André Rømcke made changes -
          Sprint Ventoux Sprint 4 [ 25 ]
          André Rømcke made changes -
          Rank Ranked higher
          Hide
          Petar Spanja (Inactive) added a comment -

          As can be seen in https://github.com/ezsystems/ezpublish-kernel/pull/758, we have a need to load both embedded Content and embedded Location. This is all about checking correct permissions, so I would suggest:

          1. ContentService::loadEmbeddedContent() - checks content/view_embed on Content
          2. LocationService::loadEmbeddedLocation() - checks content/view_embed on Content in a Location

          Regarding ContentService::loadRelations(), since Relation contains ContentInfo and not Content, it can check and return Relation for both content/view and content/view_embed.

          Show
          Petar Spanja (Inactive) added a comment - As can be seen in https://github.com/ezsystems/ezpublish-kernel/pull/758 , we have a need to load both embedded Content and embedded Location. This is all about checking correct permissions, so I would suggest: ContentService::loadEmbeddedContent() - checks content/view_embed on Content LocationService::loadEmbeddedLocation() - checks content/view_embed on Content in a Location Regarding ContentService::loadRelations(), since Relation contains ContentInfo and not Content, it can check and return Relation for both content/view and content/view_embed.
          André Rømcke made changes -
          Fix Version/s 5.3 [ 11282 ]
          André Rømcke made changes -
          Workflow eZ Engineering Scrumban Workflow [ 60397 ] EZ* Development Workflow [ 69673 ]
          Alex Schuster made changes -
          Workflow EZ* Development Workflow [ 69673 ] EZEE Development Workflow [ 107967 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Confirmed Confirmed
          43d 23h 55m 1 André Rømcke 22/Jan/14 2:54 PM
          Confirmed Confirmed Backlog Backlog
          3s 1 André Rømcke 22/Jan/14 2:54 PM

            People

            • Assignee:
              Unassigned
              Reporter:
              Petar Spanja (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Time Tracking

                Estimated:
                Original Estimate - 5 hours
                5h
                Remaining:
                Remaining Estimate - 5 hours
                5h
                Logged:
                Time Spent - Not Specified
                Not Specified