#435: display statistics by issues and pull-requests

app/assets/javascripts/angularjs/controllers/statistics_controller.js.coffee

@@ -137,15 +137,14 @@ RosaABF.controller 'StatisticsController', ['$scope', '$http', ($scope, $http) -
   $scope.initPullRequestsChart = ->
     $scope.dateChart '#pull_requests_chart', [
       $scope.statistics.pull_requests.open,
+      $scope.statistics.pull_requests.merged
       $scope.statistics.pull_requests.closed,
-      $scope.statistics.pull_requests.approved
     ]
 
   $scope.initIssuesChart = ->
     $scope.dateChart '#issues_chart', [
       $scope.statistics.issues.open,
-      $scope.statistics.issues.closed,
+      $scope.statistics.issues.closed
-      $scope.statistics.issues.approved
     ]
 
 ]

app/controllers/api/v1/pull_requests_controller.rb

@@ -49,10 +49,11 @@ class Api::V1::PullRequestsController < Api::V1::BaseController
 
     @pull = @project.pull_requests.new
     @pull.build_issue title: pull_params[:title], body: pull_params[:body]
-    @pull.from_project = @project
+    @pull.from_project            = @project
-    @pull.to_ref, @pull.from_ref = pull_params[:to_ref], pull_params[:from_ref]
+    @pull.to_ref, @pull.from_ref  = pull_params[:to_ref], pull_params[:from_ref]
-    @pull.issue.assignee_id = pull_params[:assignee_id] if can?(:write, @project)
+    @pull.issue.assignee_id       = pull_params[:assignee_id] if can?(:write, @project)
     @pull.issue.user, @pull.issue.project = current_user, @project
+    @pull.issue.new_pull_request  = true
     render_validation_error(@pull, "#{@pull.class.name} has not been created") && return unless @pull.valid?
 
     @pull.save # set pull id

app/controllers/projects/pull_requests_controller.rb

@@ -39,8 +39,9 @@ class Projects::PullRequestsController < Projects::BaseController
     @pull = to_project.pull_requests.new pull_params
     @pull.issue.assignee_id = (params[:issue] || {})[:assignee_id] if can?(:write, to_project)
     @pull.issue.user, @pull.issue.project, @pull.from_project = current_user, to_project, @project
-    @pull.from_project_owner_uname = @pull.from_project.owner.uname
+    @pull.from_project_owner_uname  = @pull.from_project.owner.uname
-    @pull.from_project_name = @pull.from_project.name
+    @pull.from_project_name         = @pull.from_project.name
+    @pull.issue.new_pull_request    = true
 
     if @pull.valid? # FIXME more clean/clever logics
       @pull.save # set pull id

app/models/issue.rb

@@ -1,16 +1,37 @@
 class Issue < ActiveRecord::Base
   include Feed::Issue
-  STATUSES = ['open', 'closed']
+
+  STATUSES = [
+    STATUS_OPEN   = 'open',
+    STATUS_CLOSED = 'closed'
+  ]
+  HASH_TAG_REGEXP = /([a-zA-Z0-9\-_]*\/)?([a-zA-Z0-9\-_]*)?#([0-9]+)/
 
   belongs_to :project
   belongs_to :user
-  belongs_to :assignee, class_name: 'User', foreign_key: 'assignee_id'
+  belongs_to :assignee,
-  belongs_to :closer, class_name: 'User', foreign_key: 'closed_by'
+              class_name:   'User',
+              foreign_key:  'assignee_id'
+
+  belongs_to :closer,
+              class_name:   'User',
+              foreign_key:  'closed_by'
+
+  has_many :comments,
+            as:         :commentable,
+            dependent:  :destroy
+
+  has_many :subscribes,
+            as:         :subscribeable,
+            dependent:  :destroy
+
+  has_many :labelings,
+            dependent:  :destroy
+
+  has_many :labels,
+            -> { uniq },
+            through:    :labelings
 
-  has_many :comments, as: :commentable, dependent: :destroy
-  has_many :subscribes, as: :subscribeable, dependent: :destroy
-  has_many :labelings, dependent: :destroy
-  has_many :labels, -> { uniq }, through: :labelings
   has_one :pull_request#, dependent: :destroy
 
   validates :title, :body, :project_id, presence: true
@@ -19,21 +40,28 @@ class Issue < ActiveRecord::Base
   after_create :subscribe_users
   after_update :subscribe_issue_assigned_user
 
+  before_create :update_statistic
+  before_update :update_statistic
+
   attr_accessible :labelings_attributes, :title, :body, :assignee_id
   accepts_nested_attributes_for :labelings, allow_destroy: true
 
-  scope :opened, -> { where(status: 'open') }
+  scope :opened, -> { where(status: STATUS_OPEN) }
-  scope :closed, -> { where(status: 'closed') }
+  scope :closed, -> { where(status: STATUS_CLOSED) }
 
-  scope :needed_checking, -> { where(issues: {status: ['open', 'blocked', 'ready', 'already']}) }
+  scope :needed_checking,       -> { where(issues: { status: %w(open blocked ready already) }) }
-  scope :not_closed_or_merged, -> { needed_checking }
+  scope :not_closed_or_merged,  -> { needed_checking }
-  scope :closed_or_merged, -> { where(issues: {status: ['closed', 'merged']}) }
+  scope :closed_or_merged,      -> { where(issues: { status: %w(closed merged) }) }
   # Using mb_chars for correct transform to lowercase ('Русский Текст'.downcase => "Русский Текст")
-  scope :search, ->(q) { where("#{table_name}.title ILIKE ?", "%#{q.mb_chars.downcase}%") if q.present? }
+  scope :search,                ->(q) {
+    where("#{table_name}.title ILIKE ?", "%#{q.mb_chars.downcase}%") if q.present?
+  }
   scope :without_pull_requests, -> {
     where('NOT EXISTS (select null from pull_requests as pr where pr.issue_id = issues.id)')
   }
 
+  attr_accessor :new_pull_request
+
   def assign_uname
     assignee.uname if assignee
   end
@@ -43,24 +71,24 @@ class Issue < ActiveRecord::Base
   end
 
   def subscribe_creator(creator_id)
-    if !self.subscribes.exists?(user_id: creator_id)
+    unless self.subscribes.exists?(user_id: creator_id)
       self.subscribes.create(user_id: creator_id)
     end
   end
 
   def closed?
-    closed_by && closed_at && status == 'closed'
+    closed_by && closed_at && status == STATUS_CLOSED
   end
 
   def set_close(closed_by)
-    self.closed_at = Time.now.utc
+    self.closed_at  = Time.now.utc
-    self.closer = closed_by
+    self.closer     = closed_by
-    self.status = 'closed'
+    self.status     = STATUS_CLOSED
   end
 
   def set_open
-    self.closed_at = self.closed_by = nil
+    self.closed_at  = self.closed_by = nil
-    self.status = 'open'
+    self.status     = STATUS_OPEN
   end
 
   def collect_recipients
@@ -69,12 +97,12 @@ class Issue < ActiveRecord::Base
     recipients
   end
 
-  def self.find_by_hash_tag hash_tag, current_ability, project
+  def self.find_by_hash_tag(hash_tag, current_ability, project)
-    hash_tag =~ /([a-zA-Z0-9\-_]*\/)?([a-zA-Z0-9\-_]*)?#([0-9]+)/
+    hash_tag =~ HASH_TAG_REGEXP
-    owner_uname = Regexp.last_match[1].presence || Regexp.last_match[2].presence || project.owner.uname
+    owner_uname   = Regexp.last_match[1].presence || Regexp.last_match[2].presence || project.owner.uname
-    project_name = Regexp.last_match[1] ? Regexp.last_match[2] : project.name
+    project_name  = Regexp.last_match[1] ? Regexp.last_match[2] : project.name
-    serial_id = Regexp.last_match[3]
+    serial_id     = Regexp.last_match[3]
-    project = Project.find_by_owner_and_name(owner_uname.chomp('/'), project_name)
+    project       = Project.find_by_owner_and_name(owner_uname.chomp('/'), project_name)
     return nil unless project
     return nil unless current_ability.can? :show, project
     project.issues.where(serial_id: serial_id).first
@@ -82,6 +110,16 @@ class Issue < ActiveRecord::Base
 
   protected
 
+  def update_statistic
+    key = (pull_request || new_pull_request) ? Statistic::KEY_PULL_REQUEST : Statistic::KEY_ISSUE
+    Statistic.statsd_increment(
+      activity_at:  Time.now,
+      key:          "#{key}.#{status}",
+      project_id:   project_id,
+      user_id:      closed_by || user_id,
+    ) if new_record? || status_changed?
+  end
+
   def set_serial_id
     self.serial_id = self.project.issues.count
     self.save!

app/models/pull_request.rb

@@ -1,10 +1,40 @@
 class PullRequest < ActiveRecord::Base
-  STATUSES = %w(ready already blocked merged closed)
+  STATUSES = [
-  belongs_to :issue, autosave: true, dependent: :destroy, touch: true, validate: true
+    STATUS_OPEN     = 'open',
-  belongs_to :to_project, class_name: 'Project', foreign_key: 'to_project_id'
+    STATUS_READY    = 'ready',
-  belongs_to :from_project, class_name: 'Project', foreign_key: 'from_project_id'
+    STATUS_ALREADY  = 'already',
-  delegate :user, :user_id, :title, :body, :serial_id, :assignee, :status, :to_param,
+    STATUS_BLOCKED  = 'blocked',
-    :created_at, :updated_at, :comments, :status=, to: :issue, allow_nil: true
+    STATUS_MERGED   = 'merged',
+    STATUS_CLOSED   = 'closed'
+  ]
+
+  belongs_to :issue,
+              autosave:     true,
+              dependent:    :destroy,
+              touch:        true,
+              validate:     true
+
+  belongs_to :to_project,
+              class_name:   'Project',
+              foreign_key:  'to_project_id'
+
+  belongs_to :from_project,
+              class_name:   'Project',
+              foreign_key:  'from_project_id'
+
+  delegate  :user,
+            :user_id,
+            :title,
+            :body,
+            :serial_id,
+            :assignee,
+            :status,
+            :to_param,
+            :created_at,
+            :updated_at,
+            :comments,
+            :status=,
+            to: :issue, allow_nil: true
 
   validates :from_project, :to_project, presence: true
   validate :uniq_merge, if: ->(pull) { pull.to_project.present? }
@@ -19,9 +49,9 @@ class PullRequest < ActiveRecord::Base
   accepts_nested_attributes_for :issue
   attr_accessible :issue_attributes, :to_ref, :from_ref
 
-  scope :needed_checking, -> { includes(:issue).where(issues: {status: ['open', 'blocked', 'ready']}) }
+  scope :needed_checking,       -> { includes(:issue).where(issues: { status: [STATUS_OPEN, STATUS_BLOCKED, STATUS_READY] }) }
-  scope :not_closed_or_merged, -> { needed_checking }
+  scope :not_closed_or_merged,  -> { needed_checking }
-  scope :closed_or_merged, -> { where(issues: {status: ['closed', 'merged']}) }
+  scope :closed_or_merged,      -> { where(issues: { status: [STATUS_CLOSED, STATUS_MERGED] }) }
 
   state_machine :status, initial: :open do
     event :ready do
@@ -53,8 +83,8 @@ class PullRequest < ActiveRecord::Base
     FileUtils.mv path(old_from_project_name), path, force: true if old_from_project_name
     return unless Dir.exists?(path)
     Dir.chdir(path) do
-      system 'git', 'remote', 'set-url', 'origin', to_project.path
+      system 'git', 'remote', 'set-url', 'origin',  to_project.path
-      system 'git', 'remote', 'set-url', 'head', from_project.path if cross_pull?
+      system 'git', 'remote', 'set-url', 'head',    from_project.path if cross_pull?
     end
   end
   later :update_relations, queue: :middle
@@ -87,7 +117,7 @@ class PullRequest < ActiveRecord::Base
       new_status == 'already' ? (ready; merging) : send(new_status)
       self.update_inline_comments
     else
-      self.status = new_status == 'block' ? 'blocked' : new_status
+      self.status = new_status == 'block' ? STATUS_BLOCKED : new_status
     end
   end
 
@@ -140,7 +170,7 @@ class PullRequest < ActiveRecord::Base
 
   def set_user_and_time user
     issue.closed_at = Time.now.utc
-    issue.closer = user
+    issue.closer    = user
   end
 
   def self.check_ref(record, attr, value)
@@ -163,8 +193,7 @@ class PullRequest < ActiveRecord::Base
   end
 
   def repo
-    return @repo if @repo.present? #&& !id_changed?
+    @repo ||= Grit::Repo.new(path)
-    @repo = Grit::Repo.new path
   end
 
   def from_commit
@@ -243,6 +272,6 @@ class PullRequest < ActiveRecord::Base
 
   def set_add_data
     self.from_project_owner_uname = from_project.owner.uname
-    self.from_project_name = from_project.name
+    self.from_project_name        = from_project.name
   end
 end

app/models/statistic.rb

@@ -5,7 +5,14 @@ class Statistic < ActiveRecord::Base
     KEY_BUILD_LIST_BUILD_STARTED    = "#{KEY_BUILD_LIST}.#{BuildList::BUILD_STARTED}",
     KEY_BUILD_LIST_SUCCESS          = "#{KEY_BUILD_LIST}.#{BuildList::SUCCESS}",
     KEY_BUILD_LIST_BUILD_ERROR      = "#{KEY_BUILD_LIST}.#{BuildList::BUILD_ERROR}",
-    KEY_BUILD_LIST_BUILD_PUBLISHED  = "#{KEY_BUILD_LIST}.#{BuildList::BUILD_PUBLISHED}"
+    KEY_BUILD_LIST_BUILD_PUBLISHED  = "#{KEY_BUILD_LIST}.#{BuildList::BUILD_PUBLISHED}",
+    KEY_ISSUE                       = 'issue',
+    KEY_ISSUES_OPEN                 = "#{KEY_ISSUE}.#{Issue::STATUS_OPEN}",
+    KEY_ISSUES_CLOSED               = "#{KEY_ISSUE}.#{Issue::STATUS_CLOSED}",
+    KEY_PULL_REQUEST                = 'pull_request',
+    KEY_PULL_REQUESTS_OPEN          = "#{KEY_PULL_REQUEST}.#{PullRequest::STATUS_OPEN}",
+    KEY_PULL_REQUESTS_MERGED        = "#{KEY_PULL_REQUEST}.#{PullRequest::STATUS_MERGED}",
+    KEY_PULL_REQUESTS_CLOSED        = "#{KEY_PULL_REQUEST}.#{PullRequest::STATUS_CLOSED}",
   ]
 
   belongs_to :user
@@ -48,7 +55,11 @@ class Statistic < ActiveRecord::Base
   scope :build_lists_error,     -> { where(key: KEY_BUILD_LIST_BUILD_ERROR) }
   scope :build_lists_published, -> { where(key: KEY_BUILD_LIST_BUILD_PUBLISHED) }
   scope :commits,               -> { where(key: KEY_COMMIT) }
-
+  scope :issues_open,           -> { where(key: KEY_ISSUES_OPEN) }
+  scope :issues_closed,         -> { where(key: KEY_ISSUES_CLOSED) }
+  scope :pull_requests_open,    -> { where(key: KEY_PULL_REQUESTS_OPEN) }
+  scope :pull_requests_merged,  -> { where(key: KEY_PULL_REQUESTS_MERGED) }
+  scope :pull_requests_closed,  -> { where(key: KEY_PULL_REQUESTS_CLOSED) }
 
 
   def self.now_statsd_increment(activity_at: nil, user_id: nil, project_id: nil, key: nil, counter: 1)

app/presenters/statistic_presenter.rb

@@ -24,6 +24,22 @@ class StatisticPresenter < ApplicationPresenter
       commits: {
         chart:                  prepare_collection(commits_chart),
         commits_count:          commits_chart.sum(&:count)
+      },
+      issues: {
+        open:                   prepare_collection(issues_open),
+        closed:                 prepare_collection(issues_closed),
+
+        open_count:             issues_open.sum(&:count),
+        closed_count:           issues_closed.sum(&:count)
+      },
+      pull_requests: {
+        open:                   prepare_collection(pull_requests_open),
+        merged:                 prepare_collection(pull_requests_merged),
+        closed:                 prepare_collection(pull_requests_closed),
+
+        open_count:             pull_requests_open.sum(&:count),
+        merged_count:           pull_requests_merged.sum(&:count),
+        closed_count:           pull_requests_closed.sum(&:count)
       }
     }
   end
@@ -36,6 +52,26 @@ class StatisticPresenter < ApplicationPresenter
       group("date_trunc('#{ unit }', activity_at)").order('activity_at')
   end
 
+  def issues_open
+    @issues_open ||= scope.issues_open.to_a
+  end
+
+  def issues_closed
+    @issues_closed ||= scope.issues_closed.to_a
+  end
+
+  def pull_requests_open
+    @pull_requests_open ||= scope.pull_requests_open.to_a
+  end
+
+  def pull_requests_merged
+    @pull_requests_merged ||= scope.pull_requests_merged.to_a
+  end
+
+  def pull_requests_closed
+    @pull_requests_closed ||= scope.pull_requests_closed.to_a
+  end
+
   def commits_chart
     @commits_chart ||= scope.commits.to_a
   end

app/views/statistics/_issues.html.haml

@@ -0,0 +1,37 @@
+.row
+  .span12
+    %h3.text-info
+      = t('.header')
+
+.row
+  .span8
+    .graph-wrapper
+      %h3
+        %span.graph-key-color1
+        = t('.open_title')
+        %span.graph-key-color2
+        = t('.closed_title')
+      .centered.graph-loading{ ng_show: 'loading' }
+        = image_tag 'loading-large.gif'
+      .no-data{ ng_hide: 'loading || statistics.issues' }
+        = t('.no_data')
+      %canvas#issues_chart{ ng_show: 'statistics.issues' }
+
+  .span3
+    .panel-wrapper
+      %h3
+        = t('.total_open')
+      .panel-data
+        = image_tag 'loading-small.gif', ng_show: 'loading'
+        .no-data{ ng_hide: 'loading || statistics.issues.open_count >= 0' }
+          = t('.no_data')
+        {{ statistics.issues.open_count | number }}
+
+    .panel-wrapper
+      %h3
+        = t('.total_closed')
+      .panel-data
+        = image_tag 'loading-small.gif', ng_show: 'loading'
+        .no-data{ ng_hide: 'loading || statistics.issues.closed_count >= 0' }
+          = t('.no_data')
+        {{ statistics.issues.closed_count | number }}

app/views/statistics/_pull_requests.html.haml

@@ -0,0 +1,48 @@
+.row
+  .span12
+    %h3.text-info
+      = t('.header')
+
+.row
+  .span8
+    .graph-wrapper
+      %h3
+        %span.graph-key-color1
+        = t('.open_title')
+        %span.graph-key-color2
+        = t('.merged_title')
+        %span.graph-key-color3
+        = t('.closed_title')
+      .centered.graph-loading{ ng_show: 'loading' }
+        = image_tag 'loading-large.gif'
+      .no-data{ ng_hide: 'loading || statistics.pull_requests' }
+        = t('.no_data')
+      %canvas#pull_requests_chart{ ng_show: 'statistics.pull_requests' }
+
+  .span3
+    .panel-wrapper
+      %h3
+        = t('.total_open')
+      .panel-data
+        = image_tag 'loading-small.gif', ng_show: 'loading'
+        .no-data{ ng_hide: 'loading || statistics.pull_requests.open_count >= 0' }
+          = t('.no_data')
+        {{ statistics.pull_requests.open_count | number }}
+
+    .panel-wrapper
+      %h3
+        = t('.total_merged')
+      .panel-data
+        = image_tag 'loading-small.gif', ng_show: 'loading'
+        .no-data{ ng_hide: 'loading || statistics.pull_requests.merged_count >= 0' }
+          = t('.no_data')
+        {{ statistics.pull_requests.merged_count | number }}
+
+    .panel-wrapper
+      %h3
+        = t('.total_closed')
+      .panel-data
+        = image_tag 'loading-small.gif', ng_show: 'loading'
+        .no-data{ ng_hide: 'loading || statistics.pull_requests.closed_count >= 0' }
+          = t('.no_data')
+        {{ statistics.pull_requests.closed_count | number }}

app/views/statistics/index.html.haml

@@ -5,4 +5,6 @@
   = render 'filter'
   = render 'build_lists'
   = render 'commits'
+  = render 'issues'
+  = render 'pull_requests'
 

config/locales/models/statistics.en.yml

@@ -29,6 +29,26 @@ en:
       total_commits:                              "Total commits"
       no_data:                                    "No data available"
 
+    issues:
+      header:                                     "Issues"
+      open_title:                                 "Open"
+      closed_title:                               "Closed"
+
+      total_open:                                 "Total open"
+      total_closed:                               "Total closed"
+      no_data:                                    "No data available"
+
+    pull_requests:
+      header:                                     "Pull Requests"
+      open_title:                                 "Open"
+      merged_title:                               "Merged"
+      closed_title:                               "Closed"
+
+      total_open:                                 "Total open"
+      total_merged:                               "Total merged"
+      total_closed:                               "Total closed"
+      no_data:                                    "No data available"
+
     helper:
       period:
         twenty_four_hours:                        "the last 24 hours"

config/locales/models/statistics.ru.yml

@@ -29,6 +29,26 @@ ru:
       total_commits:                              "Всего коммитов"
       no_data:                                    "Нет данных"
 
+    issues:
+      header:                                     "Задачи"
+      open_title:                                 "Открыто"
+      closed_title:                               "Закрыто"
+
+      total_open:                                 "Всего открыто"
+      total_closed:                               "Всего закрыто"
+      no_data:                                    "Нет данных"
+
+    pull_requests:
+      header:                                     "Пулл реквесты"
+      open_title:                                 "Открыто"
+      merged_title:                               "Принято"
+      closed_title:                               "Закрыто"
+
+      total_open:                                 "Всего открыто"
+      total_merged:                               "Всего принято"
+      total_closed:                               "Всего закрыто"
+      no_data:                                    "Нет данных"
+
     helper:
       period:
         twenty_four_hours:                        "последние 24 часа"

spec/factories/pull_request.rb

@@ -1,9 +1,9 @@
 FactoryGirl.define do
   factory :pull_request do
-    title { FactoryGirl.generate(:string) }
+    association :issue,         factory: :issue
-    body { FactoryGirl.generate(:string) }
+    association :from_project,  factory: :project
-    association :project, factory: :project
+    association :to_project,    factory: :project
-    association :user, factory: :user
+    from_ref  { FactoryGirl.generate(:string) }
-    association :assignee, factory: :user
+    to_ref    { FactoryGirl.generate(:string) }
   end
 end

spec/models/build_list_observer_spec.rb

@@ -12,4 +12,10 @@ describe BuildListObserver do
     expect(build_list.started_at).to_not be_nil
   end
 
+  it 'updates styatistics' do
+    expect do
+      build_list
+    end.to change(Statistic, :count).by(1)
+  end
+
 end

spec/models/issue_spec.rb

@@ -16,6 +16,16 @@ describe Issue do
     stub_symlink_methods
     any_instance_of(Project, versions: ['v1.0', 'v2.0'])
   end
+
+
+  context '#update_statistic' do
+    it 'updates styatistics' do
+      expect do
+        FactoryGirl.create(:issue)
+      end.to change(Statistic, :count).by(1)
+    end
+  end
+
   context 'for project admin user' do
     before(:each) do
       set_data

spec/models/pull_request_spec.rb

@@ -14,9 +14,10 @@ def set_data_for_pull
 end
 
 describe PullRequest do
+  before { stub_symlink_methods }
+
   context 'for owner user' do
     before do
-      stub_symlink_methods
       @user = FactoryGirl.create(:user)
       set_data_for_pull
       @pull = @project.pull_requests.new(issue_attributes: {title: 'test', body: 'testing'})
@@ -127,6 +128,20 @@ describe PullRequest do
   it { should belong_to(:to_project) }
   it { should belong_to(:from_project) }
 
+  context '#update_statistic' do
+    let(:issue)         { FactoryGirl.build(:issue) }
+    let(:pull_request)  { FactoryGirl.build(:pull_request, issue: issue) }
+
+    it 'updates styatistics' do
+      allow(PullRequest).to receive(:check_ref).and_return(true)
+      issue.new_pull_request = true
+      expect do
+        pull_request.save
+      end.to change(Statistic, :count).by(1)
+      expect(Statistic.last.key).to eq "#{Statistic::KEY_PULL_REQUEST}.#{Issue::STATUS_OPEN}"
+    end
+  end
+
   after do
     FileUtils.rm_rf(APP_CONFIG['root_path'])
     FileUtils.rm_rf File.join(Rails.root, "tmp", Rails.env, "pull_requests")