Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batch Mutations for creating, updating, and deleting #438 #653

Merged
merged 3 commits into from
Dec 8, 2024

Conversation

keithhackbarth
Copy link
Contributor

@keithhackbarth keithhackbarth commented Oct 29, 2024

Batch mutations for creating, updating, and deleting.

Description

Batched mutations are mutations that mutate multiple objects at once.
Mutations with a filter function or accept a list of objects that return a list.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Issues Fixed or Closed by This PR

#438

Checklist

  • [x ] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Key Considerations When Reviewing

Previously, batch creation in Strawberry Django was an undocumented and untested feature, relying on hidden logic to adjust argument typing. To support batch operations as an official feature (including updates and deletes), I updated the syntax approach, which introduces a breaking change.

The new method explicitly requires batch mutations to take a list type as an argument. This makes the behavior more straightforward, consistent, and explicit.

Summary by Sourcery

Implement batch mutations for creating, updating, and deleting multiple objects simultaneously, requiring list-type arguments for clarity and consistency. Update documentation and add tests to support this new feature.

New Features:

  • Introduce batch mutations for creating, updating, and deleting multiple objects at once, allowing operations to accept a list of objects and return a list.

Enhancements:

  • Refactor mutation logic to explicitly require batch mutations to take a list type as an argument, improving clarity and consistency.

Documentation:

  • Update documentation to include information on using batch mutations for create, update, and delete operations.

Tests:

  • Add tests for batch mutations to ensure correct functionality for create, update, and delete operations with lists.

Copy link
Contributor

sourcery-ai bot commented Oct 29, 2024

Reviewer's Guide by Sourcery

This PR implements batch mutations for creating, updating, and deleting multiple objects in a single operation. The implementation makes batch operations an official feature by explicitly requiring list types as arguments, replacing the previous undocumented approach that relied on hidden logic for batch creation.

Sequence diagram for batch mutation process

sequenceDiagram
    actor User
    participant System

    User->>System: Send batch mutation request
    System->>System: Check if data is a list
    alt Data is a list
        System->>System: Process each item in the list
    else Data is not a list
        System->>System: Process single item
    end
    System->>User: Return mutation results
Loading

Class diagram for batch mutation changes

classDiagram
    class Mutation {
        +create_fruit: Fruit
        +create_fruits: list[Fruit]
        +patch_fruits: list[Fruit]
        +update_fruits: list[Fruit]
    }
    class FruitInput
    class FruitPartialInput
    class Fruit
    class FruitFilter

    Mutation --> FruitInput
    Mutation --> FruitPartialInput
    Mutation --> Fruit
    Mutation --> FruitFilter

    note for Mutation "Batch mutations now require list types as arguments"
Loading

File-Level Changes

Change Details Files
Refactored mutation resolver to handle batch operations
  • Added new resolver method to handle instance-level updates
  • Modified main resolver to process both single and batch operations
  • Added helper function get_vdata for data extraction
  • Removed implicit list handling for create mutations
strawberry_django/mutations/fields.py
Added documentation for batch mutations
  • Added new section explaining batch mutation syntax
  • Provided examples for batch create, update, and delete operations
docs/guide/mutations.md
Added comprehensive test coverage for batch operations
  • Added tests for batch create operations
  • Added tests for batch delete with filters
  • Added tests for batch update with filters
  • Added tests for batch patch operations
  • Added tests for empty list handling
tests/mutations/test_batch_mutations.py
tests/mutations/conftest.py
tests/mutations/test_filter_mutations.py

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @keithhackbarth - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟡 Testing: 2 issues found
  • 🟢 Complexity: all looks good
  • 🟡 Documentation: 2 issues found

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

strawberry_django/mutations/fields.py Show resolved Hide resolved
tests/mutations/test_batch_mutations.py Show resolved Hide resolved
tests/mutations/test_batch_mutations.py Show resolved Hide resolved
docs/guide/mutations.md Outdated Show resolved Hide resolved
docs/guide/mutations.md Outdated Show resolved Hide resolved
@keithhackbarth
Copy link
Contributor Author

@bellini666, @thclark, @patrick91 - What is the process for requesting a PR review here?

@bellini666
Copy link
Member

Hi @keithhackbarth ,

Sorry for taking long to review this. I'm in the middle of a relocation (just moved from Brazil to the Netherlands) and it is consuming all of my time.

I'll try to take some time during this week, or maximum next weekend to review this!

@keithhackbarth
Copy link
Contributor Author

Thanks, @bellini666 – and congrats on the move! Hope you're settling in well and enjoying the Netherlands.

Quick semi-related question: is there a specific reason we disable all DjangoOptimizerExtension for mutations? Just want to clarify the reasoning behind that approach. When I remove those lines in mutations/fields.py and rerun pytest - everything works and its more performant so trying to figure out the logic.

@keithhackbarth
Copy link
Contributor Author

Hi @bellini666,

I hope you're doing well! I wanted to kindly follow up on the review for this PR. We're starting to build features on top of this branch and want to make sure we're heading in the right direction as we already started releasing to production.

I completely understand that maintaining an open-source project is a volunteer effort and labor of love, and I truly appreciate all the work your team puts into it. That said, looking at commit log, I’ve noticed other branches are actively being reviewed and merged, and we’re struggling to get feedback on this one, even though we're financial contributors to the project.

If there’s anything I can do to make the review easier or provide more context, please let me know. Your guidance would be greatly appreciated!

Thank you so much, and I truly value your time and effort!

@bellini666
Copy link
Member

Hey @keithhackbarth ,

Sorry for taking long to be able to review... I was finally able to settle up in my new house, from now on I should be able to review stuff more fast :)

Thanks for the PR, it looks good! Going to approve and merge it!

In the future, if I take too long, feel free to ping me on Discord :)

@bellini666 bellini666 merged commit 21c99df into strawberry-graphql:main Dec 8, 2024
22 checks passed
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.

2 participants