[Patch] The custom fields for start and end date should be required

[Ryan J Ollos]

It seems like all of the custom fields are optional except for start and finish. Is there any use case or utility of the chart if these aren't defined? If not, and we checked for and threw an error when they aren't defined, it would be safe to omit checks such as if self.fields['start'].

This will lead into some other refactoring ideas I would like to discuss later on, in order to improve the robustness of the code.

0.11/tracjsgantt/tracjsgantt.py

@@ -183 +183 @@
         for field in fields:
             self.fields[field] = self.config.get('trac-jsgantt',
                                                  'fields.%s' % field)
+        
+        # Check for required fields
+        if not self.fields['start']:
+            raise TracError("""The custom field for start date must
+                               be configured in trac.ini.""")
+            
+        if not self.fields['finish']:
+            raise TracError("""The custom field for end date must
+                               be configured in trac.ini.""")
 
         # This is the format of start and finish in the Trac database
         self.dbDateFormat = str(self.config.get('trac-jsgantt', 'date_format')) 

[Ryan J Ollos]

Two of the thoughts I had about refactoring were:

• The ticket dictionary that is output from _process_tickets should have a consistent set of keys. If we mapped, for example, t[self.fields['estimate']] -> t['estimate'], we could avoid the need for the repetitive and verbose checks such as the following, which have resulted in a number of bugs: if self.fields['estimate'] and ticket.get(self.fields['estimate']):.
• In the same step, when a custom field isn't defined, we could still create an entry in the dictionary and set its value to None. That way, the only check that ever needs to be done is if ticket['estimate']:

[Ryan J Ollos]

#9508 is essentially a duplicate of this ticket.

[Chris Nelson]

Replying to rjollos:

It seems like all of the custom fields are optional except for start and finish. Is there any use case or utility of the chart if these aren't defined?

Sure. Those fields just allow the user to specify explicit start and/or finish dates for tasks. Without them you could have a Trac milestone with a due date and tickets leading up to it (with dependencies between them) which flow back ALAP to a necessary start date.

...

I'm not sure if you're saying that the chart currently misbehaves if these fields aren't configured. I'll test that. If there is a place I'm requiring them by not testing for existence, I'll plan to fix that.

[Chris Nelson]

(In [10863]) Don't require start and finish fields. Refs #9445.

Most uses were protected by checking that they were defined but when setting up milestones, there wasn't a check.

[Chris Nelson]

This works for me and I've heard no complaints in over a week.