-
Notifications
You must be signed in to change notification settings - Fork 7
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 m detach #33
Conversation
Co-authored-by: Michele Leo <[email protected]>
Co-authored-by: Michele Leo <[email protected]>
Co-authored-by: Michele Leo <[email protected]>
Co-authored-by: Michele Leo <[email protected]>
Co-authored-by: Michele Leo <[email protected]>
Co-authored-by: Michele Leo <[email protected]>
Co-authored-by: Adrien Maret <[email protected]>
Co-authored-by: Adrien Maret <[email protected]>
Next works is about making tests steps more generic to tests m* Queries |
Co-authored-by: Adrien Maret <[email protected]>
Co-authored-by: Adrien Maret <[email protected]>
Co-authored-by: Adrien Maret <[email protected]>
…kuzzle documents limits
@@ -0,0 +1,52 @@ | |||
const { When, Then } = require('cucumber'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong indentation for this file. It should be 2 spaces
lib/controllers/SensorController.ts
Outdated
@@ -76,21 +76,21 @@ export class SensorController extends CRUDController { | |||
const tenantId = this.getIndex(request); | |||
const sensorId = this.getId(request); | |||
|
|||
const document = { tenant: tenantId, id: sensorId }; | |||
const document = { tenantId: tenantId, sensorId: sensorId }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer shorthand notation
@@ -0,0 +1,89 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be name m-detach
--- | ||
code: true | ||
type: page | ||
title: detach |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
title: detach | |
title: mDetach |
description: Detach multiple sensors from multiple tenants | ||
--- | ||
|
||
# detach |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# detach | |
# mDetach |
``` js | ||
{ | ||
// Using JSON | ||
"records" [{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mDetach
action should only takes an array of sensorId
lib/services/SensorService.ts
Outdated
async detach (sensor: Sensor) { | ||
if (! sensor._source.tenantId) { | ||
throw new BadRequestError(`Sensor "${sensor._id}" is not attached to a tenant`); | ||
async mDetach (sensors: Sensor[], bulkData: SensorBulkContent[], isStrict: boolean) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as for mAttach, the option should be passed inside an object
lib/services/SensorService.ts
Outdated
}) | ||
|
||
results.successes.concat(successes); | ||
results.errors.concat(errors); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are not returning anything?
lib/services/SensorService.ts
Outdated
const results = { | ||
errors: [], | ||
successes: [], | ||
}; | ||
|
||
console.log('BBBBB', JSON.stringify(documents)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DDDDDDebug leftover 😁
lib/services/SensorService.ts
Outdated
@@ -81,27 +81,37 @@ export class SensorService { | |||
return results; | |||
} | |||
|
|||
async mDetach (sensors: Sensor[], bulkData: SensorBulkContent[], isStrict: boolean) { | |||
async mDetach (sensors: Sensor[], bulkData: SensorBulkContent[], { strict }) { | |||
console.log('CCCC', JSON.stringify(sensors)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug leftover
// Using JSON | ||
"sensorIds" ["test-id"], | ||
// Using CSV syntax | ||
"csv": "sensorIds\ntest-id" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The column name should be sensorId
since it's a csv file.
Each line will contain 1 sensor ID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, you're right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Also this PR adds mDetachTenant methods