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

Fix parameter validation when generating relative URLs to routes with domain parameters #755

Draft
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions tests/js/route.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,26 @@
expect(route('catalog-domain-param-path', ['/p/test'], false)).toBe('/catalog//p/test');
});

// Highest potential for "breaking" changes when fixing these is the second format in each example, where
// params are passed as an array, because it's less clear in that case which param is which since we're
// relying on the order they're passed. It may break some people's code but it's probably more
// important to match Laravel's behaviour correctly, this is a pretty big inconsistency.
test('handle domain parameters when generating relative URL', () => {
// Just for demonstration purposes, this is tested properly elsewhere
expect(route('team.user.show', { team: 1, id: 2 })).toBe('https://1.ziggy.dev/users/2');
expect(route('team.user.show', [1, 2])).toBe('https://1.ziggy.dev/users/2');

// Laravel route() errors here, domain {team} param is required
// We should *probably* match that behaviour and error too
expect(route('team.user.show', { id: 2 }, false)).toBe('/users/2');
expect(route('team.user.show', [2], false)).toBe('/users/2');

// Laravel route() handles these find, {team} param is recognized even though final output won't contain it
// We should *definitely* match that behaviour and handle these properly
expect(route('team.user.show', { team: 1, id: 2 }, false)).toBe('/users/2');

Check failure on line 810 in tests/js/route.test.js

View workflow job for this annotation

GitHub Actions / Ubuntu, PHP 8.1, Laravel 10

tests/js/route.test.js > route() > handle domain parameters when generating relative URL

AssertionError: expected '/users/2?team=1' to be '/users/2' // Object.is equality - Expected + Received - /users/2 + /users/2?team=1 ❯ tests/js/route.test.js:810:68

Check failure on line 810 in tests/js/route.test.js

View workflow job for this annotation

GitHub Actions / Ubuntu, PHP 8.2, Laravel 9

tests/js/route.test.js > route() > handle domain parameters when generating relative URL

AssertionError: expected '/users/2?team=1' to be '/users/2' // Object.is equality - Expected + Received - /users/2 + /users/2?team=1 ❯ tests/js/route.test.js:810:68

Check failure on line 810 in tests/js/route.test.js

View workflow job for this annotation

GitHub Actions / Ubuntu, PHP 8.3, Laravel 9

tests/js/route.test.js > route() > handle domain parameters when generating relative URL

AssertionError: expected '/users/2?team=1' to be '/users/2' // Object.is equality - Expected + Received - /users/2 + /users/2?team=1 ❯ tests/js/route.test.js:810:68

Check failure on line 810 in tests/js/route.test.js

View workflow job for this annotation

GitHub Actions / Ubuntu, PHP 8.3, Laravel 10

tests/js/route.test.js > route() > handle domain parameters when generating relative URL

AssertionError: expected '/users/2?team=1' to be '/users/2' // Object.is equality - Expected + Received - /users/2 + /users/2?team=1 ❯ tests/js/route.test.js:810:68

Check failure on line 810 in tests/js/route.test.js

View workflow job for this annotation

GitHub Actions / Ubuntu, PHP 8.3, Laravel 11

tests/js/route.test.js > route() > handle domain parameters when generating relative URL

AssertionError: expected '/users/2?team=1' to be '/users/2' // Object.is equality - Expected + Received - /users/2 + /users/2?team=1 ❯ tests/js/route.test.js:810:68
expect(route('team.user.show', [1, 2], false)).toBe('/users/2');
});

test('skip encoding some characters in route parameters', () => {
// Laravel doesn't encode these characters in route parameters: / @ : ; , = + ! * | ? & # %
expect(route('pages', 'a/b')).toBe('https://ziggy.dev/a/b');
Expand Down
Loading