Feature not deleted / created

[atrovato]

Why is the "update feature" based on feature.external_id instead of feature.id ?

Gladys/server/lib/device/device.create.js

Line 104 in efc2ef5

const featureIndex = deviceToReturn.features.findIndex((f) => f.external_id === feature.external_id);

[atrovato]

According to this test, if the incoming feature have the same external_id, we update the incoming feature, but it may have a new id.

So, I see 2 solutions :

1. test on feature.id
2. keep test on feature.external_id but set old feature id to incoming one

[Pierre-Gilles]

The external_id is not meant to change ! It's a unique way to identify a device based on external identifier. (Exemple: Philips Hue Light ID)

We don't use the id, because the service sending the device don't have this id (it's internal).

What's your exact use case/problem ?

[atrovato]

When I create a device from MQTT service, I add a light on/off feature and save my device.

Then I remove this feature, by error, and create it again.

It will call the update, but the feature does not exist anymore with this id.

[Pierre-Gilles]

There might be somethings I don't understand, I don't see what's wrong with this case 🤔

If you create a device, remove a feature, then add the feature again, it should work! Based on the external_id of course...

[atrovato]

Try it yourself:

• go to MQTT device creator
• create one with 1 feature and Feature external ID = mqtt:invalid
• save it
• go back and edit this new created device
• delete feature
• create a new one with same Feature external ID = mqtt:invalid
• click on save

==> the device is refreshed but no feature is visible.

What happens ?
Feature is deleted by its id at line 84

Gladys/server/lib/device/device.create.js

Line 84 in efc2ef5

if (!features.find((newFeature) => newFeature.id === existingFeature.id)) {

But as we are in the same transaction, when we get plain device at line 97, the feature delete query is not executed, so deleted feature is selected

Gladys/server/lib/device/device.create.js

Line 97 in efc2ef5

deviceToReturn = deviceInDb.get({ plain: true });

Then we check for an update not on the id used to delete feature, but on the external_id, feature is always there according to SQL transaction, but at the end, no feature is still present.

[Pierre-Gilles]

The bug was not exactly that, but the fix was simple !

As the external_id is the only source of "truth/unicity", I use it everywhere instead of "id".

Now it works in the case you specified. I added some tests to ensure it stays the same in the future.