Skip to content

Conversation

morgandruesne
Copy link
Contributor

What does this PR do?

This PR add the delete function and test for the Collection Controller

@morgandruesne morgandruesne self-assigned this Jun 18, 2020
@morgandruesne morgandruesne changed the title collection.delete add the collection.delete function Jun 18, 2020
@morgandruesne morgandruesne marked this pull request as draft June 18, 2020 12:26
@codecov
Copy link

codecov bot commented Jun 18, 2020

Codecov Report

Merging #523 into 7-dev will decrease coverage by 4.43%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            7-dev     #523      +/-   ##
==========================================
- Coverage   95.59%   91.16%   -4.44%     
==========================================
  Files          32       32              
  Lines        1318     1392      +74     
  Branches        0      129     +129     
==========================================
+ Hits         1260     1269       +9     
- Misses         58       98      +40     
- Partials        0       25      +25     
Impacted Files Coverage Δ
src/controllers/Index.js 100.00% <ø> (ø)
src/controllers/Collection.js 100.00% <100.00%> (ø)
src/controllers/MemoryStorage.js 96.05% <0.00%> (-0.20%) ⬇️
src/controllers/Security.js 97.03% <0.00%> (-0.03%) ⬇️
src/core/Room.js 100.00% <0.00%> (ø)
src/core/security/Profile.js
src/Kuzzle.js
src/controllers/Document.js
src/core/security/User.js
src/controllers/Auth.js
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1df77ef...2968d60. Read the comment docs.

@morgandruesne morgandruesne marked this pull request as ready for review June 19, 2020 07:06
Copy link
Contributor

@Aschen Aschen left a comment

Choose a reason for hiding this comment

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

Some nitpicking

<br/>

```js
delete(index, collection, [options]);
Copy link
Contributor

Choose a reason for hiding this comment

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

What kind of options can you pass? Also you should add this to the arguments table

code: true
type: page
title: delete
description: Delete a collection
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: Delete a collection
description: Deletes a collection


## Resolves

Resolves if the collection is successfully delete.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Resolves if the collection is successfully delete.
Resolves if the collection is successfully deleted.

template: default
expected: Success

sdk: js
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this line and the one below, we don't need them anymore

action: 'delete'
};
return this.query(request)
.then(response => response.result);
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO this should return undefined

@morgandruesne morgandruesne requested a review from Aschen July 1, 2020 07:19
action: 'delete'
};
return this.query(request)
.then(undefined);
Copy link
Contributor

@Aschen Aschen Jul 1, 2020

Choose a reason for hiding this comment

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

Then then method take a callback as a parameter, passing undefined will result to a TypeError.
The promise resolved value will be this callback return value

Suggested change
.then(undefined);
.then(() => undefined);

@scottinet scottinet changed the title add the collection.delete function Add support for the collection:delete API action Jul 22, 2020
@Aschen Aschen merged commit 005269a into 7-dev Jul 31, 2020
@Aschen Aschen deleted the collection-delete branch July 31, 2020 09:07
This was referenced Aug 18, 2020
Aschen added a commit that referenced this pull request Aug 18, 2020
# [7.4.0](https://github.com/kuzzleio/sdk-javascript/releases/tag/7.4.0) (2020-08-18)


#### New features

- [ [#542](#542) ] Finalize typescript support   ([Aschen](https://github.com/Aschen))
- [ [#529](#529) ] Add typescript support for protocols   ([Aschen](https://github.com/Aschen))

#### Enhancements

- [ [#541](#541) ] Add meaningful stacktrace   ([Aschen](https://github.com/Aschen))
- [ [#537](#537) ] Add typescript support realtime   ([Aschen](https://github.com/Aschen))
- [ [#531](#531) ] Add typescript support for Index and Collection controllers   ([Aschen](https://github.com/Aschen))
- [ [#530](#530) ] Add typescript support for search results   ([Aschen](https://github.com/Aschen))
- [ [#523](#523) ] Add support for the collection:delete API action   ([morgandruesne](https://github.com/morgandruesne))
---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants