Conditionally set attributes in MongoDB based on form fields

I'm not 100% sure there is a particularly elegant way to write this. If the code did not have a different body for each if statement, one could rewrite it like this

FORM_VALUES = {
    "priority": accident.priority,
    "status": accident.status,
    "duration": accident.duration,
    "service_status": accident.service_status
}

for key in FORM_VALUES:
    if request.form.get(key) and int(request.form[key]) != FORM_VALUES[key]:
        # As you use different functions for setting the values, this isn't possible
        request.set_value(key, request.form[key])

However, that is not the case, so don't get your hopes up. Regardless, there are still improvements to be made:

• You're not checking that the header values are valid integers. I'd recommend either checking they are by using string.isdigit() or otherwise wrapping your calls to int() in a try except block.
• You can use 'reason' in request.form instead of request.form.get('reason'); they have the same meaning in the places you've used them, but the first is easier to read.
• It might be good if you used an enumeration for the values to Accident.status. As it is, checking that accident.status == 1 doesn't tell me anything; using a meaningful identifier might solve this. You can do this quite simply, by just assigning values to each identifier: PENDING, IN_PROGRESS, FINISHED = STATUSES. I've used some random names, and assumed that there are 3 possible statuses, change this to be accurate. Then in your function, instead of if accident.status == 1, we can have if accident.status == PENDING, which is a lot easier to understand without needing to know the meaning of each value of status.
• In general, you should try and be consistent with the ordering of your arguments: your set_reason takes first a reason, then a user argument, while all of your other methods take the user argument first. The set_service_status doesn't take a user argument at all, which is fairly impressive, considering it uses a user variable in the function.

Overall not bad code. I don't agree with your comments on how @property would be inappropriate there, but it's far from the end of the world. In general, I'd try and be more careful to validate user input, but it may be that you do not need to worry about that.

I'd like to see this as my goal:¹

if accident.status == 1:
    transfer_from_form(request.form, accident, [
        ('reason', str),
        ('note', str),
    ])
else:
    transfer_from_form(request.form, accident, [
        ('priority', int),
        ('status', int),
        ('duration', int),
        ('service_status', int),
    ])

Let's see what it takes to get there! I imagine that transfer_from_form() could look like this:

def transfer_from_form(form, obj, fields):
    for field, conv in fields:
        if form.get(field) and form[field] != getattr(obj, field):
            setattr(obj, field, conv(form[field]))

Notionally, that would perform the equivalent of assignments like accident.service_status = int(request.form['service_status']) whenever the form field is set and the attribute on accident doesn't already contain that value.

Then there is the pesky issue of how to pass the user parameter. I would argue that the user should be stored in the session somewhere, and the Accident's setter method could pluck it out of the session instead of having it passed in explicitly. You didn't specify which web framework you are using, so I can't tell you exactly how to accomplish that.

The Accident class's setters appear to be wrappers around the MongoEngine fields' setters, with some audit logging inserted. Instead of defining methods like set_service_status(), you could hook in the audit logging by monkey patching the MongoEngine fields' setters. Not only would the code read more smoothly, you wouldn't be relying on the goodwill of the users of the Accident class to refrain from setting accident.service_status directly instead of going through the approved set_service_status() method.

¹ There is some redundancy here. Ideally, I shouldn't have to declare that 'reason' is a str and 'service_status' is an int, because the types should be deducible from the fact that they are a StringField and IntField, respectively. If you can figure out how to accomplish that deduction, that would be even better.

Don't forget that dict.get() can also take a default:

if request.form.get('reason', accident.reason) != accident.reason:
    accident.set_reason(request.form['reason'], current_user)

...that makes it a little shorter.

EDIT: or even

def needs_update(name):
    return request.form.get(name, getattr(accident, name)) != getattr(accident, name)

if needs_update('reason'):
    ...

Barring @BluePepper's elegant solution, you could probably build a change list and then iterate through the fields that need updating.

This is very cheesy, but you could break the value check and the update in two parts. I did this fast, sorry, but I hope the general idea is clear:

form = {'a': 1, 'b': 2, 'c': 3}
original = {'a': 1, 'b': 20, 'c': 30, 'd': 4}

# build the list of new values as a list of tuples
update = [(k, form.get(k)) for k in form.keys() if form.get(k) != original.get(k, None)]

print update

# make the updates, making special calls as necessary
for k, v in update:
    if k == 'c':
        print "DO SOMETHING SPECIAL"
        original['c'] = v
    else:
        original[k] = v

print original

Produces

[('c', 3), ('b', 2)]
DO SOMETHING SPECIAL
{'a': 1, 'c': 3, 'b': 2, 'd': 4}