API considerations

[zejn]

Reading invoice_demo.py it is not obvious no remote calls are being made. One is clearly creating API object. It is used only for signing, but this is only clear after reading api.py.

I think it would make more sense to have one API class, not three separate (FURSBaseAPI, FURSBusinessPremiseAPI and FURSInvoiceAPI), which would handle all remote calls. Using this class then makes clear remote calls are being made.

Signing functions are also used locally, it may be sensible to bring signing logic/cert handling together. Signing is now done in connector.py and in base_api.py.

Local Invoice API part could maybe benefit from a richer set of objects, such as:

class Register(object):
    invoice_serial = 0
    def __init__(self, tax_number, business_premise_id, electronic_device_id):
        self.tax_number = tax_numbe
        self.business_premise_id = business_premise_id
        self.electronic_device_id = electronic_device_id

    def invoice(self, issued_date, invoice_amount, low_tax_rate_base, low_tax_rate_amount, ...):
        # generate Invoice object, which has ZOI and EOR (after requesting) attributes
        Register.invoice_serial += 1
        return Invoice(Register.invoice_serial, issued_date, invoice_amount, ...)

Since there's a possibility of reissuing requests, you also probably want to abstract this and keep the state of invoice somewhere, maybe check presence of eor attribute.

Also consider adding warning if you see floats as input values. All kinds of madness lies in mixing floats with money.

[boris-savic]

Hi Zejn,

thanks for the PR (will check it on the weekend) and the suggestion here. I gave quite some thought to the library design and at the end I decided to follow the same grouping logic as the FURS did in their technical documentation [1]. API is split to Register and BusinessPremise - those two are intended for other developers to use - the Base is there because of DRY principles. It's split into two classes because registering of the business premise is done only once and that is usually on the serverside whereas EOR is requested for every invoice.

I agree that if one has not read the FURS documentation at [1] it might not be clear which method calls invoke HTTP requests and which don't. I tried to name the functions appropriately so that "get_invoice_eor" means it does the API call and "calculate_zoi" does what it says - calculates.

I prefer to keep the library stateless and without rich objects. It does mean there are more parameters to pass to method on call, but I think it's better that way - less prone to errors.

As for the floats - yes will have to show warnings in that case, but I also have to figure out how to serialize Decimals without getting all kinds of exceptions, since "json.dumps" is being called inside Jose library.

[1] http://www.datoteke.fu.gov.si/dpr/files/TehnicnaDokumentacijaVer1.3.pdf