-
Notifications
You must be signed in to change notification settings - Fork 204
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 incorrect pathWithParams for savedFinds. #532
base: master
Are you sure you want to change the base?
Conversation
Must include parameters for screen.
@@ -2385,7 +2385,7 @@ class ScreenRenderImpl implements ScreenRender { | |||
for (Map<String, Object> userFlf in userFlfList) { | |||
EntityValue formListFind = (EntityValue) userFlf.formListFind | |||
Map itemMap = [name:formListFind.formListFindId, title:formListFind.description, image:lastImage, imageType:lastImageType, | |||
path:lastPath, pathWithParams:(lastPath + "?formListFindId=" + formListFind.formListFindId)] | |||
path:lastPath, pathWithParams:(currentPath.toString() + (paramString.length() > 0 ? '&' : '?') + "formListFindId=" + formListFind.formListFindId)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I think I see what was broken here, in cases with parameters resulting in 2 question mark characters in the pathWithParams.
Small detail, and partly messy due to naming of things above, but it would be helpful to be consistent and use either lastPath
or currentPath.toString()
in both path and pathWithParams instead of mixed usage. Otherwise the changes look fine, I suppose another way to fix that would be to look for a ?
in lastPath.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And lastPath
is defined before add parameters to currentPath
, it doesn't include parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot found a good solution for this. LINE:2399 below this also use lastPath
for path
without parameters, and currentPath.toString
for pathWithParams
.
Maybe a utility class can concate query string will improve readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR still have a bug. If paramString
already contains a formListFindId
, We should remove it first.
e8e881a
to
5bd151b
Compare
Must include parameters for screen.