#234 | Better URL validation | Iframe board tests added (plus some refactor)

.github/PULL_REQUEST_TEMPLATE.md

@@ -31,7 +31,7 @@
 <!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
 <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
 
-- [ ] My code follows the [code style](/CONTRIBUTING.md#coding-conventions) of this project.
+- [ ] My code follows the [code style](https://github.com/Cognifide/cogboard/blob/master/CONTRIBUTING.md#coding-conventions) of this project.
 - [ ] My change requires a change to the documentation.
 - [ ] Automated functional tests have been added or modified to cover my changes (if applicable)
 - [ ] I have updated the documentation accordingly.

cogboard-app/src/main/kotlin/com/cognifide/cogboard/config/controller/ControllerFactory.kt

@@ -5,14 +5,16 @@ import io.vertx.core.eventbus.MessageConsumer
 import io.vertx.core.json.JsonObject
 
 class ControllerFactory {
-    fun create(address: String, vertx: Vertx, configuration: Map<String, (JsonObject) -> String>): MessageConsumer<JsonObject> {
-        return vertx.eventBus()
-                .consumer<JsonObject>(address)
-                .handler {
-                    val body = it.body()
-                    configuration[body.getString("method")]?.let { method ->
-                        it.reply(method(body.getJsonObject("payload")))
-                    }
+    fun create(
+        address: String,
+        vertx: Vertx,
+        configuration: Map<String, (JsonObject) -> String>
+    ): MessageConsumer<JsonObject> = vertx.eventBus()
+            .consumer<JsonObject>(address)
+            .handler {
+                val body = it.body()
+                configuration[body.getString("method")]?.let { method ->
+                    it.reply(method(body.getJsonObject("payload")))
                 }
-    }
+            }
 }

cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/persondraw/PersonDrawUtils.kt

@@ -12,23 +12,17 @@ fun <T> List<T>.getNextListIndex(currentIndex: Int): Int = when {
 }
 
 fun <T> List<T>.getRandomListIndex(usedIndexes: List<Int>): Int {
-
-    if (this.isEmpty()) {
-        return -1
-    }
-
-    val indexes = this.mapIndexed { index, _ -> index }
-            .filter { i -> !usedIndexes.contains(i) }
-
-    // If there is one index, there is no need to use random function
-    if (indexes.size == 1) {
-        return indexes[0]
-    }
-
-    return if (indexes.isNotEmpty()) {
-        indexes[Random.nextInt(0, indexes.size)]
+    return if (this.isEmpty()) {
+        -1
     } else {
-        Random.nextInt(0, this.size)
+        val indexes = this.mapIndexed { index, _ -> index }
+                .filter { i -> !usedIndexes.contains(i) }
+        when {
+            // If there is one index, there is no need to use random function
+            indexes.size == 1 -> return indexes[0]
+            indexes.isNotEmpty() -> indexes[Random.nextInt(0, indexes.size)]
+            else -> Random.nextInt(0, this.size)
+        }
     }
 }
 

cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/sonarqube/SonarQubeWidget.kt

@@ -60,7 +60,10 @@ class SonarQubeWidget(vertx: Vertx, config: JsonObject) : AsyncWidget(vertx, con
                     metrics.list
                             .stream()
                             .map { it as JsonObject }
-                            .filter { it.getString(version.getMetricKey()) != "alert_status" && it.getString(version.getMetricKey()) == metricName }
+                            .filter {
+                                it.getString(version.getMetricKey()) != "alert_status" &&
+                                        it.getString(version.getMetricKey()) == metricName
+                            }
                             .findFirst()
                             .ifPresent { result.put(metricName, version.getMetricValue(it)) }
                 }

cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/sonarqube/Version.kt

@@ -15,11 +15,8 @@ interface Version {
     fun getMetricValue(metric: JsonObject): Int
 
     companion object Test {
-        fun getVersion(version: String): Version {
-            if (version.equals("5.x"))
-                return V5x()
-            else
-                return V7x()
-        }
+        fun getVersion(version: String) =
+                if (version == "5.x") V5x()
+                else V7x()
     }
 }

cogboard-webapp/src/components/widgets/dialogFields/index.js

@@ -76,7 +76,11 @@ const dialogFields = {
     component: TextInput,
     name: 'publicUrl',
     label: 'Public URL',
-    validator: () => string().url(vm.INVALID_URL())
+    validator: () =>
+      string().matches(
+        /^(?:([A-Za-z]+):)?(\/{0,3})([0-9.\-A-Za-z]+)(?::(\d+))?(?:\/([^?#]*))?(?:\?([^#]*))?(?:#(.*))?$/,
+        { message: vm.INVALID_URL(), excludeEmptyString: true }
+      )
   },
   WidgetTypeField: {
     component: DisplayValueSelect,
@@ -210,13 +214,21 @@ const dialogFields = {
     component: TextInput,
     name: 'url',
     label: 'URL',
-    validator: () => string().url(vm.INVALID_URL())
+    validator: () =>
+      string().matches(
+        /^(?:([A-Za-z]+):)?(\/{0,3})([0-9.\-A-Za-z]+)(?::(\d+))?(?:\/([^?#]*))?(?:\?([^#]*))?(?:#(.*))?$/,
+        { message: vm.INVALID_URL(), excludeEmptyString: true }
+      )
   },
   IFrameURL: {
     component: TextInput,
     name: 'iframeUrl',
     label: 'URL',
-    validator: () => string().url(vm.INVALID_URL())
+    validator: () =>
+      string().matches(
+        /^(?:([A-Za-z]+):)?(\/{0,3})([0-9.\-A-Za-z]+)(?::(\d+))?(?:\/([^?#]*))?(?:\?([^#]*))?(?:#(.*))?$/,
+        { message: vm.INVALID_URL(), excludeEmptyString: true }
+      )
   },
   IdString: {
     component: TextInput,

functional/cypress-tests/cypress/fixtures/Dashboard.js

@@ -1,5 +1,5 @@
 export function dashboardNameGen(name = 'Dashboard') {
-  return `name_${Date.now().toString()}`;
+  return `${name}_${Date.now().toString()}`;
 }
 
 export const columnEdgeValues = ['3', '4', '20', '21'];

functional/cypress-tests/cypress/integration/dashboards.js

@@ -2,10 +2,9 @@ import {
   dashboardNameGen,
   columnEdgeValues,
   switchIntervalEdgeValues,
-  dashboardNames,
-  dashboardTypes
+  dashboardNames
 } from '../fixtures/Dashboard';
-import { addWidgetsDashboard } from '../support/dashboard';
+import { addIframeDashboard, addWidgetsDashboard } from '../support/dashboard';
 
 describe('Basic Dashboard CRUD', () => {
   const newTitle = dashboardNameGen('Edit');
@@ -15,14 +14,24 @@ describe('Basic Dashboard CRUD', () => {
     cy.login();
   });
 
-  it('Logged user can add new dashboard', () => {
+  it('Logged user can add new widgets dashboard', () => {
     addWidgetsDashboard();
   });
 
-  it('Logged user can choose dashboard', () => {
+  it('Logged user can choose widgets dashboard', () => {
     addWidgetsDashboard().canBeSelected();
   });
 
+  it('Logged user can add new iframe dashboard', () => {
+    addIframeDashboard();
+  });
+
+  it('Logged user can choose iframe dashboard', () => {
+    addIframeDashboard()
+      .canBeSelected()
+      .assertIframeExists();
+  });
+
   it('Anonymous user can choose dashboard', () => {
     const board = addWidgetsDashboard();
     cy.closeDrawer();
@@ -46,63 +55,61 @@ describe('Basic Dashboard CRUD', () => {
   });
 });
 
-describe('Dashboard Frontend Validation', () => {
+describe('Dashboard Input Validation', () => {
   beforeEach(() => {
     cy.visit('/');
     cy.login();
   });
 
-  it(' For empty dashboard name is displayed and submit is impossible', () => {
-    addWidgetsDashboard(' ', '4', '10', true).assertErrorMessageVisible(
-      'This field is required'
-    );
+  it('Name input do not accept empty strings', () => {
+    addWidgetsDashboard(' ')
+      .expectConfigToBeInvalid()
+      .assertErrorMessageVisible('This field is required');
   });
 
   dashboardNames.forEach(value => {
-    it(` For too long dashboard name is displayed and submit is impossible. Length: ${value.length}`, () => {
-      const board = addWidgetsDashboard(
-        value,
-        dashboardTypes.widgets,
-        '8',
-        '10',
-        true
-      );
+    it(`Name input accepts strings not exceeding 50 characters. Tested value: ${value.length}`, () => {
+      const board = addWidgetsDashboard(value);
       if (value.length > 50) {
-        board.assertErrorMessageVisible(
-          'Title length must be less or equal to 50.'
-        );
+        board
+          .expectConfigToBeInvalid()
+          .assertErrorMessageVisible(
+            'Title length must be less or equal to 50.'
+          );
       } else {
-        board.assertErrorNotVisible();
+        board.expectConfigToBeValid();
       }
     });
   });
 
   columnEdgeValues.forEach(value => {
-    it(` For Columns input is displayed and submit is impossible for incorrect values. Edge value : ${value}`, () => {
-      const board = addWidgetsDashboard(undefined, value, '10', true);
-      if (value === '3') {
-        board.assertErrorMessageVisible(
-          'Columns number cannot be less than 4.'
-        );
-      } else if (value === '21') {
-        board.assertErrorMessageVisible(
-          'Columns number cannot be more than 20.'
-        );
+    it(`Columns input accepts only values from 4 to 20. Tested value: ${value}`, () => {
+      const board = addWidgetsDashboard(undefined, value);
+      if (value < 4) {
+        board
+          .expectConfigToBeInvalid()
+          .assertErrorMessageVisible('Columns number cannot be less than 4.');
+      } else if (value > 20) {
+        board
+          .expectConfigToBeInvalid()
+          .assertErrorMessageVisible('Columns number cannot be more than 20.');
       } else {
-        board.assertErrorNotVisible();
+        board.expectConfigToBeValid();
       }
     });
   });
 
   switchIntervalEdgeValues.forEach(value => {
-    it(` For Switch Interval input is displayed and submit is impossible for incorrect values. Edge value: ${value}`, () => {
-      const board = addWidgetsDashboard(undefined, '8', value, true);
-      if (value === '2') {
-        board.assertErrorMessageVisible(
-          'Switch interval number cannot be less than 3.'
-        );
+    it(`Switch interval input accepts only values grater than 2. Tested value: ${value}`, () => {
+      const board = addWidgetsDashboard(undefined, undefined, value);
+      if (value < 3) {
+        board
+          .expectConfigToBeInvalid()
+          .assertErrorMessageVisible(
+            'Switch interval number cannot be less than 3.'
+          );
       } else {
-        board.assertErrorNotVisible();
+        board.expectConfigToBeValid();
       }
     });
   });

functional/cypress-tests/cypress/support/dashboard.js

@@ -1,15 +1,9 @@
 import { dashboardNameGen, dashboardTypes } from '../fixtures/Dashboard';
 
 class Dashboard {
-  constructor(
-    name = dashboardNameGen(),
-    type,
-    columns,
-    switchInterval,
-    expectedFailure
-  ) {
+  constructor(name = dashboardNameGen(), type, columns, switchInterval) {
     this.name = name;
-    cy.addDashboard(this.name, columns, switchInterval, expectedFailure, type);
+    cy.addDashboard(this.name, columns, switchInterval, type);
   }
 
   canBeSelected() {
@@ -18,6 +12,11 @@ class Dashboard {
     return this;
   }
 
+  assertIframeExists() {
+    cy.get('iframe').should('is.visible');
+    return this;
+  }
+
   openEditDialog() {
     cy.contains('[data-cy="board-card"]', this.name)
       .find('[data-cy="board-card-edit-button"]')
@@ -65,13 +64,21 @@ class Dashboard {
     cy.contains('[data-cy="board-card"]', this.name).should('not.exist');
   }
 
-  assertErrorNotVisible() {
+  assertErrorMessageVisible(message) {
+    cy.contains('[data-cy^="board-form-"]', message).should('is.visible');
+    return this;
+  }
+
+  expectConfigToBeValid() {
+    cy.contains('[data-cy="board-card"]', this.name)
+      .scrollIntoView()
+      .should('is.visible');
     cy.get('[data-cy="board-form-title-input-error"]').should('not.visible');
     return this;
   }
 
-  assertErrorMessageVisible(message) {
-    cy.contains('[data-cy^="board-form-"]', message).should('is.visible');
+  expectConfigToBeInvalid() {
+    cy.contains('h2', 'Add new board').should('is.visible');
     return this;
   }
 
@@ -81,32 +88,10 @@ class Dashboard {
   }
 }
 
-export function addWidgetsDashboard(
-  name,
-  columns,
-  switchInterval,
-  expectedFailure
-) {
-  return new Dashboard(
-    name,
-    dashboardTypes.widgets,
-    columns,
-    switchInterval,
-    expectedFailure
-  );
+export function addWidgetsDashboard(name, columns, switchInterval) {
+  return new Dashboard(name, dashboardTypes.widgets, columns, switchInterval);
 }
 
-export function addIframeDashboard(
-  name,
-  columns,
-  switchInterval,
-  expectedFailure
-) {
-  return new Dashboard(
-    name,
-    dashboardTypes.iframe,
-    columns,
-    switchInterval,
-    expectedFailure
-  );
+export function addIframeDashboard(name, columns, switchInterval) {
+  return new Dashboard(name, dashboardTypes.iframe, columns, switchInterval);
 }