Make Transaction History an optional Economy API feature

[lokka30]

I believe that Account#retrieveTransactionHistory should be an optional Economy API feature. Smaller economy providers will likely rather not deal with the hassle of implementing this feature.

[MrIvanPlays]

no

[MrNemo64]

no

Care to explain a little?

[lokka30]

Out of interest to make this an API appealing to everyone. Not to the few individuals who have all the time in the world they want to make a fully-featured economy provider plugin. I don't understand how this is a necessary feature, and I doubt it will be utilized much in the first place. Merely stating a one-word opinion does not achieve anything, this consistently happens.

[MrIvanPlays]

Big or small either way transactions are stored so this is not too hard to implement.

[lokka30]

Big or small either way transactions are stored so this is not too hard to implement.

There is no transaction storage unless one wishes to add functionality to the transaction history method

[MrIvanPlays]

Then in that case lets make currencies an optional api feature. Lets make non player accounts also an optional api feature.

Of course I'm calling these with irony and I won't support them being an optional api features, neither I will support transactions being an optional api feature. Either way you're too late to the party ; e.g we need method changes in order to make it an optional api, perhaps breaking. Fairly sure you don't want to break any of the existing api.

[lokka30]

Then in that case lets make currencies an optional api feature. Lets make non player accounts also an optional api feature.

Of course I'm calling these with irony and I won't support them being an optional api features, neither I will support transactions being an optional api feature. Either way you're too late to the party ; e.g we need method changes in order to make it an optional api, perhaps breaking. Fairly sure you don't want to break any of the existing api.

This issue's idea requires zero breaking changes. The outcomes are:

• economy providers don't have to store and retrieve transactions (but they should!)
• economy consumers should check the optional economy API feature set for the current economy provider on the server before trying to access the transaction history method

There are no methods removed or added, just an enum constant and a new line in the retrieval method's javadoc.

The alternative to this being added, is that the developers who can't/won't implement this, will either do a botchy attempt at 'making it work', or plainly fail the subscriber with the 'feature not supported' reason.

I have also thought about doing this for non-players, but I am quite cemented against such a change. I don't want to overcomplicate the API with optional features, especially those of such caliber.

[MrIvanPlays]

if we want quality economy plugins then we shouldn't have any optional api whatsoever. doesn't treasury want quality plugins? wasn't this one of the foundations of why treasury was born? if a developer can't write to (a) file(s) then in my opinion he doesn't have a basic understanding of java. there's nothing more straightforward than reading and writing to (a) file(s).

[lokka30]

if we want quality economy plugins then we shouldn't have any optional api whatsoever. doesn't treasury want quality plugins? wasn't this one of the foundations of why treasury was born? if a developer can't write to (a) file(s) then in my opinion he doesn't have a basic understanding of java. there's nothing more straightforward than reading and writing to (a) file(s).

I don't mind going this route. Do you think we should just remove the optional economy API features entirely? I don't see why one can't fire Treasury's events, nor support negative balances.

[MrIvanPlays]

supporting negative balances is not as straightforward as calling the events. negative numbers can be a bomb with a clock mechanism and that's the reason im not going to support completely ditching the optional api features, although you have a point with the events so im for making the event calls a required api, especially since we have our own events api with examples and documentation. but i wont deprecate the optional api feature now, rather id wait for 2.0.0 and directly remove it.

[MrNemo64]

if we want quality economy plugins then we shouldn't have any optional api whatsoever. doesn't treasury want quality plugins? wasn't this one of the foundations of why treasury was born? if a developer can't write to (a) file(s) then in my opinion he doesn't have a basic understanding of java. there's nothing more straightforward than reading and writing to (a) file(s).

I don't mind going this route. Do you think we should just remove the optional economy API features entirely? I don't see why one can't fire Treasury's events, nor support negative balances.

I would say that events are similar to the transaction history but not the negative balances. The history and the events are an API feature mainly used to gather information of things that other consumers may be doing. The negative balances, however, it's a provider decision, a provider may consider that an account with a negative balance makes no sense because reason A, B and C so therefore those are not supported while another provider considers that negative balances make sense because reason D, E and F so therefore they support it. History and events are api parts while negative supports is a decision that the provider makes depending on if they think it's logical to have them.

[lokka30]

Thank you both for the great opinions, I completely agree. I'll make a new issue for 'make transaction events no longer optional for the economy api'.