From dd126e58672aba6d4eafc5708d458fc0034fce86 Mon Sep 17 00:00:00 2001
From: Mohamed El Mouctar Haidara <elmhaidara@gmail.com>
Date: Fri, 13 Dec 2024 22:26:03 +0100
Subject: [PATCH] fix: Tasks in 'include_role' were being wrongly included in
 the graph by default (#222)

---
 ansibleplaybookgrapher/parser.py | 16 ++++++------
 tests/test_parser.py             | 42 +++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/ansibleplaybookgrapher/parser.py b/ansibleplaybookgrapher/parser.py
index 69dccd7e..ad4b182c 100644
--- a/ansibleplaybookgrapher/parser.py
+++ b/ansibleplaybookgrapher/parser.py
@@ -407,12 +407,14 @@ def _include_tasks_in_blocks(
                             # If we have an include_role, and we want to include its tasks, the parent node now becomes
                             # the role.
                             parent_nodes.append(role_node)
-
-                        block_list, _ = task_or_block.get_block_list(
-                            play=current_play,
-                            loader=self.data_loader,
-                            variable_manager=self.variable_manager,
-                        )
+                            block_list, _ = task_or_block.get_block_list(
+                                play=current_play,
+                                loader=self.data_loader,
+                                variable_manager=self.variable_manager,
+                            )
+                        else:
+                            # Go to the next task if we don't want to include the tasks of the role
+                            continue
                 else:
                     display.v(
                         f"An 'include_tasks' found. Including tasks from '{task_or_block.get_name()}'",
@@ -501,7 +503,7 @@ def _include_tasks_in_blocks(
                 if has_role_parent(task_or_block) and not self.include_role_tasks:
                     # skip role's task
                     display.vv(
-                        f"The task '{task_or_block.get_name()}' has a role as parent and include_role_tasks is false. "
+                        f"The task '{task_or_block.get_name()}' has a role as parent and include_role_tasks is False. "
                         "It will be skipped.",
                     )
                     # skipping
diff --git a/tests/test_parser.py b/tests/test_parser.py
index a92b7b38..2619ebf3 100644
--- a/tests/test_parser.py
+++ b/tests/test_parser.py
@@ -179,6 +179,45 @@ def test_include_role_parsing(
     assert include_role_4.has_loop(), "The third include role has a loop"
 
 
+@pytest.mark.parametrize("grapher_cli", [["group-roles-by-name.yml"]], indirect=True)
+@pytest.mark.parametrize(
+    ("include_role_tasks", "nested_include_role_tasks_count"), [(True, 4), (False, 0)]
+)
+def test_include_role_parsing_with_different_include_role_tasks(
+    include_role_tasks: bool,
+    nested_include_role_tasks_count: int,
+    grapher_cli: PlaybookGrapherCLI,
+) -> None:
+    """Test parsing of include_role with different include_role_tasks options.
+
+    :param include_role_tasks:
+    :param grapher_cli:
+    :return:
+    """
+    parser = PlaybookParser(
+        grapher_cli.options.playbooks[0],
+        include_role_tasks=include_role_tasks,
+    )
+    playbook_node = parser.parse()
+    assert len(playbook_node.plays()) == 1
+
+    play_node = playbook_node.plays()[0]
+
+    assert (
+        len(play_node.roles) == 2
+    ), "Two roles should be at the play level (the ones in the 'roles:' section)"
+
+    # The first task of the play is an include role in the block
+    assert len(play_node.tasks) == 1
+    assert isinstance(play_node.tasks[0], BlockNode)
+    assert isinstance(play_node.tasks[0].tasks[0], RoleNode)
+
+    # The first post task is an include role as well but with nested include roles
+    assert len(play_node.post_tasks) == 1
+    assert isinstance(play_node.post_tasks[0], RoleNode)
+    assert len(play_node.post_tasks[0].tasks) == nested_include_role_tasks_count
+
+
 @pytest.mark.parametrize("grapher_cli", [["include_role.yml"]], indirect=True)
 def test_include_role_parsing_with_only_roles(
     grapher_cli: PlaybookGrapherCLI,
@@ -304,7 +343,8 @@ def test_roles_usage_multi_plays(
     nb_display_some_facts: int,
     nb_nested_include_role: int,
 ) -> None:
-    """Test the role_usages method for multiple plays referencing the same roles
+    """Test the role_usages method for multiple plays referencing the same roles.
+
     :param grapher_cli:
     :param roles_number: The number of uniq roles in the graph
     :param group_roles_by_name: flag to enable grouping roles or not