Added missing segments API calls (v11)

[rcerljenko]

Hi,

In addition to my previous work, this one adds a new "Segments" API calls for Create, List and Delete actions.

[norkunas]

As discussed in #172 we should start with request payload objects and static analysis instead of data resolvers in a forward compatible way and then deprecate the old apis :)

[rcerljenko]

How do we validate payload objects? Via phpdoc ?

[norkunas]

Yes, with SA tools, so no overhead on runtime perf :)

[rcerljenko]

Ok I'll try

[norkunas]

Instead of direct arrays add an payload object so users instantiate it and get autocomplete for available methods :)

[rcerljenko]

@norkunas check it out now if that's it

[rcerljenko]

aha I see... you mean with some sort of DTO?

[norkunas]

Yes ;)

[rcerljenko]

@norkunas how about now? i'll just fix the fixer erros if that's ok

[norkunas]

Will continue tomorrow, going afk, thanks :)

[norkunas]

Looks good, now the tests should be added :)

[rcerljenko]

Looks good, now the tests should be added :)

ah yes, we love writing tests yes

[norkunas]

I think we should introduce new method as a replacement for sendRequest as throwing in previous apis will be a BC break. Maybe makeRequest :thinking:

[rcerljenko]

I think we should introduce new method as a replacement for sendRequest as throwing in previous apis will be a BC break. Maybe makeRequest 🤔

this is going to be ugly since I can't reuse sendRequest() i would need to copy entire method in order to get both the Request and Response object instances

[norkunas]

I think we should introduce new method as a replacement for sendRequest as throwing in previous apis will be a BC break. Maybe makeRequest 🤔

this is going to be ugly since I can't reuse sendRequest() i would need to copy entire method in order to get both the Request and Response object instances

yeah, but better than adding a bool flag to toggle throwing and deprecating extending OneSignal::sendRequest method without that argument which would be later just dropped..

[rcerljenko]

I think we should introduce new method as a replacement for sendRequest as throwing in previous apis will be a BC break. Maybe makeRequest 🤔

this is going to be ugly since I can't reuse sendRequest() i would need to copy entire method in order to get both the Request and Response object instances

yeah, but better than adding a bool flag to toggle throwing and deprecating extending OneSignal::sendRequest method without that argument which would be later just dropped..

done :)

[norkunas]

good :muscle: now tests, after this PR will try to spend time on this also :)

[rcerljenko]

Could you squash all commits? :)

Can't you do that on merge?

Also, I still have to write test for "Create segment" method.

[rcerljenko]

@norkunas well this is it i guess... :)

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges#squash-and-merge-your-commits

[norkunas]

Thank you :) now will need to add remaining apis and deprecate old ones for a new release

[rcerljenko]

Thank you :) now will need to add remaining apis and deprecate old ones for a new release

Yes. Bit by bit... :) But at least we now have a "modus operandi"

[norkunas]

true :)

[rcerljenko]

will you trigger a new release soon?

[norkunas]

Wont do partial releases for now

[norkunas]

If you need you can always use dev branch in composer json :)