Modify

Opened 3 years ago

Closed 3 years ago

#9445 closed enhancement (fixed)

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

Reported by: rjollos Owned by: ChrisNelson
Priority: normal Component: TracJsGanttPlugin
Severity: normal Keywords:
Cc: Trac Release: 0.11

Description

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

     
    183183        for field in fields: 
    184184            self.fields[field] = self.config.get('trac-jsgantt', 
    185185                                                 'fields.%s' % field) 
     186         
     187        # Check for required fields 
     188        if not self.fields['start']: 
     189            raise TracError("""The custom field for start date must 
     190                               be configured in trac.ini.""") 
     191             
     192        if not self.fields['finish']: 
     193            raise TracError("""The custom field for end date must 
     194                               be configured in trac.ini.""") 
    186195 
    187196        # This is the format of start and finish in the Trac database 
    188197        self.dbDateFormat = str(self.config.get('trac-jsgantt', 'date_format'))  

Attachments (0)

Change History (6)

comment:1 Changed 3 years ago by rjollos

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']:

comment:2 Changed 3 years ago by rjollos

  • Summary changed from The custom fields for start and end date should be required to [Patch] The custom fields for start and end date should be required

comment:3 Changed 3 years ago by rjollos

#9508 is essentially a duplicate of this ticket.

comment:4 in reply to: ↑ description Changed 3 years ago by ChrisNelson

  • Status changed from new to assigned

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.

comment:5 Changed 3 years ago by ChrisNelson

(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.

comment:6 Changed 3 years ago by ChrisNelson

  • Resolution set to fixed
  • Status changed from assigned to closed

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

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.