Merge branch 'master' into ce-to-ee

app/assets/javascripts/geo_nodes.js

 /* eslint-disable no-new*/
-import './smart_interval';
+import axios from 'axios';
+import SmartInterval from '~/smart_interval';
 import { parseSeconds, stringifyTime } from './lib/utils/pretty_time';
 
 const healthyClass = 'geo-node-healthy';
@@ -31,7 +32,7 @@ class GeoNodeStatus {
     this.$advancedStatus = $('.js-advanced-geo-node-status-toggler', this.$status);
     this.$advancedStatus.on('click', GeoNodeStatus.toggleShowAdvancedStatus);
 
-    this.statusInterval = new gl.SmartInterval({
+    this.statusInterval = new SmartInterval({
       callback: this.getStatus.bind(this),
       startingInterval: 30000,
       maxInterval: 120000,
@@ -59,14 +60,24 @@ class GeoNodeStatus {
 
   static formatCount(count) {
     if (count !== null) {
-      gl.text.addDelimiter(count);
+      return gl.text.addDelimiter(count);
     }
 
     return notAvailable;
   }
 
   getStatus() {
-    $.getJSON(this.endpoint, (status) => {
+    return axios.get(this.endpoint)
+      .then((response) => {
+        this.handleStatus(response.data);
+        return response;
+      })
+      .catch((err) => {
+        this.handleError(err);
+      });
+  }
+
+  handleStatus(status) {
     this.setStatusIcon(status.healthy);
     this.setHealthStatus(status.healthy);
 
@@ -111,6 +122,8 @@ class GeoNodeStatus {
 
     let eventDate = notAvailable;
     let cursorDate = notAvailable;
+    let lastEventSeen = notAvailable;
+    let lastCursorEvent = notAvailable;
 
     if (status.last_event_timestamp !== null && status.last_event_timestamp > 0) {
       eventDate = gl.utils.formatDate(new Date(status.last_event_timestamp * 1000));
@@ -120,8 +133,17 @@ class GeoNodeStatus {
       cursorDate = gl.utils.formatDate(new Date(status.cursor_last_event_timestamp * 1000));
     }
 
-      this.$lastEventSeen.text(`${status.last_event_id} (${eventDate})`);
-      this.$lastCursorEvent.text(`${status.cursor_last_event_id} (${cursorDate})`);
+    if (status.last_event_id !== null) {
+      lastEventSeen = `${status.last_event_id} (${eventDate})`;
+    }
+
+    if (status.cursor_last_event_id !== null) {
+      lastCursorEvent = `${status.cursor_last_event_id} (${cursorDate})`;
+    }
+
+    this.$lastEventSeen.text(lastEventSeen);
+    this.$lastCursorEvent.text(lastCursorEvent);
+
     if (status.health === 'Healthy') {
       this.$health.text('');
     } else {
@@ -130,7 +152,13 @@ class GeoNodeStatus {
     }
 
     this.$status.show();
-    });
+  }
+
+  handleError(err) {
+    this.setStatusIcon(false);
+    this.setHealthStatus(false);
+    this.$health.html(`<code class="geo-health">${err}</code>`);
+    this.$status.show();
   }
 
   setStatusIcon(healthy) {

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

@@ -27,7 +27,7 @@
       Geo nodes (#{@nodes.count})
     %ul.well-list.geo-nodes
       - @nodes.each do |node|
-        %li{ id: dom_id(node), class: node_class(node), data: { status_url: status_admin_geo_node_path(node) } }
+        %li{ id: dom_id(node), class: node_class(node), data: { status_url: status_admin_geo_node_path(node, format: :json) } }
           .node-block
             = node_status_icon(node)
             %strong= node.url

changelogs/unreleased-ee/dba-fix-nil-status-count.yml

+---
+title: 'Geo: Fix handling of nil values on advanced section in admin screen'
+merge_request:
+author:
+type: fixed

changelogs/unreleased/winh-subgroups-api.yml

+---
+title: Add /groups/:id/subgroups endpoint to API
+merge_request: 15142
+author: marbemac
+type: added

doc/api/groups.md

@@ -9,13 +9,13 @@ Parameters:
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `skip_groups` | array of integers | no | Skip the group IDs passes |
-| `all_available` | boolean | no | Show all the groups you have access to |
-| `search` | string | no | Return list of authorized groups matching the search criteria |
+| `skip_groups` | array of integers | no | Skip the group IDs passed |
+| `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users) |
+| `search` | string | no | Return the list of authorized groups matching the search criteria |
 | `order_by` | string | no | Order groups by `name` or `path`. Default is `name` |
 | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` |
 | `statistics` | boolean | no | Include group statistics (admins only) |
-| `owned` | boolean | no | Limit by groups owned by the current user |
+| `owned` | boolean | no | Limit to groups owned by the current user |
 
 ```
 GET /groups
@@ -80,6 +80,47 @@ You can filter by [custom attributes](custom_attributes.md) with:
 GET /groups?custom_attributes[key]=value&custom_attributes[other_key]=other_value
 ```
 
+## List a groups's subgroups
+
+Get a list of visible direct subgroups in this group.
+When accessed without authentication, only public groups are returned.
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) of the parent group |
+| `skip_groups` | array of integers | no | Skip the group IDs passed |
+| `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users) |
+| `search` | string | no | Return the list of authorized groups matching the search criteria |
+| `order_by` | string | no | Order groups by `name` or `path`. Default is `name` |
+| `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` |
+| `statistics` | boolean | no | Include group statistics (admins only) |
+| `owned` | boolean | no | Limit to groups owned by the current user |
+
+```
+GET /groups/:id/subgroups
+```
+
+```json
+[
+  {
+    "id": 1,
+    "name": "Foobar Group",
+    "path": "foo-bar",
+    "description": "An interesting group",
+    "visibility": "public",
+    "lfs_enabled": true,
+    "avatar_url": "http://gitlab.example.com/uploads/group/avatar/1/foo.jpg",
+    "web_url": "http://gitlab.example.com/groups/foo-bar",
+    "request_access_enabled": false,
+    "full_name": "Foobar Group",
+    "full_path": "foo-bar",
+    "parent_id": 123
+  }
+]
+```
+
 ## List a group's projects
 
 Get a list of projects in this group. When accessed without authentication, only

lib/api/groups.rb

@@ -34,24 +34,7 @@ module API
         optional :statistics, type: Boolean, default: false, desc: 'Include project statistics'
       end
 
-      def present_groups(groups, options = {})
-        options = options.reverse_merge(
-          with: Entities::Group,
-          current_user: current_user
-        )
-
-        groups = groups.with_statistics if options[:statistics]
-        present paginate(groups), options
-      end
-    end
-
-    resource :groups do
-      include CustomAttributesEndpoints
-
-      desc 'Get a groups list' do
-        success Entities::Group
-      end
-      params do
+      params :group_list_params do
         use :statistics_params
         optional :skip_groups, type: Array[Integer], desc: 'Array of group ids to exclude from list'
         optional :all_available, type: Boolean, desc: 'Show all group that you have access to'
@@ -61,19 +44,47 @@ module API
         optional :sort, type: String, values: %w[asc desc], default: 'asc', desc: 'Sort by asc (ascending) or desc (descending)'
         use :pagination
       end
-      get do
+
+      def find_groups(params)
         find_params = {
           all_available: params[:all_available],
-          owned: params[:owned],
-          custom_attributes: params[:custom_attributes]
+          custom_attributes: params[:custom_attributes],
+          owned: params[:owned]
         }
+        find_params[:parent] = find_group!(params[:id]) if params[:id]
 
         groups = GroupsFinder.new(current_user, find_params).execute
         groups = groups.search(params[:search]) if params[:search].present?
         groups = groups.where.not(id: params[:skip_groups]) if params[:skip_groups].present?
         groups = groups.reorder(params[:order_by] => params[:sort])
 
-        present_groups groups, statistics: params[:statistics] && current_user.admin?
+        groups
+      end
+
+      def present_groups(params, groups)
+        options = {
+          with: Entities::Group,
+          current_user: current_user,
+          statistics: params[:statistics] && current_user.admin?
+        }
+
+        groups = groups.with_statistics if options[:statistics]
+        present paginate(groups), options
+      end
+    end
+
+    resource :groups do
+      include CustomAttributesEndpoints
+
+      desc 'Get a groups list' do
+        success Entities::Group
+      end
+      params do
+        use :group_list_params
+      end
+      get do
+        groups = find_groups(params)
+        present_groups params, groups
       end
 
       desc 'Create a group. Available only for users who can create groups.' do
@@ -199,6 +210,17 @@ module API
         present paginate(projects), with: entity, current_user: current_user
       end
 
+      desc 'Get a list of subgroups in this group.' do
+        success Entities::Group
+      end
+      params do
+        use :group_list_params
+      end
+      get ":id/subgroups" do
+        groups = find_groups(params)
+        present_groups params, groups
+      end
+
       desc 'Transfer a project to the group namespace. Available only for admin.' do
         success Entities::GroupDetail
       end

spec/ee/spec/requests/git_http_geo_spec.rb

@@ -41,7 +41,7 @@ describe "Git HTTP requests (Geo)" do
       it { travel_to(2.minutes.ago) { is_expected.to have_gitlab_http_status(:unauthorized) } }
     end
 
-    xcontext 'expired Geo JWT token' do
+    context 'expired Geo JWT token' do
       let(:env) { valid_geo_env }
 
       it { travel_to(Time.now + 2.minutes) { is_expected.to have_gitlab_http_status(:unauthorized) } }

spec/javascripts/api_spec.js

@@ -171,7 +171,7 @@ describe('Api', () => {
     it('creates a new group label', (done) => {
       const namespace = 'some namespace';
       const labelData = { some: 'data' };
-      const expectedUrl = `${dummyUrlRoot}/groups/${namespace}/labels`;
+      const expectedUrl = Api.buildUrl(Api.groupLabelsPath).replace(':namespace_path', namespace);
       const expectedData = {
         label: labelData,
       };

spec/requests/api/groups_spec.rb

@@ -471,6 +471,142 @@ describe API::Groups do
     end
   end
 
+  describe 'GET /groups/:id/subgroups', :nested_groups do
+    let!(:subgroup1) { create(:group, parent: group1) }
+    let!(:subgroup2) { create(:group, :private, parent: group1) }
+    let!(:subgroup3) { create(:group, :private, parent: group2) }
+
+    context 'when unauthenticated' do
+      it 'returns only public subgroups' do
+        get api("/groups/#{group1.id}/subgroups")
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+        expect(json_response.first['id']).to eq(subgroup1.id)
+        expect(json_response.first['parent_id']).to eq(group1.id)
+      end
+
+      it 'returns 404 for a private group' do
+        get api("/groups/#{group2.id}/subgroups")
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    context 'when authenticated as user' do
+      context 'when user is not member of a public group' do
+        it 'returns no subgroups for the public group' do
+          get api("/groups/#{group1.id}/subgroups", user2)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(0)
+        end
+
+        context 'when using all_available in request' do
+          it 'returns public subgroups' do
+            get api("/groups/#{group1.id}/subgroups", user2), all_available: true
+
+            expect(response).to have_gitlab_http_status(200)
+            expect(json_response).to be_an Array
+            expect(json_response.length).to eq(1)
+            expect(json_response[0]['id']).to eq(subgroup1.id)
+            expect(json_response[0]['parent_id']).to eq(group1.id)
+          end
+        end
+      end
+
+      context 'when user is not member of a private group' do
+        it 'returns 404 for the private group' do
+          get api("/groups/#{group2.id}/subgroups", user1)
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+
+      context 'when user is member of public group' do
+        before do
+          group1.add_guest(user2)
+        end
+
+        it 'returns private subgroups' do
+          get api("/groups/#{group1.id}/subgroups", user2)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to include_pagination_headers
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(2)
+          private_subgroups = json_response.select { |group| group['visibility'] == 'private' }
+          expect(private_subgroups.length).to eq(1)
+          expect(private_subgroups.first['id']).to eq(subgroup2.id)
+          expect(private_subgroups.first['parent_id']).to eq(group1.id)
+        end
+
+        context 'when using statistics in request' do
+          it 'does not include statistics' do
+            get api("/groups/#{group1.id}/subgroups", user2), statistics: true
+
+            expect(response).to have_gitlab_http_status(200)
+            expect(json_response).to be_an Array
+            expect(json_response.first).not_to include 'statistics'
+          end
+        end
+      end
+
+      context 'when user is member of private group' do
+        before do
+          group2.add_guest(user1)
+        end
+
+        it 'returns subgroups' do
+          get api("/groups/#{group2.id}/subgroups", user1)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(1)
+          expect(json_response.first['id']).to eq(subgroup3.id)
+          expect(json_response.first['parent_id']).to eq(group2.id)
+        end
+      end
+    end
+
+    context 'when authenticated as admin' do
+      it 'returns private subgroups of a public group' do
+        get api("/groups/#{group1.id}/subgroups", admin)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(2)
+      end
+
+      it 'returns subgroups of a private group' do
+        get api("/groups/#{group2.id}/subgroups", admin)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+      end
+
+      it 'does not include statistics by default' do
+        get api("/groups/#{group1.id}/subgroups", admin)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.first).not_to include('statistics')
+      end
+
+      it 'includes statistics if requested' do
+        get api("/groups/#{group1.id}/subgroups", admin), statistics: true
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.first).to include('statistics')
+      end
+    end
+  end
+
   describe "POST /groups" do
     context "when authenticated as user without group permissions" do
       it "does not create group" do