[Gitlab] Due date override in issues is overridden by empty milestone due date

[rubdos]

Due to this logic:

if milestone and (
        self.extra['type'] == 'issue' or
        (self.extra['type'] == 'merge_request' and duedate is None)):
    duedate = milestone['due_date']

an issue like this one does not have a due date in TW. The issue has a due date (end of this week), but the milestone does not. Nevertheless, the logic above says that an issue's due date gets overridden by any milestone due date, even if the milestone does not have a due date.

Three proposals:

1. Get rid of the discrepancy between MR and issue altogether (but I'm not sure what the reasoning and semantics where for this decision):

if milestone and duedate is None:
    duedate = milestone['due_date']

2. Don't assign an empty due date

if milestone and milestone['due_date'] is not None and (
        self.extra['type'] == 'issue' or
        (self.extra['type'] == 'merge_request' and duedate is None)):
    duedate = milestone['due_date']

3. Take the earliest of both due dates (which is an extension of option 2). No code example since that's a lot more involved.

Feel free to give me a choice and I'll create a PR in here :-)

[ryneeverett]

1 seems like a clear and straightforward improvement. Looking at the git history, it seems that the discrepancy is a historical artifact of a previous implementation of this method which had separate code paths for issues, merge requests, todos, etc.

I can see the appeal of 3 but I could also see it not working well with everyone's workflow. For example, it seems common to miss milestones and start ignoring them, which would be annoying to bugwarrior users who don't necessarily have control over the milestones.

Any thoughts @fmauch?

[fmauch]

My intuitive impression would be that an issue's due date should have preference. An assigned milestone's due date should only be the fallback in my opinion.

[ryneeverett]

The discussion in #385 would suggest that automatically updating the taskwarrior duedate based on data returned from the service is wrong.

[fmauch]

Yes, I have to agree that this comment makes a lot of sense to me.

[fmauch]

@ryneeverett should I start working on this? I could address this while looking at my old #818...

[ryneeverett]

@fmauch Go for it, though I'm not sure what direction we should take. Whether it's worth breaking backwards compatibility to remove automatic duedate assignment or if we should just accept the conservative simplification of 1 is an open question. What do you think?