Automatic merge of gitlab-org/gitlab-ce master

app/helpers/storage_helper.rb

@@ -6,4 +6,14 @@ module StorageHelper
 
     number_to_human_size(size_in_bytes, delimiter: ',', precision: precision, significant: false)
   end
+
+  def storage_counters_details(statistics)
+    counters = {
+      counter_repositories: storage_counter(statistics.repository_size),
+      counter_build_artifacts: storage_counter(statistics.build_artifacts_size),
+      counter_lfs_objects: storage_counter(statistics.lfs_objects_size)
+    }
+
+    _("%{counter_repositories} repositories, %{counter_build_artifacts} build artifacts, %{counter_lfs_objects} LFS") % counters
+  end
 end

app/models/namespace.rb

@@ -77,7 +77,8 @@ class Namespace < ApplicationRecord
         'COALESCE(SUM(ps.storage_size), 0) AS storage_size',
         'COALESCE(SUM(ps.repository_size), 0) AS repository_size',
         'COALESCE(SUM(ps.lfs_objects_size), 0) AS lfs_objects_size',
-        'COALESCE(SUM(ps.build_artifacts_size), 0) AS build_artifacts_size'
+        'COALESCE(SUM(ps.build_artifacts_size), 0) AS build_artifacts_size',
+        'COALESCE(SUM(ps.packages_size), 0) AS packages_size'
       )
   end
 

app/models/project_services/chat_message/deployment_message.rb

@@ -2,27 +2,31 @@
 
 module ChatMessage
   class DeploymentMessage < BaseMessage
+    attr_reader :commit_title
     attr_reader :commit_url
     attr_reader :deployable_id
     attr_reader :deployable_url
     attr_reader :environment
     attr_reader :short_sha
     attr_reader :status
+    attr_reader :user_url
 
     def initialize(data)
       super
 
+      @commit_title = data[:commit_title]
       @commit_url = data[:commit_url]
       @deployable_id = data[:deployable_id]
       @deployable_url = data[:deployable_url]
       @environment = data[:environment]
       @short_sha = data[:short_sha]
       @status = data[:status]
+      @user_url = data[:user_url]
     end
 
     def attachments
       [{
-        text: "#{project_link}\n#{deployment_link}, SHA #{commit_link}, by #{user_combined_name}",
+        text: "#{project_link} with job #{deployment_link} by #{user_link}\n#{commit_link}: #{commit_title}",
         color: color
       }]
     end
@@ -55,7 +59,11 @@ module ChatMessage
     end
 
     def deployment_link
-      link("Job ##{deployable_id}", deployable_url)
+      link("##{deployable_id}", deployable_url)
+    end
+
+    def user_link
+      link(user_combined_name, user_url)
     end
 
     def commit_link

app/models/project_statistics.rb

@@ -7,7 +7,7 @@ class ProjectStatistics < ApplicationRecord
   before_save :update_storage_size
 
   COLUMNS_TO_REFRESH = [:repository_size, :lfs_objects_size, :commit_count].freeze
-  INCREMENTABLE_COLUMNS = { build_artifacts_size: %i[storage_size] }.freeze
+  INCREMENTABLE_COLUMNS = { build_artifacts_size: %i[storage_size], packages_size: %i[storage_size] }.freeze
 
   def total_repository_size
     repository_size + lfs_objects_size
@@ -36,8 +36,13 @@ class ProjectStatistics < ApplicationRecord
     self.lfs_objects_size = project.lfs_objects.sum(:size)
   end
 
+  # older migrations fail due to non-existent attribute without this
+  def packages_size
+    has_attribute?(:packages_size) ? super.to_i : 0
+  end
+
   def update_storage_size
-    self.storage_size = repository_size + lfs_objects_size + build_artifacts_size
+    self.storage_size = repository_size + lfs_objects_size + build_artifacts_size + packages_size
   end
 
   # Since this incremental update method does not call update_storage_size above,

app/views/admin/groups/show.html.haml

@@ -44,12 +44,10 @@
 
         %li
           %span.light= _('Storage:')
-          - counter_storage = storage_counter(@group.storage_size)
-          - counter_repositories = storage_counter(@group.repository_size)
-          - counter_build_artifacts = storage_counter(@group.build_artifacts_size)
-          - counter_lfs_objects = storage_counter(@group.lfs_objects_size)
-          %strong
-            = _("%{counter_storage} (%{counter_repositories} repositories, %{counter_build_artifacts} build artifacts, %{counter_lfs_objects} LFS)") % { counter_storage: counter_storage, counter_repositories: counter_repositories, counter_build_artifacts: counter_build_artifacts, counter_lfs_objects: counter_lfs_objects }
+          %strong= storage_counter(@group.storage_size)
+          (
+          = storage_counters_details(@group)
+          )
 
         %li
           %span.light= _('Group Git LFS status:')

app/views/admin/projects/show.html.haml

@@ -73,15 +73,10 @@
               = @project.repository.relative_path
 
           %li
-            %span.light Storage used:
+            %span.light= _('Storage:')
             %strong= storage_counter(@project.statistics.storage_size)
             (
-            = storage_counter(@project.statistics.repository_size)
-            repository,
-            = storage_counter(@project.statistics.build_artifacts_size)
-            build artifacts,
-            = storage_counter(@project.statistics.lfs_objects_size)
-            LFS
+            = storage_counters_details(@project.statistics)
             )
 
           %li

changelogs/unreleased/ac-package-storage-stats.yml

+---
+title: Add packages_size to ProjectStatistics
+merge_request: 27373
+author:
+type: added

changelogs/unreleased/issue-61038-deploy-chat-message-update.yml

+---
+title: Update deployment event chat notification message
+merge_request: 27972
+author:
+type: changed

db/migrate/20190415095825_add_packages_size_to_project_statistics.rb

+# frozen_string_literal: true
+
+class AddPackagesSizeToProjectStatistics < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :project_statistics, :packages_size, :bigint
+  end
+end

lib/gitlab/data_builder/deployment.rb

@@ -15,7 +15,9 @@ module Gitlab
           project: deployment.project.hook_attrs,
           short_sha: deployment.short_sha,
           user: deployment.user.hook_attrs,
-          commit_url: Gitlab::UrlBuilder.build(deployment.commit)
+          user_url: Gitlab::UrlBuilder.build(deployment.user),
+          commit_url: Gitlab::UrlBuilder.build(deployment.commit),
+          commit_title: deployment.commit.title
         }
       end
     end

lib/gitlab/url_builder.rb

@@ -32,6 +32,8 @@ module Gitlab
         milestone_url(object)
       when ::Ci::Build
         project_job_url(object.project, object)
+      when User
+        user_url(object)
       else
         raise NotImplementedError.new("No URL builder defined for #{object.class}")
       end

locale/gitlab.pot

@@ -142,7 +142,7 @@ msgstr ""
 msgid "%{commit_author_link} authored %{commit_timeago}"
 msgstr ""
 
-msgid "%{counter_storage} (%{counter_repositories} repositories, %{counter_build_artifacts} build artifacts, %{counter_lfs_objects} LFS)"
+msgid "%{counter_repositories} repositories, %{counter_build_artifacts} build artifacts, %{counter_lfs_objects} LFS"
 msgstr ""
 
 msgid "%{count} approval required from %{name}"

spec/helpers/storage_helper_spec.rb

@@ -18,4 +18,28 @@ describe StorageHelper do
       expect(helper.storage_counter(100_000_000_000_000_000_000_000)).to eq("86,736.2 EB")
     end
   end
+
+  describe "#storage_counters_details" do
+    let(:namespace) { create :namespace }
+    let(:project) do
+      create(:project,
+             namespace: namespace,
+             statistics: build(:project_statistics,
+                               repository_size:      10.kilobytes,
+                               lfs_objects_size:     20.gigabytes,
+                               build_artifacts_size: 30.megabytes))
+    end
+
+    let(:message) { '10 KB repositories, 30 MB build artifacts, 20 GB LFS' }
+
+    it 'works on ProjectStatistics' do
+      expect(helper.storage_counters_details(project.statistics)).to eq(message)
+    end
+
+    it 'works on Namespace.with_statistics' do
+      namespace_stats = Namespace.with_statistics.find(project.namespace.id)
+
+      expect(helper.storage_counters_details(namespace_stats)).to eq(message)
+    end
+  end
 end

spec/lib/gitlab/data_builder/deployment_spec.rb

@@ -19,6 +19,7 @@ describe Gitlab::DataBuilder::Deployment do
       deployment = create(:deployment, status: :failed, environment: environment, sha: commit.sha, project: project)
       deployable = deployment.deployable
       expected_deployable_url = Gitlab::Routing.url_helpers.project_job_url(deployable.project, deployable)
+      expected_user_url = Gitlab::Routing.url_helpers.user_url(deployment.user)
       expected_commit_url = Gitlab::UrlBuilder.build(commit)
 
       data = described_class.build(deployment)
@@ -30,7 +31,9 @@ describe Gitlab::DataBuilder::Deployment do
       expect(data[:project]).to eq(project.hook_attrs)
       expect(data[:short_sha]).to eq(deployment.short_sha)
       expect(data[:user]).to eq(deployment.user.hook_attrs)
+      expect(data[:user_url]).to eq(expected_user_url)
       expect(data[:commit_url]).to eq(expected_commit_url)
+      expect(data[:commit_title]).to eq(commit.title)
     end
   end
 end

spec/models/namespace_spec.rb

@@ -146,20 +146,20 @@ describe Namespace do
       create(:project,
              namespace: namespace,
              statistics: build(:project_statistics,
-                               storage_size:         606,
                                repository_size:      101,
                                lfs_objects_size:     202,
-                               build_artifacts_size: 303))
+                               build_artifacts_size: 303,
+                               packages_size:        404))
     end
 
     let(:project2) do
       create(:project,
              namespace: namespace,
              statistics: build(:project_statistics,
-                               storage_size:         60,
                                repository_size:      10,
                                lfs_objects_size:     20,
-                               build_artifacts_size: 30))
+                               build_artifacts_size: 30,
+                               packages_size:        40))
     end
 
     it "sums all project storage counters in the namespace" do
@@ -167,10 +167,11 @@ describe Namespace do
       project2
       statistics = described_class.with_statistics.find(namespace.id)
 
-      expect(statistics.storage_size).to eq 666
+      expect(statistics.storage_size).to eq 1110
       expect(statistics.repository_size).to eq 111
       expect(statistics.lfs_objects_size).to eq 222
       expect(statistics.build_artifacts_size).to eq 333
+      expect(statistics.packages_size).to eq 444
     end
 
     it "correctly handles namespaces without projects" do
@@ -180,6 +181,7 @@ describe Namespace do
       expect(statistics.repository_size).to eq 0
       expect(statistics.lfs_objects_size).to eq 0
       expect(statistics.build_artifacts_size).to eq 0
+      expect(statistics.packages_size).to eq 0
     end
   end
 

spec/models/project_services/chat_message/deployment_message_spec.rb

@@ -89,8 +89,10 @@ describe ChatMessage::DeploymentMessage do
           name: "Jane Person",
           username: "jane"
         },
+        user_url: "user_url",
         short_sha: "12345678",
-        commit_url: "commit_url"
+        commit_url: "commit_url",
+        commit_title: "commit title text"
       }.merge(params)
     end
 
@@ -104,12 +106,13 @@ describe ChatMessage::DeploymentMessage do
       deployment = create(:deployment, :success, deployable: ci_build, environment: environment, project: project, user: user, sha: commit.sha)
       job_url = Gitlab::Routing.url_helpers.project_job_url(project, ci_build)
       commit_url = Gitlab::UrlBuilder.build(deployment.commit)
+      user_url = Gitlab::Routing.url_helpers.user_url(user)
       data = Gitlab::DataBuilder::Deployment.build(deployment)
 
       message = described_class.new(data)
 
       expect(message.attachments).to eq([{
-        text: "[myspace/myproject](#{project.web_url})\n[Job ##{ci_build.id}](#{job_url}), SHA [#{deployment.short_sha}](#{commit_url}), by John Smith (smith)",
+        text: "[myspace/myproject](#{project.web_url}) with job [##{ci_build.id}](#{job_url}) by [John Smith (smith)](#{user_url})\n[#{deployment.short_sha}](#{commit_url}): #{commit.title}",
         color: "good"
       }])
     end
@@ -120,7 +123,7 @@ describe ChatMessage::DeploymentMessage do
       message = described_class.new(data)
 
       expect(message.attachments).to eq([{
-        text: "[project_path_with_namespace](project_web_url)\n[Job #3](deployable_url), SHA [12345678](commit_url), by Jane Person (jane)",
+        text: "[project_path_with_namespace](project_web_url) with job [#3](deployable_url) by [Jane Person (jane)](user_url)\n[12345678](commit_url): commit title text",
         color: "danger"
       }])
     end
@@ -131,7 +134,7 @@ describe ChatMessage::DeploymentMessage do
       message = described_class.new(data)
 
       expect(message.attachments).to eq([{
-        text: "[project_path_with_namespace](project_web_url)\n[Job #3](deployable_url), SHA [12345678](commit_url), by Jane Person (jane)",
+        text: "[project_path_with_namespace](project_web_url) with job [#3](deployable_url) by [Jane Person (jane)](user_url)\n[12345678](commit_url): commit title text",
         color: "warning"
       }])
     end
@@ -142,7 +145,7 @@ describe ChatMessage::DeploymentMessage do
       message = described_class.new(data)
 
       expect(message.attachments).to eq([{
-        text: "[project_path_with_namespace](project_web_url)\n[Job #3](deployable_url), SHA [12345678](commit_url), by Jane Person (jane)",
+        text: "[project_path_with_namespace](project_web_url) with job [#3](deployable_url) by [Jane Person (jane)](user_url)\n[12345678](commit_url): commit title text",
         color: "#334455"
       }])
     end

spec/models/project_statistics_spec.rb

@@ -124,16 +124,30 @@ describe ProjectStatistics do
   end
 
   describe '.increment_statistic' do
-    it 'increases the statistic by that amount' do
-      expect { described_class.increment_statistic(project.id, :build_artifacts_size, 13) }
-        .to change { statistics.reload.build_artifacts_size }
-        .by(13)
+    shared_examples 'a statistic that increases storage_size' do
+      it 'increases the statistic by that amount' do
+        expect { described_class.increment_statistic(project.id, stat, 13) }
+          .to change { statistics.reload.send(stat) || 0 }
+          .by(13)
+      end
+
+      it 'increases also storage size by that amount' do
+        expect { described_class.increment_statistic(project.id, stat, 20) }
+         .to change { statistics.reload.storage_size }
+         .by(20)
+      end
     end
 
-    it 'increases also storage size by that amount' do
-      expect { described_class.increment_statistic(project.id, :build_artifacts_size, 20) }
-        .to change { statistics.reload.storage_size }
-        .by(20)
+    context 'when adjusting :build_artifacts_size' do
+      let(:stat) { :build_artifacts_size }
+
+      it_behaves_like 'a statistic that increases storage_size'
+    end
+
+    context 'when adjusting :packages_size' do
+      let(:stat) { :packages_size }
+
+      it_behaves_like 'a statistic that increases storage_size'
     end
 
     context 'when the amount is 0' do