Results 1 to 10 of 10

Thread: Model.save after failed Model.erase tries to delete again

    Wait! Looks like we don't have enough information to add this to bug database. Please follow this template bug format.
  1. #1
    Ext JS Premium Member
    Join Date
    Jun 2009
    Posts
    63

    Default Model.save after failed Model.erase tries to delete again

    Fiddle:

    https://fiddle.sencha.com/#fiddle/1cqv

    Ext version tested:

    • 5.1.2.748
    • 5.1.3.228

    Description:
    Calling Model.save after a Model.erase failed another destroy request is sent to the proxy.

    Steps to reproduce the problem:

    • record.erase()
    • DELETE gets sent to BE
    • BE answers with non success
    • user changes something in record and wants to save it
    • record.save()
    • another DELETE is sent to BE

    The result that was expected:

    • An UPDATE is sent

    The result that occurs instead:
    • A delete is sent


    Fix:
    Code:
    Ext.define('Perbility.fix.data.Model2', {
        override: 'Ext.data.Model',
        
        erase: function(options) {
            var me = this;
            
            options = options || {};
            
            options.failure = Ext.Function.createSequence(function() {
                me.dropped = false;
            }, options.failure);
            
            me.callParent([options]);
        }
    });

  2. #2
    Sencha User
    Join Date
    Feb 2013
    Location
    California
    Posts
    11,985

    Default

    Thanks for the report. Can you please post a test case which reproduces this issue?
    https://fiddle.sencha.com/#home

  3. #3
    Ext JS Premium Member
    Join Date
    Jun 2009
    Posts
    63

    Default

    I have added a fiddle and updated the text of my first post.

  4. #4
    Sencha Premium User
    Join Date
    Apr 2007
    Location
    Sydney, Australia
    Posts
    19,258

    Default

    I can see how you may want the behaviour you'd describe, but I wouldn't necessarily say it's a bug. The server returning a failure could be a network failure or something else. I think it's fairly reasonable to leave it as you found it.

  5. #5
    Ext JS Premium Member
    Join Date
    Jun 2009
    Posts
    63

    Default

    No matter where the failure comes from, the record was not deleted. So it should not be marked as dropped.

    Let's consider the following use case: The user wants to delete an object. The BE does not allow it, because some dependencies are not met (some links to other objects have to be removed, ore some setting changed, so that this object becomes deletable). The user then tries to change the object accordingly to meet the dependencies and be able to delete it, therefore the user has to save the changes. Which in (unfixed) ExtJs he cant, because when the save is called another delete is executed.

    I find your answer pretty poor, in my opinion.

  6. #6
    Sencha Premium User
    Join Date
    Apr 2007
    Location
    Sydney, Australia
    Posts
    19,258

    Default

    Not really sure what about my response was poor. As I said above, I understand why you may want something like that, however I think you're taking a narrow view of the issue because you're trying to achieve a specific business rule.

    To elaborate, there are 2 things to consider here:

    1) Consistent behaviour

    See this fiddle: https://fiddle.sencha.com/#fiddle/1cu0

    When the update returns a failure, the behaviour is to leave the record untouched. The same is true for creation. Using your logic above, if the server rejects the update, then it should also rollback any changes. So making a change to drop should, for consistency, carry over to other operations, which would mean a larger change. Are you suggesting that updates/creates should also modify state on failure?

    2) Generic perspective

    In general, it's difficult to derive meaning from "failure". In your application it may have a very well defined meaning, however as framework developers we can't make assumptions like that. A failure could mean the user internet dropped out. It could mean the DB was unavailable. It could mean some business rule vetoed it permanently, or it could be vetoed temporarily. As such, it seems safer to leave the current state and let the developer decide how to proceed.


    Again, as I said I can understand why you may want something like that, but saying that it's a bug that needs fixing is somewhat short-sighted. What I would suggest is that you can implement your own erase override, or implement a new method that implements the behaviour you describe. It should be fairly minimal to do so, seems like you've already got the code to do it.

  7. #7
    Sencha User
    Join Date
    Jan 2016
    Posts
    2

    Default

    Yes, from the framework perspective leaving the record untouched makes sense.
    But calling save() should never repeat a previously failed erase() call...

  8. #8
    Sencha Premium User
    Join Date
    Apr 2007
    Location
    Sydney, Australia
    Posts
    19,258

    Default

    You haven't really provided any reason for this other than "that's how we want the app to work".

    - Why should a drop operation behave differently to create/update?
    - We can't ascribe a reason for failure in the same way an application can. The safer option is to leave it as is.

    But calling save() should never repeat a previously failed erase() call...
    What if there's a network failure? Surely you'd want to retry then?

  9. #9
    Sencha User
    Join Date
    Jan 2016
    Posts
    2

    Default

    It's the job of the result handler (callback) to decide what to do if the previous operation failed.

    The callback can:
    - recall the previous method (save, erase, ...)
    - Can just inform the user and leave everything untouched
    - ...

    The framework should leave the record data and state unchanged. (As currently implemented)

    Calling erase() should always lead to an DELETE request. If the app wants a PUT it has to call save() instead.

    It does not make any sense to send a DELETE request doing a save() call.

  10. #10
    Sencha Premium User
    Join Date
    Apr 2007
    Location
    Sydney, Australia
    Posts
    19,258

    Default

    Ok, so it seems like we can agree that the state should be left as-is if a server call fails.

    In that case, you've got a scenario where you're calling save() on a record that is marked as dropped (pending deletion), you could:

    a) Call erase()
    b) Send it back to the server
    c) Throw an exception (force developer to "fix" this)

    I'd say that a) seems like a more logical choice, mostly on the grounds of consistency. When you call save() on a phantom model, it sends a create. When you call save() on a dirty record, it sends an update. When you call save() on a dropped record, it sends a destroy. The docs also outline this behaviour.

    Making updates on a record that is supposed to be deleted seems like a less intuitive thing to do.

Similar Threads

  1. Ext.data.Model.{save|erase} accepts params
    By ffayolle in forum Sencha Documentation
    Replies: 1
    Last Post: 15 Jun 2016, 7:23 AM
  2. Model.erase and DELETE request with empty body
    By chris-mac in forum Sencha Ext JS Q&A
    Replies: 2
    Last Post: 9 Mar 2016, 10:40 AM
  3. [INFOREQ] Erase model with writeAllFields in writer
    By ebett in forum Ext 5: Bugs
    Replies: 4
    Last Post: 9 Oct 2014, 9:16 AM
  4. Replies: 1
    Last Post: 4 Nov 2013, 12:54 PM
  5. Replies: 4
    Last Post: 7 Oct 2013, 5:32 AM

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •