Skip to content

Conversation

@ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Oct 21, 2025

Mutations to create nodes no longer have required attributes or relationships, the reason is that we want to be able to use Templates that populate this information using the template itself. The Upsert mutations still have these requirements in place and as such it's not possible to create an object using an Upsert mutation that requires a template to fill in the missing information. This PR removes the requirements for attributes and relationships from the Upsert mutation.

Fixes #7398

Summary by CodeRabbit

  • Bug Fixes

    • Upsert mutations now permit required fields to be omitted when templates can provide values.
  • Tests

    • Added test coverage for template-based field population in mutations.

@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Oct 21, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Walkthrough

This pull request modifies the GraphQL upsert input generation to remove required field constraints, allowing required relationships and attributes to be omitted when they can be supplied by templates. The changes remove conditional required logic from both attribute and relationship field creation in the upsert input builder. Test coverage is added with a new test case validating upsert behavior with template-supplied relationships. A changelog entry documents this as a fix to enable upsert mutations when using templates that provide required fields.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Loosen requirements for Upsert mutations (for templates)" accurately and concisely summarizes the main change across the codebase. The modifications to the GraphQL manager remove required constraints on attributes and relationships in upsert input generation, directly enabling templates to supply mandatory fields. The title is specific, clear, and reflects the primary objective without unnecessary details or vague language.
Linked Issues Check ✅ Passed The code changes directly address all coding objectives from issue #7398. The modifications to backend/infrahub/graphql/manager.py remove per-field required logic and conditional required constraints for attributes and relationships in upsert input generation, enabling templates to supply missing required fields. The test additions in test_mutation_upsert.py validate that upsert mutations now work with templates providing required relationships, and the test infrastructure changes in tshirt.py enable template generation for testing. These changes align with the requirement to allow object creation via upsert when relying on templates to supply mandatory data.
Out of Scope Changes Check ✅ Passed All changes in the pull request are directly aligned with the stated objectives of issue #7398. The modifications to the GraphQL manager, test schemas, and test cases all focus on enabling upsert mutations to work with templates supplying required fields. The changelog entry documents the fix appropriately. No changes were detected that fall outside the scope of loosening upsert requirements to support template-provided mandatory fields.
✨ 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 pog-upsert-requirements-IFC-1911

📜 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 13a1fcb and 1206927.

📒 Files selected for processing (4)
  • backend/infrahub/graphql/manager.py (1 hunks)
  • backend/tests/helpers/schema/tshirt.py (1 hunks)
  • backend/tests/unit/graphql/test_mutation_upsert.py (2 hunks)
  • changelog/7398.fixed.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{md,mdx}

📄 CodeRabbit inference engine (CLAUDE.md)

Lint Markdown/MDX files with markdownlint using .markdownlint.yaml

Files:

  • changelog/7398.fixed.md
backend/tests/**/*

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Place backend tests in backend/tests/

Files:

  • backend/tests/helpers/schema/tshirt.py
  • backend/tests/unit/graphql/test_mutation_upsert.py
backend/**/*

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Run backend tests with pytest or via invoke tasks

Files:

  • backend/tests/helpers/schema/tshirt.py
  • backend/infrahub/graphql/manager.py
  • backend/tests/unit/graphql/test_mutation_upsert.py
**/*.py

📄 CodeRabbit inference engine (.github/instructions/python-docstring.instructions.md)

**/*.py: Use triple double quotes (""") for all Python docstrings
Write docstrings in Google-style format
Include a brief one-line description at the top of each docstring
Add a detailed description section when additional context is needed
Document function/method parameters under an Args/Parameters section without typing information
Include a Returns section describing the return value
Include a Raises section listing possible exceptions
Provide an Examples section demonstrating usage when helpful

Use ruff and mypy to validate and lint Python files

**/*.py: Use type hints for all Python function parameters and return values
Prefer asynchronous code in Python when feasible
Define asynchronous Python functions with async def
Use await for asynchronous calls in Python
Use Pydantic models instead of standard dataclasses
Use ruff and mypy for linting and type checking

**/*.py: Use type hints for all Python function parameters and return values
Use async/await whenever possible in Python code
Define asynchronous functions with async def
Await asynchronous calls with await
Use Pydantic models instead of standard dataclasses for data modeling
Use triple quotes (""") for all Python docstrings
Write docstrings in Google-style format
Include docstring sections when applicable: one-line summary, optional details, Args (without types), Returns, Raises, Examples
Validate and lint Python with ruff and mypy

Files:

  • backend/tests/helpers/schema/tshirt.py
  • backend/infrahub/graphql/manager.py
  • backend/tests/unit/graphql/test_mutation_upsert.py
🧬 Code graph analysis (2)
backend/infrahub/graphql/manager.py (2)
backend/infrahub/core/schema/relationship_schema.py (1)
  • internal_peer (53-54)
backend/infrahub/core/constants/__init__.py (1)
  • RelationshipCardinality (277-279)
backend/tests/unit/graphql/test_mutation_upsert.py (3)
backend/infrahub/core/schema/__init__.py (1)
  • SchemaRoot (47-99)
backend/infrahub/graphql/initialization.py (1)
  • prepare_graphql_params (80-126)
backend/tests/helpers/graphql.py (1)
  • graphql (26-61)
🔇 Additional comments (5)
backend/tests/helpers/schema/tshirt.py (1)

14-14: LGTM! Template generation enabled for TShirt schema.

This change correctly enables template generation for the TShirt node, allowing templates to supply required fields like the color relationship defined at lines 39-44.

backend/tests/unit/graphql/test_mutation_upsert.py (2)

15-15: LGTM! Import additions support the new test.

The addition of COLOR and TSHIRT imports is necessary for the new test function test_upsert_with_required_relationship_from_template at lines 678-754.


678-754: Excellent test coverage for template-driven upsert behavior.

This test comprehensively validates the PR objective:

  1. Verifies that upserting without a required relationship fails with an appropriate error message (lines 701-722)
  2. Confirms that providing a template allows the upsert to succeed with the template supplying the required color relationship (lines 724-754)
  3. Validates that the template-supplied value is correctly applied to the created node (lines 752-754)

The test structure is clear, well-documented, and provides good coverage for both success and failure paths.

changelog/7398.fixed.md (1)

1-1: LGTM! Changelog entry accurately describes the fix.

The changelog entry clearly explains that upsert mutation requirements have been loosened to allow templates to supply required fields, which aligns with the PR objectives and fixes issue #7398.

backend/infrahub/graphql/manager.py (1)

783-802: Based on my thorough analysis of the codebase, I have verified the runtime validation logic and template handling. The verification is complete.

Runtime validation for required fields is properly implemented and working correctly.

The validation logic in backend/infrahub/core/node/__init__.py at lines 454-479 checks mandatory attributes and relationships by verifying they exist in the fields dictionary. Critically, template values are merged into the fields dictionary via handle_object_template() (called at line 450) BEFORE validation occurs.

This means:

  • When a template is provided with a required field, that field is merged into the fields dict before validation runs, so validation passes
  • When neither input nor template provides a required field, the field is absent from the fields dict, and validation correctly raises the "is mandatory" error
  • The test at backend/tests/unit/graphql/test_mutation_upsert.py:722 confirms this behavior with the expected error: "color is mandatory for TestingTShirt at color"

The removal of GraphQL schema-level required flags correctly defers validation to runtime, and the runtime implementation properly handles all scenarios including missing required fields when templates don't supply them.


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

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

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 21, 2025

CodSpeed Performance Report

Merging #7455 will not alter performance

Comparing pog-upsert-requirements-IFC-1911 (1206927) with stable (13a1fcb)

Summary

✅ 10 untouched

@ogenstad ogenstad force-pushed the pog-upsert-requirements-IFC-1911 branch from e7dce27 to 1206927 Compare October 23, 2025 06:45
@ogenstad ogenstad marked this pull request as ready for review October 23, 2025 08:08
@ogenstad ogenstad requested a review from a team as a code owner October 23, 2025 08:08
@ogenstad ogenstad merged commit f533403 into stable Oct 23, 2025
63 of 64 checks passed
@ogenstad ogenstad deleted the pog-upsert-requirements-IFC-1911 branch October 23, 2025 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/backend Issue related to the backend (API Server, Git Agent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Upsert fails when using a template due to missing required relationship

3 participants