KLO-271 : Remove environment from iot-console#312
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @nxtcoder19 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟡 Testing: 5 issues found
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @@ -1,23 +1,24 @@ | |||
| --- | |||
There was a problem hiding this comment.
suggestion (code_refinement): Consider consolidating device-related operations under a unified naming convention.
The mutation names like Iot_createDevice, Iot_updateDevice, etc., are clear but consider a naming convention that groups all device-related operations more closely, for example, using a prefix or a more consistent naming pattern.
| --- | |
| Device_create | |
| Device_update | |
| Device_delete | |
| Device_get | |
| Device_list |
| ) | ||
|
|
||
| func (d *domain) findDevice(ctx IotResourceContext, name string) (*entities.IOTDevice, error) { | ||
| func (d *domain) findDevice(ctx IotResourceContext, name string, deviceBlueprintName string) (*entities.IOTDevice, error) { |
There was a problem hiding this comment.
suggestion (testing): Consider adding tests for findDevice method to cover device not found scenario.
It's important to ensure that the method behaves correctly when a device with the specified name and deviceBlueprintName does not exist.
| func (d *domain) findDevice(ctx IotResourceContext, name string, deviceBlueprintName string) (*entities.IOTDevice, error) { | |
| if dev == nil { | |
| return nil, errors.Newf("no device with name=%q and deviceBlueprintName=%q found", name, deviceBlueprintName) | |
| } |
| return dev, nil | ||
| } | ||
|
|
||
| func (d *domain) ListDevices(ctx IotResourceContext, deviceBlueprintName string, search map[string]repos.MatchFilter, pq repos.CursorPagination) (*repos.PaginatedRecord[*entities.IOTDevice], error) { |
There was a problem hiding this comment.
suggestion (testing): Ensure there are tests for ListDevices method covering various filter combinations.
Given the method supports filtering, it would be beneficial to have tests that verify the correct devices are returned for different filter criteria.
| } | ||
|
|
||
| func (d *domain) CreateDevice(ctx IotResourceContext, device entities.IOTDevice) (*entities.IOTDevice, error) { | ||
| func (d *domain) CreateDevice(ctx IotResourceContext, deviceBlueprintName string, device entities.IOTDevice) (*entities.IOTDevice, error) { |
There was a problem hiding this comment.
suggestion (testing): Add tests for CreateDevice to validate device creation with invalid inputs.
Testing with invalid inputs (e.g., empty deviceBlueprintName or device fields) helps ensure robust error handling and validation.
| func (d *domain) CreateDevice(ctx IotResourceContext, deviceBlueprintName string, device entities.IOTDevice) (*entities.IOTDevice, error) { | |
| func (d *domain) CreateDevice(ctx IotResourceContext, deviceBlueprintName string, device entities.IOTDevice) (*entities.IOTDevice, error) { | |
| if deviceBlueprintName == "" { | |
| return nil, errors.New("deviceBlueprintName cannot be empty") | |
| } | |
| if device.Name == "" { | |
| return nil, errors.New("device name cannot be empty") | |
| } | |
| // Existing code for CreateDevice continues here... | |
| } |
| return upDev, nil | ||
| } | ||
|
|
||
| func (d *domain) UpdateDevice(ctx IotResourceContext, deviceBlueprintName string, device entities.IOTDevice) (*entities.IOTDevice, error) { |
There was a problem hiding this comment.
suggestion (testing): Missing tests for UpdateDevice method to verify successful and failed updates.
It's crucial to test both scenarios where the device update is successful and where it fails (e.g., device not found or invalid update parameters).
| } | ||
|
|
||
| func (d *domain) DeleteDevice(ctx IotResourceContext, name string) error { | ||
| func (d *domain) DeleteDevice(ctx IotResourceContext, deviceBlueprintName string, name string) error { |
There was a problem hiding this comment.
suggestion (testing): Add unit tests for DeleteDevice to cover non-existent device scenarios.
Ensuring that attempts to delete non-existent devices are handled correctly is important for maintaining data integrity.
- remove environment resource - tag device to deployment
nxtcoder19
force-pushed
the
update/iot-console
branch
from
40b0d7d to
2466970
Compare
KLO-271 : Remove environment from iot-console
2 participants
Resolves kloudlite/kloudlite#199