General Settings view for Devices #624

kschiffer · 2019-05-02T15:54:10Z

Summary

Closes #584

Changes

Refactor device data form into a dedicated container
Add general settings view for devices
Add option to use <Tab />'s as nav elements
Add submit logic for updating devices
Add disabled styling to checkboxes and radio buttons
A couple of minor styling tweaks

Notes for Reviewers

I have added the link prop to the <Tabs /> which will render the tab element as <NavLink />, as such also handling active state by itself. I decided to not make it a dedicated component, as the functionality could quite easily be added and it saves us some redundancy. Hope that's ok with you.

Release Notes

Added "General Settings" view for end devices

cc @htdvisser

coveralls · 2019-05-02T16:03:03Z

Coverage increased (+0.02%) to 73.592% when pulling 474ea80 on feature/584-webui-device-settings into 0534064 on master.

bafonins · 2019-05-03T11:18:50Z

pkg/webui/locales/xx.json

-  "console.views.device-add.messages.abp": "Xxxxxxxxxx Xx Xxxxxxxxxxxxxxx (XXX)",
-  "console.views.device-add.messages.resetWarning": "Xxxxxxxx xx xxxxxxxx xxx xxxxx xxxx xxxxxx xxxxxxxxxxx xxx xxxxxx xxxxxxx",
-  "console.views.device-add.messages.couldNotRetrieveFrequencyPlans": "Xxxxx xxx xxxxxxxx xxx xxxx xx xxxxxxxxx xxxxxxxxx xxxxx",
+  "console.views.device-general-settings.index.updateSuccess": "Xxxxxxxxxxxx xxxxxxx xxx xxxxxx",
  "console.views.device-overview.index.hardware": "Xxxxxxxx",
  "console.views.device-overview.index.brand": "Xxxxx",
  "console.views.device-overview.index.model": "Xxxxx",


please move these to shared-messages.js. The gateway model has them as well, see https://github.com/TheThingsNetwork/lorawan-stack/blob/master/api/gateway.proto#L48-L53

bafonins · 2019-05-03T11:19:46Z

pkg/webui/console/views/device-general-settings/index.js

+  async handleSubmit (values, { setSubmitting, resetForm }) {
+    const { device } = this.props
+    const { application_id } = device.ids.application_ids
+    const updatedDevice = Object.assign({}, values)


Suggested change

const updatedDevice = Object.assign({}, values)

const updatedDevice = {...values}

bafonins · 2019-05-03T11:21:16Z

pkg/webui/console/views/device-general-settings/index.js

+
+  async handleSubmit (values, { setSubmitting, resetForm }) {
+    const { device } = this.props
+    const { application_id } = device.ids.application_ids


you can selector here, see https://github.com/TheThingsNetwork/lorawan-stack/blob/master/pkg/webui/lib/selectors/id.js#L17-L20

bafonins · 2019-05-03T11:26:39Z

pkg/webui/console/views/device-general-settings/index.js

+        updatedDevice.ids.dev_addr = updatedDevice.session.dev_addr
+      }
+    }
+    delete updatedDevice.activation_mode


const { activation_mode, ...updatedDevice } = values

and you can remove this + simplifies check on L51

bafonins · 2019-05-03T11:27:00Z

pkg/webui/console/views/device-general-settings/index.js

+export default class DeviceGeneralSettings extends React.Component {
+
+  state = {
+    error: undefined,


Suggested change

error: undefined,

error: '',

bafonins · 2019-05-03T11:33:51Z

pkg/webui/console/containers/device-data-form/index.js

+          max={16}
+          placeholder={m.leaveBlankPlaceholder}
+          description={m.nwkKeyDescription}
+


bafonins · 2019-05-03T11:34:23Z

pkg/webui/console/containers/device-data-form/index.js

+}
+
+DeviceDataForm.propTypes = {
+  onSubmit: PropTypes.func,


not required?

bafonins · 2019-05-03T11:34:33Z

pkg/webui/console/containers/device-data-form/index.js

+
+DeviceDataForm.propTypes = {
+  onSubmit: PropTypes.func,
+  error: PropTypes.error,


add default value

bafonins · 2019-05-03T11:34:39Z

pkg/webui/console/containers/device-data-form/index.js

+DeviceDataForm.propTypes = {
+  onSubmit: PropTypes.func,
+  error: PropTypes.error,
+  update: PropTypes.bool,


add default value

bafonins · 2019-05-03T11:36:20Z

pkg/webui/console/api/index.js

@@ -101,6 +101,7 @@ export default {
    list: ttnClient.Applications.Devices.getAll.bind(ttnClient.Applications.Devices),
    get: ttnClient.Applications.Devices.getById.bind(ttnClient.Applications.Devices),
    create: ttnClient.Applications.Devices.create.bind(ttnClient.Applications.Devices),
+    update: ttnClient.Applications.Devices.updateById.bind(ttnClient.Applications.Devices),


please see how the applications and application definitions are structured. lets be consistent

bafonins

please check the <Tabs /> storybook, changing tabs doesnot work anymore

bafonins · 2019-05-03T15:30:10Z

pkg/webui/lib/selectors/id.js

@@ -16,7 +16,7 @@ import getByPath from '../get-by-path'

 export const getApplicationId = function (application = {}) {
  return getByPath(application, 'application_id')
-    || getByPath(application, 'application_ids.application_id')
+    || getByPath(application, 'ids.application_id')


this breaks the events component.
my suggestion would be to add an additional case:

Suggested change

|| getByPath(application, 'ids.application_id')

|| (getByPath(application, 'application_ids.application_id')

|| (getByPath(application, ''ids.application_id'))

This also causes the issue we discussed in slack:

checkPropTypes.js:19 Warning: Failed prop type: The prop `emitter` is marked as required in `Event`, but its value is `undefined`. in Event (created by DefaultEvent) in DefaultEvent (created by List) in li (created by ListItem) in ListItem (created by List) in ol (created by List) in div (created by List) in List (created by EventsWidget) in aside (created by EventsWidget) in EventsWidget (created by EventsSubscription) in EventsSubscription (created by Connect(EventsSubscription))

Ahh indeed. Was looking whether it was used elsewhere, but couldn't find it. Will add a case then.

bafonins · 2019-05-03T15:32:30Z

pkg/webui/console/views/device-general-settings/index.js

+      const changed = diff(device, updatedDevice)
+      await api.device.update(appId, device_id, changed)
+
+      resetForm({ ...values })


Suggested change

resetForm({ ...values })

resetForm(values)

should work as well

kschiffer added the c/console This is related to the Console label May 2, 2019

kschiffer added this to the May 2019 milestone May 2, 2019

kschiffer requested a review from bafonins May 2, 2019 15:54

kschiffer self-assigned this May 2, 2019

kschiffer mentioned this pull request May 3, 2019

Console MVP #629

Closed

48 tasks

bafonins suggested changes May 3, 2019

View reviewed changes

kschiffer requested a review from bafonins May 3, 2019 14:06

bafonins suggested changes May 3, 2019

View reviewed changes

kschiffer force-pushed the feature/584-webui-device-settings branch from db7b48f to dddc699 Compare May 6, 2019 08:04

kschiffer requested a review from bafonins May 6, 2019 08:05

bafonins approved these changes May 6, 2019

View reviewed changes

kschiffer added 18 commits May 6, 2019 13:43

console,oauth: Add link option to tabs

9008b67

console: Improve submit bar styling

166877a

console: Add general settings route

34dd244

console: Add device general-settings view

7919403

console: Refactor add device form into container

f3e1494

console: Add initial value composition

ec89d18

console: Use different string literal for mac versions

ef819f2

console: Add disabled mode to checkbox and radio buttons

cb7ded5

console: Add device update to api tree

1796efb

console: Add update option to device data form

1c8d698

console: Add submit logic to device settings

6c47bbd

console: Add more selectors to device request

37bc3a6

console: Fix settings for abp devices

381c838

console: Move version info messages

c103299

console: Fix error default prop

4ba7c17

console: Update stories

f8dea19

console: Refactor api tree mapping

802dba1

console: Extend application selector

3487d5f

kschiffer added 4 commits May 6, 2019 13:46

console: Fix initial activation mode

d6e205b

console: Add initial value to freq plan container

fa817d7

util: Fix entity mapping of dev_addr value

6423472

dev: Update locales

474ea80

kschiffer force-pushed the feature/584-webui-device-settings branch from 1d7013e to 474ea80 Compare May 6, 2019 11:50

kschiffer merged commit b89085b into master May 6, 2019

kschiffer deleted the feature/584-webui-device-settings branch May 6, 2019 12:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

General Settings view for Devices #624

General Settings view for Devices #624

kschiffer commented May 2, 2019

coveralls commented May 2, 2019 •

edited

Loading

bafonins May 3, 2019

bafonins May 3, 2019

bafonins May 3, 2019

bafonins May 3, 2019

bafonins May 3, 2019

bafonins May 3, 2019

bafonins May 3, 2019

bafonins May 3, 2019

bafonins May 3, 2019

bafonins May 3, 2019

bafonins left a comment

bafonins May 3, 2019

bafonins May 3, 2019

kschiffer May 3, 2019

bafonins May 3, 2019

	const updatedDevice = Object.assign({}, values)
	const updatedDevice = {...values}

	\|\| getByPath(application, 'ids.application_id')
	\|\| (getByPath(application, 'application_ids.application_id')
	\|\| (getByPath(application, ''ids.application_id'))

General Settings view for Devices #624

General Settings view for Devices #624

Conversation

kschiffer commented May 2, 2019

Summary

Changes

Notes for Reviewers

Release Notes

coveralls commented May 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bafonins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 2, 2019 •

edited

Loading