diff --git a/app/controllers/admin/cohorts_controller.rb b/app/controllers/admin/cohorts_controller.rb index 947afe3a028..9b77c554908 100644 --- a/app/controllers/admin/cohorts_controller.rb +++ b/app/controllers/admin/cohorts_controller.rb @@ -1,9 +1,11 @@ class Admin::CohortsController < Admin::ApplicationController def index - if ApplicationSetting.current.usage_ping_enabled - @cohorts = Rails.cache.fetch('cohorts', expires_in: 1.day) do + if current_application_settings.usage_ping_enabled + cohorts_results = Rails.cache.fetch('cohorts', expires_in: 1.day) do CohortsService.new.execute end + + @cohorts = CohortsSerializer.new.represent(cohorts_results) end end end diff --git a/app/serializers/cohort_activity_month_entity.rb b/app/serializers/cohort_activity_month_entity.rb new file mode 100644 index 00000000000..e6788a8b596 --- /dev/null +++ b/app/serializers/cohort_activity_month_entity.rb @@ -0,0 +1,11 @@ +class CohortActivityMonthEntity < Grape::Entity + include ActionView::Helpers::NumberHelper + + expose :total do |cohort_activity_month| + number_with_delimiter(cohort_activity_month[:total]) + end + + expose :percentage do |cohort_activity_month| + number_to_percentage(cohort_activity_month[:percentage], precision: 0) + end +end diff --git a/app/serializers/cohort_entity.rb b/app/serializers/cohort_entity.rb new file mode 100644 index 00000000000..7cdba5b0484 --- /dev/null +++ b/app/serializers/cohort_entity.rb @@ -0,0 +1,17 @@ +class CohortEntity < Grape::Entity + include ActionView::Helpers::NumberHelper + + expose :registration_month do |cohort| + cohort[:registration_month].strftime('%b %Y') + end + + expose :total do |cohort| + number_with_delimiter(cohort[:total]) + end + + expose :inactive do |cohort| + number_with_delimiter(cohort[:inactive]) + end + + expose :activity_months, using: CohortActivityMonthEntity +end diff --git a/app/serializers/cohorts_entity.rb b/app/serializers/cohorts_entity.rb new file mode 100644 index 00000000000..98f5995ba6f --- /dev/null +++ b/app/serializers/cohorts_entity.rb @@ -0,0 +1,4 @@ +class CohortsEntity < Grape::Entity + expose :months_included + expose :cohorts, using: CohortEntity +end diff --git a/app/serializers/cohorts_serializer.rb b/app/serializers/cohorts_serializer.rb new file mode 100644 index 00000000000..fe9367b13d8 --- /dev/null +++ b/app/serializers/cohorts_serializer.rb @@ -0,0 +1,3 @@ +class CohortsSerializer < AnalyticsGenericSerializer + entity CohortsEntity +end diff --git a/app/services/cohorts_service.rb b/app/services/cohorts_service.rb index e7f8a50605f..a7963f01176 100644 --- a/app/services/cohorts_service.rb +++ b/app/services/cohorts_service.rb @@ -1,11 +1,19 @@ class CohortsService MONTHS_INCLUDED = 12 - # Get a hash that looks like: + def execute + { + months_included: MONTHS_INCLUDED, + cohorts: cohorts + } + end + + # Get an array of hashes that looks like: # - # { - # month => { - # months: [3, 2, 1], + # [ + # { + # registration_month: Date.new(2017, 3), + # activity_months: [3, 2, 1], # total: 3 # inactive: 0 # }, @@ -13,29 +21,26 @@ class CohortsService # # The `months` array is always from oldest to newest, so it's always # non-strictly decreasing from left to right. - # - def execute - cohorts = {} + def cohorts months = Array.new(MONTHS_INCLUDED) { |i| i.months.ago.beginning_of_month.to_date } - MONTHS_INCLUDED.times do - created_at_month = months.last - activity_months = running_totals(months, created_at_month) + Array.new(MONTHS_INCLUDED) do + registration_month = months.last + activity_months = running_totals(months, registration_month) # Even if no users registered in this month, we always want to have a # value to fill in the table. - inactive = counts_by_month[[created_at_month, nil]].to_i - - cohorts[created_at_month] = { - months: activity_months, - total: activity_months.first, - inactive: inactive - } + inactive = counts_by_month[[registration_month, nil]].to_i months.pop - end - cohorts + { + registration_month: registration_month, + activity_months: activity_months, + total: activity_months.first[:total], + inactive: inactive + } + end end private @@ -44,11 +49,20 @@ class CohortsService # count as active in this month, too. Start with the most recent month first, # for calculating the running totals, and then reverse for displaying in the # table. - def running_totals(all_months, created_at_month) - all_months - .map { |activity_month| counts_by_month[[created_at_month, activity_month]] } - .reduce([]) { |result, total| result << result.last.to_i + total.to_i } - .reverse + # + # Each month has a total, and a percentage of the overall total, as keys. + def running_totals(all_months, registration_month) + month_totals = + all_months + .map { |activity_month| counts_by_month[[registration_month, activity_month]] } + .reduce([]) { |result, total| result << result.last.to_i + total.to_i } + .reverse + + overall_total = month_totals.first + + month_totals.map do |total| + { total: total, percentage: total.zero? ? 0 : 100 * total / overall_total } + end end # Get a hash that looks like: @@ -60,9 +74,8 @@ class CohortsService # } # # created_at_month can never be nil, but current_sign_in_at_month can (when a - # user has never logged in, just been created). This covers the last twelve - # months. - # + # user has never logged in, just been created). This covers the last + # MONTHS_INCLUDED months. def counts_by_month @counts_by_month ||= begin @@ -80,7 +93,7 @@ class CohortsService def column_to_date(column) if Gitlab::Database.postgresql? "CAST(DATE_TRUNC('month', #{column}) AS date)" - elsif Gitlab::Database.mysql? + else "STR_TO_DATE(DATE_FORMAT(#{column}, '%Y-%m-01'), '%Y-%m-%d')" end end diff --git a/app/views/admin/cohorts/_cohorts_table.html.haml b/app/views/admin/cohorts/_cohorts_table.html.haml index 38795583a8c..c3b37bcf8ec 100644 --- a/app/views/admin/cohorts/_cohorts_table.html.haml +++ b/app/views/admin/cohorts/_cohorts_table.html.haml @@ -1,8 +1,8 @@ .bs-callout.clearfix %p - User cohorts are shown for the last twelve months. Only users with - activity are counted in the cohort total; inactive users are counted - separately. + User cohorts are shown for the last #{@cohorts[:months_included]} + months. Only users with activity are counted in the cohort total; inactive + users are counted separately. = link_to icon('question-circle'), help_page_path('administration/usage_ping_and_cohorts', anchor: 'cohorts'), title: 'About this feature', target: '_blank' .table-holder @@ -12,27 +12,17 @@ %th Registration month %th Inactive users %th Cohort total - %th Month 0 - %th Month 1 - %th Month 2 - %th Month 3 - %th Month 4 - %th Month 5 - %th Month 6 - %th Month 7 - %th Month 8 - %th Month 9 - %th Month 10 - %th Month 11 + - @cohorts[:months_included].times do |i| + %th Month #{i} %tbody - - @cohorts.each do |registration_month, cohort| + - @cohorts[:cohorts].each do |cohort| %tr - %td= registration_month.strftime('%b %Y') - %td= number_with_delimiter(cohort[:inactive]) - %td= number_with_delimiter(cohort[:total]) - - cohort[:months].each do |running_total| + %td= cohort[:registration_month] + %td= cohort[:inactive] + %td= cohort[:total] + - cohort[:activity_months].each do |activity_month| %td - - next if cohort[:total].zero? - = number_to_percentage(100 * running_total / cohort[:total], precision: 0) + - next if cohort[:total] == '0' + = activity_month[:percentage] %br - (#{number_with_delimiter(running_total)}) + = activity_month[:total] diff --git a/spec/services/cohorts_service_spec.rb b/spec/services/cohorts_service_spec.rb index 878e9fcea05..5dd89e3e341 100644 --- a/spec/services/cohorts_service_spec.rb +++ b/spec/services/cohorts_service_spec.rb @@ -17,22 +17,83 @@ describe CohortsService do create(:user) # this user is inactive and belongs to the current month - expected = { - month_start(11) => { months: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], total: 0, inactive: 0 }, - month_start(10) => { months: [2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1], total: 2, inactive: 0 }, - month_start(9) => { months: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0], total: 0, inactive: 0 }, - month_start(8) => { months: [2, 1, 1, 1, 1, 1, 1, 1, 1], total: 2, inactive: 0 }, - month_start(7) => { months: [0, 0, 0, 0, 0, 0, 0, 0], total: 0, inactive: 0 }, - month_start(6) => { months: [2, 1, 1, 1, 1, 1, 1], total: 2, inactive: 0 }, - month_start(5) => { months: [0, 0, 0, 0, 0, 0], total: 0, inactive: 0 }, - month_start(4) => { months: [2, 1, 1, 1, 1], total: 2, inactive: 0 }, - month_start(3) => { months: [0, 0, 0, 0], total: 0, inactive: 0 }, - month_start(2) => { months: [2, 1, 1], total: 2, inactive: 0 }, - month_start(1) => { months: [0, 0], total: 0, inactive: 0 }, - month_start(0) => { months: [2], total: 2, inactive: 1 } - } + expected_cohorts = [ + { + registration_month: month_start(11), + activity_months: Array.new(12) { { total: 0, percentage: 0 } }, + total: 0, + inactive: 0 + }, + { + registration_month: month_start(10), + activity_months: [{ total: 2, percentage: 100 }] + Array.new(10) { { total: 1, percentage: 50 } }, + total: 2, + inactive: 0 + }, + { + registration_month: month_start(9), + activity_months: Array.new(10) { { total: 0, percentage: 0 } }, + total: 0, + inactive: 0 + }, + { + registration_month: month_start(8), + activity_months: [{ total: 2, percentage: 100 }] + Array.new(8) { { total: 1, percentage: 50 } }, + total: 2, + inactive: 0 + }, + { + registration_month: month_start(7), + activity_months: Array.new(8) { { total: 0, percentage: 0 } }, + total: 0, + inactive: 0 + }, + { + registration_month: month_start(6), + activity_months: [{ total: 2, percentage: 100 }] + Array.new(6) { { total: 1, percentage: 50 } }, + total: 2, + inactive: 0 + }, + { + registration_month: month_start(5), + activity_months: Array.new(6) { { total: 0, percentage: 0 } }, + total: 0, + inactive: 0 + }, + { + registration_month: month_start(4), + activity_months: [{ total: 2, percentage: 100 }] + Array.new(4) { { total: 1, percentage: 50 } }, + total: 2, + inactive: 0 + }, + { + registration_month: month_start(3), + activity_months: Array.new(4) { { total: 0, percentage: 0 } }, + total: 0, + inactive: 0 + }, + { + registration_month: month_start(2), + activity_months: [{ total: 2, percentage: 100 }] + Array.new(2) { { total: 1, percentage: 50 } }, + total: 2, + inactive: 0 + }, + { + registration_month: month_start(1), + activity_months: Array.new(2) { { total: 0, percentage: 0 } }, + total: 0, + inactive: 0 + }, + { + registration_month: month_start(0), + activity_months: [{ total: 2, percentage: 100 }], + total: 2, + inactive: 1 + }, + ] - expect(described_class.new.execute).to eq(expected) + expect(described_class.new.execute).to eq(months_included: 12, + cohorts: expected_cohorts) end end end