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 m detach #33

Merged
merged 59 commits into from
Mar 18, 2021
Merged

Feat/add m detach #33

merged 59 commits into from
Mar 18, 2021

Conversation

rolljee
Copy link
Contributor

@rolljee rolljee commented Mar 4, 2021

⚠️ This PR is deeply link into #31 and the PR must be merged before this one.

Also this PR adds mDetachTenant methods

  • Code
  • Tests
  • Docs

@rolljee rolljee linked an issue Mar 4, 2021 that may be closed by this pull request
@rolljee rolljee marked this pull request as ready for review March 5, 2021 09:59
@rolljee rolljee marked this pull request as draft March 10, 2021 10:09
@rolljee
Copy link
Contributor Author

rolljee commented Mar 10, 2021

Next works is about making tests steps more generic to tests m* Queries

@rolljee rolljee marked this pull request as ready for review March 12, 2021 10:47
@rolljee rolljee requested a review from Aschen March 12, 2021 10:47
@Aschen Aschen changed the base branch from 1-dev to feat/add-m-attach-tenant March 15, 2021 09:37
@@ -0,0 +1,52 @@
const { When, Then } = require('cucumber');
Copy link
Contributor

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

@@ -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 };
Copy link
Contributor

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 @@
---
Copy link
Contributor

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
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
title: detach
title: mDetach

description: Detach multiple sensors from multiple tenants
---

# detach
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
# detach
# mDetach

``` js
{
// Using JSON
"records" [{
Copy link
Contributor

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

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) {
Copy link
Contributor

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

})

results.successes.concat(successes);
results.errors.concat(errors);
Copy link
Contributor

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?

Base automatically changed from feat/add-m-attach-tenant to 1-dev March 16, 2021 08:50
const results = {
errors: [],
successes: [],
};

console.log('BBBBB', JSON.stringify(documents));
Copy link
Contributor

Choose a reason for hiding this comment

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

DDDDDDebug leftover 😁

@@ -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));
Copy link
Contributor

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"
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, you're right

@rolljee rolljee requested a review from Aschen March 18, 2021 14:43
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.

LGTM 👍

@Aschen Aschen merged commit 3077036 into 1-dev Mar 18, 2021
@Aschen Aschen deleted the feat/add-m-detach branch March 18, 2021 19:07
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.

Add bulk API actions
2 participants