Problem/Motivation

The FetchUrlSchema plugin displays a link to show the raw content in a new window. To build the url of that link, the container ID is needed, but it is not available in build() so the current request url is parsed. That is bad because the container may possibly be viewed in another context than the clean canonical container url.

Proposed resolution

Maybe pass the whole CollectDataItem object to build().
Or implement them as contextual links?

Remaining tasks

User interface changes

API changes

Comments

arla’s picture

Actually, isn't the link a duplicate of #2449371: Link to container raw data?

arla’s picture

Discussed with @miro_dietiker and this is not a duplicate.

The other issue is about providing the complete, escaped, raw data in the UI. The purpose is to be able to review the data e.g. for schema/property management.

The fetched content link is about providing the content for downloading, or viewing outside the Drupal UI.

mbovan’s picture

Status: Active » Needs review
StatusFileSize
new801 bytes

After #2448225: Filter container list for schema URI and origin URI this problem has activated. Because url is parsed from current request there is a problem if we click on "Show content in a new button" from this url http://.../admin/content/collect/11?destination=/admin/content/collect (url is created if we click on 'View' button on container listing page).
In uploaded patch, replaced url parsing with container id from the collect container object, which we can get from current request.

miro_dietiker’s picture

Status: Needs review » Needs work

Yeah, that looks ugly. The proposed solution already looks better... But it still looks like a workaround.

We need a test that exactly shows how it is currently broken and fixed after with that destination parameter.

mbovan’s picture

Status: Needs work » Needs review
StatusFileSize
new888 bytes
new1.65 KB
new888 bytes

Added tests to assert that generated web page is accessible. Also provided test-only patch.

berdir’s picture

Status: Needs review » Needs work
+++ b/src/Plugin/collect/Schema/FetchUrlSchema.php
@@ -96,8 +96,7 @@ class FetchUrlSchema extends SchemaBase implements SpecializedDisplaySchemaInter
+    $collect_container_id = \Drupal::requestStack()->getCurrentRequest()->get('collect_container')->id();

\Drupal::routeMatch()->getRawParameter('collect_container') should give you the same.

And yeah not pretty. It's still relying on the current route, which will be a problem if this is every called in a different way?

Apparently we are relying on it, shouldn't we pass the entity or $items into this method? Or have a generic $context array, if the container is there, use it, if not, don't display this link?

mbovan’s picture

Status: Needs work » Needs review
StatusFileSize
new5.84 KB
new4.93 KB

Added collect container as a second parameter in build() method (SpecializedDisplaySchemaInterface class) so we can always get needed collect container id.

miro_dietiker’s picture

Status: Needs review » Fixed

Fixxxed. Thx!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.