Generate new form data for each page load #829
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors the TaxBrain form logic so that the page form data is re-generated each time a page is loaded. In order to save user input data as a JSON object instead of columns (see #822), we need to dynamically add fields to the
ModelForm
subclass. We must do this because theModelForm
class expects there to be a mapping from the database fields to the form fields. But, what we have is a mapping from form fields to items in a JSON object which is stored in the database more or less as a blob of data. Thus, the form fields were dynamically created from the keys in the blob of data. This was done by directly modifying thefields
attribute of an instance of (subclass of?)django.ModelForms
.The previous logic created all of the form data once--on start up--in an inner class
Meta
. Then, if the user changed the start year, all of the data was updated according to the Tax-Calculatorcurrent_law_policy.json
document. After implementing PR #822, we were updating form data inself.fields
(thinking ofself
is in aTaxBrainForms
instance) and in theself._meta.widgets
object which I think was created from theMeta
class. We were essentially updating two different objects with the same data. This had some strange side effects that I missed while developing and testing locally via REPL type methods (i.e. change some code, print some output, load a page or two, change some more code, etc.).This method of development involved saving and reloading the modules pretty regularly. Once I got the desired behavior, I merged my changes, ran some tests on the test app, pushed to production, ran a few quick tests immediately after, and was quite surprised when I checked this morning and saw that all of the CPI flags were off. After much head scratching over the fact that the production app had a bug while the test app and my local instance did not, I realized that the test app and my local instance had the same bug. After much more head scratching, I realized that everything worked just fine on the test app immediately after I deployed and locally immediately after I made a change (i.e. triggered Django to save and reload all of the modules and classes). I came to the conclusion that the same data was being updated and manipulated, and it was not being reloaded as I assumed. Apparently, this is how inner classes like
Meta
work.The changes in this PR do two things:
self.widgets
andself.fields
. I think that one of the problems was operating on the data that is either stored in different places or is the same but is accessed differently. That is accessingself.fields
andself._meta.widgets
separately.