Update device summary store structure #26
Conversation
|
Please hold off on this one. I'd like to continue going through it more thoroughly |
Sure. Overall I feel like the device summary doesn't need to hold all the info. And I also thought about having another Map type holds the information of <deviceid, digitaltwininterfaceids>. But we are not using the digitalinterfaceids information anywhere else, might as well not going through redux at all like the device events. |
src/app/devices/deviceList/components/addDevice/components/addDevice.tsx
Show resolved
Hide resolved
src/app/devices/deviceList/components/addDevice/components/addDevice.tsx
Show resolved
Hide resolved
| <span className={`device-list-cell-item ${!device.isPnpDevice && 'last'}`}>{`${context.t(ResourceKeys.deviceLists.columns.statusUpdatedTime)}: `}<span className="data">{device.statusUpdatedTime || context.t(ResourceKeys.deviceLists.noData)}</span></span> | ||
| {!!device.isPnpDevice && | ||
| <span className={`device-list-cell-item ${this.state.interfaceIds.length === 0 && 'last'}`}>{`${context.t(ResourceKeys.deviceLists.columns.statusUpdatedTime)}: `}<span className="data">{device.statusUpdatedTime || context.t(ResourceKeys.deviceLists.noData)}</span></span> | ||
| {this.state.interfaceIds.length !== 0 && |
There was a problem hiding this comment.
Is this the right condition? Before pnp devices were devices that implemented an interface other than ModelDiscovery. I think now every device is a PNP device, if that's the case do we still need the icon?
There was a problem hiding this comment.
Same concerns here. Reached out again to the PMs, and that's keep it as is, and see what they would decide.
Removed digital twin related information from device summary, which have created some unncessary complexity and introduced a few bugs.
Instead, the component now directly calls digital twin api to update itself when user expand the device list cell, without going through the saga and redux.
By doing so, we removed the complex logic of checking whether device list component needs an update which caused couple bugs during bugbash.
The device list in the store is still using immutable all the way down, but there is no point having a List<Map<string, any>> as we lost all the intellisense in the device list reducer. Now it is simply a immutable map.
Bugs related:
https://msazure.visualstudio.com/One/_workitems/edit/4991936
https://msazure.visualstudio.com/One/_workitems/edit/4992402
https://msazure.visualstudio.com/One/_workitems/edit/4992707