From fe419f295a56dc4c8796059c1fbb1c75f44e44d0 Mon Sep 17 00:00:00 2001 From: Sylvain Date: Mon, 10 Oct 2022 12:20:39 +0200 Subject: [PATCH 1/3] (bug) erroneous statistics The date of the statistics data was using the date of the regenerate command parameter. This was ok for the nightly builds but definitly not for bulk regeneration --- CHANGELOG.md | 3 + app/services/statistics/cleaner_service.rb | 10 ++- .../statistics/concerns/helpers_concern.rb | 8 +- app/services/statistics/fetcher_service.rb | 20 ++--- test/services/statistic_service_test.rb | 75 ++++++++++++++----- 5 files changed, 85 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37fbe40b9..fcb57616b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog Fab-manager +- Fix a bug: erroneous statistics +- [TODO DEPLOY] `rails fablab:maintenance:regenerate_statistics[2021,6]` + ## v5.4.21 2022 October 05 - Ability to dismiss a user to a lower privileged role diff --git a/app/services/statistics/cleaner_service.rb b/app/services/statistics/cleaner_service.rb index f40a5d9e0..671700869 100644 --- a/app/services/statistics/cleaner_service.rb +++ b/app/services/statistics/cleaner_service.rb @@ -12,7 +12,15 @@ class Statistics::CleanerService client.delete_by_query( index: model.index_name, type: model.document_type, - body: { query: { match: { date: format_date(options[:start_date]) } } } + body: { + query: { + terms: { + date: (to_date(options[:start_date]).to_date..to_date(options[:end_date]).to_date) + .to_a + .map { |d| format_date(d) } + } + } + } ) end end diff --git a/app/services/statistics/concerns/helpers_concern.rb b/app/services/statistics/concerns/helpers_concern.rb index 5ec077db1..e07b8c441 100644 --- a/app/services/statistics/concerns/helpers_concern.rb +++ b/app/services/statistics/concerns/helpers_concern.rb @@ -17,10 +17,14 @@ module Statistics::Concerns::HelpersConcern end def format_date(date) + to_date(date).strftime('%Y-%m-%d') + end + + def to_date(date) if date.is_a?(String) - Date.strptime(date, '%Y%m%d').strftime('%Y-%m-%d') + Date.strptime(date, '%Y%m%d') else - date.strftime('%Y-%m-%d') + date end end diff --git a/app/services/statistics/fetcher_service.rb b/app/services/statistics/fetcher_service.rb index 91cd0bc07..0569e9ecc 100644 --- a/app/services/statistics/fetcher_service.rb +++ b/app/services/statistics/fetcher_service.rb @@ -23,7 +23,7 @@ class Statistics::FetcherService ca /= 100.00 profile = sub.statistic_profile p = sub.plan - result.push({ date: options[:start_date].to_date, + result.push({ date: i.created_at.to_date, plan: p.group.slug, plan_id: p.id, plan_interval: p.interval, @@ -48,7 +48,7 @@ class Statistics::FetcherService next unless r.reservable profile = r.statistic_profile - result.push({ date: options[:start_date].to_date, + result.push({ date: r.created_at.to_date, reservation_id: r.id, machine_id: r.reservable.id, machine_type: r.reservable.friendly_id, @@ -69,7 +69,7 @@ class Statistics::FetcherService next unless r.reservable profile = r.statistic_profile - result.push({ date: options[:start_date].to_date, + result.push({ date: r.created_at.to_date, reservation_id: r.id, space_id: r.reservable.id, space_name: r.reservable.name, @@ -91,7 +91,7 @@ class Statistics::FetcherService profile = r.statistic_profile slot = r.slots.first - result.push({ date: options[:start_date].to_date, + result.push({ date: r.created_at.to_date, reservation_id: r.id, training_id: r.reservable.id, training_type: r.reservable.friendly_id, @@ -114,7 +114,7 @@ class Statistics::FetcherService profile = r.statistic_profile slot = r.slots.first - result.push({ date: options[:start_date].to_date, + result.push({ date: r.created_at.to_date, reservation_id: r.id, event_id: r.reservable.id, event_type: r.reservable.category.slug, @@ -140,7 +140,7 @@ class Statistics::FetcherService next unless r.reservable reservations_ca_list.push( - { date: options[:start_date].to_date, ca: calcul_ca(r.original_invoice) || 0 }.merge(user_info(r.statistic_profile)) + { date: r.created_at.to_date, ca: calcul_ca(r.original_invoice) || 0 }.merge(user_info(r.statistic_profile)) ) end Avoir.where('invoices.created_at >= :start_date AND invoices.created_at <= :end_date', options) @@ -148,12 +148,12 @@ class Statistics::FetcherService .each do |i| # the following line is a workaround for issue #196 profile = i.statistic_profile || i.main_item.object&.wallet&.user&.statistic_profile - avoirs_ca_list.push({ date: options[:start_date].to_date, ca: calcul_avoir_ca(i) || 0 }.merge(user_info(profile))) + avoirs_ca_list.push({ date: i.created_at.to_date, ca: calcul_avoir_ca(i) || 0 }.merge(user_info(profile))) end reservations_ca_list.concat(subscriptions_ca_list).concat(avoirs_ca_list).each do |e| profile = StatisticProfile.find(e[:statistic_profile_id]) u = find_or_create_user_info(profile, users_list) - u[:date] = options[:start_date].to_date + u[:date] = e[:date] add_ca(u, e[:ca], users_list) end users_list @@ -167,7 +167,7 @@ class Statistics::FetcherService .each do |sp| next if sp.user&.need_completion? - result.push({ date: options[:start_date].to_date }.merge(user_info(sp))) + result.push({ date: sp.created_at.to_date }.merge(user_info(sp))) end result end @@ -177,7 +177,7 @@ class Statistics::FetcherService Project.where('projects.published_at >= :start_date AND projects.published_at <= :end_date', options) .eager_load(:licence, :themes, :components, :machines, :project_users, author: [:group]) .each do |p| - result.push({ date: options[:start_date].to_date }.merge(user_info(p.author)).merge(project_info(p))) + result.push({ date: p.created_at.to_date }.merge(user_info(p.author)).merge(project_info(p))) end result end diff --git a/test/services/statistic_service_test.rb b/test/services/statistic_service_test.rb index ef1556fa1..e04412be6 100644 --- a/test/services/statistic_service_test.rb +++ b/test/services/statistic_service_test.rb @@ -14,9 +14,46 @@ class StatisticServiceTest < ActionDispatch::IntegrationTest end test 'build stats' do - # Create a reservation to generate an invoice + # Create a reservation to generate an invoice (2 days ago) machine = Machine.find(1) slot = Availability.find(19).slots.first + travel_to(2.days.ago) + post '/api/local_payment/confirm_payment', params: { + customer_id: @user.id, + items: [ + { + reservation: { + reservable_id: machine.id, + reservable_type: machine.class.name, + slots_reservations_attributes: [ + { + slot_id: slot.id + } + ] + } + } + ] + }.to_json, headers: default_headers + travel_back + + # Create a subscription to generate another invoice (1 day ago) + plan = Plan.find_by(group_id: @user.group.id, type: 'Plan') + travel_to(1.day.ago) + post '/api/local_payment/confirm_payment', + params: { + customer_id: @user.id, + items: [ + { + subscription: { + plan_id: plan.id + } + } + ] + }.to_json, headers: default_headers + travel_back + + # Crate another machine reservation (today) + slot = Availability.find(19).slots.last post '/api/local_payment/confirm_payment', params: { customer_id: @user.id, items: [ @@ -34,26 +71,27 @@ class StatisticServiceTest < ActionDispatch::IntegrationTest ] }.to_json, headers: default_headers - # Create a subscription to generate another invoice - plan = Plan.find_by(group_id: @user.group.id, type: 'Plan') - post '/api/local_payment/confirm_payment', - params: { - customer_id: @user.id, - items: [ - { - subscription: { - plan_id: plan.id - } - } - ] - }.to_json, headers: default_headers - - # Build the stats for today, we expect the above invoices (reservation+subscription) to appear in the resulting stats - ::Statistics::BuilderService.generate_statistic({ start_date: DateTime.current.beginning_of_day, + # Build the stats for the last 3 days, we expect the above invoices (reservations+subscription) to appear in the resulting stats + ::Statistics::BuilderService.generate_statistic({ start_date: 2.days.ago.beginning_of_day, end_date: DateTime.current.end_of_day }) Stats::Machine.refresh_index! + # first machine reservation (2 days ago) + stat_booking = Stats::Machine.search(query: { bool: { must: [{ term: { date: 2.days.ago.to_date.iso8601 } }, + { term: { type: 'booking' } }] } }).first + assert_not_nil stat_booking + assert_equal machine.friendly_id, stat_booking['subType'] + check_statistics_on_user(stat_booking) + + stat_hour = Stats::Machine.search(query: { bool: { must: [{ term: { date: 2.days.ago.to_date.iso8601 } }, + { term: { type: 'hour' } }] } }).first + + assert_not_nil stat_hour + assert_equal machine.friendly_id, stat_hour['subType'] + check_statistics_on_user(stat_hour) + + # second machine reservation (today) stat_booking = Stats::Machine.search(query: { bool: { must: [{ term: { date: DateTime.current.to_date.iso8601 } }, { term: { type: 'booking' } }] } }).first assert_not_nil stat_booking @@ -67,9 +105,10 @@ class StatisticServiceTest < ActionDispatch::IntegrationTest assert_equal machine.friendly_id, stat_hour['subType'] check_statistics_on_user(stat_hour) + # subscription Stats::Subscription.refresh_index! - stat_subscription = Stats::Subscription.search(query: { bool: { must: [{ term: { date: DateTime.current.to_date.iso8601 } }, + stat_subscription = Stats::Subscription.search(query: { bool: { must: [{ term: { date: 1.day.ago.to_date.iso8601 } }, { term: { type: plan.find_statistic_type.key } }] } }).first assert_not_nil stat_subscription From d74e95f2d9bb9c48a27c97b53abd906fcf7b72af Mon Sep 17 00:00:00 2001 From: Sylvain Date: Mon, 10 Oct 2022 15:40:22 +0200 Subject: [PATCH 2/3] (doc) fix typos --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcb57616b..61d9ec460 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -566,7 +566,7 @@ - Improved stripe 3D secure payment on payment schedules - Disable monthly payment for the subscription with interval 1 month - Fix a bug: unable to show statistics module in nav menu after login -- Fix a bug: plans page show an error if admin dont create any plans +- Fix a bug: plans page show an error if admin don't create any plans ## v5.0.12 2021 August 24 @@ -575,7 +575,7 @@ ## v5.0.11 2021 August 6 - Fix a bug: unable to generate avoir of wallet -- Fix a bug: manager cant reserve training for user +- Fix a bug: manager can't reserve any training for users ## v5.0.10 2021 August 2 @@ -1785,7 +1785,7 @@ - Fix a bug: when deleting an availability just after its creation, the indexer workers crash and retries for a month - [TODO DEPLOY] remove possible value `application/` in `ALLOWED_MIME_TYPES` list, in environment variable - [TODO DEPLOY] `rails runner StatisticCustomAggregation.destroy_all`, then `rake db:seed`, then `rake fablab:es:build_availabilities_index` (1) -- [TODO DEPLOY] `rake fablab:es:generate_stats[1095]` if you already has regenerated the statistics in the past, then they are very likely corrupted. Run this task to fix (2) +- [TODO DEPLOY] `rake fablab:es:generate_stats[1095]` if you already had regenerated the statistics in the past, then they are very likely corrupted. Run this task to fix (2) ## v2.4.8 2016 December 15 From c38c621e79b64accee5d52d92cd39e841447d7ba Mon Sep 17 00:00:00 2001 From: Sylvain Date: Mon, 10 Oct 2022 15:43:21 +0200 Subject: [PATCH 3/3] Version 5.4.22 --- CHANGELOG.md | 2 ++ package.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61d9ec460..c25efb942 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Changelog Fab-manager +## v5.4.22 2022 October 10 + - Fix a bug: erroneous statistics - [TODO DEPLOY] `rails fablab:maintenance:regenerate_statistics[2021,6]` diff --git a/package.json b/package.json index 73097400e..9fcd05602 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "fab-manager", - "version": "5.4.21", + "version": "5.4.22", "description": "Fab-manager is the FabLab management solution. It provides a comprehensive, web-based, open-source tool to simplify your administrative tasks and your marker's projects.", "keywords": [ "fablab",