Multiple path parameters do not get replaced in view url #211
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit ec5f47c.
pekura
requested review from
antiheld,
dariadomagala-sap,
hardl,
jesusreal,
kwiatekus,
maxmarkus,
parostatkiem-zz and
y-kkamil
as code owners
jesusreal
suggested changes
Nov 12, 2018
core/src/services/routing.js
Outdated
| @@ -123,20 +123,30 @@ const escapeRegExp = string => { | |||
| return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | |||
| }; | |||
|
|
|||
| const replaceVars = (viewUrl, params, prefix) => { | |||
| const replaceVars = (viewUrl, params, prefix, noParenthesis) => { | |||
There was a problem hiding this comment.
If my tests were correct here we are replacing just the second dynamic parameter. If this is true, where are we replacing the first dynamic parameter and why aren´t we replacing all parameters at once in the same place?
There was a problem hiding this comment.
Right, it is a bit weird. It comes from a place that needs to get refactored. I created a dedicated issue for that: #213.
There was a problem hiding this comment.
Could you please explain this issue in detail so that we make sure this is fixed in that ticket?
There was a problem hiding this comment.
It is explained in the related issue #213.
jesusreal
approved these changes
Nov 13, 2018
y-kkamil
approved these changes
Nov 14, 2018
pekura
deleted the
Multiple-path-parameters-do-not-get-replaced-in-view-url
branch
stanleychh
pushed a commit
to stanleychh/luigi
that referenced
this pull request
Dec 30, 2021
Multiple path parameter replacement in the viewUrl fixed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for: multiple path parameters do not get replaced in view url #208.