Table-mode: sort ips numerically #2007

fbarl · 2016-11-11T10:56:49Z

No description provided.

client/app/scripts/components/node-details/node-details-table.js

@@ -50,6 +51,16 @@ const COLUMN_WIDTHS = {
 };


+// TODO: Consider introducing the 'ip' dataType for
+// the fields instead of maintaining a list here.
+const IP_FIELDS = ['kubernetes_ip'];


davkal

Good first stab! Have a couple of requests.

client/app/scripts/components/node-details/node-details-table.js

@@ -74,13 +80,22 @@ function maybeToLower(value) {
 }


+function toArrayOfThreeDigitStrings(value) {


client/app/scripts/components/node-details/node-details-table.js

@@ -74,13 +80,22 @@ function maybeToLower(value) {
 }


+function toArrayOfThreeDigitStrings(value) {
+  const padToThreeDigits = (n) => `000${n}`.slice(-3);


client/app/scripts/components/node-details/__tests__/node-details-table-test.js

+  function matchColumnValues(columnLabel, expectedValues) {
+    const columnIndex = columns.findIndex(column => column.id === columnLabel);
+    const values = TestUtils.scryRenderedDOMComponentsWithClass(component, 'node-details-table-node-value').map(d => d.title);
+    const filteredValues = values.filter((element, index) => index % columns.length === columnIndex);


client/app/scripts/components/node-details/__tests__/node-details-table-test.js

+    describe('kubernetes_ip', () => {
+      it('sorts ascendingly', () => {
+        component = TestUtils.renderIntoDocument(
+          <Provider store={configureStore()}>


client/app/scripts/components/node-details/__tests__/node-details-table-test.js

+      {
+        id: 'node-1',
+        metadata: [
+          { id: 'kubernetes_ip', label: 'IP', value: '10.244.253.24' },


fbarl

@davkal PTAL

client/app/scripts/components/node-details/node-details-table.js

@@ -74,13 +80,22 @@ function maybeToLower(value) {
 }


+function toArrayOfThreeDigitStrings(value) {
+  const padToThreeDigits = (n) => `000${n}`.slice(-3);


client/app/scripts/utils/__tests__/string-utils-test.js

@@ -20,4 +20,13 @@ describe('StringUtils', () => {
      expect(fun(['prefix', 'suffix'])).toBe('');
    });
  });
+
+  describe('iPtoPaddedString', () => {
+    const fun = StringUtils.iPtoPaddedString;


client/app/scripts/utils/string-utils.js

@@ -75,6 +80,11 @@ export function longestCommonPrefix(strArr) {
  return (new LCP(strArr)).lcp();
 }

+// Converts IPs from '10.244.253.4' to '010.244.253.004' format.
+export function iPtoPaddedString(value) {


foot · 2016-11-15T09:47:02Z

Nicely done! Keeping the ip as a padded string rather than converting it into a number works really well and still allows for comparisons w/ domains etc too. 👍

I'm not sure if we really need to render the components to test the sorting though.. you could export function getSortedNodes in node-details-table and test that instead (i think?).

foot · 2016-11-15T13:17:04Z

Ooops! The rebase looks like its gone a bit funny (commits from master now in this branch),

You wanna do something like:

git fetch
git rebase origin/master
git push -f

fbarl · 2016-11-17T10:13:26Z

@davkal PTAL before I merge (I will squash all the commits into one as Simon suggested).

davkal · 2016-11-22T09:58:11Z

LGTM

fbarl self-assigned this Nov 11, 2016

davkal reviewed Nov 11, 2016

View reviewed changes

fbarl force-pushed the 1746-sort-ips-numerically branch from a5c682f to bceee78 Compare November 14, 2016 14:48

fbarl changed the title ~~[WIP] Table-mode: sort ips numerically~~ Table-mode: sort ips numerically Nov 14, 2016

fbarl assigned foot and davkal Nov 14, 2016

davkal suggested changes Nov 14, 2016

View reviewed changes

davkal unassigned foot and davkal Nov 14, 2016

fbarl force-pushed the 1746-sort-ips-numerically branch from da8c70f to d9e46c5 Compare November 15, 2016 09:14

fbarl commented Nov 15, 2016

View reviewed changes

client/app/scripts/components/node-details/node-details-table.js

@@ -74,13 +80,22 @@ function maybeToLower(value) {

}

function toArrayOfThreeDigitStrings(value) {

const padToThreeDigits = (n) => `000${n}`.slice(-3);

This comment was marked as abuse.

Sign in to view

foot reviewed Nov 15, 2016

View reviewed changes

fbarl force-pushed the 1746-sort-ips-numerically branch from d9e46c5 to 3128bc7 Compare November 15, 2016 09:59

fbarl added 10 commits November 16, 2016 18:07

Added /tmp folder to .gitignore

8e37d4c

Sorting kubernetes_ip column properly.

91f8599

Wrote the NodeDetailsTable component test for sorting IPs.

8dd3261

Addressed @davkal's comment

670b1bf

Fix #1746

b0e8b7a

Addressed some @davkal's comments.

712c5aa

Moved iPtoPaddedString function to string-utils.

97779a9

Wrote the test for iptoPaddedString.

9f8003b

Done the renaming.

01649d2

Addressed all the comments

3a8b81c

fbarl force-pushed the 1746-sort-ips-numerically branch from 3128bc7 to 3a8b81c Compare November 17, 2016 10:05

fbarl assigned davkal Nov 18, 2016

davkal approved these changes Nov 22, 2016

View reviewed changes

fbarl merged commit d15e884 into master Nov 22, 2016

fbarl deleted the 1746-sort-ips-numerically branch January 17, 2017 10:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Table-mode: sort ips numerically #2007

Table-mode: sort ips numerically #2007

fbarl commented Nov 11, 2016

This comment was marked as abuse.

This comment was marked as abuse.

davkal left a comment

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

fbarl left a comment

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

foot commented Nov 15, 2016

foot commented Nov 15, 2016

fbarl commented Nov 17, 2016

davkal commented Nov 22, 2016

		@@ -74,13 +80,22 @@ function maybeToLower(value) {
		}


		function toArrayOfThreeDigitStrings(value) {

Table-mode: sort ips numerically #2007

Table-mode: sort ips numerically #2007

Conversation

fbarl commented Nov 11, 2016

This comment was marked as abuse.

This comment was marked as abuse.

davkal left a comment

Choose a reason for hiding this comment

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

fbarl left a comment

Choose a reason for hiding this comment

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

foot commented Nov 15, 2016

foot commented Nov 15, 2016

fbarl commented Nov 17, 2016

davkal commented Nov 22, 2016