Multiple Wishlist bug

[nethrelm]

We had some errors show up related to Wishlists not being 1:1 with Users. Here is the error log detail:

An error has occured at https://www.onlinescuba.com/Login.aspx
Exception: Exception of type 'System.Web.HttpUnhandledException' was thrown. Stack Trace: at System.Web.UI.Page.HandleError(Exception e) at System.Web.UI.Page.ProcessRequestMain(Boolean includeStagesBeforeAsyncPoint, Boolean includeStagesAfterAsyncPoint) at System.Web.UI.Page.ProcessRequest(Boolean includeStagesBeforeAsyncPoint, Boolean includeStagesAfterAsyncPoint) at System.Web.UI.Page.ProcessRequest() at System.Web.UI.Page.ProcessRequest(HttpContext context) at System.Web.HttpApplication.CallHandlerExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute() at System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously) Inner Exception: query did not return a unique result: 2 Inner Exception Stack Trace: at NHibernate.Impl.AbstractQueryImpl.UniqueElement(IList list) at NHibernate.Impl.CriteriaImpl.UniqueResult[T]() at CommerceBuilder.Users.Wishlist.Transfer(Int32 sourceId, Int32 targetId, Boolean transferEmptyWishlist) at CommerceBuilder.Users.User.Migrate(User oldUser, User newUser, Boolean includeOrderData, Boolean isNewUserAccount, Boolean migrateBasketShipments) at CommerceBuilder.Users.User.Migrate(User oldUser, User newUser) at AbleCommerce.ConLib.LoginDialog.LoginButton_Click(Object sender, EventArgs e) at System.Web.UI.WebControls.Button.RaisePostBackEvent(String eventArgument) at System.Web.UI.Page.ProcessRequestMain(Boolean includeStagesBeforeAsyncPoint, Boolean includeStagesAfterAsyncPoint)

I looked into it and it appears that there is nothing stopping the Wishlists table from creating multiple records for a given user, however the Wishlist.Transfer method runs a query that requires a UniqueResult<Wishlist>();

The user's wishlists are actually queried more than once - the first time it queries correctly, returning a list of wishlists for the user. The second time it imposes the UniqueResult contraint, which is causing an error. This is in CommerceBuilder > Users > Wishlist.cs, lines 61-97 of the latest build (R8b7345)

Code: Select all

        public static void Transfer(int sourceId, int targetId, bool transferEmptyWishlist)
        {
            if (sourceId != targetId)
            {
                // GET THE DEFAULT BASKET FOR THE SOURCE USER
                IList<Wishlist> sourceWishlists = AbleContext.Resolve<IWishlistRepository>().LoadForUser(sourceId); /**** THIS IS CORRECT ****/
                if (sourceWishlists.Count == 0) return;
                Wishlist sourceWishlist = sourceWishlists[0];
                if (!transferEmptyWishlist)
                {
                    // WE SHOULD NOT TRANSFER EMPTY BASKETS, COUNT THE SOURCE ITEMS
                    int sourceCount = AbleContext.Resolve<IWishlistItemRepository>().CountForWishlist(sourceWishlist.Id);
                    if (sourceCount == 0) return;
                }

                // DETERMINE WHETHER USER HAS A WISHLIST ALREADY
                Wishlist wishlist = NHibernateHelper.CreateCriteria<Wishlist>()
                    .Add(Restrictions.Eq("User.Id", targetId))
                    .UniqueResult<Wishlist>(); /**** THIS IS THE SOURCE OF THE ERROR ****/

                int targetWishlistId = wishlist != null ? wishlist.Id : 0;
                if (targetWishlistId == 0)
                {
                    // USER HAS NO WISHLIST, SO MOVE THE SOURCE WISHLIST TO NEW USER
                    sourceWishlist.User = AbleContext.Resolve<IUserRepository>().Load(targetId);
                    sourceWishlist.Save();
                }
                else
                {
                    // USER HAS A WISHLIST, JUST MOVE ITEMS FROM SOURCE TO TARGET
                    NHibernateHelper.CreateSQLQuery("update ac_WishlistItems set WishlistId = :targetWishlistId where WishlistId = :sourceWishlistId")
                        .SetInt32("targetWishlistId", targetWishlistId)
                        .SetInt32("sourceWishlistId", sourceWishlist.Id)
                        .ExecuteUpdate();
                }
            }
        }

I haven't looked deep enough into the code to determine if the intention is for users to be able to have multiple wishlists or if they were supposed to be unique, but whichever it is the code needs to be consistent when loading them, and if they are supposed to be unique then there needs to be mechanisms in place to prevent multiple wishlists from being created.

[jguengerich]

Did you submit this as a bug report (Feedback tab in Admin, choose Report Issue at top)?

[jmestep]

How are you able to create more than one wishlist for a customer? I'm looking in R8 and can't figure out how to do it.

[nethrelm]

I don't really know how multiple wishlists got created, but it happened. The database does not enforce a unique constraint on the UserId column of ac_Wishlists, so whatever series of steps led to an attempt to save a second wishlist, it succeeded. Once that happens, this error can occur.

[jmestep]

It looks like you should report it as a bug. The nHibernate mapping file defines it as a many to one, but it looks like the code doesn't handle it, as you have pointed out.

Code: Select all

 <many-to-one name="User" column="UserId" not-null="true" class="CommerceBuilder.Users.User,CommerceBuilder" />

[mazhar]

We kept our schema and entity mappings so that we can have multiple wishlists per users due to some considerations. Although in application we only have one wishlist per user. When ever you want to deal with Wishlist of user you should use

Code: Select all

user.PrimaryWishlist

This property will make sure to get you a wishlist without creating any duplicates.

[nethrelm]

Well, as I said, I'm not sure where the source of the multiple wishlists came from, but I know that the error that was thrown as a result came from the CommerceBuilder library code, which I showed in my first post. The library is not respecting the wishlist database schema, so it's a bug.

[mazhar]

Well, as I said, I'm not sure where the source of the multiple wishlists came from, but I know that the error that was thrown as a result came from the CommerceBuilder library code, which I showed in my first post. The library is not respecting the wishlist database schema, so it's a bug.

I just reported it in our logs.