From 22af36dd2d847624e0cac0ab3652354e4bd18931 Mon Sep 17 00:00:00 2001 From: Urban Rotnik Date: Mon, 15 Apr 2019 16:38:19 +0200 Subject: [PATCH] Fix for exports_left, fix for some edge cases --- app/controllers/teams_controller.rb | 8 ++-- app/models/team_zip_export.rb | 4 ++ app/models/user.rb | 18 +++++--- spec/models/user_spec.rb | 64 ++++++++++++++++++++++++++--- 4 files changed, 78 insertions(+), 16 deletions(-) diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb index b8e1cc3ca..7464b8f49 100644 --- a/app/controllers/teams_controller.rb +++ b/app/controllers/teams_controller.rb @@ -251,15 +251,13 @@ class TeamsController < ApplicationController def export_projects_modal if @exp_projects.present? - limit = (ENV['EXPORT_ALL_LIMIT_24_HOURS'] || 3).to_i - curr_num = current_user.export_vars['num_of_export_all_last_24_hours'] if current_user.has_available_exports? render json: { html: render_to_string( partial: 'projects/export/modal.html.erb', locals: { num_projects: @exp_projects.size, - limit: limit, - num_of_requests_left: limit - curr_num - 1 } + limit: TeamZipExport.exports_limit, + num_of_requests_left: current_user.exports_left } ), title: t('projects.export_projects.modal_title') } @@ -267,7 +265,7 @@ class TeamsController < ApplicationController render json: { html: render_to_string( partial: 'projects/export/error.html.erb', - locals: { limit: limit } + locals: { limit: TeamZipExport.exports_limit } ), title: t('projects.export_projects.error_title'), status: 'error' diff --git a/app/models/team_zip_export.rb b/app/models/team_zip_export.rb index 1112f130f..4e9beb1c6 100644 --- a/app/models/team_zip_export.rb +++ b/app/models/team_zip_export.rb @@ -44,6 +44,10 @@ class TeamZipExport < ZipExport handle_asynchronously :generate_exportable_zip, queue: :team_zip_export + def self.exports_limit + (Rails.application.secrets.export_all_limit_24h || 3).to_i + end + private # Export all functionality diff --git a/app/models/user.rb b/app/models/user.rb index bdc20ed82..dcc659415 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -518,24 +518,32 @@ class User < ApplicationRecord def increase_daily_exports_counter! range = Time.now.utc.beginning_of_day.to_i..Time.now.utc.end_of_day.to_i last_export = export_vars[:last_export_timestamp] || 0 + export_vars[:num_of_export_all_last_24_hours] ||= 0 if range.cover?(last_export) export_vars[:num_of_export_all_last_24_hours] += 1 else - export_vars[:last_export_timestamp] = Time.now.utc.to_i export_vars[:num_of_export_all_last_24_hours] = 1 end + export_vars[:last_export_timestamp] = Time.now.utc.to_i save end def has_available_exports? - limit = (Rails.application.secrets.export_all_limit_24h || 3).to_i - last_export = export_vars[:last_export_timestamp] + last_export_timestamp = export_vars[:last_export_timestamp] || 0 # limit 0 means unlimited exports - return true if limit.zero? || last_export < Time.now.utc.beginning_of_day.to_i + return true if TeamZipExport.exports_limit.zero? || last_export_timestamp < Time.now.utc.beginning_of_day.to_i - export_vars[:num_of_export_all_last_24_hours] < limit + exports_left.positive? + end + + def exports_left + if (export_vars[:last_export_timestamp] || 0) < Time.now.utc.beginning_of_day.to_i + return TeamZipExport.exports_limit + end + + TeamZipExport.exports_limit - export_vars[:num_of_export_all_last_24_hours] end def global_activity_filter(filters, search_query) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5cb709f25..20c46deab 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -259,19 +259,71 @@ describe User, type: :model do }.from(2).to(1) end end + + context 'when num_of_export not exists' do + it 'starts count reports with 1' do + user.export_vars.delete('num_of_export_all_last_24_hours') + user.save + + expect { user.increase_daily_exports_counter! } + .to change { + user.reload.export_vars['num_of_export_all_last_24_hours'] + }.from(nil).to(1) + end + end end - describe 'has_available_exports?' do + describe '#has_available_exports?' do let(:user) { create :user } - it 'returns true when user has avaiable export' do - expect(user.has_available_exports?).to be_truthy + context 'when user have export_vars' do + it 'returns true when user has avaiable export' do + expect(user.has_available_exports?).to be_truthy + end + + it 'returns false when user has no avaiable export' do + user.export_vars['num_of_export_all_last_24_hours'] = 3 + + expect(user.has_available_exports?).to be_falsey + end end - it 'returns false when user has no avaiable export' do - user.export_vars['num_of_export_all_last_24_hours'] = 3 + context 'when user do not have export_vars' do + it 'returns false when user has no avaiable export' do + user.export_vars = {} - expect(user.has_available_exports?).to be_falsey + expect(user.has_available_exports?).to be_truthy + end + end + end + + describe '#exports_left' do + let(:user) { create :user } + context 'when user do have export_vars' do + it 'returns 3 when user has all exports avaible' do + expect(user.exports_left).to be == 3 + end + + it 'returns 0 when user has no avaiable export' do + user.export_vars['num_of_export_all_last_24_hours'] = 3 + + expect(user.exports_left).to be == 0 + end + + it 'returns 3 when user has invalid number from the past' do + user.export_vars['num_of_export_all_last_24_hours'] = -4 + user.export_vars['last_export_timestamp'] = Date.yesterday.to_time.utc.to_i + + expect(user.exports_left).to be == 3 + end + end + + context 'when user do not have export_vars' do + it 'returns false when user has no avaiable export' do + user.export_vars = {} + + expect(user.exports_left).to be == 3 + end end end