From 73fd907333b9cb6f123bf56d6674782ab0df38d4 Mon Sep 17 00:00:00 2001 From: Aashish Passrija Date: Sat, 10 Oct 2020 21:31:01 +0530 Subject: [PATCH 01/18] Data Export To CSV for user : backend apis --- Gemfile | 8 ++ Gemfile.lock | 16 ++++ app/assets/stylesheets/users/reports.scss | 3 + app/controllers/users/reports_controller.rb | 25 +++++ app/helpers/users/reports_helper.rb | 25 +++++ app/models/allyship.rb | 9 ++ app/models/application_record.rb | 13 +++ app/models/care_plan_contact.rb | 9 ++ app/models/category.rb | 11 +++ app/models/group.rb | 9 ++ app/models/group_member.rb | 9 ++ app/models/medication.rb | 23 ++++- app/models/meeting_member.rb | 10 ++ app/models/moment.rb | 17 ++++ app/models/mood.rb | 11 +++ app/models/notification.rb | 9 ++ app/models/strategy.rb | 14 +++ app/models/user.rb | 70 ++++++++++++++ app/models/users.rb | 5 + app/models/users/data_request.rb | 92 +++++++++++++++++++ app/workers/delete_stale_data_worker.rb | 14 +++ app/workers/process_data_request_worker.rb | 10 ++ config/env/development.example.env | 4 + config/initializers/sidekiq.rb | 14 +++ config/routes.rb | 13 ++- config/sidekiq.yml | 12 +++ config/sidekiq_schedule.yml | 6 ++ ...201008170831_create_users_data_requests.rb | 23 +++++ db/schema.rb | 14 ++- spec/factories/users/data_requests.rb | 17 ++++ spec/helpers/users/reports_helper_spec.rb | 15 +++ spec/models/users/data_request_spec.rb | 17 ++++ spec/requests/users/reports_request_spec.rb | 5 + spec/workers/delete_stale_data_worker_spec.rb | 4 + .../process_data_request_worker_spec.rb | 4 + 35 files changed, 556 insertions(+), 4 deletions(-) create mode 100644 app/assets/stylesheets/users/reports.scss create mode 100644 app/controllers/users/reports_controller.rb create mode 100644 app/helpers/users/reports_helper.rb create mode 100644 app/models/users.rb create mode 100644 app/models/users/data_request.rb create mode 100644 app/workers/delete_stale_data_worker.rb create mode 100644 app/workers/process_data_request_worker.rb create mode 100644 config/initializers/sidekiq.rb create mode 100644 config/sidekiq.yml create mode 100644 config/sidekiq_schedule.yml create mode 100644 db/migrate/20201008170831_create_users_data_requests.rb create mode 100644 spec/factories/users/data_requests.rb create mode 100644 spec/helpers/users/reports_helper_spec.rb create mode 100644 spec/models/users/data_request_spec.rb create mode 100644 spec/requests/users/reports_request_spec.rb create mode 100644 spec/workers/delete_stale_data_worker_spec.rb create mode 100644 spec/workers/process_data_request_worker_spec.rb diff --git a/Gemfile b/Gemfile index a8b95d00b9..0cfa78dda8 100644 --- a/Gemfile +++ b/Gemfile @@ -52,6 +52,14 @@ gem 'selenium-webdriver', '~> 3.142.3' gem 'rubyzip', '~> 1.3.0' +gem 'sidekiq', '5.0.5' +# Uniqueness in sidekiq jobs +gem 'sidekiq-middleware' +# Keep track of failed jobs in sidekiq +gem 'sidekiq-failures' +#corn job for sidekiq +gem "sidekiq-cron", "~> 1.1" + group :development, :test do gem 'bundler-audit' gem 'dotenv-rails', '~> 2.7.2' diff --git a/Gemfile.lock b/Gemfile.lock index 606f9637f5..e15b21a40f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -452,6 +452,18 @@ GEM faraday (>= 0.7.6, < 1.0) shoulda-matchers (4.4.1) activesupport (>= 4.2.0) + sidekiq (5.0.5) + concurrent-ruby (~> 1.0) + connection_pool (~> 2.2, >= 2.2.0) + rack-protection (>= 1.5.0) + redis (>= 3.3.4, < 5) + sidekiq-cron (1.2.0) + fugit (~> 1.1) + sidekiq (>= 4.2.1) + sidekiq-failures (1.0.0) + sidekiq (>= 4.0.0) + sidekiq-middleware (0.3.0) + sidekiq (>= 2.12.4) signet (0.14.0) addressable (~> 2.3) faraday (>= 0.17.3, < 2.0) @@ -580,6 +592,10 @@ DEPENDENCIES selenium-webdriver (~> 3.142.3) sentry-raven shoulda-matchers + sidekiq (= 5.0.5) + sidekiq-cron (~> 1.1) + sidekiq-failures + sidekiq-middleware simplecov (= 0.16.1) spring turbolinks (~> 5.2.0) diff --git a/app/assets/stylesheets/users/reports.scss b/app/assets/stylesheets/users/reports.scss new file mode 100644 index 0000000000..ea4277635a --- /dev/null +++ b/app/assets/stylesheets/users/reports.scss @@ -0,0 +1,3 @@ +// Place all the styles related to the Users::Reports controller here. +// They will automatically be included in application.css. +// You can use Sass (SCSS) here: http://sass-lang.com/ diff --git a/app/controllers/users/reports_controller.rb b/app/controllers/users/reports_controller.rb new file mode 100644 index 0000000000..f4904ae05c --- /dev/null +++ b/app/controllers/users/reports_controller.rb @@ -0,0 +1,25 @@ +class Users::ReportsController < ApplicationController + + before_action :authenticate_user! + include Users::ReportsHelper + + def submit_request + status, response = submit_request_helper(current_user) + render json: response, status: status + end + + def fetch_request_status + status, response = fetch_request_status_helper(current_user, params[:request_id]) + render json: response, status: status + end + + def download_data + status, response = download_data_helper(current_user, params[:request_id]) + if status != 200 + render json: response, status: status + else + send_file(response, status: 200) + end + end + +end diff --git a/app/helpers/users/reports_helper.rb b/app/helpers/users/reports_helper.rb new file mode 100644 index 0000000000..d5d5b8c753 --- /dev/null +++ b/app/helpers/users/reports_helper.rb @@ -0,0 +1,25 @@ +module Users::ReportsHelper + + def submit_request_helper(user) + begin + return 200, {request_id: user.generate_data_request()} + rescue => e + return 422, {error: e.message} + end + end + + def fetch_request_status_helper(user, request_id) + return 400, {error: "Request id can be blank."} if request_id.blank? + data_request = user.data_requests.find_by(request_id: request_id) + return 404, {error: "No such request exists for current user."} if data_request.blank? + return 200, {current_status: data_request.status_id} + end + + def download_data_helper(user, request_id) + return 400, {error: "Request id can be blank."} if request_id.blank? + data_request = user.data_requests.find_by(request_id: request_id, status_id: Users::DataRequest::STATUS[:success]) + return 404, {error: "Requested csv not found."} if (data_request.blank? || !File.exist?("#{data_request.file_path}")) + return 200, data_request.file_path + end + +end diff --git a/app/models/allyship.rb b/app/models/allyship.rb index 096c79d9d1..16ac4aa167 100644 --- a/app/models/allyship.rb +++ b/app/models/allyship.rb @@ -12,6 +12,15 @@ # class Allyship < ApplicationRecord + + DISPLAY_ATTRIBUTES = %w{ + id + created_at + updated_at + ally_id + status + }.map!(&:freeze).freeze + enum status: { accepted: 0, pending_from_user: 1, pending_from_ally: 2 } validate :different_users diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 71fbba5b32..b76393133e 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -2,4 +2,17 @@ class ApplicationRecord < ActiveRecord::Base self.abstract_class = true + + def self.build_csv_rows(objects) + return [] if objects.blank? + klass = objects.klass + data = [["#{klass.name.underscore}_info"]] + diplay_attributes = klass.const_defined?("DISPLAY_ATTRIBUTES") ? klass.const_get("DISPLAY_ATTRIBUTES") : klass.column_names + data << diplay_attributes + objects.each do |object| + data << diplay_attributes.map { |attribute| object.send(attribute.to_sym) } + end + return data + end + end diff --git a/app/models/care_plan_contact.rb b/app/models/care_plan_contact.rb index 70bbc595a5..77700620ce 100644 --- a/app/models/care_plan_contact.rb +++ b/app/models/care_plan_contact.rb @@ -12,6 +12,15 @@ # class CarePlanContact < ApplicationRecord + + DISPLAY_ATTRIBUTES = %w{ + id + name + phone + created_at + updated_at + }.map!(&:freeze).freeze + validates :user_id, :name, presence: true belongs_to :user end diff --git a/app/models/category.rb b/app/models/category.rb index 02a520ba33..8187f88288 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -23,4 +23,15 @@ class Category < ApplicationRecord has_many :moments_categories, dependent: :destroy has_many :strategies_categories, dependent: :destroy validates :visible, inclusion: [true, false] + + DISPLAY_ATTRIBUTES = %w{ + id + name + description + created_at + updated_at + slug + visible + }.map!(&:freeze).freeze + end diff --git a/app/models/group.rb b/app/models/group.rb index 4f3c2ff86f..d5f22612ff 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -23,6 +23,15 @@ class Group < ApplicationRecord through: :group_members, source: :user after_destroy :destroy_notifications + DISPLAY_ATTRIBUTES = %w{ + id + name + created_at + updated_at + description + slug + }.map!(&:freeze).freeze + def led_by?(user) leaders.include? user end diff --git a/app/models/group_member.rb b/app/models/group_member.rb index a4fa161f25..118ca237e4 100644 --- a/app/models/group_member.rb +++ b/app/models/group_member.rb @@ -12,6 +12,15 @@ # class GroupMember < ApplicationRecord + + DISPLAY_ATTRIBUTES = %w{ + id + group_id + leader + created_at + updated_at + }.map!(&:freeze).freeze + after_destroy :destroy_meeting_memberships validates :group_id, :user_id, presence: true diff --git a/app/models/medication.rb b/app/models/medication.rb index 161e26f632..4913d5280c 100644 --- a/app/models/medication.rb +++ b/app/models/medication.rb @@ -18,14 +18,33 @@ # comments :text # slug :string # add_to_google_cal :boolean default(FALSE) -# weekly_dosage :integer -# default(["0", "1", "2", "3", "4", "5", "6"]), is an Array +# weekly_dosage :integer default(["0", "1", "2", "3", "4", "5", "6"]), is an Array +# class Medication < ApplicationRecord # dosage: amount of medication taken at one time # total: total quantity of medication # strength: strength of medication + DISPLAY_ATTRIBUTES = %w{ + id + name + dosage + refill + created_at + updated_at + user_id + total + strength + strength_unit + dosage_unit + total_unit + comments + slug + add_to_google_cal + weekly_dosage + }.map!(&:freeze).freeze + extend FriendlyId friendly_id :name belongs_to :user, foreign_key: :user_id diff --git a/app/models/meeting_member.rb b/app/models/meeting_member.rb index 94031181c0..3c75e1ff7b 100644 --- a/app/models/meeting_member.rb +++ b/app/models/meeting_member.rb @@ -13,6 +13,16 @@ # class MeetingMember < ApplicationRecord + + DISPLAY_ATTRIBUTES = %w{ + id + meeting_id + leader + created_at + updated_at + google_cal_event_id + }.map!(&:freeze).freeze + validates :meeting_id, :user_id, presence: true validates :leader, inclusion: [true, false] diff --git a/app/models/moment.rb b/app/models/moment.rb index f630c67490..633c68e95b 100644 --- a/app/models/moment.rb +++ b/app/models/moment.rb @@ -25,6 +25,23 @@ class Moment < ApplicationRecord include CommonMethods extend FriendlyId + DISPLAY_ATTRIBUTES = %w{ + id + name + why + fix + created_at + updated_at + viewers + comment + slug + secret_share_identifier + secret_share_expires_at + published_at + bookmarked + resource_recommendations + }.map!(&:freeze).freeze + friendly_id :name serialize :viewers, Array diff --git a/app/models/mood.rb b/app/models/mood.rb index 5ffb829025..cb2293a2fd 100644 --- a/app/models/mood.rb +++ b/app/models/mood.rb @@ -14,6 +14,17 @@ # class Mood < ApplicationRecord + + DISPLAY_ATTRIBUTES = %w{ + id + name + description + created_at + updated_at + slug + visible + }.map!(&:freeze).freeze + extend FriendlyId friendly_id :name validates :user_id, :name, presence: true diff --git a/app/models/notification.rb b/app/models/notification.rb index 6832d3bb1d..9dc1f05b5a 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -12,6 +12,15 @@ # class Notification < ApplicationRecord + + DISPLAY_ATTRIBUTES = %w{ + id + uniqueid + data + created_at + updated_at + }.map!(&:freeze).freeze + validates :user_id, :uniqueid, :data, presence: true belongs_to :user, foreign_key: :user_id diff --git a/app/models/strategy.rb b/app/models/strategy.rb index 03d35ab023..7a3732650f 100644 --- a/app/models/strategy.rb +++ b/app/models/strategy.rb @@ -22,6 +22,20 @@ class Strategy < ApplicationRecord include CommonMethods extend FriendlyId + DISPLAY_ATTRIBUTES = %w{ + id + description + viewers + comment + created_at + updated_at + name + slug + published_at + visible + bookmarked + }.map!(&:freeze).freeze + friendly_id :name serialize :viewers, Array diff --git a/app/models/user.rb b/app/models/user.rb index 9482c04f1f..619a21c8df 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -51,6 +51,32 @@ class User < ApplicationRecord OAUTH_TOKEN_URL = 'https://accounts.google.com/o/oauth2/token' + DISPLAY_ATTRIBUTES = %w{ + id + email + sign_in_count + current_sign_in_at + last_sign_in_at + current_sign_in_ip + last_sign_in_ip + created_at + updated_at + name + location + timezone + about + conditions + uid + provider + comment_notify + ally_notify + group_notify + meeting_notify + locale + banned + admin + }.map!(&:freeze).freeze + # Include default devise modules. Others available are: # :confirmable, :lockable, :timeoutable and :omniauthable devise :invitable, :database_authenticatable, :registerable, :uid, @@ -72,6 +98,7 @@ class User < ApplicationRecord has_many :categories has_many :care_plan_contacts has_many :password_histories, dependent: :destroy + has_many :data_requests, class_name: "Users::DataRequest", foreign_key: :user_id belongs_to :invited_by, class_name: 'User' after_initialize :set_defaults, unless: :persisted? @@ -136,6 +163,49 @@ def update_access_token new_access_token end + def build_csv_data() + user_data = [["user_info"]] + user_data << DISPLAY_ATTRIBUTES + user_data << DISPLAY_ATTRIBUTES.map { |attribute| self.send(attribute.to_sym) } + user_data += Group.build_csv_rows(self.groups) + user_data += GroupMember.build_csv_rows(self.group_members) + user_data += Category.build_csv_rows(self.categories) + user_data += Medication.build_csv_rows(self.medications) + user_data += Strategy.build_csv_rows(self.strategies) + user_data += Moment.build_csv_rows(self.moments) + user_data += Notification.build_csv_rows(self.notifications) + user_data += Mood.build_csv_rows(self.moods) + user_data += CarePlanContact.build_csv_rows(self.care_plan_contacts) + user_data += Allyship.build_csv_rows(self.allyships) + user_data += MeetingMember.build_csv_rows(self.meeting_members) + return user_data + end + + def generate_data_request() + ActiveRecord::Base.transaction do + self.lock! + data_request = self.data_requests + .where(status_id: Users::DataRequest::STATUS[:enqueued]) + .first_or_initialize + return data_request.request_id if data_request.request_id.present? + data_request.request_id = SecureRandom.uuid + data_request.save! + return data_request.request_id + end + end + + def delete_stale_data_file() + successful_data_requests = self.data_requests.where(status_id: Users::DataRequest::STATUS[:success]).order("updated_at desc") + return if successful_data_requests.count < 2 + ActiveRecord::Base.transaction do + stale_data_requests = successful_data_requests.where.not(id: successful_data_requests.first) + stale_data_requests.each do |dr| + File.delete(dr.file_path) if File.exist?(dr.file_path) + dr.update!(status_id: Users::DataRequest::STATUS[:deleted]) + end + end + end + private_class_method def self.update_access_token_fields(user:, access_token:) user.update!( provider: access_token.provider, diff --git a/app/models/users.rb b/app/models/users.rb new file mode 100644 index 0000000000..c7af3cf2c9 --- /dev/null +++ b/app/models/users.rb @@ -0,0 +1,5 @@ +module Users + def self.table_name_prefix + 'users_' + end +end diff --git a/app/models/users/data_request.rb b/app/models/users/data_request.rb new file mode 100644 index 0000000000..1d2a0deac5 --- /dev/null +++ b/app/models/users/data_request.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: users_data_requests +# +# id :bigint not null, primary key +# request_id :string not null +# status_id :integer not null +# user_id :bigint not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class Users::DataRequest < ApplicationRecord + + STATUS = { + enqueued: 1, + success: 2, + failed: 3, + deleted: 4 + }.freeze + + ASSOCIATIONS_TO_EXPORT = %i{ + allyships + group_members + groups + categories + medications + strategies + moments + notifications + moods + care_plan_contacts + meeting_members + } + + DEFAULT_FILE_PATH = "#{Rails.root}/tmp/csv_data/".freeze + + belongs_to :user, class_name: '::User', foreign_key: "user_id" + + after_commit :after_commit_tasks + + validates_uniqueness_of :user_id, scope: [:status_id], + message: "There's already a request enqueued for this user.", if: -> {self.status_id == STATUS[:enqueued]} + + validates_uniqueness_of :request_id, + message: "There's already a request with this request_id." + + validates :status_id, + inclusion: { + in: STATUS.values, + message: proc { |request| "'#{request.status_id}' is not valid."} + }, + presence: true + + validates :request_id, presence: true + + attr_accessor :file_path + + def after_commit_tasks() + enqueue_download_request() if (self.saved_change_to_id? && self.status_id == STATUS[:enqueued]) + end + + def enqueue_download_request() + ProcessDataRequestWorker.perform_async(self.request_id) + end + + def create_csv() + Dir.mkdir(DEFAULT_FILE_PATH) unless File.exist?(DEFAULT_FILE_PATH) + user = User.includes(*ASSOCIATIONS_TO_EXPORT).find(self.user_id) + begin + require 'csv' + csv_rows = user.build_csv_data() + CSV.open("#{self.file_path}","wb") do |csv_row| + csv_rows.each do |row| + csv_row << row + end + end + self.status_id = STATUS[:success] + self.save! + user.delete_stale_data_file() + rescue => e + File.delete(self.file_path) if (self.file_path.present? && File.exist?(self.file_path)) + self.status_id = STATUS[:failed] + self.save! + end + end + + def file_path() + return "#{DEFAULT_FILE_PATH}#{self.request_id}.csv" + end +end diff --git a/app/workers/delete_stale_data_worker.rb b/app/workers/delete_stale_data_worker.rb new file mode 100644 index 0000000000..b205733630 --- /dev/null +++ b/app/workers/delete_stale_data_worker.rb @@ -0,0 +1,14 @@ +class DeleteStaleDataWorker + include Sidekiq::Worker + + ACTIVE_DURATION = 30.days + + def perform() + Users::DataRequest.where("updated_at < ?", (Time.current() - 30.days)) + .where(status_id: Users::DataRequest::STATUS[:success]) + .each do |dr| + File.delete(self.file_path) if (self.file_path.present? && File.exist?(self.file_path)) + dr.update(status_id: Users::DataRequest::STATUS[:deleted]) + end + end +end diff --git a/app/workers/process_data_request_worker.rb b/app/workers/process_data_request_worker.rb new file mode 100644 index 0000000000..d8c9f8377f --- /dev/null +++ b/app/workers/process_data_request_worker.rb @@ -0,0 +1,10 @@ +class ProcessDataRequestWorker + include Sidekiq::Worker + sidekiq_options queue: 'critical' + + def perform(request_id) + data_request = Users::DataRequest.find_by(request_id: request_id) + return if (data_request.blank? || data_request.status_id == Users::DataRequest::STATUS[:success]) + data_request.create_csv() + end +end diff --git a/config/env/development.example.env b/config/env/development.example.env index cdc310b186..1a12422d8a 100644 --- a/config/env/development.example.env +++ b/config/env/development.example.env @@ -39,3 +39,7 @@ RAISE_DELIVERY_ERRORS="false" PSQL_HOST="" PSQL_USERNAME="" PSQL_PASSWORD="" + +# REDIS +REDIS_HOST="" +REDIS_PORT="" diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb new file mode 100644 index 0000000000..f557bbb0b0 --- /dev/null +++ b/config/initializers/sidekiq.rb @@ -0,0 +1,14 @@ +Sidekiq.configure_client do |config| + config.redis = { url: "redis://#{ENV['REDIS_HOST']}:#{ENV['REDIS_PORT']}/0", namespace: 'ifme' } +end + +Sidekiq.configure_server do |config| + config.redis = { url: "redis://#{ENV['REDIS_HOST']}:#{ENV['REDIS_PORT']}/0", namespace: 'ifme' } + schedule_file = "config/sidekiq_schedule.yml" + + if File.exist?(schedule_file) + Sidekiq::Cron::Job.load_from_hash YAML.load_file(schedule_file) + end +end + +Sidekiq.default_worker_options = { :backtrace => true, :unique => :all, :failures => true } diff --git a/config/routes.rb b/config/routes.rb index 5582cc61b9..586a8575da 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true - +require 'sidekiq/web' +require 'sidekiq/cron/web' Rails.application.routes.draw do get 'errors/not_found' get 'errors/internal_server_error' @@ -7,6 +8,10 @@ get '/404' => 'errors#not_found' get '/500' => 'errors#internal_server_error' + authenticate :user, lambda { |u| u.admin? } do + mount Sidekiq::Web => '/sidekiq' + end + resources :allies, only: :index do collection do post 'add' @@ -126,6 +131,12 @@ invitations: 'users/invitations', sessions: :sessions } + namespace :users do + post "/data" => "reports#submit_request" + get "/data/status" => "reports#fetch_request_status" + get "/data/download" => "reports#download_data" + end + post 'pusher/auth' Rails.configuration.i18n.available_locales.each do |locale| diff --git a/config/sidekiq.yml b/config/sidekiq.yml new file mode 100644 index 0000000000..e471576c4d --- /dev/null +++ b/config/sidekiq.yml @@ -0,0 +1,12 @@ +--- +:concurrency: 1 +:logfile: log/sidekiq.log +staging: + :concurrency: 1 +production: + :concurrency: 1 + :timeout: 10800 +:queues: + - critical + - default + - low diff --git a/config/sidekiq_schedule.yml b/config/sidekiq_schedule.yml new file mode 100644 index 0000000000..9582ceb791 --- /dev/null +++ b/config/sidekiq_schedule.yml @@ -0,0 +1,6 @@ + +delete_stale_csv: + cron: "0 0 0 * * * *" + class: "DeleteStaleDataWorker" + queue: default + diff --git a/db/migrate/20201008170831_create_users_data_requests.rb b/db/migrate/20201008170831_create_users_data_requests.rb new file mode 100644 index 0000000000..a50ff95b38 --- /dev/null +++ b/db/migrate/20201008170831_create_users_data_requests.rb @@ -0,0 +1,23 @@ +class CreateUsersDataRequests < ActiveRecord::Migration[6.0] + def up + unless table_exists? :users_data_requests + create_table :users_data_requests do |t| + t.string :request_id, null: false + t.integer :status_id, null: false + t.references :user, foreign_key: true, null: false + t.timestamps + end + add_index :users_data_requests, :request_id, unique: true unless index_exists?(:users_data_requests, :request_id) + add_index :users_data_requests, :user_id unless index_exists?(:users_data_requests, :user_id) + execute "CREATE UNIQUE INDEX IF NOT EXISTS index_users_data_requests_on_users_id_and_status_uniq + ON users_data_requests(user_id, status_id) WHERE (status_id = #{Users::DataRequest::STATUS[:enqueued]})" + end + end + + def down + execute "DROP INDEX IF EXISTS index_users_data_requests_on_users_id_and_status_uniq" + remove_index :users_data_requests, :request_id if index_exists?(:users_data_requests, :request_id) + remove_index :users_data_requests, :user_id if index_exists?(:users_data_requests, :user_id) + drop_table :users_data_requests if table_exists? :users_data_requests + end +end diff --git a/db/schema.rb b/db/schema.rb index a386f16b8c..36ec2b85dd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_05_27_035711) do +ActiveRecord::Schema.define(version: 2020_10_08_170831) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -309,6 +309,17 @@ t.index ["uid"], name: "index_users_on_uid", unique: true end + create_table "users_data_requests", force: :cascade do |t| + t.string "request_id", null: false + t.integer "status_id", null: false + t.bigint "user_id", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["request_id"], name: "index_users_data_requests_on_request_id", unique: true + t.index ["user_id", "status_id"], name: "index_users_data_requests_on_users_id_and_status_uniq", unique: true, where: "(status_id = 1)" + t.index ["user_id"], name: "index_users_data_requests_on_user_id" + end + add_foreign_key "moments_categories", "categories" add_foreign_key "moments_categories", "moments" add_foreign_key "moments_moods", "moments" @@ -318,4 +329,5 @@ add_foreign_key "password_histories", "users" add_foreign_key "strategies_categories", "categories" add_foreign_key "strategies_categories", "strategies" + add_foreign_key "users_data_requests", "users" end diff --git a/spec/factories/users/data_requests.rb b/spec/factories/users/data_requests.rb new file mode 100644 index 0000000000..81e2f4ef73 --- /dev/null +++ b/spec/factories/users/data_requests.rb @@ -0,0 +1,17 @@ +# == Schema Information +# +# Table name: users_data_requests +# +# id :bigint not null, primary key +# request_id :string not null +# status_id :integer not null +# user_id :bigint not null +# created_at :datetime not null +# updated_at :datetime not null +# + +FactoryBot.define do + factory :users_data_request, class: 'Users::DataRequest' do + + end +end diff --git a/spec/helpers/users/reports_helper_spec.rb b/spec/helpers/users/reports_helper_spec.rb new file mode 100644 index 0000000000..095d5a1356 --- /dev/null +++ b/spec/helpers/users/reports_helper_spec.rb @@ -0,0 +1,15 @@ +require 'rails_helper' + +# Specs in this file have access to a helper object that includes +# the Users::ReportsHelper. For example: +# +# describe Users::ReportsHelper do +# describe "string concat" do +# it "concats two strings with spaces" do +# expect(helper.concat_strings("this","that")).to eq("this that") +# end +# end +# end +RSpec.describe Users::ReportsHelper, type: :helper do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/models/users/data_request_spec.rb b/spec/models/users/data_request_spec.rb new file mode 100644 index 0000000000..0d1af34810 --- /dev/null +++ b/spec/models/users/data_request_spec.rb @@ -0,0 +1,17 @@ +# == Schema Information +# +# Table name: users_data_requests +# +# id :bigint not null, primary key +# request_id :string not null +# status_id :integer not null +# user_id :bigint not null +# created_at :datetime not null +# updated_at :datetime not null +# + +require 'rails_helper' + +RSpec.describe Users::DataRequest, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/requests/users/reports_request_spec.rb b/spec/requests/users/reports_request_spec.rb new file mode 100644 index 0000000000..9a6ee54f4b --- /dev/null +++ b/spec/requests/users/reports_request_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe "Users::Reports", type: :request do + +end diff --git a/spec/workers/delete_stale_data_worker_spec.rb b/spec/workers/delete_stale_data_worker_spec.rb new file mode 100644 index 0000000000..8a380b1212 --- /dev/null +++ b/spec/workers/delete_stale_data_worker_spec.rb @@ -0,0 +1,4 @@ +require 'rails_helper' +RSpec.describe DeleteStaleDataWorker, type: :worker do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/workers/process_data_request_worker_spec.rb b/spec/workers/process_data_request_worker_spec.rb new file mode 100644 index 0000000000..a6dfad54cd --- /dev/null +++ b/spec/workers/process_data_request_worker_spec.rb @@ -0,0 +1,4 @@ +require 'rails_helper' +RSpec.describe ProcessDataRequestWorker, type: :worker do + pending "add some examples to (or delete) #{__FILE__}" +end From e7b8fe509e7c4f4f70c8f472eacfe82f5dd6737f Mon Sep 17 00:00:00 2001 From: Aashish Passrija Date: Sun, 11 Oct 2020 21:59:09 +0530 Subject: [PATCH 02/18] rubocop --- app/controllers/comments_controller.rb | 2 - app/controllers/users/reports_controller.rb | 41 ++++--- app/helpers/users/reports_helper.rb | 48 +++++--- app/models/allyship.rb | 5 +- app/models/application_record.rb | 14 ++- app/models/care_plan_contact.rb | 7 +- app/models/category.rb | 5 +- app/models/group.rb | 4 +- app/models/group_member.rb | 5 +- app/models/medication.rb | 4 +- app/models/meeting_member.rb | 7 +- app/models/moment.rb | 4 +- app/models/mood.rb | 7 +- app/models/notification.rb | 5 +- app/models/strategy.rb | 6 +- app/models/user.rb | 62 +++++----- app/models/users.rb | 1 + app/models/users/data_request.rb | 124 ++++++++++---------- app/workers/delete_stale_data_worker.rb | 12 +- app/workers/process_data_request_worker.rb | 7 +- 20 files changed, 199 insertions(+), 171 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 056346fb66..dd9480804f 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -45,7 +45,6 @@ def handle_create(comment_params) comment.id end - # rubocop:disable Metrics/MethodLength def handle_delete(comment) if %w[moment strategy].include?(comment.commentable_type) raise RuntimeError unless CommentViewersService.deletable?( @@ -61,5 +60,4 @@ def handle_delete(comment) remove_meeting_notification(comment, meeting) end end - # rubocop:enable Metrics/MethodLength end diff --git a/app/controllers/users/reports_controller.rb b/app/controllers/users/reports_controller.rb index f4904ae05c..0de28ce1b7 100644 --- a/app/controllers/users/reports_controller.rb +++ b/app/controllers/users/reports_controller.rb @@ -1,25 +1,28 @@ -class Users::ReportsController < ApplicationController +# frozen_string_literal: true +module Users + class ReportsController < ApplicationController + before_action :authenticate_user! + include Users::ReportsHelper - before_action :authenticate_user! - include Users::ReportsHelper - - def submit_request - status, response = submit_request_helper(current_user) - render json: response, status: status - end - - def fetch_request_status - status, response = fetch_request_status_helper(current_user, params[:request_id]) - render json: response, status: status - end + def submit_request + status, response = submit_request_helper(current_user) + render json: response, status: status + end - def download_data - status, response = download_data_helper(current_user, params[:request_id]) - if status != 200 + def fetch_request_status + status, response = fetch_request_status_helper(current_user, + params[:request_id]) render json: response, status: status - else - send_file(response, status: 200) end - end + def download_data + status, response = download_data_helper(current_user, + params[:request_id]) + if status != 200 + render json: response, status: status + else + send_file(response, status: 200) + end + end + end end diff --git a/app/helpers/users/reports_helper.rb b/app/helpers/users/reports_helper.rb index d5d5b8c753..183f1c710f 100644 --- a/app/helpers/users/reports_helper.rb +++ b/app/helpers/users/reports_helper.rb @@ -1,25 +1,35 @@ -module Users::ReportsHelper +# frozen_string_literal: true +module Users + module ReportsHelper + def submit_request_helper(user) + [200, { request_id: user.generate_data_request }] + rescue StandardError => e + [422, { error: e.message }] + end + + def fetch_request_status_helper(user, request_id) + return 400, { error: 'Request id can be blank.' } if request_id.blank? - def submit_request_helper(user) - begin - return 200, {request_id: user.generate_data_request()} - rescue => e - return 422, {error: e.message} + data_request = user.data_requests.find_by(request_id: request_id) + if data_request.blank? + return 404, { error: 'No such request exists for current user.' } + end + + [200, { current_status: data_request.status_id }] end - end - def fetch_request_status_helper(user, request_id) - return 400, {error: "Request id can be blank."} if request_id.blank? - data_request = user.data_requests.find_by(request_id: request_id) - return 404, {error: "No such request exists for current user."} if data_request.blank? - return 200, {current_status: data_request.status_id} - end + def download_data_helper(user, request_id) + return 400, { error: 'Request id can be blank.' } if request_id.blank? - def download_data_helper(user, request_id) - return 400, {error: "Request id can be blank."} if request_id.blank? - data_request = user.data_requests.find_by(request_id: request_id, status_id: Users::DataRequest::STATUS[:success]) - return 404, {error: "Requested csv not found."} if (data_request.blank? || !File.exist?("#{data_request.file_path}")) - return 200, data_request.file_path - end + data_request = user.data_requests.find_by( + request_id: request_id, + status_id: Users::DataRequest::STATUS[:success] + ) + if data_request.blank? || !File.exist?(data_request.file_path.to_s) + return 404, { error: 'Requested csv not found.' } + end + [200, data_request.file_path] + end + end end diff --git a/app/models/allyship.rb b/app/models/allyship.rb index 16ac4aa167..b2f7fd71c3 100644 --- a/app/models/allyship.rb +++ b/app/models/allyship.rb @@ -12,14 +12,13 @@ # class Allyship < ApplicationRecord - - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id created_at updated_at ally_id status - }.map!(&:freeze).freeze + ].map!(&:freeze).freeze enum status: { accepted: 0, pending_from_user: 1, pending_from_ally: 2 } diff --git a/app/models/application_record.rb b/app/models/application_record.rb index b76393133e..ab5651199f 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -5,14 +5,18 @@ class ApplicationRecord < ActiveRecord::Base def self.build_csv_rows(objects) return [] if objects.blank? + klass = objects.klass data = [["#{klass.name.underscore}_info"]] - diplay_attributes = klass.const_defined?("DISPLAY_ATTRIBUTES") ? klass.const_get("DISPLAY_ATTRIBUTES") : klass.column_names - data << diplay_attributes + attributes = if klass.const_defined?('DISPLAY_ATTRIBUTES') + klass.const_get('DISPLAY_ATTRIBUTES') + else + klass.column_names + end + data << attributes objects.each do |object| - data << diplay_attributes.map { |attribute| object.send(attribute.to_sym) } + data << attributes.map { |attribute| object.send(attribute.to_sym) } end - return data + data end - end diff --git a/app/models/care_plan_contact.rb b/app/models/care_plan_contact.rb index 77700620ce..eabbc17a4b 100644 --- a/app/models/care_plan_contact.rb +++ b/app/models/care_plan_contact.rb @@ -12,15 +12,14 @@ # class CarePlanContact < ApplicationRecord - - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id name phone created_at updated_at - }.map!(&:freeze).freeze - + ].map!(&:freeze).freeze + validates :user_id, :name, presence: true belongs_to :user end diff --git a/app/models/category.rb b/app/models/category.rb index 8187f88288..c45d5e0182 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -24,7 +24,7 @@ class Category < ApplicationRecord has_many :strategies_categories, dependent: :destroy validates :visible, inclusion: [true, false] - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id name description @@ -32,6 +32,5 @@ class Category < ApplicationRecord updated_at slug visible - }.map!(&:freeze).freeze - + ].map!(&:freeze).freeze end diff --git a/app/models/group.rb b/app/models/group.rb index d5f22612ff..515f0f4a01 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -23,14 +23,14 @@ class Group < ApplicationRecord through: :group_members, source: :user after_destroy :destroy_notifications - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id name created_at updated_at description slug - }.map!(&:freeze).freeze + ].map!(&:freeze).freeze def led_by?(user) leaders.include? user diff --git a/app/models/group_member.rb b/app/models/group_member.rb index 118ca237e4..8628fd6986 100644 --- a/app/models/group_member.rb +++ b/app/models/group_member.rb @@ -12,14 +12,13 @@ # class GroupMember < ApplicationRecord - - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id group_id leader created_at updated_at - }.map!(&:freeze).freeze + ].map!(&:freeze).freeze after_destroy :destroy_meeting_memberships diff --git a/app/models/medication.rb b/app/models/medication.rb index 4913d5280c..c14dac8ae9 100644 --- a/app/models/medication.rb +++ b/app/models/medication.rb @@ -26,7 +26,7 @@ class Medication < ApplicationRecord # total: total quantity of medication # strength: strength of medication - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id name dosage @@ -43,7 +43,7 @@ class Medication < ApplicationRecord slug add_to_google_cal weekly_dosage - }.map!(&:freeze).freeze + ].map!(&:freeze).freeze extend FriendlyId friendly_id :name diff --git a/app/models/meeting_member.rb b/app/models/meeting_member.rb index 3c75e1ff7b..eb7db3a98f 100644 --- a/app/models/meeting_member.rb +++ b/app/models/meeting_member.rb @@ -13,16 +13,15 @@ # class MeetingMember < ApplicationRecord - - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id meeting_id leader created_at updated_at google_cal_event_id - }.map!(&:freeze).freeze - + ].map!(&:freeze).freeze + validates :meeting_id, :user_id, presence: true validates :leader, inclusion: [true, false] diff --git a/app/models/moment.rb b/app/models/moment.rb index 633c68e95b..6983e62552 100644 --- a/app/models/moment.rb +++ b/app/models/moment.rb @@ -25,7 +25,7 @@ class Moment < ApplicationRecord include CommonMethods extend FriendlyId - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id name why @@ -40,7 +40,7 @@ class Moment < ApplicationRecord published_at bookmarked resource_recommendations - }.map!(&:freeze).freeze + ].map!(&:freeze).freeze friendly_id :name serialize :viewers, Array diff --git a/app/models/mood.rb b/app/models/mood.rb index cb2293a2fd..4f6f440e30 100644 --- a/app/models/mood.rb +++ b/app/models/mood.rb @@ -14,8 +14,7 @@ # class Mood < ApplicationRecord - - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id name description @@ -23,8 +22,8 @@ class Mood < ApplicationRecord updated_at slug visible - }.map!(&:freeze).freeze - + ].map!(&:freeze).freeze + extend FriendlyId friendly_id :name validates :user_id, :name, presence: true diff --git a/app/models/notification.rb b/app/models/notification.rb index 9dc1f05b5a..57ce031581 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -12,14 +12,13 @@ # class Notification < ApplicationRecord - - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id uniqueid data created_at updated_at - }.map!(&:freeze).freeze + ].map!(&:freeze).freeze validates :user_id, :uniqueid, :data, presence: true belongs_to :user, foreign_key: :user_id diff --git a/app/models/strategy.rb b/app/models/strategy.rb index 7a3732650f..bd3ccc9acb 100644 --- a/app/models/strategy.rb +++ b/app/models/strategy.rb @@ -22,7 +22,7 @@ class Strategy < ApplicationRecord include CommonMethods extend FriendlyId - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id description viewers @@ -34,8 +34,8 @@ class Strategy < ApplicationRecord published_at visible bookmarked - }.map!(&:freeze).freeze - + ].map!(&:freeze).freeze + friendly_id :name serialize :viewers, Array diff --git a/app/models/user.rb b/app/models/user.rb index 619a21c8df..1f32ae40d9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -51,7 +51,7 @@ class User < ApplicationRecord OAUTH_TOKEN_URL = 'https://accounts.google.com/o/oauth2/token' - DISPLAY_ATTRIBUTES = %w{ + DISPLAY_ATTRIBUTES = %w[ id email sign_in_count @@ -74,8 +74,8 @@ class User < ApplicationRecord meeting_notify locale banned - admin - }.map!(&:freeze).freeze + admin + ].map!(&:freeze).freeze # Include default devise modules. Others available are: # :confirmable, :lockable, :timeoutable and :omniauthable @@ -98,7 +98,7 @@ class User < ApplicationRecord has_many :categories has_many :care_plan_contacts has_many :password_histories, dependent: :destroy - has_many :data_requests, class_name: "Users::DataRequest", foreign_key: :user_id + has_many :data_requests, class_name: 'Users::DataRequest', foreign_key: :user_id belongs_to :invited_by, class_name: 'User' after_initialize :set_defaults, unless: :persisted? @@ -163,42 +163,50 @@ def update_access_token new_access_token end - def build_csv_data() - user_data = [["user_info"]] + def build_csv_data + user_data = [['user_info']] user_data << DISPLAY_ATTRIBUTES - user_data << DISPLAY_ATTRIBUTES.map { |attribute| self.send(attribute.to_sym) } - user_data += Group.build_csv_rows(self.groups) - user_data += GroupMember.build_csv_rows(self.group_members) - user_data += Category.build_csv_rows(self.categories) - user_data += Medication.build_csv_rows(self.medications) - user_data += Strategy.build_csv_rows(self.strategies) - user_data += Moment.build_csv_rows(self.moments) - user_data += Notification.build_csv_rows(self.notifications) - user_data += Mood.build_csv_rows(self.moods) - user_data += CarePlanContact.build_csv_rows(self.care_plan_contacts) - user_data += Allyship.build_csv_rows(self.allyships) - user_data += MeetingMember.build_csv_rows(self.meeting_members) - return user_data + user_data << DISPLAY_ATTRIBUTES.map { |attribute| send(attribute.to_sym) } + user_data += Group.build_csv_rows(groups) + user_data += GroupMember.build_csv_rows(group_members) + user_data += Category.build_csv_rows(categories) + user_data += Medication.build_csv_rows(medications) + user_data += Strategy.build_csv_rows(strategies) + user_data += Moment.build_csv_rows(moments) + user_data += Notification.build_csv_rows(notifications) + user_data += Mood.build_csv_rows(moods) + user_data += CarePlanContact.build_csv_rows(care_plan_contacts) + user_data += Allyship.build_csv_rows(allyships) + user_data += MeetingMember.build_csv_rows(meeting_members) + user_data end - def generate_data_request() + def generate_data_request ActiveRecord::Base.transaction do - self.lock! - data_request = self.data_requests - .where(status_id: Users::DataRequest::STATUS[:enqueued]) - .first_or_initialize + lock! + data_request = data_requests + .where(status_id: Users::DataRequest::STATUS[:enqueued]) + .first_or_initialize return data_request.request_id if data_request.request_id.present? + data_request.request_id = SecureRandom.uuid data_request.save! return data_request.request_id end end - def delete_stale_data_file() - successful_data_requests = self.data_requests.where(status_id: Users::DataRequest::STATUS[:success]).order("updated_at desc") + def delete_stale_data_file + successful_data_requests = data_requests + .where( + status_id: Users::DataRequest::STATUS[:success] + ) + .order('updated_at desc') return if successful_data_requests.count < 2 + ActiveRecord::Base.transaction do - stale_data_requests = successful_data_requests.where.not(id: successful_data_requests.first) + stale_data_requests = successful_data_requests.where.not( + id: successful_data_requests.first + ) stale_data_requests.each do |dr| File.delete(dr.file_path) if File.exist?(dr.file_path) dr.update!(status_id: Users::DataRequest::STATUS[:deleted]) diff --git a/app/models/users.rb b/app/models/users.rb index c7af3cf2c9..f6d6b4a8a7 100644 --- a/app/models/users.rb +++ b/app/models/users.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true module Users def self.table_name_prefix 'users_' diff --git a/app/models/users/data_request.rb b/app/models/users/data_request.rb index 1d2a0deac5..fcd9b94f32 100644 --- a/app/models/users/data_request.rb +++ b/app/models/users/data_request.rb @@ -11,82 +11,86 @@ # updated_at :datetime not null # -class Users::DataRequest < ApplicationRecord +module Users + class DataRequest < ApplicationRecord + STATUS = { + enqueued: 1, + success: 2, + failed: 3, + deleted: 4 + }.freeze - STATUS = { - enqueued: 1, - success: 2, - failed: 3, - deleted: 4 - }.freeze + ASSOCIATIONS_TO_EXPORT = %i[ + allyships + group_members + groups + categories + medications + strategies + moments + notifications + moods + care_plan_contacts + meeting_members + ].freeze - ASSOCIATIONS_TO_EXPORT = %i{ - allyships - group_members - groups - categories - medications - strategies - moments - notifications - moods - care_plan_contacts - meeting_members - } + DEFAULT_FILE_PATH = Rails.root.join('tmp/csv_data/') - DEFAULT_FILE_PATH = "#{Rails.root}/tmp/csv_data/".freeze + belongs_to :user, class_name: '::User', foreign_key: 'user_id' - belongs_to :user, class_name: '::User', foreign_key: "user_id" - - after_commit :after_commit_tasks + after_commit :after_commit_tasks - validates_uniqueness_of :user_id, scope: [:status_id], - message: "There's already a request enqueued for this user.", if: -> {self.status_id == STATUS[:enqueued]} + validates :user_id, uniqueness: { + scope: :status_id, + message: 'There is already a request enqueued for this user.' + }, + if: -> { status_id == STATUS[:enqueued] } + + validates :request_id, uniqueness: { + message: 'There is already a request with this request_id.' + } - validates_uniqueness_of :request_id, - message: "There's already a request with this request_id." - - validates :status_id, - inclusion: { + validates :status_id, inclusion: { in: STATUS.values, - message: proc { |request| "'#{request.status_id}' is not valid."} + message: proc { |request| "'#{request.status_id}' is not valid." } }, - presence: true + presence: true - validates :request_id, presence: true + validates :request_id, presence: true - attr_accessor :file_path + def after_commit_tasks + return unless saved_change_to_id? && status_id == STATUS[:enqueued] - def after_commit_tasks() - enqueue_download_request() if (self.saved_change_to_id? && self.status_id == STATUS[:enqueued]) - end + enqueue_download_request + end - def enqueue_download_request() - ProcessDataRequestWorker.perform_async(self.request_id) - end + def enqueue_download_request + ProcessDataRequestWorker.perform_async(request_id) + end - def create_csv() - Dir.mkdir(DEFAULT_FILE_PATH) unless File.exist?(DEFAULT_FILE_PATH) - user = User.includes(*ASSOCIATIONS_TO_EXPORT).find(self.user_id) - begin - require 'csv' - csv_rows = user.build_csv_data() - CSV.open("#{self.file_path}","wb") do |csv_row| - csv_rows.each do |row| - csv_row << row + def create_csv + Dir.mkdir(DEFAULT_FILE_PATH) unless File.exist?(DEFAULT_FILE_PATH) + user = User.includes(*ASSOCIATIONS_TO_EXPORT).find(user_id) + begin + require 'csv' + csv_rows = user.build_csv_data + CSV.open(file_path, 'wb') do |csv_row| + csv_rows.each do |row| + csv_row << row + end end + self.status_id = STATUS[:success] + save! + user.delete_stale_data_file + rescue StandardError + File.delete(file_path) if file_path.present? && File.exist?(file_path) + self.status_id = STATUS[:failed] + save! end - self.status_id = STATUS[:success] - self.save! - user.delete_stale_data_file() - rescue => e - File.delete(self.file_path) if (self.file_path.present? && File.exist?(self.file_path)) - self.status_id = STATUS[:failed] - self.save! end - end - def file_path() - return "#{DEFAULT_FILE_PATH}#{self.request_id}.csv" + def file_path + "#{DEFAULT_FILE_PATH}#{request_id}.csv" + end end end diff --git a/app/workers/delete_stale_data_worker.rb b/app/workers/delete_stale_data_worker.rb index b205733630..cb9a533217 100644 --- a/app/workers/delete_stale_data_worker.rb +++ b/app/workers/delete_stale_data_worker.rb @@ -1,13 +1,17 @@ +# frozen_string_literal: true class DeleteStaleDataWorker include Sidekiq::Worker - + ACTIVE_DURATION = 30.days - def perform() - Users::DataRequest.where("updated_at < ?", (Time.current() - 30.days)) + def perform + Users::DataRequest + .where('updated_at < ?', (Time.current - 30.days)) .where(status_id: Users::DataRequest::STATUS[:success]) .each do |dr| - File.delete(self.file_path) if (self.file_path.present? && File.exist?(self.file_path)) + if dr.file_path.present? && File.exist?(dr.file_path) + File.delete(dr.file_path) + end dr.update(status_id: Users::DataRequest::STATUS[:deleted]) end end diff --git a/app/workers/process_data_request_worker.rb b/app/workers/process_data_request_worker.rb index d8c9f8377f..be9af42f66 100644 --- a/app/workers/process_data_request_worker.rb +++ b/app/workers/process_data_request_worker.rb @@ -1,10 +1,13 @@ +# frozen_string_literal: true class ProcessDataRequestWorker include Sidekiq::Worker sidekiq_options queue: 'critical' def perform(request_id) data_request = Users::DataRequest.find_by(request_id: request_id) - return if (data_request.blank? || data_request.status_id == Users::DataRequest::STATUS[:success]) - data_request.create_csv() + return if data_request.blank? + return if data_request.status_id == Users::DataRequest::STATUS[:success] + + data_request.create_csv end end From 3c0195ba34761ed9f0d40454def096922d7b9f74 Mon Sep 17 00:00:00 2001 From: Aashish Passrija <40358683+akp2603@users.noreply.github.com> Date: Thu, 15 Oct 2020 23:32:51 +0530 Subject: [PATCH 03/18] Delete reports.scss --- app/assets/stylesheets/users/reports.scss | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 app/assets/stylesheets/users/reports.scss diff --git a/app/assets/stylesheets/users/reports.scss b/app/assets/stylesheets/users/reports.scss deleted file mode 100644 index ea4277635a..0000000000 --- a/app/assets/stylesheets/users/reports.scss +++ /dev/null @@ -1,3 +0,0 @@ -// Place all the styles related to the Users::Reports controller here. -// They will automatically be included in application.css. -// You can use Sass (SCSS) here: http://sass-lang.com/ From f88ab4a9b6788e578366d439b0763cb92595e602 Mon Sep 17 00:00:00 2001 From: Aashish Passrija Date: Fri, 16 Oct 2020 03:09:32 +0530 Subject: [PATCH 04/18] changed variable name --- Gemfile | 3 --- app/models/allyship.rb | 2 +- app/models/application_record.rb | 4 ++-- app/models/care_plan_contact.rb | 2 +- app/models/category.rb | 2 +- app/models/group.rb | 2 +- app/models/group_member.rb | 2 +- app/models/medication.rb | 2 +- app/models/meeting_member.rb | 2 +- app/models/moment.rb | 2 +- app/models/mood.rb | 2 +- app/models/notification.rb | 2 +- app/models/strategy.rb | 2 +- app/models/user.rb | 6 +++--- 14 files changed, 16 insertions(+), 19 deletions(-) diff --git a/Gemfile b/Gemfile index d985059d12..741ed62331 100644 --- a/Gemfile +++ b/Gemfile @@ -53,11 +53,8 @@ gem 'selenium-webdriver', '~> 3.142.3' gem 'rubyzip', '~> 1.3.0' gem 'sidekiq', '5.0.5' -# Uniqueness in sidekiq jobs gem 'sidekiq-middleware' -# Keep track of failed jobs in sidekiq gem 'sidekiq-failures' -#corn job for sidekiq gem "sidekiq-cron", "~> 1.1" group :development, :test do diff --git a/app/models/allyship.rb b/app/models/allyship.rb index b2f7fd71c3..284672c088 100644 --- a/app/models/allyship.rb +++ b/app/models/allyship.rb @@ -12,7 +12,7 @@ # class Allyship < ApplicationRecord - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id created_at updated_at diff --git a/app/models/application_record.rb b/app/models/application_record.rb index ab5651199f..55263e298a 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -8,8 +8,8 @@ def self.build_csv_rows(objects) klass = objects.klass data = [["#{klass.name.underscore}_info"]] - attributes = if klass.const_defined?('DISPLAY_ATTRIBUTES') - klass.const_get('DISPLAY_ATTRIBUTES') + attributes = if klass.const_defined?('USER_DATA_ATTRIBUTES') + klass.const_get('USER_DATA_ATTRIBUTES') else klass.column_names end diff --git a/app/models/care_plan_contact.rb b/app/models/care_plan_contact.rb index eabbc17a4b..35fd4e2974 100644 --- a/app/models/care_plan_contact.rb +++ b/app/models/care_plan_contact.rb @@ -12,7 +12,7 @@ # class CarePlanContact < ApplicationRecord - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id name phone diff --git a/app/models/category.rb b/app/models/category.rb index c45d5e0182..b497b9eb4e 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -24,7 +24,7 @@ class Category < ApplicationRecord has_many :strategies_categories, dependent: :destroy validates :visible, inclusion: [true, false] - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id name description diff --git a/app/models/group.rb b/app/models/group.rb index 515f0f4a01..4dc18e5bc0 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -23,7 +23,7 @@ class Group < ApplicationRecord through: :group_members, source: :user after_destroy :destroy_notifications - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id name created_at diff --git a/app/models/group_member.rb b/app/models/group_member.rb index 8628fd6986..d335f0ab49 100644 --- a/app/models/group_member.rb +++ b/app/models/group_member.rb @@ -12,7 +12,7 @@ # class GroupMember < ApplicationRecord - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id group_id leader diff --git a/app/models/medication.rb b/app/models/medication.rb index c14dac8ae9..8e752626b8 100644 --- a/app/models/medication.rb +++ b/app/models/medication.rb @@ -26,7 +26,7 @@ class Medication < ApplicationRecord # total: total quantity of medication # strength: strength of medication - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id name dosage diff --git a/app/models/meeting_member.rb b/app/models/meeting_member.rb index eb7db3a98f..069c88dcf9 100644 --- a/app/models/meeting_member.rb +++ b/app/models/meeting_member.rb @@ -13,7 +13,7 @@ # class MeetingMember < ApplicationRecord - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id meeting_id leader diff --git a/app/models/moment.rb b/app/models/moment.rb index 6983e62552..c62b71072e 100644 --- a/app/models/moment.rb +++ b/app/models/moment.rb @@ -25,7 +25,7 @@ class Moment < ApplicationRecord include CommonMethods extend FriendlyId - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id name why diff --git a/app/models/mood.rb b/app/models/mood.rb index 4f6f440e30..98093167ed 100644 --- a/app/models/mood.rb +++ b/app/models/mood.rb @@ -14,7 +14,7 @@ # class Mood < ApplicationRecord - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id name description diff --git a/app/models/notification.rb b/app/models/notification.rb index 57ce031581..b2fc8283d2 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -12,7 +12,7 @@ # class Notification < ApplicationRecord - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id uniqueid data diff --git a/app/models/strategy.rb b/app/models/strategy.rb index bd3ccc9acb..53c2933772 100644 --- a/app/models/strategy.rb +++ b/app/models/strategy.rb @@ -22,7 +22,7 @@ class Strategy < ApplicationRecord include CommonMethods extend FriendlyId - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id description viewers diff --git a/app/models/user.rb b/app/models/user.rb index 1f32ae40d9..bc26204b04 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -51,7 +51,7 @@ class User < ApplicationRecord OAUTH_TOKEN_URL = 'https://accounts.google.com/o/oauth2/token' - DISPLAY_ATTRIBUTES = %w[ + USER_DATA_ATTRIBUTES = %w[ id email sign_in_count @@ -165,8 +165,8 @@ def update_access_token def build_csv_data user_data = [['user_info']] - user_data << DISPLAY_ATTRIBUTES - user_data << DISPLAY_ATTRIBUTES.map { |attribute| send(attribute.to_sym) } + user_data << USER_DATA_ATTRIBUTES + user_data << USER_DATA_ATTRIBUTES.map { |attribute| send(attribute.to_sym) } user_data += Group.build_csv_rows(groups) user_data += GroupMember.build_csv_rows(group_members) user_data += Category.build_csv_rows(categories) From aebe22b1b5cd0cbf5568ae82f4cd586546ab60db Mon Sep 17 00:00:00 2001 From: Aashish Passrija Date: Sat, 17 Oct 2020 04:33:18 +0530 Subject: [PATCH 05/18] request spec added --- spec/requests/users/reports_request_spec.rb | 92 ++++++++++++++++++++- 1 file changed, 90 insertions(+), 2 deletions(-) diff --git a/spec/requests/users/reports_request_spec.rb b/spec/requests/users/reports_request_spec.rb index 9a6ee54f4b..3d2405b2e9 100644 --- a/spec/requests/users/reports_request_spec.rb +++ b/spec/requests/users/reports_request_spec.rb @@ -1,5 +1,93 @@ -require 'rails_helper' - RSpec.describe "Users::Reports", type: :request do + let(:user) { create(:user) } + + describe '#submit_request' do + context 'when the user is logged in' do + before { sign_in user } + + it 'creates a data download request' do + post "/users/data" + expect(status).to eq(200) + data_request = Users::DataRequest.last + expect(JSON.parse(response.body)['request_id']).to eq(data_request.request_id) + expect(user.id).to eq(data_request.user_id) + end + end + + context 'when the user is not logged in' do + before { post "/users/data" } + it_behaves_like :with_no_logged_in_user + end + end + + describe '#fetch_request_status' do + context 'when the user is logged in' do + before { sign_in user } + + it 'fetches the status of data request with a blank request_id' do + get "/users/data/status" + expect(status).to eq(400) + expect(JSON.parse(response.body)).to have_key('error') + end + + it 'fetches the status of data request with a random request_id' do + params = { request_id: SecureRandom.uuid } + get "/users/data/status", params: params + expect(status).to eq(404) + expect(JSON.parse(response.body)).to have_key('error') + end + + it "creates a data request and then fetches it's status" do + post "/users/data" + expect(status).to eq(200) + params = { request_id: JSON.parse(response.body)['request_id'] } + get "/users/data/status", params: params + expect(status).to eq(200) + expect(JSON.parse(response.body)).to have_key('current_status') + end + end + + context 'when the user is not logged in' do + before { get "/users/data/status" } + it_behaves_like :with_no_logged_in_user + end + end + + describe '#download_data' do + context 'when the user is logged in' do + before { sign_in user } + + it 'fetches the file with a blank request_id' do + get "/users/data/download" + expect(status).to eq(400) + expect(JSON.parse(response.body)).to have_key('error') + end + + it 'fetches the file with a random request_id' do + params = { request_id: SecureRandom.uuid } + get "/users/data/download", params: params + expect(status).to eq(404) + expect(JSON.parse(response.body)).to have_key('error') + end + + it "creates a data request and then fetches it's status and then fetches the file" do + post "/users/data" + expect(status).to eq(200) + params = { request_id: JSON.parse(response.body)['request_id'] } + get "/users/data/status", params: params + expect(status).to eq(200) + expect(JSON.parse(response.body)).to have_key('current_status') + ProcessDataRequestWorker.new.perform(params[:request_id]) + data_request = Users::DataRequest.find_by(request_id: params[:request_id]) + get "/users/data/download", params: params + expect(status).to eq(200) + expect(File.exist?(data_request.file_path.to_s)).to be(true) + end + end + context 'when the user is not logged in' do + before { get "/users/data/download" } + it_behaves_like :with_no_logged_in_user + end + end end From 38cd98be2824f7bf202a9583de7ca2de450be805 Mon Sep 17 00:00:00 2001 From: Aashish Passrija <40358683+akp2603@users.noreply.github.com> Date: Sun, 18 Oct 2020 00:14:49 +0530 Subject: [PATCH 06/18] Delete delete_stale_data_worker_spec.rb --- spec/workers/delete_stale_data_worker_spec.rb | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 spec/workers/delete_stale_data_worker_spec.rb diff --git a/spec/workers/delete_stale_data_worker_spec.rb b/spec/workers/delete_stale_data_worker_spec.rb deleted file mode 100644 index 8a380b1212..0000000000 --- a/spec/workers/delete_stale_data_worker_spec.rb +++ /dev/null @@ -1,4 +0,0 @@ -require 'rails_helper' -RSpec.describe DeleteStaleDataWorker, type: :worker do - pending "add some examples to (or delete) #{__FILE__}" -end From e3546dbd3c84974674b0c4ce4c51c0a7210d7b9b Mon Sep 17 00:00:00 2001 From: Aashish Passrija <40358683+akp2603@users.noreply.github.com> Date: Sun, 18 Oct 2020 00:15:05 +0530 Subject: [PATCH 07/18] Delete process_data_request_worker_spec.rb --- spec/workers/process_data_request_worker_spec.rb | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 spec/workers/process_data_request_worker_spec.rb diff --git a/spec/workers/process_data_request_worker_spec.rb b/spec/workers/process_data_request_worker_spec.rb deleted file mode 100644 index a6dfad54cd..0000000000 --- a/spec/workers/process_data_request_worker_spec.rb +++ /dev/null @@ -1,4 +0,0 @@ -require 'rails_helper' -RSpec.describe ProcessDataRequestWorker, type: :worker do - pending "add some examples to (or delete) #{__FILE__}" -end From 12c1a64449e092e8b3733a1bbfa22f1c2e0b93b7 Mon Sep 17 00:00:00 2001 From: Aashish Passrija <40358683+akp2603@users.noreply.github.com> Date: Sun, 18 Oct 2020 00:15:47 +0530 Subject: [PATCH 08/18] Delete reports_helper_spec.rb --- spec/helpers/users/reports_helper_spec.rb | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 spec/helpers/users/reports_helper_spec.rb diff --git a/spec/helpers/users/reports_helper_spec.rb b/spec/helpers/users/reports_helper_spec.rb deleted file mode 100644 index 095d5a1356..0000000000 --- a/spec/helpers/users/reports_helper_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require 'rails_helper' - -# Specs in this file have access to a helper object that includes -# the Users::ReportsHelper. For example: -# -# describe Users::ReportsHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# expect(helper.concat_strings("this","that")).to eq("this that") -# end -# end -# end -RSpec.describe Users::ReportsHelper, type: :helper do - pending "add some examples to (or delete) #{__FILE__}" -end From 1967ff83c2763bdc192e654513ad54d7024595ab Mon Sep 17 00:00:00 2001 From: Aashish Passrija Date: Sun, 18 Oct 2020 01:57:07 +0530 Subject: [PATCH 09/18] data request model specs added --- app/workers/delete_stale_data_worker.rb | 2 +- spec/factories/users/data_requests.rb | 22 ++++++++++++-- spec/models/users/data_request_spec.rb | 30 +++++++++++++++++-- spec/requests/users/reports_request_spec.rb | 1 + spec/workers/delete_stale_data_worker_spec.rb | 1 - .../process_data_request_worker_spec.rb | 1 - 6 files changed, 49 insertions(+), 8 deletions(-) diff --git a/app/workers/delete_stale_data_worker.rb b/app/workers/delete_stale_data_worker.rb index cb9a533217..47ef77dbb9 100644 --- a/app/workers/delete_stale_data_worker.rb +++ b/app/workers/delete_stale_data_worker.rb @@ -6,7 +6,7 @@ class DeleteStaleDataWorker def perform Users::DataRequest - .where('updated_at < ?', (Time.current - 30.days)) + .where('updated_at < ?', (Time.current - ACTIVE_DURATION)) .where(status_id: Users::DataRequest::STATUS[:success]) .each do |dr| if dr.file_path.present? && File.exist?(dr.file_path) diff --git a/spec/factories/users/data_requests.rb b/spec/factories/users/data_requests.rb index 81e2f4ef73..0f4c8f996c 100644 --- a/spec/factories/users/data_requests.rb +++ b/spec/factories/users/data_requests.rb @@ -10,8 +10,26 @@ # updated_at :datetime not null # +EXAMPLE_UUID = "89ebd1e0-c500-4bda-b879-370e01f7f7f9" + FactoryBot.define do - factory :users_data_request, class: 'Users::DataRequest' do - + factory :partial_data_request, class: 'Users::DataRequest' do + transient do + use_example_uuid { false } + end + user { create(:user) } + request_id { use_example_uuid ? EXAMPLE_UUID : SecureRandom.uuid } + end + + factory :enqueued_data_request, parent: :partial_data_request do + status_id {Users::DataRequest::STATUS[:enqueued]} + end + + factory :invalid_status_data_request, parent: :partial_data_request do + status_id { 10 } + end + + factory :empty_request_id_data_request, parent: :enqueued_data_request do + request_id { nil } end end diff --git a/spec/models/users/data_request_spec.rb b/spec/models/users/data_request_spec.rb index 0d1af34810..88fdfd0dc5 100644 --- a/spec/models/users/data_request_spec.rb +++ b/spec/models/users/data_request_spec.rb @@ -10,8 +10,32 @@ # updated_at :datetime not null # -require 'rails_helper' - RSpec.describe Users::DataRequest, type: :model do - pending "add some examples to (or delete) #{__FILE__}" + context 'validations' do + it 'is invalid without a request_id' do + data_request = build(:empty_request_id_data_request) + expect(data_request).to have(1).error_on(:request_id) + end + + it 'is invalid without a status_id' do + data_request = build(:partial_data_request) + expect(data_request).to have(2).error_on(:status_id) + end + + it 'is invalid with an invalid status_id' do + data_request = build(:invalid_status_data_request) + expect(data_request).to have(1).error_on(:status_id) + end + + it 'is a valid data request object' do + data_request = create(:enqueued_data_request) + expect(data_request.valid?).to be(true) + end + + it 'fails unique request_id check' do + data_request1 = create(:enqueued_data_request, use_example_uuid: true) + data_request2 = build(:enqueued_data_request, use_example_uuid: true) + expect(data_request2.valid?).to be(false) + end + end end diff --git a/spec/requests/users/reports_request_spec.rb b/spec/requests/users/reports_request_spec.rb index 3d2405b2e9..3ef6485b56 100644 --- a/spec/requests/users/reports_request_spec.rb +++ b/spec/requests/users/reports_request_spec.rb @@ -82,6 +82,7 @@ get "/users/data/download", params: params expect(status).to eq(200) expect(File.exist?(data_request.file_path.to_s)).to be(true) + File.delete(data_request.file_path.to_s) if File.exist?(data_request.file_path.to_s) end end diff --git a/spec/workers/delete_stale_data_worker_spec.rb b/spec/workers/delete_stale_data_worker_spec.rb index 8a380b1212..8a265ea85c 100644 --- a/spec/workers/delete_stale_data_worker_spec.rb +++ b/spec/workers/delete_stale_data_worker_spec.rb @@ -1,4 +1,3 @@ -require 'rails_helper' RSpec.describe DeleteStaleDataWorker, type: :worker do pending "add some examples to (or delete) #{__FILE__}" end diff --git a/spec/workers/process_data_request_worker_spec.rb b/spec/workers/process_data_request_worker_spec.rb index a6dfad54cd..fc3af88102 100644 --- a/spec/workers/process_data_request_worker_spec.rb +++ b/spec/workers/process_data_request_worker_spec.rb @@ -1,4 +1,3 @@ -require 'rails_helper' RSpec.describe ProcessDataRequestWorker, type: :worker do pending "add some examples to (or delete) #{__FILE__}" end From aa369ed8c07b27b047a6732f5925bca27e32cc83 Mon Sep 17 00:00:00 2001 From: Aashish Passrija Date: Sun, 18 Oct 2020 02:22:16 +0530 Subject: [PATCH 10/18] rubocop fixes --- app/models/medication.rb | 2 ++ app/models/user.rb | 5 ++++- app/models/users/data_request.rb | 4 ++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/models/medication.rb b/app/models/medication.rb index 8e752626b8..889fa205dd 100644 --- a/app/models/medication.rb +++ b/app/models/medication.rb @@ -18,7 +18,9 @@ # comments :text # slug :string # add_to_google_cal :boolean default(FALSE) +# rubocop:disable Layout/LineLength # weekly_dosage :integer default(["0", "1", "2", "3", "4", "5", "6"]), is an Array +# rubocop:enable Layout/LineLength # class Medication < ApplicationRecord diff --git a/app/models/user.rb b/app/models/user.rb index bc26204b04..72c1862147 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -44,7 +44,7 @@ # admin :boolean default(FALSE) # third_party_avatar :text # - +# rubocop:disable Metrics/ClassLength class User < ApplicationRecord include PasswordValidator include AllyConcern @@ -98,7 +98,9 @@ class User < ApplicationRecord has_many :categories has_many :care_plan_contacts has_many :password_histories, dependent: :destroy + # rubocop:disable Layout/LineLength has_many :data_requests, class_name: 'Users::DataRequest', foreign_key: :user_id + # rubocop:enable Layout/LineLength belongs_to :invited_by, class_name: 'User' after_initialize :set_defaults, unless: :persisted? @@ -234,3 +236,4 @@ def access_token_expired? !access_expires_at || Time.zone.now > access_expires_at end end +# rubocop:enable Metrics/ClassLength diff --git a/app/models/users/data_request.rb b/app/models/users/data_request.rb index fcd9b94f32..c1c67f87d3 100644 --- a/app/models/users/data_request.rb +++ b/app/models/users/data_request.rb @@ -34,7 +34,7 @@ class DataRequest < ApplicationRecord meeting_members ].freeze - DEFAULT_FILE_PATH = Rails.root.join('tmp/csv_data/') + DEFAULT_FILE_PATH = Rails.root.join('tmp', 'csv_data') belongs_to :user, class_name: '::User', foreign_key: 'user_id' @@ -90,7 +90,7 @@ def create_csv end def file_path - "#{DEFAULT_FILE_PATH}#{request_id}.csv" + DEFAULT_FILE_PATH.join("#{request_id}.csv").to_s end end end From 9b2fa1f40692285e251896bf08c2ad188686d0e2 Mon Sep 17 00:00:00 2001 From: Aashish Passrija Date: Sun, 18 Oct 2020 02:28:10 +0530 Subject: [PATCH 11/18] refactoring --- app/models/users/data_request.rb | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/app/models/users/data_request.rb b/app/models/users/data_request.rb index c1c67f87d3..72df8f1f56 100644 --- a/app/models/users/data_request.rb +++ b/app/models/users/data_request.rb @@ -74,11 +74,7 @@ def create_csv begin require 'csv' csv_rows = user.build_csv_data - CSV.open(file_path, 'wb') do |csv_row| - csv_rows.each do |row| - csv_row << row - end - end + write_to_csv(csv_rows) self.status_id = STATUS[:success] save! user.delete_stale_data_file @@ -92,5 +88,15 @@ def create_csv def file_path DEFAULT_FILE_PATH.join("#{request_id}.csv").to_s end + + private + + def write_to_csv(csv_rows) + CSV.open(file_path, 'wb') do |csv_row| + csv_rows.each do |row| + csv_row << row + end + end + end end end From 47b0af0a082adce81622721d461fc886c1a1eebd Mon Sep 17 00:00:00 2001 From: Aashish Passrija Date: Sun, 18 Oct 2020 02:32:21 +0530 Subject: [PATCH 12/18] refactoring --- app/models/users/data_request.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/users/data_request.rb b/app/models/users/data_request.rb index 72df8f1f56..2c2fefad2b 100644 --- a/app/models/users/data_request.rb +++ b/app/models/users/data_request.rb @@ -61,6 +61,8 @@ class DataRequest < ApplicationRecord def after_commit_tasks return unless saved_change_to_id? && status_id == STATUS[:enqueued] + Dir.mkdir(DEFAULT_FILE_PATH) unless File.exist?(DEFAULT_FILE_PATH) + enqueue_download_request end @@ -69,7 +71,6 @@ def enqueue_download_request end def create_csv - Dir.mkdir(DEFAULT_FILE_PATH) unless File.exist?(DEFAULT_FILE_PATH) user = User.includes(*ASSOCIATIONS_TO_EXPORT).find(user_id) begin require 'csv' From 79085ff53dbb655721d343c6662aea8eda2bd2ae Mon Sep 17 00:00:00 2001 From: Aashish Passrija <40358683+akp2603@users.noreply.github.com> Date: Sun, 18 Oct 2020 02:42:51 +0530 Subject: [PATCH 13/18] Delete delete_stale_data_worker_spec.rb --- spec/workers/delete_stale_data_worker_spec.rb | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 spec/workers/delete_stale_data_worker_spec.rb diff --git a/spec/workers/delete_stale_data_worker_spec.rb b/spec/workers/delete_stale_data_worker_spec.rb deleted file mode 100644 index 8a265ea85c..0000000000 --- a/spec/workers/delete_stale_data_worker_spec.rb +++ /dev/null @@ -1,3 +0,0 @@ -RSpec.describe DeleteStaleDataWorker, type: :worker do - pending "add some examples to (or delete) #{__FILE__}" -end From 27a6b7d7f787f1fd39a117816c06c3a175d4cfc5 Mon Sep 17 00:00:00 2001 From: Aashish Passrija <40358683+akp2603@users.noreply.github.com> Date: Sun, 18 Oct 2020 02:43:05 +0530 Subject: [PATCH 14/18] Delete process_data_request_worker_spec.rb --- spec/workers/process_data_request_worker_spec.rb | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 spec/workers/process_data_request_worker_spec.rb diff --git a/spec/workers/process_data_request_worker_spec.rb b/spec/workers/process_data_request_worker_spec.rb deleted file mode 100644 index fc3af88102..0000000000 --- a/spec/workers/process_data_request_worker_spec.rb +++ /dev/null @@ -1,3 +0,0 @@ -RSpec.describe ProcessDataRequestWorker, type: :worker do - pending "add some examples to (or delete) #{__FILE__}" -end From cbf22e152a2e48d3ee6f562be3101d4def77d6ed Mon Sep 17 00:00:00 2001 From: Aashish Passrija Date: Sun, 18 Oct 2020 03:16:49 +0530 Subject: [PATCH 15/18] added test.example.env --- config/env/test.example.env | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/config/env/test.example.env b/config/env/test.example.env index cdc310b186..caa7dc5f9d 100644 --- a/config/env/test.example.env +++ b/config/env/test.example.env @@ -39,3 +39,7 @@ RAISE_DELIVERY_ERRORS="false" PSQL_HOST="" PSQL_USERNAME="" PSQL_PASSWORD="" + +# REDIS +REDIS_HOST="" +REDIS_PORT="" \ No newline at end of file From 9e04274ae1ecda9c374729457ebc0cc76b47c4de Mon Sep 17 00:00:00 2001 From: Julia Nguyen Date: Sat, 17 Oct 2020 14:41:28 -0700 Subject: [PATCH 16/18] Setup Redis in Circle CI config --- .circleci/config.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index bd5e40954a..a9961d4ec0 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -17,6 +17,11 @@ defaults: &defaults POSTGRES_USER: circleci POSTGRES_DB: ifme_test POSTGRES_PASSWORD: "" + - image: redis:4.2.2 + environment: + REDIS_HOST: redis + # The default Redis port + REDIS_PORT: 6379 steps: - checkout jobs: From b1502a6db3fcd2990fc861d52b82813e368f15ee Mon Sep 17 00:00:00 2001 From: Aashish Passrija <40358683+akp2603@users.noreply.github.com> Date: Sun, 18 Oct 2020 03:34:40 +0530 Subject: [PATCH 17/18] Update config.yml --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a9961d4ec0..c3e14aa5cc 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -17,7 +17,7 @@ defaults: &defaults POSTGRES_USER: circleci POSTGRES_DB: ifme_test POSTGRES_PASSWORD: "" - - image: redis:4.2.2 + - image: redis environment: REDIS_HOST: redis # The default Redis port @@ -207,4 +207,4 @@ workflows: branches: ignore: /.*/ tags: - only: /^v[0-9]+\.[0-9]+\.[0-9]+/ \ No newline at end of file + only: /^v[0-9]+\.[0-9]+\.[0-9]+/ From ba3c682f4972459a3f86a6f56e559fef8bede76c Mon Sep 17 00:00:00 2001 From: Aashish Passrija Date: Sun, 18 Oct 2020 08:07:41 +0530 Subject: [PATCH 18/18] review changes --- app/helpers/users/reports_helper.rb | 4 ++-- app/workers/process_data_request_worker.rb | 4 ++-- spec/requests/users/reports_request_spec.rb | 26 ++++++++++----------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/app/helpers/users/reports_helper.rb b/app/helpers/users/reports_helper.rb index 183f1c710f..dfd3ea05f0 100644 --- a/app/helpers/users/reports_helper.rb +++ b/app/helpers/users/reports_helper.rb @@ -8,7 +8,7 @@ def submit_request_helper(user) end def fetch_request_status_helper(user, request_id) - return 400, { error: 'Request id can be blank.' } if request_id.blank? + return 400, { error: "Request id can't be blank." } if request_id.blank? data_request = user.data_requests.find_by(request_id: request_id) if data_request.blank? @@ -19,7 +19,7 @@ def fetch_request_status_helper(user, request_id) end def download_data_helper(user, request_id) - return 400, { error: 'Request id can be blank.' } if request_id.blank? + return 400, { error: "Request id can't be blank." } if request_id.blank? data_request = user.data_requests.find_by( request_id: request_id, diff --git a/app/workers/process_data_request_worker.rb b/app/workers/process_data_request_worker.rb index be9af42f66..eaaa8e61bc 100644 --- a/app/workers/process_data_request_worker.rb +++ b/app/workers/process_data_request_worker.rb @@ -5,8 +5,8 @@ class ProcessDataRequestWorker def perform(request_id) data_request = Users::DataRequest.find_by(request_id: request_id) - return if data_request.blank? - return if data_request.status_id == Users::DataRequest::STATUS[:success] + return if data_request.blank? || + data_request.status_id == Users::DataRequest::STATUS[:success] data_request.create_csv end diff --git a/spec/requests/users/reports_request_spec.rb b/spec/requests/users/reports_request_spec.rb index 3ef6485b56..3228e48147 100644 --- a/spec/requests/users/reports_request_spec.rb +++ b/spec/requests/users/reports_request_spec.rb @@ -6,7 +6,7 @@ before { sign_in user } it 'creates a data download request' do - post "/users/data" + post users_data_path expect(status).to eq(200) data_request = Users::DataRequest.last expect(JSON.parse(response.body)['request_id']).to eq(data_request.request_id) @@ -15,7 +15,7 @@ end context 'when the user is not logged in' do - before { post "/users/data" } + before { post users_data_path } it_behaves_like :with_no_logged_in_user end end @@ -25,30 +25,30 @@ before { sign_in user } it 'fetches the status of data request with a blank request_id' do - get "/users/data/status" + get users_data_status_path expect(status).to eq(400) expect(JSON.parse(response.body)).to have_key('error') end it 'fetches the status of data request with a random request_id' do params = { request_id: SecureRandom.uuid } - get "/users/data/status", params: params + get users_data_status_path, params: params expect(status).to eq(404) expect(JSON.parse(response.body)).to have_key('error') end it "creates a data request and then fetches it's status" do - post "/users/data" + post users_data_path expect(status).to eq(200) params = { request_id: JSON.parse(response.body)['request_id'] } - get "/users/data/status", params: params + get users_data_status_path, params: params expect(status).to eq(200) expect(JSON.parse(response.body)).to have_key('current_status') end end context 'when the user is not logged in' do - before { get "/users/data/status" } + before { get users_data_status_path } it_behaves_like :with_no_logged_in_user end end @@ -58,28 +58,28 @@ before { sign_in user } it 'fetches the file with a blank request_id' do - get "/users/data/download" + get users_data_download_path expect(status).to eq(400) expect(JSON.parse(response.body)).to have_key('error') end it 'fetches the file with a random request_id' do params = { request_id: SecureRandom.uuid } - get "/users/data/download", params: params + get users_data_download_path, params: params expect(status).to eq(404) expect(JSON.parse(response.body)).to have_key('error') end it "creates a data request and then fetches it's status and then fetches the file" do - post "/users/data" + post users_data_path expect(status).to eq(200) params = { request_id: JSON.parse(response.body)['request_id'] } - get "/users/data/status", params: params + get users_data_status_path, params: params expect(status).to eq(200) expect(JSON.parse(response.body)).to have_key('current_status') ProcessDataRequestWorker.new.perform(params[:request_id]) data_request = Users::DataRequest.find_by(request_id: params[:request_id]) - get "/users/data/download", params: params + get users_data_download_path, params: params expect(status).to eq(200) expect(File.exist?(data_request.file_path.to_s)).to be(true) File.delete(data_request.file_path.to_s) if File.exist?(data_request.file_path.to_s) @@ -87,7 +87,7 @@ end context 'when the user is not logged in' do - before { get "/users/data/download" } + before { get users_data_download_path } it_behaves_like :with_no_logged_in_user end end