Skip to content

Conversation

@chunlinyao
Copy link
Member

Must include parameters for screen.

Must include parameters for screen.
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)]
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member Author

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.

Copy link
Member Author

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.

@chunlinyao chunlinyao force-pushed the master branch 2 times, most recently from e8e881a to 5bd151b Compare May 23, 2023 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants