Add article, article and client property value, article and client tag and some documentation.

[JohnDalyGivve]

I’ve added some code to support the way we plan to use Billomat and added some documentation for my coworkers. I’m interested in getting my current (and future) work mainlined. My changes are:

added list and paed list methods to base model
added utils and moved to_array and count to it

added models:
article
article property value
article tag
client property value
client tag

added documentation for:
articles
clients
invoices

[Jack12816]

Please take care of the copy paste glitches. Billomat::Actions::Complete all over the place, correct to Billomat::Actions::Correct.

[Jack12816]

Conventionally this helper should be named out_of_bound? as it returns a boolean.

    # @param [Hash] paging_data The response from get_paging_data
    # @return [Boolean] whenever the given paging data is out of bound or not
    def self.out_of_bound?(paging_data)

[Jack12816]

Conventionally Ruby methods do not prefix get_ or set_ - just name it paging_data.

[Jack12816]

This method/design is not robust. Unfortunately we do not require/added ActiveSupport as runtime dependency - so we do not have String#pluralize, but it would be better to return the fully processed input string instead of returning a string suffix.

For future ActiveSupport addition it would be great if you could refactor this like so:

def self.pluralized_resource(name)
  # NOTE: When ActiveSupport gets included convert this to +String.pluralize+.

  return 'taxes' if name == 'tax'

  "#{name}s"
end

Then call it like this later:

resp[pluralized_resource(name)]['@total'].to_i

It is also not required prefix Billomat::Utils here, inside the same file to use a static method.

[sos4nt]

Since there's already resource_name, why not handle it within the model(s), e.g.:

module Billomat
  module Models
    class Tax < Base
      def self.base_path
        '/taxes'
      end

      def self.base_name
        'taxes'
      end

      def self.resource_name
        'tax'
      end
    end
  end
end

base_path is so generic that it can probably be moved into Billomat::Models::Base:

def self.base_path
  "/#{base_name}"
end

This way, you'd have:

resp[@resource.base_name]['@total'].to_i

# or

resp[@resource.base_name][@resource.resource_name]

[Jack12816]

I'm fine with this, too!

[Jack12816]

A mapped object with method access would be nicer to read/use at the clients.

article_tags = Billomat::Models::ArticleTag.paged_list(page = 2, per_page = 10)

pp article_tags.data
# => [Array<Billomat::Models::ArticleTag>]

pp article_tags.page
# => Int
pp article_tags.per_page
# => Int
pp article_tags.total
# => Int

[Jack12816]

Thank you very much for your effort! This PR is a great enhancement for the project, nice! 🎖️

I just left some minor comments and refactorings, but nothing to fancy.

Please add new line(s) describing the changes (all suffixed with (#20)) at the https://github.com/hausgold/billomat/blob/master/CHANGELOG.md file.

Another good thing would be some specs - at least for the new models like: https://github.com/hausgold/billomat/blob/master/spec/models/invoice_spec.rb
And some specs for all new utility methods (and maybe to_array/count, too).

[JohnDalyGivve]

Hi Jack,

I'll look into the suggestions.