views:

32

answers:

2

I have a repeater which binds a set of data. Within this repeater is a column with various controls for updating, deleting, etc. These are image buttons which fire an onclick event such as "DeleteRecord". All this does is fire a stored procedure, passing in the ID of the record to delete from the CommandArgument of the object.

This works wonderfully... except for one rather huge problem. Once you delete a record, if you refresh the page, the record where the first deleted record used to be gets deleted.

For instance... if I have 4 records

1 Record1
2 Record2
3 Record3
4 Record4

and I delete record 2... The page reloads with (which is fine):

1 Record1
3 Record3
4 Record4

...if I then hit refresh...

1 Record1
4 Record4

I assume this is because the erroneously deleted object (record3) is now in the same hierarchical place as the old object used to be and .net therefore doesn't know the difference, the page refreshes and fires the onlick event, grabbing out the command argument of the new object and deletes based on the ID as obtained from the commandargument of the new object.

This is obviously a huge problem, if a client did this it would destroy data erroneously and I'm at a loss here.

Is there any way to stop this from happening? I'm not sure if there is a better way to go about doing things or not. If there isn't, I need some sort of way to tell the page not to execute the event or to cross reference the ID of the object that is intended for deletion against the object itself...

Code below for convenience...

EDIT Wrapped a LinkButton around it because I have some jquery code in here as well which stops the page execution to wait for user confirmation. Pressing "ok" continues page execution.

<asp:LinkButton ID="oDeleteLink" CssClass="oDeleteIcon" CommandName="Delete" CommandArgument='<%# Eval("iAccountID") %>' runat="server">
     <asp:ImageButton ImageUrl="/files/system/icons/trash-steel-16.png" ToolTip="Delete This Account" AlternateText="Delete" ID="oDeleteIcon" runat="server" />
</asp:LinkButton>

    protected void oAccounts_ItemCommand(Object Sender, RepeaterCommandEventArgs e) {
        if (e.CommandName == "Delete") {
            int ID = e.CommandArgument.ToString().Numeric();
            db.SPs.SpDeleteAccount(ID).Execute();
            UI.Confirm(uiBroadcast, "Account has been deleted", "300px");
            BindAccounts();
        }
    }

Would appreciate any feedback you folks could give. Thanks!

+1  A: 

Use ItemCommand event and CommandName/CommandArgument instead:

<!-- CommandArgument binding would be the same as whatever you are binding your ID label text value to -->
<asp:ImageButton 
    CommandName="Delete" CommandArgument='<%# Eval("ID") %>'
    ImageUrl="/files/system/icons/trash-steel-16.png"  
    ToolTip="Delete This Account" 
    AlternateText="Delete" CssClass="oDeleteIcon"  
    ID="oDeleteIcon" runat="server" /> 

And the event handler:

void Repeater_ItemCommand(Object Sender, RepeaterCommandEventArgs e) {        
    if( e.CommandName == "Delete" ) {
        int id = e.CommandArgument.ToString().Numeric();

        db.SPs.SpDeleteAccount(id).Execute();
        // the rest of your code
    }
} 

This way the delete button itself is indicating what ID to delete, as opposed to relying on positional elements in your repeater.

Adam Sills
Thanks for the response Adam. Unfortunately it does the same thing.
Mark
I've done exactly this quite a bit and I've never had a problem. In fact, I just built a simple example and it works correctly. Something else in your code is causing your problem.
Adam Sills
Also note you can always Response.Redirect back to the same page to avoid any refresh issues.
Adam Sills
@Adam This is a pretty critical bug so I think I will use the response.redirect method for now, but that's not really ideal. Would you do me a favour and throw up your simple example? Even better, zip it and send it to me so I can see where it is I am going wrong. Would be much appreciated. Thanks for your assistance.
Mark
It's a real simple example. It uses static data and shows that no matter how often you refresh after deleting an item, the same command arg is sent to the server (put a breakpoint inside the event handler to prove it).
Adam Sills
Make a new web project, put this ASPX inside your default page: http://pastebin.com/EHv4PAEu
Adam Sills
And use this code: http://pastebin.com/m78VGynL
Adam Sills
You're awesome. Thanks for the help Adam. I ran your code and it works the way one would think - upon further examination I discovered that the difference between my code and yours is I'm calling my repeater binding in the Page_Load, you are calling yours in the Page_Init. I moved your binding member to the Page_load and ran your code again and indeed it exhibits the same behaviour as mine does. That is a pretty easy fix. Your help is appreciated mate! This was bugging me for the longest time.
Mark
Ah. Yeah, if you are going to rebind a data control and rely on events it raises, you should rebind during Page_Init. The page lifecycle between Init and Load loads viewstate, postback data, etc, and running your binding too late can overwrite some of that.
Adam Sills
+1  A: 

You need to follow the Post-Redirect-Get pattern [1]

Basically show the data, take the command request and redirect back to show the data, do not show the update in response to the post. Any refresh will re-request the data not re-perform the post

adam

[1] http://en.wikipedia.org/wiki/Post/Redirect/Get

adam straughan
+1 - I use this a lot also.
Adam Sills