Fix Issue-171 by adding a validation to check for querystring paramet…

[espierre]

Fix #171
Fix Issue-171 by adding a validation to check for querystring parameters before adding the trailing slash

[marisks]

@espierre Could you please add tests for this?

[espierre]

@marisks Added tests to cover issue-171 changes.

[lanorkin]

@marisks here is a somewhat related PR from me with tests #168 which received no attention; I described some cases with incorrect behavior like this one in TODOs in tests in that PR

For this PR, there are more cases to be fixed - @espierre might be interesting for you, several TODOs starting from here https://github.com/Geta/404handler/pull/168/files#diff-c3448f2e875671b96f1597ec3936a893R139

[marisks]

@lanorkin The main issue with #168 is that it has a lot of TODOs. I understood that it is under development. As no improvements came to fix TODOs, I didn't look at it anymore.
Now it also has conflicts with the main branch too.

[lanorkin]

@marisks well idea was to write more tests to document current (well, current at that moment year ago) behavior; TODOs meant to highlight incorrect (but existing) behavior, which should have been fixed with future bugs & PRs like this one.

pure PR with tests, like it is said in title; I refer to it here, because it contains more cases that can be fixed in regard to trailing slash issue

[marisks]

@lanorkin I can't approve PR that has failing tests. That PR can be kept open for the devs to look at cases that can be fixed.

[lanorkin]

@marisks all the tests were passing at the moment; that's the idea of documenting behavior - you have passing tests for all the cases, even if that cases does not look OK from business perspective; TODOs added to highlight that though test is passing now, it is something which should be changed in future to reflect real business needs.

let's switch to discussion in #168 if you need any details; @espierre sorry for discussing it here

[espierre]

I saw that this PR was merged int your code base. Do yo know how long it would take to have it available in the Episerver nuget feed?

[valdisiljuconoks]

Sorry guys for delays (vacations). I'll make it happen so you can get official release.

Stay safe!