Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow "open" date range in AmountsExtension balance method via defaults #56

Open
davidkuhta opened this issue Dec 18, 2015 · 1 comment

Comments

@davidkuhta
Copy link

Hi @mbulat
Currently the balance method in AmountsExtension supports two cases:

  1. All - No date hash provided; return sum of all amounts
  2. Closed Range - Hash provided with from_date and to_date; return sum of all amounts between these dates.

I believe that a reasonable argument could be made for provision of two additional cases, based on the frequency with which to_date = Date.today is used and eliminating the need to find the earliest entry date to set a from_date:

  1. Open Range (after) - Hash provided with from_date; return sum of all amounts after this date (up to today).
  2. Open Range (before) - Hash provided with to_date; return sum of all amounts before this date (down to the earliest entry).

Current balance method:

   def balance(hash={})
      if hash[:from_date] && hash[:to_date]
        from_date = hash[:from_date].kind_of?(Date) ? hash[:from_date] : Date.parse(hash[:from_date])
        to_date = hash[:to_date].kind_of?(Date) ? hash[:to_date] : Date.parse(hash[:to_date])
        includes(:entry).where('plutus_entries.date' => from_date..to_date).sum(:amount)
      else
        sum(:amount)
      end
    end

Revised method:

   def balance(hash={})
      if hash[:from_date] && hash[:to_date]
        from_date = hash[:from_date].kind_of?(Date) ? hash[:from_date] : Date.parse(hash[:from_date])
        to_date = hash[:to_date].kind_of?(Date) ? hash[:to_date] : Date.parse(hash[:to_date])
        includes(:entry).where('plutus_entries.date' => from_date..to_date).sum(:amount)
      elsif hash[:from_date]
        # if only :from_date provided default assumes all amounts requested after that date
        from_date = hash[:from_date].kind_of?(Date) ? hash[:from_date] : Date.parse(hash[:from_date])
        includes(:entry).where('plutus_entries.date >= ?, from_date).sum(:amount)
      elsif hash[:to_date]
        # if only :to_date provided default assumes all amounts requested before that date
        to_date = hash[:to_date].kind_of?(Date) ? hash[:to_date] : Date.parse(hash[:to_date])
        includes(:entry).where('plutus_entries.date <= ?, to_date).sum(:amount)
      else
        sum(:amount)
      end
    end

Some class methods might make this neater

   def balance(hash={})
      if hash[:from_date] && hash[:to_date]
        from_date = hash[:from_date].kind_of?(Date) ? hash[:from_date] : Date.parse(hash[:from_date])
        to_date = hash[:to_date].kind_of?(Date) ? hash[:to_date] : Date.parse(hash[:to_date])
        between(from_date, to_date)
      elsif hash[:from_date]
        # if only :from_date provided default assumes all amounts requested after that date
        from_date = hash[:from_date].kind_of?(Date) ? hash[:from_date] : Date.parse(hash[:from_date])
        after(from_date)
      elsif hash[:to_date]
        # if only :to_date provided default assumes all amounts requested before that date
        to_date = hash[:to_date].kind_of?(Date) ? hash[:to_date] : Date.parse(hash[:to_date])
        before(to_date)
      else
        sum(:amount)
      end
    end

  def self.between(from_date, to_date)
    includes(:entry).where('plutus_entries.date' => from_date..to_date).sum(:amount)
  end
  def self.after(from_date, to_date)
    includes(:entry).where('plutus_entries.date >= ?, from_date).sum(:amount)
  end
  def self.before(to_date)
    includes(:entry).where('plutus_entries.date <= ?, to_date).sum(:amount)
  end

I don't see any impact to existing installations either as the existing if hash[:from_date] && hash[:to_date] and else catch-all remain unchanged.

Let me know if you'd like a PR.

@mbulat
Copy link
Owner

mbulat commented May 12, 2016

Yes, I like this implementation. PR is welcome! Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants