Bug in Admin Category Sorting page
Posted: Wed Aug 15, 2012 2:22 pm
In the Admin Sort Category page (/Admin/Catalog/SortCategory.aspx), there is a bug in the way the sort is handled.
The page uses the CatalogNodeCollection to populate a listbox that is resorted through javascript calls to the functions UP(int) and DN(int). The javascript functions are responsible for reordering the listbox, and the SaveButton_Click event reads the resulting listbox state and attempts to save the changes.
The bug is in how these list items are created.
The value field is bound to CatalogNodeId, which is NOT a unique identifier for items in the list. A catalog node can be one of 4 different objects:
Each of these objects is stored in a separate database table and has a separate identity pool. Because of this, duplicate values could exist across the identity pools. Therefore, it is insufficient to use the identity value by itself (catalognodeid) without qualifying which type of node (catalognodetypeid) it is. The listbox does not distinguish the node types in any way.
Assume we have 2 categories and 4 products:
Category A (Id = 1)
Category B (Id = 2)
Product 1 (Id = 1)
Product 2 (Id = 2)
Product 3 (Id = 3)
Product 4 (Id = 4)
Their current sort order in the listbox is such:
Category A (Id = 1)
Product 1 (Id = 1)
Category B (Id = 2)
Product 2 (Id = 2)
Product 3 (Id = 3)
Product 4 (Id = 4)
If we use the FIRST, ^,v, and LAST buttons, we see that the item moves appropriately up and down the list. If we move Category B to the top, we now have:
Category B (Id = 2)
Category A (Id = 1)
Product 1 (Id = 1)
Product 2 (Id = 2)
Product 3 (Id = 3)
Product 4 (Id = 4)
Then we click save. But we notice that Category B moves back above Product 2. Why? Lets look at the SaveButton_Click code:
SortOrder is the hidden field that holds the comma-separated list of ids in the desired sort order. Right now, it looks like this:
2,1,1,2,3,4
Lets see what happens next in the SaveButton_Click:
Here we are going to go through each id and attempt to locate it in the catalog nodes, and when we find it, assign a new OrderBy value and increment the order value for the next item. You can see that, because of the duplication of Id values in the SortOrder string, simply looping through catalog nodes and checking to see if the Id matches is not sufficient. The catalog node type has to be checked as well. Unfortunately, the listbox has no knowledge of the catalog node type after it is bound. The listbox has to be able to distinguish products from categories from webpages from links in order to perform the sort properly. this could be accomplished through using both id and catalog node type in the value field, separated by some delimiter other than a comma, and then split the values and do the checking of both id and node type as you loop through the catalog nodes.
Has this bug been reported? Is there an expected release of a fix?
The page uses the CatalogNodeCollection to populate a listbox that is resorted through javascript calls to the functions UP(int) and DN(int). The javascript functions are responsible for reordering the listbox, and the SaveButton_Click event reads the resulting listbox state and attempts to save the changes.
The bug is in how these list items are created.
Code: Select all
<asp:ListBox ID="CatalogNodeList" runat="server" DataTextField="Name" DataValueField="CatalogNodeId" Rows="20">
</asp:ListBox>
Code: Select all
public enum CatalogNodeType
{
Category = 0,
Product = 1,
Webpage = 2,
Link = 3,
}
Assume we have 2 categories and 4 products:
Category A (Id = 1)
Category B (Id = 2)
Product 1 (Id = 1)
Product 2 (Id = 2)
Product 3 (Id = 3)
Product 4 (Id = 4)
Their current sort order in the listbox is such:
Category A (Id = 1)
Product 1 (Id = 1)
Category B (Id = 2)
Product 2 (Id = 2)
Product 3 (Id = 3)
Product 4 (Id = 4)
If we use the FIRST, ^,v, and LAST buttons, we see that the item moves appropriately up and down the list. If we move Category B to the top, we now have:
Category B (Id = 2)
Category A (Id = 1)
Product 1 (Id = 1)
Product 2 (Id = 2)
Product 3 (Id = 3)
Product 4 (Id = 4)
Then we click save. But we notice that Category B moves back above Product 2. Why? Lets look at the SaveButton_Click code:
Code: Select all
protected void SaveButton_Click(object sender, EventArgs e)
{
if (!string.IsNullOrEmpty(SortOrder.Value))
{
2,1,1,2,3,4
Lets see what happens next in the SaveButton_Click:
Code: Select all
CatalogNodeCollection catalogNodes = _CatalogNodes;
string[] catalogNodesIds = SortOrder.Value.Split(",".ToCharArray());
int order = 0;
int tempId = 0;
int index = -1;
foreach (string sPartId in catalogNodesIds)
{
foreach (CatalogNode cn in _CatalogNodes)
{
tempId = AlwaysConvert.ToInt(sPartId);
if (cn.CatalogNodeId == tempId)
{
index = catalogNodes.IndexOf(cn);
if (index > -1)
catalogNodes[index].OrderBy = (short)order;
order++;
}
}
}
catalogNodes.Save();
}
Response.Redirect("Browse.aspx?CategoryId=" + _CategoryId.ToString());
}
Has this bug been reported? Is there an expected release of a fix?