Bug 1

If I create a controller action as follows...

public void List(int category)

... and use the MVCToolkit HtmlHelper.ActionLink<T> as follows...

<%= Html.ActionLink<ProductController>( x => x.List(Category), "View Products for this category" ) %>

... it generates the URL http://server/Product/List/1 instead of http://server/Product/List/?category=1 and when i browse to the URL it says "A value is required for parameter 'category' in action 'List'"

[this is due to LinkExtensions.BuildQueryStringFromBLOCKED EXPRESSION always assuming the first parameter should be simply appended to the url]

Bug 2 

If I create a controller action as follows...

public void View(int Id)

... and use the MVCToolkit HtmlHelper.ActionLink<T> as follows...

<%= Html.ActionLink<ProductController>( x => x.View(ProductId), "View Product" ) %>

... it correctly generates the URL http://server/Product/View/1. However, when you navigate to that URL the same link now becomes http://server/Product/View/1/1. 

[this is because, in LinkExtensions.BuildUrl(), the call to UrlHelper.Action() parses the current URL and includes any current values (like Id) if it's a call to the same action. So maybe this is a bug with the way UrlHelper.Action works?? ]

 

[apologies for the BLOCKED EXPRESSION (does anyone understand why?) it should read LinkExtensions.BuildQueryStringFromE x p r e s s i o n()]

Just wanted to check that people are experiencing the same problems as me. 0 replies isn't a good sign.

 WRT bug 1 - you have to make sure the params are named the same as your routing. In this case I suspect that you're routing rule is:

[controller]/[action]/[id]

If it is, you need to make sure the argument is named "id" since that's how it's being set (name matching) when the action is invoked. The URL generated is proper for this. To make this work, you need to change the arg name from (int category) to (int id).

I think bug 2 is a casing issue - I'll see if I can reproduce it but ideally you want to make sure your casing matches your routes.

Thanks for these reports - do let me know if they solve your issue and more importantly, what you were expecting. It will help us as we build this stuff out.
 

WRT bug 1

It does not matter what my routing rule is!! The fact is that the function Html.ActionLink<ProductsController>( x => x.List(1), "..." ) should return the URL which maps to controller = "Products", action = "List", category = 1. no matter how the routing rules are defined. If category is matched in the URL then is should be /Products/List/1 if not then it should be /Products/List?category=1.

WRT bug 2

I'm now convinced that RouteCollection.GetUrl is faulty (in several different ways). Try simply writing <%= Url.Action( new {controller = "Products", action = "Edit"} ) %> This will return /Products/Edit until you actually navigate to /Products/Edit/12, for instance, at which point it will then return the full url including the [id] part.

tgmdbm:

 

It does not matter what my routing rule is!!

It does - unless I misunderstand the issue here, which is entirely possible :).

tgmdbm:

The fact is that the function Html.ActionLink<ProductsController>( x => x.List(1), "..." ) should return the URL which maps to controller = "Products", action = "List", category = 1

Unfortunately in this case, that's not how the Url routing engine works just yet. If you're routing rule specifies "id" as the argument, then that's what is looked for with Method.Invoke() - if it doesn't see it, it won't set it when passed in. To verify this, rename the argument in the List() action from "category" to "id" and see if it works.

I'd like to point out that we're still CTP and if this is confusing there's lots of room for improvement.

tgmdbm:

I'm now convinced that RouteCollection.GetUrl is faulty (in several different ways). Try simply writing <%= Url.Action( new {controller = "Products", action = "Edit"} ) %> This will return /Products/Edit until you actually navigate to /Products/Edit/12, for instance, at which point it will then return the full url including the [id] part.

Could be, and I'd like to see if we can get as much detail as possible here. I don't think I follow all of the second part of the issue - to me, if you are requiring an id for Edit(), then you should get an error (null ref). If you're not - I'm confused as to what you want to see.

Thanks again for helping out with this :). 

I'm a firm believer that changing the routes should under no circumstances break the application. The app should even work with no routes (as described below, see the comments). you should be able to develop the app without knowing or caring about the routes. i could go on but i think you get the idea.

I've written the following example which shows all the URL related bugs I've found so far. 5 in total.

Create a default MVC Application and add the following to Site.Master, inside the menu <div>

--- 

            <!-- 
              All of these links will break if "page = 1" is added to the Defaults of "[controller]/[action]/[id]"
              See topic - http://forums.asp.net/t/1197234.aspx
            -->
            <ul>
                <li><%= Html.ActionLink( "Home", "Index", "Home" ) %></li>
                <li><%= Html.ActionLink( "About Us", "About", "Home" ) %></li>
                <li><%= Html.ActionLink( "List Products", new { Action = "List", Controller = "Product" } ) %></li>
                <li><%= Html.ActionLink( "Edit Product", new { Action = "Edit", Controller = "Product", Id = 1 } ) %></li>
                <!-- 
                  The following link does not work because "category" is not mapped in the Route.
                  This SHOULD generate /Product/ListByCategory?category=3
                  This would allow the Routes to change and all the links would still work.
                  You could also have NO ROUTES and it would still work, i.e. /?controller=A&action=B&category=C
                -->
                <li><i><%= Html.ActionLink( "List Products", new { Action = "ListByCategory", Controller = "Product", category = 3 } ) %></i></li>
                <!--
                  The following doesn't work because "id" is "special"
                  If your route was "[controller]/[action]/[blah]" then both /a/b/c AND /a/b?blah=c would work identically.
                  The only reason I can see for this difference in behaviour is that "id" is special. (unless i'm doing something wrong)
                -->
                <li><i><a href="/Product/EditProduct?Id=5">Edit Product</a></i></li>
                <!-- 
                  The following link does not work because the first parameter to ListByCategory is not called "Id" 
                  This SHOULD generate /Product/ListByCategory?category=5
                  See Bug 1 - http://forums.asp.net/t/1197244.aspx 
                -->
                <li><i><%= Html.ActionLink<ProductController>( x => x.ListByCategory( 5 ), "List Products" )%></i></li>
                <!-- 
                  The following link works UNLESS you are already at /Product/EditProduct/[id]
                  It will generate /Product/EditProduct/[id]/1
                  See Bug 2 - http://forums.asp.net/t/1197244.aspx 
                -->
                <li><i><%= Html.ActionLink<ProductController>( x => x.EditProduct( 1 ), "Edit Product" )%></i></li>
            </ul>

--- 

Add the following controller (As you can see we don't care about the actual Views, it's all about the URL resolution.)

--- 

  public class ProductController : Controller
  {

    [ControllerAction]
    public void Index()
    {
      RenderView( "Index" );
    }

    [ControllerAction]
    public void List()
    {
      RenderView( "Index" );
    }

    [ControllerAction]
    public void ListByCategory(int category)
    {
      RenderView( "Index" );
    }

    [ControllerAction]
    public void Edit()
    {
      RenderView( "Index" );
    }

    [ControllerAction]
    public void EditProduct(int Id)
    {
      RenderView( "Index" );
    }
  }

 ---

Read the comments for a description of each bug.

[edited by: tgmdbm at 11:52 AM on Tue, Dec 25 2007]
The 3rd bug, regarding "id" being "special". I WAS doing something wrong. ?blah=c was working because i had a non-null default on blah. I'm not sure i consider this a bug now. 

Thanks again - I appreciate the code sample but I'm still having a problem discerning between a "bug" and a point of confusion. I'm going to have to go back over this thread and work up some code (I'll try that tomorrow) but I'll be honest and tell you I'm having a bit of a hard time understanding the problem. It seems that at it's core, you're expecting to see things in the QueryString versus appended to the Url - can you add some detailt to this expectation? Also - I'll ask for your patience again as I try to get to the meat of the issue here - I don't mean to be thick :) I'm just not grokking the problem very well.
 

ok, when you get to test the code tomorrow you should get an appreciation for which of the 5 things you think are bugs and which you don't. But to give you a feel for my thinking behind routes ...

if your route is, for example...

      RouteTable.Routes.Add( new Route
      {
        Url = "[controller]/list/[category]/[page]",
        Defaults = new { action = "List", category = "oranges", page = 1 },
        RouteHandler = typeof( MvcRouteHandler )
      } );

... you could navigate to /Products/list/apples/5 and that would take you to the ProductsController, action "List", with category equal to "appels" and page equal to 5.

<%= Url.Action( new { controller = "Products", action = "List", category = "apples", page = 5 }) %> should generate the same URL, and it does. However, if you change the route to the default...

      RouteTable.Routes.Add( new Route
      {
        Url = "[controller]/[action]/[id]",
        Defaults = new { action = "Index", id = (string)null },
        RouteHandler = typeof( MvcRouteHandler )
      } );

... It no longer works, you have to do something like <%= Url.Action( new { controller = "Products", action = "List" } ) %>?category=apples&page=5

The fact is that changing the routes has broken my code. Something I am very much against (as you can probably tell.)

All the 5 bugs stem from that one premise. Everything that generates a URL should generate the correct URL no matter how the routes are set up.

 

 I disagree, I haven't tested your code but looking at your most recent post I think you are misunderstanding how routing is supposed to work.  Changing the route to drop category and page parameters should not generate the same url!  How do you expect the routingcenter to know where the parameters are supposed to go, they are not defined anywhere in the route?

No, you misunderstood me. I'm sorry if I wasn't clear. I never said it should generate the SAME url, it should generate the RIGHT url, no matter what the route is.
 

 So given this route;

tgmdbm:

      RouteTable.Routes.Add( new Route
      {
        Url = "[controller]/[action]/[id]",
        Defaults = new { action = "Index", id = (string)null },
        RouteHandler = typeof( MvcRouteHandler )
      } );

And this action link:

tgmdbm:

<%= Url.Action( new { controller = "Products", action = "List", category = "apples", page = 5 }) %>

What is the "right" url that should be generated?

/Products/List?category=apples&page=5

Simply put, if it's not matched in the Url of the Route then it's part of the querystring.

If i'm not mistaken, that is how the arguments of the Action are generated. This just makes complete sense to me.

 +1... That makes sense, extra params should be query string.

abombss:

 +1... That makes sense, extra params should be query string.

+2

This then matches the request processing logic where query parameters are mapped to named parameters of the action method. 

Thanks for the positive feedback. Much appreciated.

Rob, did you get chance to test the sample code i posted?

If anybody else has tested it, I wouldn't mind a discussion on the individual bugs, if you have views on them.