Skip to content

BUGFIX: Correct asset download url#262

Merged
Sebobo merged 1 commit intoFlowpack:2.xfrom
jcarstens-cnlbz:bugfix/issue-259
Sep 16, 2025
Merged

BUGFIX: Correct asset download url#262
Sebobo merged 1 commit intoFlowpack:2.xfrom
jcarstens-cnlbz:bugfix/issue-259

Conversation

@jcarstens-cnlbz
Copy link
Contributor

What I did
fix that download always dowloaded the preview png instead of file
#259

How I did it
changed $asset to $assetProxy in condition
I found ProvidesOriginalUriInterface is only implemented in NeosAssetProxy so that makes sense

How to verify it
Download of pdf works again for example

@jonnitto jonnitto requested a review from Sebobo September 8, 2025 08:59
Copy link
Member

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

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

Thx for trying to fix this, but your change cannot be right in my opinion.

The $asset can never be a ProvidesOriginalUriInterface as it is a Types\Asset and only the NeosAssetProxy has this method as it implements the interface.
So the condition in line 112 can never be true and it instead always returns the preview uri.

So IMO the correct fix would be to switch from $asset to $assetProxy in the condition.

Also we need to target the 2.x branch with this.

@jcarstens-cnlbz
Copy link
Contributor Author

@Sebobo Yes, I thought so too, see my first commit. Changed it to assetProxy again

@Sebobo Sebobo changed the title Issue #259 - Fix Download BUGFIX: Correct asset download url Sep 16, 2025
@Sebobo Sebobo changed the base branch from main to 2.x September 16, 2025 07:06
Copy link
Member

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

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

Thx works!

@Sebobo Sebobo merged commit cb4d000 into Flowpack:2.x Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants