Задание по рефакторингу

readme.md

Задание по рефакторингу

Инструкции

Для выполнения задания требуется:

• Отрефакторить данный пример в тот вид, который считаешь приемлимым
• Добавить описание:
  • того, что изменилось и почему
  • с какими проблемами столкнулся в процессе
  • любые идеи и мысли, чтобы еще сделал при наличии дополнительного времени, и т.д.

История

Нам захотелось попробовать возможность отправки различных промо сообщений новым пользователям. Поэтому, мы разработали функционал, который позволяет администраторам:

• Создавать и отправлять промо сообщения пользователям, которые опубликовали свое первое объявление в указанный промежуток времени.
• Скачать CSV файл со списком получателей.

Для упрощения модели и контроллеры находятся в одном файле (example.rb), что должно дать общее представление для выполнения задания.

example.rb

# По стилю
# - компактные декларативные методы с по возможности понятными именами
# - предпочтительно guard`ы вместо ветвления условий
# - одинарные кавычки вместо двойных
# - длинна строки в 100 (минорно, дело вкуса)

# Модели
# - использовать базовый класс ApplicationRecord и от него наследовать модели
# - предпочтительно использовать методы active record вместо sql
# - `with_posts_published` - выглядит не совсем корректным, тут нужно уточнить структуру базы
#    есть ли в ней колонка/индекс published_ads_count и тогда либо работать с ним,
#    либо менять логику

# Контроллеры
# - не забивать sidekiq redis целыми обьектами + в интервале schedule->execute данные могут быть
#   изменены - использовать object.id
# - отдельный ресурс/контроллер для promo messages/отдельный для просмотра/экспорта промо
#   пользователей
# - public_send вместо send
# - PromoUsers concern, который можно использовать для работы с юзерами для promo рассылки

class ApplicationRecord < ActiveRecord::Base
end

class PromoMessage < ApplicationRecord
end

class User < ApplicationRecord
  has_many :ads
  scope :recent, -> { order(created_at: :desc) }

  # TODO: observe db structure and fix/refactor
  def with_posts_published(from:, to:)
    joins(:ads)
      .where(ads: { published_at: from..to })
      .group('ads.user_id').where('`published_ads_count` = 1')
  end
end

class Ad < ApplicationRecord
end

#
class PromoMessagesController
  include PromoUsers

  def new
    build_message
    load_users
  end

  def create
    build_message
    load_users

    if save_message
      schedule_sending
      render_success
    else
      render_errors
    end
  end

  private

  def build_message
    @message = PromoMessage.new(promo_params)
  end

  def save_message
    @message.save
  end

  def render_success
    redirect_to promo_messages_path, notice: 'Messages scheduled for sending.'
  end

  def render_errors
    render :new # TODO: make sure view is rendering errors
  end

  def schedule_sending
    @users.each do |user|
      PromoMessagesSendJob.perform_later(user.id) # TODO: update job to get user.phone
    end
  end

  def promo_params
    promo_params = params[:message]
    promo_params ? message_params.permit(:body, :date_from, :date_to) : {}
  end
end

class PromoUsersController
  include PromoUsers

  def index
    load_users
    render_csv
  end

  private

  def report_filename
    "promotion-users-#{Time.zone.today}.csv"
  end

  def render_csv
    send_data csv(@users), filename: report_filename
  end

  private

  # TODO: extract to NewUsersCsv if going to share/structure is going to be more complicated
  def csv(data)
    header = %w(id phone name)

    CSV.generate(headers: true) do |csv|
      csv << header

      data.each do |user|
        csv << attributes.map { |attr| user.public_send(attr) }
      end
    end
  end

  def promo_params
    promo_params = params[:filters]
    promo_params ? promo_params.permit(:date_from, :date_to) : {}
  end
end

# Loading users applicable for promo messages sending
module PromoUsers
  private

  def load_users
    return unless (date_from && date_to)

    @users = User.recent.with_posts_published(from: date_from, to: date_to).page(params[:page])
  end

  # TODO: define date format and use strptime instead of parse
  def date_from
    Date.parse(promo_params[:date_from]) rescue nil
  end

  def date_to
    Date.parse(promo_params[:date_to]) rescue nil
  end

  def promo_params
    raise NotImplementedError, self
  end
end