Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Treat text and number correctly in device jobs #1082

Merged
merged 2 commits into from
Sep 4, 2018
Merged

Treat text and number correctly in device jobs #1082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
416107f
treat text and number correctly in device jobs
Aug 31, 2018
342ec2c
fix formatting nit
Aug 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/pages/devices/flyouts/deviceJobs/deviceJobProperties.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {

update.extend('$autoArray', (val, obj) => update(obj || [], val));

const isNumeric = value => typeof value === 'number' || !isNaN(parseInt(value, 10));
const isNumeric = value => typeof value === 'number';
const isAlphaNumericRegex = /^[a-zA-Z0-9]*$/;
const nonAlphaNumeric = x => !x.match(isAlphaNumericRegex);

Expand Down
2 changes: 1 addition & 1 deletion src/components/pages/devices/flyouts/deviceJobs/deviceJobTags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {

update.extend('$autoArray', (val, obj) => update(obj || [], val));

const isNumeric = value => typeof value === 'number' || !isNaN(parseInt(value, 10));
const isNumeric = value => typeof value === 'number';
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it guaranteed that isNumeric will only be used on values that are actually numbers? If a string value comes in that represents a number var num = "30", do we want that to also be a number? Which I think is why the !isNaN was there. Just want to make sure it isn't being used in this way.

Copy link
Contributor Author

@mechaffin mechaffin Aug 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Just because a value looks like a number doesn't mean that the actual type is a number. The tags and properties can be either text or number... and there is a drop down for the user to specify which one they want. We were not respecting this drop down at all in the previous version of the code.

Copy link
Contributor

@ppathan ppathan Sep 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

BTW when user selects a number but enters a string do we check?

const isAlphaNumericRegex = /^[a-zA-Z0-9]*$/;
const nonAlphaNumeric = x => !x.match(isAlphaNumericRegex);

Expand Down
4 changes: 2 additions & 2 deletions src/components/shell/header/breadcrumbs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ export const Breadcrumbs = ({ t, crumbsConfig }) => (
<Switch>
{
crumbsConfig.map(({ path, crumbs }) =>
<Route exact path={path} render={props => {
return crumbs.map((crumb, idx) => <CrumbFromConfig {...props} t={t} crumb={crumb} isLast={idx === crumbs.length - 1} />);
<Route key={path} exact path={path} render={props => {
return crumbs.map((crumb, idx) => <CrumbFromConfig {...props} key={crumb.to} t={t} crumb={crumb} isLast={idx === crumbs.length - 1} />);
}} />
)
}
Expand Down
13 changes: 9 additions & 4 deletions src/services/models/iotHubManagerModels.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import update from 'immutability-helper';
import dot from 'dot-object';
import { camelCaseReshape, getItems } from 'utilities';
import { camelCaseReshape, getItems, float } from 'utilities';
import uuid from 'uuid/v4';

// Contains methods for converting service response
Expand Down Expand Up @@ -64,7 +64,10 @@ export const toSubmitTagsJobRequestModel = (devices, { jobName, updatedTags, del
const jobId = jobName ? jobName + '-' + uuid() : uuid();
const deviceIds = devices.map(({ id }) => `'${id}'`).join(',');
const Tags = {};
updatedTags.forEach(({ name, value }) => (Tags[name] = value));
// Ensure type passed to server is correct as specified: number or text.
// The toString call is necessary when a number should be saved as text.
updatedTags.forEach(({ name, value, type }) =>
(Tags[name] = type === 'Number' ? float(value) : value.toString()));
deletedTags.forEach((name) => (Tags[name] = null));
const request = {
JobId: jobId,
Expand All @@ -81,7 +84,10 @@ export const toSubmitPropertiesJobRequestModel = (devices, { jobName, updatedPro
const jobId = jobName ? jobName + '-' + uuid() : uuid();
const deviceIds = devices.map(({ id }) => `'${id}'`).join(',');
const Desired = {};
updatedProperties.forEach(({ name, value }) => (Desired[name] = value));
// Ensure type passed to server is correct as specified: number or text.
// The toString call is necessary when a number should be saved as text.
updatedProperties.forEach(({ name, value, type }) =>
(Desired[name] = type === 'Number' ? float(value) : value.toString()));
deletedProperties.forEach((name) => (Desired[name] = null));
const request = {
JobId: jobId,
Expand Down Expand Up @@ -140,7 +146,6 @@ export const authenticationTypeOptions = {
};

export const toNewDeviceRequestModel = ({
count,
deviceId,
isGenerateId,
isSimulated,
Expand Down
3 changes: 3 additions & 0 deletions src/utilities/methods.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ export const isEmptyObject = obj => Object.keys(obj).length === 0 && obj.constru
/** Converts a value to an integer */
export const int = (num) => parseInt(num, 10);

/** Converts a value to a float */
export const float = (num) => parseFloat(num, 10);

/** Merges css classnames into a single string */
export const joinClasses = (...classNames) => classNames.filter(name => !!name).join(' ').trim();

Expand Down