Container tasks are not allowed an effort or duration attribute. Init a custom logger instance

Author: JasperCraeghs

src/mlx/jira_juggler/jira_juggler.py

@@ -27,6 +27,9 @@
 
 TAB = ' ' * 4
 
+# Module logger
+LOGGER = logging.getLogger('jira-juggler')
+
 
 def fetch_credentials():
     """ Fetches the credentials from the .env file by default or, alternatively, from the user's input
@@ -42,7 +45,7 @@ def fetch_credentials():
     if not api_token:
         password = config('JIRA_PASSWORD', default='')
         if password:
-            logging.warning('Basic authentication with a JIRA password may be deprecated. '
+            LOGGER.warning('Basic authentication with a JIRA password may be deprecated. '
                             'Consider defining an API token as environment variable JIRA_API_TOKEN instead.')
             return username, password
         else:
@@ -59,7 +62,13 @@ def set_logging_level(loglevel):
     numeric_level = getattr(logging, loglevel.upper(), None)
     if not isinstance(numeric_level, int):
         raise ValueError('Invalid log level: %s' % loglevel)
-    logging.basicConfig(level=numeric_level)
+    # Configure the named logger
+    LOGGER.setLevel(numeric_level)
+    if not LOGGER.handlers:
+        handler = logging.StreamHandler()
+        formatter = logging.Formatter('%(levelname)s:%(name)s:%(message)s')
+        handler.setFormatter(formatter)
+        LOGGER.addHandler(handler)
 
 
 def to_identifier(key):
@@ -152,8 +161,8 @@ def determine_username(user):
     elif getattr(user, 'displayName', ''):
         full_name = user.displayName
         username = f'"{full_name}"'
-        logging.error(f"Failed to fetch email address of {full_name!r}: they restricted its visibility; "
-                      f"using identifier {username!r} as fallback value.")
+        LOGGER.error(f"Failed to fetch email address of {full_name!r}: they restricted its visibility; "
+                     f"using identifier {username!r} as fallback value.")
     else:
         raise Exception(f"Failed to determine username of {user}")
     return username
@@ -168,8 +177,8 @@ def determine_default_links(link_types_per_name):
                 default_links.append(link)
                 break
         else:
-            logging.warning("Failed to find any of these default jira-juggler issue link types in your Jira project "
-                            f"configuration: {list(link_types)}. Use --links if you think this is a problem.")
+            LOGGER.warning("Failed to find any of these default jira-juggler issue link types in your Jira project "
+                           f"configuration: {list(link_types)}. Use --links if you think this is a problem.")
     return default_links
 
 
@@ -183,7 +192,7 @@ def determine_links(jira_link_types, input_links):
         all_jira_links = chain.from_iterable((link_type.inward, link_type.outward) for link_type in jira_link_types)
         missing_links = unique_input_links.difference(all_jira_links)
         if missing_links:
-            logging.warning(f"Failed to find links {missing_links} in your configuration in Jira")
+            LOGGER.warning(f"Failed to find links {missing_links} in your configuration in Jira")
         valid_links = unique_input_links - missing_links
     return valid_links
 
@@ -327,7 +336,7 @@ def load_from_jira_issue(self, jira_issue):
                 self.value = 0
         else:
             self.value = self.DEFAULT_VALUE
-            logging.warning('No estimate found for %s, assuming %s%s', jira_issue.key, self.DEFAULT_VALUE, self.UNIT)
+            LOGGER.warning('No estimate found for %s, assuming %s%s', jira_issue.key, self.DEFAULT_VALUE, self.UNIT)
 
     def validate(self, task, tasks):
         """Validates (and corrects) the current task property
@@ -337,11 +346,16 @@ def validate(self, task, tasks):
             tasks (list): Modifiable list of JugglerTask instances to which the current task belongs. Will be used to
                 verify relations to other tasks.
         """
+        # If effort is None, this is a container task; assert and skip validation.
+        if self.value is None:
+            assert getattr(task, 'children', None), (
+                f"Effort is None only allowed for container tasks; {task.key} has no children")
+            return
         if self.value == 0:
-            logging.warning('Estimate for %s, is 0. Excluding', task.key)
+            LOGGER.warning('Estimate for %s, is 0. Excluding', task.key)
             tasks.remove(task)
         elif self.value < self.MINIMAL_VALUE:
-            logging.warning('Estimate %s%s too low for %s, assuming %s%s', self.value, self.UNIT, task.key, self.MINIMAL_VALUE, self.UNIT)
+            LOGGER.warning('Estimate %s%s too low for %s, assuming %s%s', self.value, self.UNIT, task.key, self.MINIMAL_VALUE, self.UNIT)
             self.value = self.MINIMAL_VALUE
 
 
@@ -387,7 +401,7 @@ def validate(self, task, tasks):
         task_ids = [to_identifier(tsk.key) for tsk in tasks]
         for val in list(self.value):
             if val not in task_ids:
-                logging.warning('Removing link to %s for %s, as not within scope', val, task.key)
+                LOGGER.warning('Removing link to %s for %s, as not within scope', val, task.key)
                 self.value.remove(val)
 
     def __str__(self):
@@ -457,7 +471,7 @@ class JugglerTask:
 
     def __init__(self, jira_issue=None):
         if jira_issue:
-            logging.info('Create JugglerTask for %s', jira_issue.key)
+            LOGGER.info('Create JugglerTask for %s', jira_issue.key)
 
         self.key = self.DEFAULT_KEY
         self.summary = self.DEFAULT_SUMMARY
@@ -505,7 +519,7 @@ def validate(self, tasks, property_identifier):
             property_identifier (str): Identifier of property type
         """
         if self.key == self.DEFAULT_KEY:
-            logging.error('Found a task which is not initialized')
+            LOGGER.error('Found a task which is not initialized')
         self.properties[property_identifier].validate(self, tasks)
 
     def __str__(self):
@@ -619,19 +633,11 @@ def calculate_rolled_up_effort(self):
         Returns:
             float: Total effort including children
         """
-        base_effort = self.properties['effort'].value if not self.properties['effort'].is_empty else 0
-
-        # For epics and parent tasks, include child effort
+        # For container tasks (any task with children), the effort is the sum of the children only.
+        # Parent/container tasks are not allowed to have their own effort.
         if self.children:
-            child_effort = sum(child.calculate_rolled_up_effort() for child in self.children)
-            # If this is a container task (epic or parent with children),
-            # the effort should be the sum of children unless it has its own effort
-            if self.is_epic or (self.children and base_effort == JugglerTaskEffort.DEFAULT_VALUE):
-                return child_effort
-            else:
-                return base_effort + child_effort
-
-        return base_effort
+            return sum(child.calculate_rolled_up_effort() for child in self.children)
+        return self.properties['effort'].value if not self.properties['effort'].is_empty else 0
 
 
 class JiraJuggler:
@@ -649,11 +655,11 @@ def __init__(self, endpoint, user, token, query, links=None):
         """
         global id_to_username_mapping
         id_to_username_mapping = {}
-        logging.info('Jira endpoint: %s', endpoint)
+        LOGGER.info('Jira endpoint: %s', endpoint)
 
         global jirahandle
         jirahandle = JIRA(endpoint, basic_auth=(user, token))
-        logging.info('Query: %s', query)
+        LOGGER.info('Query: %s', query)
         self.query = query
         self.issue_count = 0
 
@@ -683,48 +689,72 @@ def load_issues_from_jira(self, depend_on_preceding=False, sprint_field_name='',
             list: A list of JugglerTask instances
         """
         tasks = []
-        busy = True
-        while busy:
+        next_page_token = None
+
+        while True:
             try:
-                issues = jirahandle.search_issues(self.query, maxResults=JIRA_PAGE_SIZE, startAt=self.issue_count,
-                                                  expand='changelog')
+                # Use enhanced_search_issues for API v3 compatibility
+                result = jirahandle.enhanced_search_issues(
+                    jql_str=self.query,
+                    maxResults=JIRA_PAGE_SIZE,
+                    nextPageToken=next_page_token,
+                    expand='changelog'
+                )
+
+                # enhanced_search_issues returns a ResultList, extract issues
+                if hasattr(result, 'iterable'):
+                    issues = list(result.iterable)
+                else:
+                    issues = list(result)
+
             except JIRAError as err:
-                logging.error(f'Failed to query JIRA: {err}')
+                LOGGER.error(f'Failed to query JIRA: {err}')
                 if err.status_code == 401:
-                    logging.error('Please check your JIRA credentials in the .env file or environment variables.')
+                    LOGGER.error('Please check your JIRA credentials in the .env file or environment variables.')
                 elif err.status_code == 403:
-                    logging.error('You do not have permission to access this JIRA project or query.')
+                    LOGGER.error('You do not have permission to access this JIRA project or query.')
                 elif err.status_code == 404:
-                    logging.error('The JIRA endpoint is not found. Please check the endpoint URL.')
-                elif err.status_code == 400:
+                    LOGGER.error('The JIRA endpoint is not found. Please check the endpoint URL.')
+                elif err.status_code == 400 or err.status_code == 410:
                     # Parse and display the specific JQL errors more clearly
                     try:
                         error_data = err.response.json()
                         if 'errorMessages' in error_data:
                             for error_msg in error_data['errorMessages']:
-                                logging.error(f'JIRA query error: {error_msg}')
+                                LOGGER.error(f'JIRA query error: {error_msg}')
                     except Exception:
                         pass  # Fall back to generic error if JSON parsing fails
 
-                    logging.error('Invalid JQL query syntax. Please check your query.')
+                    if err.status_code == 410:
+                        LOGGER.error('JIRA API v2 has been deprecated. Using enhanced search API.')
+                    else:
+                        LOGGER.error('Invalid JQL query syntax. Please check your query.')
                 else:
-                    logging.error(f'An unexpected error occurred: {err}')
+                    LOGGER.error(f'An unexpected error occurred: {err}')
                 return None
 
             if len(issues) <= 0:
-                busy = False
+                break
 
             self.issue_count += len(issues)
             for issue in issues:
-                logging.debug(f'Retrieved {issue.key}: {issue.fields.summary}')
+                LOGGER.debug(f'Retrieved {issue.key}: {issue.fields.summary}')
                 tasks.append(JugglerTask(issue))
 
-        self.validate_tasks(tasks)
+            # Check if there are more pages
+            if hasattr(result, 'nextPageToken') and result.nextPageToken:
+                next_page_token = result.nextPageToken
+            else:
+                break
 
-        # Build hierarchical relationships if enabled
+        # Build hierarchical relationships if enabled BEFORE validation so epic rules
+        # can consider zero-effort children.
         if enable_epics:
             tasks = self.build_hierarchical_tasks(tasks)
 
+        # Now validate tasks (may exclude remaining zero-effort tasks where appropriate)
+        self.validate_tasks(tasks)
+
         if sprint_field_name:
             self.sort_tasks_on_sprint(tasks, sprint_field_name)
         tasks.sort(key=cmp_to_key(self.compare_status))
@@ -750,20 +780,21 @@ def build_hierarchical_tasks(self, tasks):
             if task.parent_key and task.parent_key in task_by_key:
                 parent_task = task_by_key[task.parent_key]
                 parent_task.add_child(task)
-                logging.debug(f'Added {task.key} as child of parent {parent_task.key}')
+                LOGGER.debug(f'Added {task.key} as child of parent {parent_task.key}')
 
             # Handle story/task -> epic relationship
             elif task.epic_key and task.epic_key in task_by_key:
                 epic_task = task_by_key[task.epic_key]
                 epic_task.add_child(task)
-                logging.debug(f'Added {task.key} as child of epic {epic_task.key}')
+                LOGGER.debug(f'Added {task.key} as child of epic {epic_task.key}')
 
-        # Update effort calculations for parent tasks
+        # Process epics with special logic
+        tasks = self._process_epic_logic(tasks, task_by_key)
+
+        # Container tasks are not allowed an effort attribute; drop it for any task with children
         for task in tasks:
-            if task.children:
-                rolled_up_effort = task.calculate_rolled_up_effort()
-                task.properties['effort'].value = rolled_up_effort
-                logging.debug(f'Updated effort for {task.key} to {rolled_up_effort}d (including children)')
+            if task.children and 'effort' in task.properties:
+                task.properties['effort'].value = None
 
         # Return only top-level tasks (those without parents)
         top_level_tasks = []
@@ -778,9 +809,76 @@ def build_hierarchical_tasks(self, tasks):
             if not is_child:
                 top_level_tasks.append(task)
 
-        logging.info(f'Built hierarchy: {len(tasks)} total tasks, {len(top_level_tasks)} top-level tasks')
+        LOGGER.info(f'Built hierarchy: {len(tasks)} total tasks, {len(top_level_tasks)} top-level tasks')
         return top_level_tasks
 
+    def _process_epic_logic(self, tasks, task_by_key):
+        """Process epic-specific logic for effort estimation and child handling.
+
+        Implements the following rules:
+        1. If an epic has one or more children with effort at 0, discard the children and treat the epic as a single task
+        2. If that epic has no effort estimate set, exclude it and warn about it like we do for regular tasks
+        3. When both epic and children have estimates and the sum doesn't match the epic estimate, log a warning with the difference
+
+        Args:
+            tasks (list): List of JugglerTask instances
+            task_by_key (dict): Dictionary mapping task keys to task instances
+
+        Returns:
+            list: Modified list of tasks after applying epic logic
+        """
+        tasks_to_remove = set()
+
+        for task in tasks:
+            if not task.is_epic or not task.children:
+                continue
+
+            # Check if any children have zero (or effectively zero) effort
+            # Consider effectively-zero when original Jira estimates are None/0 and computed effort <= MINIMAL_VALUE
+            def _is_effectively_zero(child_task):
+                try:
+                    fields = child_task.issue.fields if child_task.issue else None
+                    orig = getattr(fields, 'timeoriginalestimate', None) if fields else None
+                    rem = getattr(fields, 'timeestimate', None) if fields else None
+                    eff = child_task.properties['effort'].value
+                    return (orig in (None, 0)) and (rem in (None, 0)) and (eff is not None and eff <= JugglerTaskEffort.MINIMAL_VALUE)
+                except Exception:
+                    return False
+
+            children_with_zero_effort = [child for child in task.children if _is_effectively_zero(child)]
+
+            if children_with_zero_effort:
+                # Rule 1: Epic has children with 0 effort - discard all children and treat epic as single task
+                LOGGER.info(f'Epic {task.key} has {len(children_with_zero_effort)} children with 0 effort. '
+                            f'Discarding all children and treating epic as single task.')
+
+                # Remove children from the epic
+                for child in task.children:
+                    tasks_to_remove.add(child)
+                    LOGGER.debug(f'Removing child {child.key} from epic {task.key}')
+
+                task.children = []
+
+                # Rule 2: If epic has no effort estimate, exclude it and warn
+                if task.properties['effort'].value == 0:
+                    LOGGER.warning(f'Estrimate for epic {task.key}, is 0. Excluding')
+                    tasks_to_remove.add(task)
+                    continue
+
+            else:
+                # Rule 3: Check if epic effort matches sum of children efforts
+                epic_effort = task.properties['effort'].value
+                children_total_effort = sum(child.calculate_rolled_up_effort() for child in task.children)
+
+                if (epic_effort is not None and epic_effort > 0) and children_total_effort > 0 and abs(epic_effort - children_total_effort) > 0.01:
+                    # Epic and children both have estimates, but they don't match
+                    difference = epic_effort - children_total_effort
+                    LOGGER.warning(f'Epic {task.key} effort estimate ({epic_effort}d) differs from sum of children '
+                                   f'({children_total_effort}d) by {difference:+.2f}d')
+
+        # Remove tasks that should be excluded
+        return [task for task in tasks if task not in tasks_to_remove]
+
     def juggle(self, output=None, **kwargs):
         """Queries JIRA and generates task-juggler output from given issues
 
@@ -893,7 +991,7 @@ def sort_tasks_on_sprint(self, tasks, sprint_field_name):
                                 task.sprint_priority = prio
                                 if hasattr(sprint_info, 'startDate'):
                                     task.sprint_start_date = parser.parse(sprint_info.startDate)
-        logging.debug("Sorting tasks based on sprint information...")
+        LOGGER.debug("Sorting tasks based on sprint information...")
         tasks.sort(key=cmp_to_key(self.compare_sprint_priority))
 
     @staticmethod
@@ -914,7 +1012,7 @@ def extract_start_date(sprint_info, issue_key):
                 try:
                     return parser.parse(start_date_match.group(1))
                 except parser.ParserError as err:
-                    logging.debug("Failed to parse start date of sprint of issue %s: %s", issue_key, err)
+                    LOGGER.debug("Failed to parse start date of sprint of issue %s: %s", issue_key, err)
                     return None
 
     @staticmethod