Custom ticket field edits from None to '' show as deletion

[thatch]

I started this ​on trac-dev but convinced myself that it was a real issue. See that message for steps to reproduce

Just as a followup, templates/ticket_view.html displays field_name deleted when a custom field goes from None to ''. This is caused by a custom field being added after a ticket is created. I don't believe this to be a change (much less a deletion) so propose the following:

trac/ticket/web_ui.py

@@ -849 +849 @@
                     else:
                         this_num = old
                     current['cnum'] = int(this_num)
-            else:
+            elif old or new:
                 current['fields'][field] = {'old': old, 'new': new}
         if current:
             yield current

The unit tests appear to still pass as expected.

---

The ticket_change table is storing:

434|1166731555|<my username>|due_date||

And pysqlite2 is giving me the tuple

(434, 1166731555, u'<my username>', u'due_date', None, u'')

[cboos]

The problem is that we can't distinguish between a new custom field not set and a new custom field explicitly set to blank, otherwise I would have said we shouldn't write the custom field in the first place, therefore we would even avoid the problem.

But this could perhaps be an alternative solution. What would happen if we would not save fields with blank values (they'd simply be not there in the ticket_custom ). Wouldn't that work?

[thatch]

Not writing None-to-'' on saving tickets works for me. Basically if ticket_custom doesn't have a value already for the relevant field, it wouldn't be added as an entry into ticket_change when the new value is ''?

[thatch]

See attachment:t4447-dont-save-empty-field-changes.diff​ for an implementation of cboos' suggestion. I think it's doing a string comparison so the only way the change won't get written is if they're both in (None, ).

[cboos]

I believe that change is correct, please commit.

[ThurnerRupert]

no regression, so leave at 0.11.1 as it seems to block 0.11 unnecessarily.

[osimons]

Just ran into this one as well today - tested it and, and the custom field no longer appear as deleted just because it had been created in the timespan between comments. Good.

Anyone committing this, or should I just go ahead? thatch?

[cboos]

Yes, just go ahead.

[thatch]

It's fine to me, so please go ahead and apply, osimons.

[osimons]

OK. Applied as [6330].