show_hidden_initial needs to "remember" initial hidden_initial when rendering form from POST data

[Margie Roginski]

I think there is a "thinko" in the way that the show_hidden_initial concept has been implemented, specifically with respect to what happens when a form is rendered from POST data (ie, in the case where there's an error in the form). Sorry if I'm wordy in this description, but understanding this I think requires a conceptual example.

I have an app where the user fills out a form that is saved to the database. Multiple users may be working at once, and I have the following not-uncommon situtation:

  Joe views object X on his screen
  Mary views object X on her screen
  Joe modifies the 'deadline' field of object X and saves it to the db 
  Mary is still looking at her original view of object X which shows the old deadline
  Mary modifies the 'priority' field of object X and saves it to the db

I want the end result to be that Joe has modified the deadline field and Mary has modified the priority field, and this all works fine using show_hidden_initial. By using show_hidden_initial, when Mary's post goes through, hasChanged() returns the priority field, but not the deadline field, and that's exactly what I want. For comparison, if I do not use show_hidden_initial, then when Mary's save is posted, the deadline field looks changed (though she hasn't changed it in her form) due to the fact that it is different from what is currently in the database (ie, what Joe posted).

Ok - so all of the above works just fine. Here's the glitch. Suppose Mary posts her form and her post contains an unrelated error (like she unintentionally deletes the contents of the 'name' field and that is marked as requird). When the form is re-rendered with errors, the hidden initial value for the priority field has been reset to whatever she entered for that field. So now when she fixes her error and reposts, the posted priority field no longer looks modified since it is now the same as the posted intial-priority field. The result is that Mary's change to the priority field does not cause has_changed() to return the priority field.

This seems inconsistent. IE, it seems like the point of hidden_initial is to allow exactly the fields that the user has changed in the form to be detected as changed, but in this case where there is an error, fields that the user has changed are not being detected as changed. Of course, I am just hypothesizing on the whole purpose of this hidden initial stuff - I don't believe it is documented, but I have never the less fouund it extremely useful.

I think that the way to fix this is to populate the initial inputs that are sent to the client with saved hidden initial data, if it exists. In other words, once the hidden initial data is taken from the object during the GET, when the form is recreated during a POST, if the intial data is in the POST, the new hidden initial data should be taken from the initial data in the post, not from data posted by the user.

Here's what I've added which seems to work for me. In forms/forms.py:

Original code in as_widget()

            if isinstance(self.field, FileField) and self.data is None:
                data = self.form.initial.get(self.name, self.field.initial)
            else:
                data = self.data

My changes to as_widget()

            if isinstance(self.field, FileField) and self.data is None:
                data = self.form.initial.get(self.name, self.field.initial)
            else:
                if only_initial:
                    hidden_initial = self.form.hidden_initial.get(self.name)
                    if hidden_initial != None:
                        data = hidden_initial
                    else:
                        data = self.data

                else:
                    data = self.data

I'm not as familiar with the django code that initializes the form, so I'm quite sure where the appropriate place to initialize the hidden_initial dict is, so I curently have it in my own task form, like this:

class CommonForm(forms.ModelForm):

    def __init__(self, *args, **kwargs):
        super(CommonForm, self).__init__(*args, **kwargs)
        data = kwargs.get("data")
        if data:
            self.hidden_initial = {}
            for fieldName in self._meta.fields:
                fieldInitialData = data.get(self.add_initial_prefix(fieldName))
                if fieldInitialData != None:
                    self.hidden_initial[fieldName] = fieldInitialData

Maybe this would go in BaseForm()?

Anyway, sorry for this being somewhere between a bug report and a patch. This is not ready for primetime as a patch, but since I understand it pretty well now, I thought I would pass on as much of what I understand as I could. I just don't have the time it would take to turn it into a real patch with running tests and all. But if someone else thinks it's important, I wanted to give enough info for them to run with it.

[Karen Tracey]

Regarding: "it seems like the point of hidden_initial is to allow exactly the fields that the user has changed in the form to be detected as changed, but in this case where there is an error, fields that the user has changed are not being detected as changed. Of course, I am just hypothesizing on the whole purpose of this hidden initial stuff - I don't believe it is documented, but I have never the less fouund it extremely useful."

I believe show_hidden_initial was introduced to fix #7975. That's a somewhat different problem than the one you are trying to solve, and likely explains why it doesn't fully meet the requirements of your problem. Perhaps it could be adjusted to do so. I'm too tired at the moment to say or even fully follow the changes you propose, but thought you might be interested in knowing where show_hidden_initial came from.

[Russell Keith-Magee]

Duplicate of #11652

[Margie Roginski]

I don't really think that this is a duplicate of 11652. 11652 is specifically an admin issue, whereas the issue/enhancment I am discussing here is a general django enhancement that would improve the developers' ability to use django in a multiuser environment. The specific enhancement is when a widget has show_hidden_initial set, then when a form is rendered from POST data, the value of the hidden initial data should come from the POST data, similar to the way that the value of the field itself comes from the POST data in this case. Currently the value of the hidden initial data always comes from the initial data, regardless of whether the form is being rendered from POST data or now.

This change allows one to use show_hidden_initial very effectively to implement a multi user environment where multiple users are both modifying the same task, but working on different fields. With the current django code base, one ends up in a situation where the whole form gets saved when any field in the form has changed. So if user A modifies field A and user B modifies field B, then if B saves last, all of user B's form data is written to the instance. This means that although user B didn't modify field A, whatever was in their form for field A (which may be old data) is written. The change I am suggesting would allow one to detect what data has actually changed on the client side. One can then override the save method to save only the fields that have changed, and this gives a much nicer multiuser experience with far less unintentional overwrites. However, it still does not address the problem from 11652 - ie, what happens when two users to write the same field and how to detect that condition. To solve that, assuming we have the new bahavior of populating the hidden initial data from the POST, then we could solve the 11652 issue by detecting that the posted data is the same as the hidden initial data, but is different from the data in the model. This would be an indication that some other user has written the data and in that case an error could be returned. Assuming that the hidden initial data was populated from teh POST data, this could certainly be done by the user overriding save(). Again - this is just one piece, and I believe what I am suggesting is much broader than 11652.

Anyway, as I said, this is really an enhancement. I am posting this info not to complain about the fact that it has been closed, but just to elaborate on why I don't think it is a dup of 11652 and to note that I think this would be a valuable enhancement. This issue of making the code "do the right thing" in a multiuser environment is a hard issue, and I think the fix I'm proposing could be very powerful, so I hope someone can take the time to mull it over.

Margie