Fix/nocond

[yissellokta]

changing NoCond = "None" to be empty, removing the overhead in openfga to rewrite from "" to None and viceversa

Description

What problem is being solved?

How is it being solved?

What changes are made to solve it?

References

Review Checklist

• I have clicked on "allow edits by maintainers".
• I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
• The correct base branch is being used, if not main
• I have added tests to validate that the change in functionality is working as expected

Summary by CodeRabbit

• New Features

  • Added public method for retrieving edges associated with a specific node ID, providing direct access to graph connections.

• Changes

  • Updated handling of empty conditions to use empty string representation instead of a default value.

• Tests

  • Added comprehensive test suite covering edge retrieval scenarios including edge existence, ordering, and edge type variations.

[coderabbitai]

Walkthrough

This PR modifies edge condition handling in the graph package by changing the NoCond constant value from "none" to an empty string, removing empty condition defaulting in edge creation logic, adding a new public method to retrieve edges by node ID, and updating test assertions accordingly.

Changes

Cohort / File(s)	Summary
Edge condition constant pkg/go/graph/weighted_graph_edge.go	Updated NoCond constant value from "none" to ""
Edge creation and retrieval logic pkg/go/graph/graph_builder.go, pkg/go/graph/weighted_graph.go	Removed defaulting logic that substituted empty condition strings with NoCond in upsertEdge; added public method GetEdgesFromNodeId to retrieve edges by node ID; simplified edge map access in calculateUsersetWeights
Test updates pkg/go/graph/weighted_graph_builder_test.go, pkg/go/graph/weighted_graph_test.go	Updated test assertions to use NoCond constant instead of literal "none"; added comprehensive TestGetEdgesFromNodeId suite with 7 subtests covering existence, non-existence, multiple edges, conditions, operator nodes, and order preservation

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Pay attention to the removal of empty condition defaulting behavior—verify this change doesn't create inconsistencies in edge creation across the codebase
• Review the new GetEdgesFromNodeId method implementation for correctness and consistency with existing edge retrieval patterns
• Verify test coverage adequately validates the NoCond semantic change and new retrieval method behavior

Possibly related PRs

• Feat/grouping userset ttus #493: Directly modifies edge-condition handling in the graph package, changing NoCond constant semantics and edge defaulting logic in weighted_graph files
• Feat/relation definition in edges #494: Modifies edge construction logic in pkg/go/graph, changing UpsertEdge/AddEdge behavior and edge metadata in weighted_graph files

Suggested reviewers

• adriantam
• justincoh

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check	❓ Inconclusive	The title "Fix/nocond" references NoCond, which is indeed central to the main changes in this PR (changing the NoCond constant from "none" to an empty string). However, the title reads more like a git branch name than a descriptive PR title and is too vague to clearly communicate what the PR accomplishes. A teammate scanning git history would not immediately understand that this PR changes the NoCond constant value, removes rewriting overhead, or adds a new GetEdgesFromNodeId method. The title uses non-descriptive phrasing that fails to convey meaningful information about the specific changes being made.	Consider updating the title to be more descriptive and specific, such as "Change NoCond constant from 'none' to empty string" or "Update NoCond constant and add GetEdgesFromNodeId method" to clearly communicate the primary changes and their purpose to other team members.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/nocond

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c774550 and 4eb61e4.

📒 Files selected for processing (5)

• pkg/go/graph/graph_builder.go (0 hunks)
• pkg/go/graph/weighted_graph.go (2 hunks)
• pkg/go/graph/weighted_graph_builder_test.go (7 hunks)
• pkg/go/graph/weighted_graph_edge.go (1 hunks)
• pkg/go/graph/weighted_graph_test.go (1 hunks)

💤 Files with no reviewable changes (1)

• pkg/go/graph/graph_builder.go

🧰 Additional context used

🧬 Code graph analysis (3)

pkg/go/graph/weighted_graph_builder_test.go (1)

pkg/go/graph/weighted_graph_edge.go (1)

• NoCond (55-55)

pkg/go/graph/weighted_graph.go (1)

pkg/go/graph/weighted_graph_edge.go (1)

• WeightedAuthorizationModelEdge (58-76)

pkg/go/graph/weighted_graph_test.go (3)

pkg/go/graph/weighted_graph.go (1)

• NewWeightedAuthorizationModelGraph (49-54)

pkg/go/graph/weighted_graph_node.go (4)

• SpecificTypeAndRelation (7-7)
• SpecificType (6-6)
• IntersectionOperator (14-14)
• OperatorNode (8-8)

pkg/go/graph/weighted_graph_edge.go (5)

• DirectEdge (11-11)
• ComputedEdge (26-26)
• RewriteEdge (16-16)
• TTUEdge (21-21)
• EdgeType (3-3)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (go)

🔇 Additional comments (4)

pkg/go/graph/weighted_graph_builder_test.go (1)

654-725: LGTM! Consistent use of NoCond constant.

The test assertions have been properly updated to use the NoCond constant instead of the hardcoded "none" string. This improves maintainability and aligns with the constant value change.

pkg/go/graph/weighted_graph.go (2)

33-36: LGTM! Useful addition for direct node ID access.

The new GetEdgesFromNodeId method provides a convenient way to retrieve edges by node ID without requiring a node reference. The implementation is consistent with the existing GetEdgesFromNode method.

---

830-830: LGTM! Safe simplification.

Removing the ok-check is safe here since the subsequent len(edges) check on line 831 handles both nil and empty slices correctly in Go.

pkg/go/graph/weighted_graph_test.go (1)

1217-1353: LGTM! Comprehensive test coverage.

The new test suite for GetEdgesFromNodeId is well-structured and thorough. It covers all important scenarios including edge cases (non-existent nodes, nodes without edges), different edge types, conditions, operator nodes, and edge order preservation.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}