Skip to content

fix: filter() without an amount produces a double slash - #58

Merged
Baspa merged 5 commits into
mainfrom
fix/filter-optional-amount
Jun 18, 2026
Merged

Baspa merged 5 commits into
mainfrom
fix/filter-optional-amount

Conversation

@Baspa

@Baspa Baspa commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Bug

In src/Transformations/Filter.php, generateUrl() unconditionally appended the amount:

$url .= '-/filter/' . $values['name'] . '/' . $values['amount'] . '/';

When filter('adaris') is called without an amount, $values['amount'] is null, which PHP casts to an empty string, yielding -/filter/adaris// — a double slash that produces an invalid Uploadcare URL.

Fix

Conditionally omit the amount segment when it is null:

if ($values['amount'] === null) {
    $url .= '-/filter/' . $values['name'] . '/';
} else {
    $url .= '-/filter/' . $values['name'] . '/' . $values['amount'] . '/';
}

The transform() validation is unchanged — null amount is permitted.

Test added

it('omits the amount segment in filter when no amount is given', function () {
    $uuid = '12a3456b-c789-1234-1de2-3cfa83096e25';

    $url = (string) uploadcare($uuid)->filter('adaris');
    expect($url)->toContain('-/filter/adaris/');
    expect($url)->not->toContain('adaris//');

    $url = (string) uploadcare($uuid)->filter('adaris', 50);
    expect($url)->toContain('-/filter/adaris/50/');
});

Note: filter triggers automatic -/preview/ insertion (the getUrl() logic), so assertions use toContain rather than exact toBe matching.

@Baspa Baspa changed the title Fix: filter() without an amount produces a double slash fix: filter() without an amount produces a double slash Jun 18, 2026
Baspa added 3 commits June 18, 2026 15:16
# Conflicts:
#	tests/TransformationTest.php
# Conflicts:
#	tests/TransformationTest.php
@Baspa
Baspa merged commit 87fb7f9 into main Jun 18, 2026
20 checks passed
@Baspa
Baspa deleted the fix/filter-optional-amount branch June 18, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant