Merge branch '1975-geo-page-ux' into 'master'

Resolve "Improve UX of Geo admin page"

Closes #1975

See merge request !1777

app/assets/javascripts/geo_nodes.js

 /* eslint-disable no-new*/
 import './smart_interval';
 
-const healthyClass = 'geo-node-icon-healthy';
-const unhealthyClass = 'geo-node-icon-unhealthy';
+const healthyClass = 'geo-node-healthy';
+const unhealthyClass = 'geo-node-unhealthy';
+const healthyIcon = 'fa-check';
+const unhealthyIcon = 'fa-close';
 
 class GeoNodeStatus {
   constructor(el) {
     this.$el = $(el);
     this.$icon = $('.js-geo-node-icon', this.$el);
+    this.$healthStatus = $('.js-health-status', this.$el);
     this.$status = $('.js-geo-node-status', this.$el);
     this.$repositoriesSynced = $('.js-repositories-synced', this.$status);
     this.$repositoriesFailed = $('.js-repositories-failed', this.$status);
@@ -29,11 +32,16 @@ class GeoNodeStatus {
   getStatus() {
     $.getJSON(this.endpoint, (status) => {
       this.setStatusIcon(status.healthy);
+      this.setHealthStatus(status.healthy);
       this.$repositoriesSynced.html(`${status.repositories_synced_count}/${status.repositories_count} (${status.repositories_synced_in_percentage})`);
       this.$repositoriesFailed.html(status.repositories_failed_count);
       this.$lfsObjectsSynced.html(`${status.lfs_objects_synced_count}/${status.lfs_objects_count} (${status.lfs_objects_synced_in_percentage})`);
       this.$attachmentsSynced.html(`${status.attachments_synced_count}/${status.attachments_count} (${status.attachments_synced_in_percentage})`);
-      this.$health.html(status.health);
+      if (status.health === 'Healthy') {
+        this.$health.html('');
+      } else {
+        this.$health.html(`<code class="geo-health">${status.health}</code>`);
+      }
 
       this.$status.show();
     });
@@ -41,16 +49,26 @@ class GeoNodeStatus {
 
   setStatusIcon(healthy) {
     if (healthy) {
-      this.$icon.removeClass(unhealthyClass)
-                .addClass(healthyClass)
+      this.$icon.removeClass(`${unhealthyClass} ${unhealthyIcon}`)
+                .addClass(`${healthyClass} ${healthyIcon}`)
                 .attr('title', 'Healthy');
     } else {
-      this.$icon.removeClass(healthyClass)
-                .addClass(unhealthyClass)
+      this.$icon.removeClass(`${healthyClass} ${healthyIcon}`)
+                .addClass(`${unhealthyClass} ${unhealthyIcon}`)
                 .attr('title', 'Unhealthy');
     }
+  }
 
-    this.$icon.tooltip('fixTitle');
+  setHealthStatus(healthy) {
+    if (healthy) {
+      this.$healthStatus.removeClass(unhealthyClass)
+                        .addClass(healthyClass)
+                        .html('Healthy');
+    } else {
+      this.$healthStatus.removeClass(healthyClass)
+                        .addClass(unhealthyClass)
+                        .html('Unhealthy');
+    }
   }
 }
 

app/assets/stylesheets/pages/geo_nodes.scss

-.geo-node-icon-healthy {
+.geo-node-healthy {
   color: $gl-success;
 }
 
-.geo-node-icon-unhealthy {
+.geo-node-unhealthy {
   color: $gl-danger;
 }
 
-.geo-node-icon-disabled {
+.geo-node-disabled {
   color: $gray-darkest;
 }
+
+.well-list.geo-nodes {
+  li {
+    position: relative;
+
+    &:hover {
+      background: $white-light;
+    }
+
+    &.node-disabled,
+    &.node-disabled:hover {
+      background-color: $gray-lightest;
+    }
+  }
+}
+
+.node-info {
+  color: $gl-text-color;
+}
+
+.geo-health {
+  display: inline-block;
+  margin-top: 5px;
+}
+
+.node-badge {
+  color: $white-light;
+  display: inline-block;
+  margin-left: 5px;
+  padding: 0 5px;
+  border-radius: 3px;
+
+  &.primary-node {
+    background-color: $blue-300;
+  }
+
+  &.current-node {
+    background-color: $green-400;
+  }
+}
+
+.node-actions {
+  margin-top: 10px;
+
+  @media (min-width: $screen-md-min) {
+    position: absolute;
+    right: 15px;
+    top: 0;
+  }
+
+  .btn:not(:first-of-type) {
+    margin-left: 10px;
+  }
+}

app/helpers/ee/geo_helper.rb

 module EE
   module GeoHelper
     def node_status_icon(node)
-      if node.primary?
-        icon 'star fw', class: 'has-tooltip', title: 'Primary node'
-      else
+      unless node.primary?
         status = node.enabled? ? 'healthy' : 'disabled'
+        icon = status == 'healthy' ? 'check' : 'times'
 
-        icon 'globe fw',
-             class: "js-geo-node-icon geo-node-icon-#{status} has-tooltip",
+        icon "#{icon} fw",
+             class: "js-geo-node-icon geo-node-#{status}",
              title: status.capitalize
       end
     end
 
+    def node_class(node)
+      klass = []
+      klass << 'js-geo-secondary-node' if node.secondary?
+      klass << 'node-disabled' unless node.enabled?
+      klass
+    end
+
     def toggle_node_button(node)
       btn_class, title, data =
         if node.enabled?
-          ['warning', 'Disable node', { confirm: 'Disabling a node stops the sync process. Are you sure?' }]
+          ['warning', 'Disable', { confirm: 'Disabling a node stops the sync process. Are you sure?' }]
         else
-          ['success', 'Enable node']
+          %w[success Enable]
         end
 
-      link_to icon('power-off fw', text: title),
+      link_to title,
               toggle_admin_geo_node_path(node),
               method: :post,
-              class: "btn btn-sm btn-#{btn_class} prepend-left-10 has-tooltip",
+              class: "btn btn-sm btn-#{btn_class}",
               title: title,
               data: data
     end

app/models/geo_node.rb

@@ -35,6 +35,10 @@ class GeoNode < ActiveRecord::Base
                  mode: :per_attribute_iv,
                  encode: true
 
+  def current?
+    Gitlab::Geo.current_node == self
+  end
+
   def secondary?
     !primary
   end

app/serializers/geo_node_status_entity.rb

@@ -5,7 +5,7 @@ class GeoNodeStatusEntity < Grape::Entity
 
   expose :healthy?, as: :healthy
   expose :health do |node|
-    node.healthy? ? 'No Health Problems Detected' : node.health
+    node.healthy? ? 'Healthy' : node.health
   end
 
   expose :attachments_count

app/views/admin/geo_nodes/index.html.haml

@@ -16,42 +16,46 @@
       Geo nodes (#{@nodes.count})
     %ul.well-list.geo-nodes
       - @nodes.each do |node|
-        %li{ id: dom_id(node), class: ('js-geo-secondary-node' if node.secondary?), data: { status_url: status_admin_geo_node_path(node) } }
-          .list-item-name
-            %span
+        %li{ id: dom_id(node), class: node_class(node), data: { status_url: status_admin_geo_node_path(node) } }
+          .node-block
             = node_status_icon(node)
             %strong= node.url
+            - if node.current?
+              .node-badge.current-node Current node
             - if node.primary?
+              .node-badge.primary-node Primary
               %span.help-block Primary node
             - else
               .js-geo-node-status{ style: 'display: none' }
+                - if node.enabled?
+                  %p
+                    %span.help-block
+                      Health Status:
+                      %span.js-health-status
                 %p
                   %span.help-block
                     Repositories synced:
-                    %span.js-repositories-synced
+                    %strong.node-info.js-repositories-synced
                 %p
                   %span.help-block
                     Repositories failed:
-                    %span.js-repositories-failed
+                    %strong.node-info.js-repositories-failed
                 %p
                   %span.help-block
                     LFS objects synced:
-                    %span.js-lfs-objects-synced
+                    %strong.node-info.js-lfs-objects-synced
                 %p
                   %span.help-block
                     Attachments synced:
-                    %span.js-attachments-synced
+                    %strong.node-info.js-attachments-synced
                 %p
-                  %span.help-block.js-health
+                  .js-health
 
-          .pull-right
+          - if Gitlab::Geo.primary?
+            .node-actions
               - if Gitlab::Geo.license_allows?
                 - if node.missing_oauth_application?
-                = link_to repair_admin_geo_node_path(node), method: :post, title: 'OAuth application is missing', class: 'btn btn-default btn-sm prepend-left-10' do
-                  = icon('exclamation-triangle fw')
-                  Repair authentication
+                  = link_to "Repair authentication", repair_admin_geo_node_path(node), method: :post, title: 'OAuth application is missing', class: 'btn btn-default btn-sm'
                 - if node.secondary?
                   = toggle_node_button(node)
-            = link_to admin_geo_node_path(node), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm prepend-left-10' do
-              = icon 'trash'
-              Remove
+              = link_to "Remove", admin_geo_node_path(node), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm'

spec/models/geo_node_spec.rb

@@ -111,6 +111,22 @@ describe GeoNode, type: :model do
     end
   end
 
+  describe '#current?' do
+    subject { described_class.new }
+
+    it 'returns true when node is the current node' do
+      allow(Gitlab::Geo).to receive(:current_node) { subject }
+
+      expect(subject.current?).to eq true
+    end
+
+    it 'returns false when node is not the current node' do
+      allow(Gitlab::Geo).to receive(:current_node) { double }
+
+      expect(subject.current?).to eq false
+    end
+  end
+
   describe '#uri' do
     context 'when all fields are filled' do
       it 'returns an URI object' do

spec/serializers/geo_node_status_entity_spec.rb

@@ -61,7 +61,7 @@ describe GeoNodeStatusEntity do
   describe '#health' do
     context 'when node is healthy' do
       it 'exposes the health message' do
-        expect(subject[:health]).to eq 'No Health Problems Detected'
+        expect(subject[:health]).to eq 'Healthy'
       end
     end