Skip to content

fix: prevent duplicate activities in array and self-referential relat… - #44

Open
pisum-sativum wants to merge 3 commits into
kathan-majithia:mainfrom
pisum-sativum:fix-duplicate-activities
Open

fix: prevent duplicate activities in array and self-referential relat…#44
pisum-sativum wants to merge 3 commits into
kathan-majithia:mainfrom
pisum-sativum:fix-duplicate-activities

Conversation

@pisum-sativum

Copy link
Copy Markdown

Description:

Fixes #16

This PR addresses the bug where the library was silently accepting duplicate activity names and self-referential dependencies, which could eventually lead to a RecursionError when calling find_probable_paths().

Changes Made:

  1. Added Duplicate Array Validation: Modified add_activities_relations to check for duplicate items within the activities array parameter itself during the initial validation loop. It now properly raises a ValueError immediately rather than attempting to insert invalid structures.
  2. Prevented Self-Referential Dependencies: Added explicit checks inside add_relation and add_activities_relations to ensure a node cannot be set as a predecessor to itself (e.g., A -> A). If such an attempt is made, it will immediately raise a ValueError preventing any self-referential cycles before traversal begins.

Testing:

  • Validated that supplying activities = ['A', 'A'] safely raises a ValueError: Activity 'A' already exists. Duplicate names are not allowed.
  • Validated that defining self-referential dependencies directly via cpm.add_relation('A', 'A') correctly raises a ValueError: Self-referential dependency detected: 'A' cannot be a predecessor of itself.

@pisum-sativum

Copy link
Copy Markdown
Author

@kathan-majithia I have done the necessary changes kindly check and merge and kindly provide the tags gssoc and gssoc:approved.

@kathan-majithia

Copy link
Copy Markdown
Owner

Traceback (most recent call last):
File "D:\Projects\networkdiagram\networkdiagram\networkdiagram\networkdiagram.py", line 837, in
cpm.add_activities_relations(
File "D:\Projects\networkdiagram\networkdiagram\networkdiagram\networkdiagram.py", line 237, in add_activities_relations
self.add_activity(activities[i], durations_with_origin[i + 1],
File "D:\Projects\networkdiagram\networkdiagram\networkdiagram\networkdiagram.py", line 167, in add_activity
self._validate_duplicate(name)
File "D:\Projects\networkdiagram\networkdiagram\networkdiagram\networkdiagram.py", line 128, in _validate_duplicate
raise ValueError(f"Activity '{name}' already exists. Duplicate names are not allowed.")

Entire trackback should not be displayed. Also provide some test cases to verify your solution.

…ions (kathan-majithia#44)

- Prevent full traceback on validation errors by wrapping main block in try-except
- Add unit tests for duplicate activities and self-referential relations checks
@pisum-sativum

Copy link
Copy Markdown
Author

Traceback (most recent call last): File "D:\Projects\networkdiagram\networkdiagram\networkdiagram\networkdiagram.py", line 837, in cpm.add_activities_relations( File "D:\Projects\networkdiagram\networkdiagram\networkdiagram\networkdiagram.py", line 237, in add_activities_relations self.add_activity(activities[i], durations_with_origin[i + 1], File "D:\Projects\networkdiagram\networkdiagram\networkdiagram\networkdiagram.py", line 167, in add_activity self._validate_duplicate(name) File "D:\Projects\networkdiagram\networkdiagram\networkdiagram\networkdiagram.py", line 128, in _validate_duplicate raise ValueError(f"Activity '{name}' already exists. Duplicate names are not allowed.")

Entire trackback should not be displayed. Also provide some test cases to verify your solution.

@kathan-majithia I have done the necessary changes kindly check and merge with the tags gssoc and gssoc: approved pls.

@kathan-majithia

Copy link
Copy Markdown
Owner

I checked, but still got same result only.

@pisum-sativum
pisum-sativum force-pushed the fix-duplicate-activities branch from 86cb5f2 to eda7fc5 Compare June 26, 2026 22:40
@pisum-sativum

Copy link
Copy Markdown
Author

I checked, but still got same result only.

@kathan-majithia I have done the required changes pls check and merge with gssoc and gssoc:approved tags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Duplicate activity names create self-referential dependencies causing RecursionError

2 participants