Preventing data mutation in format_fields

[mamico]

def format_fields(self, fields):

The fields object passed as an argument is modified within the function, which can lead to issues if the function is called multiple times, as it may result in broken or unintended data.

For example, in the following snippet:

                if self.block["subblocks"][field_index].get("field_type") == "date":
                    field["value"] = api.portal.get_localized_time(field["value"])

It seems get_localized_time expects a datetime object or an ISO-formatted string, not a localized string.

A quick fix to avoid mutating the original fields object is to add a deepcopy at the start of the function:

def format_fields(self, fields):
    fields = deepcopy(fields)

@JeffersonBledsoe thoughts ?

see #34

[JeffersonBledsoe]

@mamico I noticed a similar problem when getting #37 up-to-date . In this PR which reworks the API with fields to be a dedicated field object, I avoid the problem by re-getting the fields every time format_fields is called. This didn't seem to have any noticeable impact on performance (we have some 30 question forms), but I haven't done any actual time measurements.

Originally I wanted to compute all the fields once at the start after validation so as to only have to do this work once, however it looks like the PostEvent is currently allowed to make whatever changes it wants to the form data and so you can't really guarantee the state of the field data until after this has happened.

[mamico]

@JeffersonBledsoe Do you see any contraindications to using deepcopy as suggested above?
The issue is that it seems counterintuitive to me, for developers, that the 'format' method modifies the data passed as a parameter.

[JeffersonBledsoe]

@mamico I agree that it's counter-intuitive. I mentioned my other PR because it should already solve this problem by assigning all of the values in the passed in fields value to a dedicated field class which has functions for getting the modified versions of each field so as not to mutate the data. Until that PR is merged though, I'd agree that the deepcopy is a good quick solution for this. Unless the forms on a site are high traffic + the forms are really long, I don't imagine the memory impact being too significant