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

feat: Add ability to delete multiple documents, using multiple ids #196

Merged
merged 4 commits into from
Feb 15, 2022

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Feb 8, 2022

Resolves #164
Resolves #165

Adds the ability to delete multiple documents, using multiple ids.

We can now delete multiple documents using multiple dockeys (i.e. ids).

For example:

mutation {
    delete_user(ids: ["bae-3a1a496e-24eb-5ae3-9c17-524c146a393e" , "bae-6a6482a8-24e1-5c73-a237-ca569e41507d"]) {
        KeyOfDeletedDocument: _key
    }
}
    

Gives this on successful deletion of all documents:

{
  "data": [
    {
      "KeyOfDeletedDocument": "bae-3a1a496e-24eb-5ae3-9c17-524c146a393e"
    },
    {
      "KeyOfDeletedDocument": "bae-6a6482a8-24e1-5c73-a237-ca569e41507d"
    }
  ]
}

@shahzadlone shahzadlone self-assigned this Feb 8, 2022
@shahzadlone shahzadlone added the feature New feature or request label Feb 8, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.2.1 milestone Feb 8, 2022
@codecov
Copy link

codecov bot commented Feb 8, 2022

Codecov Report

Merging #196 (20a0052) into develop (cdaad72) will increase coverage by 0.94%.
The diff coverage is 78.04%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #196      +/-   ##
===========================================
+ Coverage    56.96%   57.90%   +0.94%     
===========================================
  Files           99       99              
  Lines         9664     9571      -93     
===========================================
+ Hits          5505     5542      +37     
+ Misses        3547     3413     -134     
- Partials       612      616       +4     
Impacted Files Coverage Δ
db/collection_update.go 41.87% <ø> (+0.47%) ⬆️
db/collection_delete.go 56.12% <76.92%> (+28.05%) ⬆️
query/graphql/schema/generate.go 81.37% <100.00%> (+0.02%) ⬆️
query/graphql/planner/delete.go 73.61% <0.00%> (+8.33%) ⬆️

@jsimnz
Copy link
Member

jsimnz commented Feb 11, 2022

Lets get tests on this, can just continue the same structure of tests the first PR for the delete added.

Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

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

Just tests and we're good! solid stuff

db/collection_delete.go Outdated Show resolved Hide resolved
@shahzadlone shahzadlone force-pushed the lone/feat/delete-documents-with-multi-keys branch from 9158ad2 to dbb9cf8 Compare February 11, 2022 11:34
@shahzadlone
Copy link
Member Author

Added tests and adhered to the code review suggestion. @jsimnz

Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

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

Beautiful work, small request on the file structure for the tests.

Im approving now tho. Just make the change and merge whenever.

👍

db/tests/mutation/simple/delete/simple_test.go Outdated Show resolved Hide resolved
@shahzadlone shahzadlone force-pushed the lone/feat/delete-documents-with-multi-keys branch from dbb9cf8 to 480a000 Compare February 15, 2022 06:57
@shahzadlone shahzadlone merged commit 8ffd2b5 into develop Feb 15, 2022
@shahzadlone shahzadlone deleted the lone/feat/delete-documents-with-multi-keys branch February 15, 2022 07:18
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…ourcenetwork#196)

### ISSUE:

Resolves sourcenetwork#164
Resolves sourcenetwork#165

### DESCRIPTION:

Adds the ability to delete multiple documents, using multiple ids.

We can now delete multiple documents using multiple dockeys (i.e. ids).

For example:
```
mutation {
    delete_user(ids: ["bae-3a1a496e-24eb-5ae3-9c17-524c146a393e" , "bae-6a6482a8-24e1-5c73-a237-ca569e41507d"]) {
        KeyOfDeletedDocument: _key
    }
}
    
```

Gives this on successful deletion of all documents:
```
{
  "data": [
    {
      "KeyOfDeletedDocument": "bae-3a1a496e-24eb-5ae3-9c17-524c146a393e"
    },
    {
      "KeyOfDeletedDocument": "bae-6a6482a8-24e1-5c73-a237-ca569e41507d"
    }
  ]
}
```


### COMMITS:

* feat: Add implementation for deleting multiple documents.

* test: Add tests for the multiple document deletion using multiple keys.

* refactor: Split tests into seperate files for the deletion mutuation
command.

* fix: suppress linter error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests for Multiple Delete Documents Operation Delete Multiple Documents Operation
2 participants