Beginnings of #47 #50
Conversation
| @@ -117,7 +117,10 @@ public RouteValueDictionary ShouldRedirectTo(MethodInfo method, RouteValueDictio | |||
| var redirectResult = (RedirectToRouteResult)ActionResult; | |||
|
|
|||
| if (redirectResult.RouteValues["Controller"] == null) | |||
There was a problem hiding this comment.
Should this be redirectResult.RouteValues["Controller"] == null && typeof(TController) == typeof(T)?
Otherwise, if the action returned a redirect to the same controller, but you were checking for a different one then it wouldn't throw when it should.
You can probably add a test case for that if there isn't one already.
There was a problem hiding this comment.
Not sure off-hand.
I will spend some time to understand the code more deeply this afternoon and then issue another commit.
Thank you.
|
Yep. You were right. I changed the implementation and added a test to represent the scenario you described. What do you think now? |
|
Yep, that looks good :)
|
|
I will merge this now but please don't deploy yet. I think this qualifies as a breaking change, yes? I also think the implementation of #45 will be a breaking change too. I think we should minimize the MAJOR version number bump by deploying them together. |
|
Thank you once more for all your input Rob -- I learn lots. |
|
Agree ex was literally just thinking the same thing. You should update nextversion.txt now though and the breaking changes file. That way the repository reflects current reality.
|
|
Ok I will submit a pull request for that now. |
No description provided.