amcintosh / freshbooks-java-sdk

FreshBooks API wrapper
MIT License
2 stars 2 forks source link

Support Other Income endpoint #3 #13

Closed raernon closed 3 years ago

raernon commented 3 years ago

Here it is the Other Income support. I had no information about the categories of Other Incomes, whether they tied to the api defined variants or they can be any free text, so I created an enum with these values: advertising, in_person_sales, online_sales, rentals, other. If the enum isn't appropriate for this field, it's easier to delete the OtherIncomeCategory enum and write back the model to a simple String category field. Because of the lack of detailed informations about DELETE method and the api shows us that it won't return anything, therefore I didn't write test for DELETE Other Income. Maybe it can be tested with assertDoesNotThrow(), but this method can be found only in JUnit5. I hope this code will be appropriate, but please let me know if I have to alter it somehow.

amcintosh commented 3 years ago

Hi @raernon, The category enum should be fine. The official node sdk does the same: https://github.com/freshbooks/freshbooks-nodejs-sdk/blob/master/packages/api/src/models/OtherIncome.ts#L8-L14

Some accounting endpoints at FreshBooks use the vis_state PUT to delete, but a few have had DELETE calls. Invoices is one which I tested here: https://github.com/amcintosh/freshbooks-java-sdk/blob/master/lib/src/test/java/net/amcintosh/freshbooks/resources/InvoicesTest.java#L253-L269 The test isn't ideal as it's just counting on the network fail if the mock isn't triggered, but we could do that for now in OtherIncome. I like the idea of assertDoesNotThrow() and I should probably look into moving to JUnit5. Thanks.

amcintosh commented 3 years ago

Thank you for this!