From #2725247: Review gateway plugins, sort out what to do with \Drupal\sms\Plugin\SmsGatewayPluginInterface::balance(). It is not used by SMS Framework nor does it have test coverage.

Change name from balance() to getBalance() (similar to getDeliveryReports()) or getCreditsBalance() (preferred, similar to new reports). The 'get' prefix implies we need an external request, and should be used sparingly.

Make signature return float|NULL. NULL if unknown.

Add an annotation to gateway which determines whether the gateway supports balance inquiries.

Where can we use the balance system in SMS framework?

  • Informational: In gateway list?, status page?
  • Can we take action in any way based on known balance, or is it just informational?
  • A more intelligent SMS routing system can use this information to shortlist gateways that are available. (from #3)
  • A notification system can use this to inform the administrator that the available credits is running out (from #3)
CommentFileSizeAuthor
#6 gateway-balance-2798161-6-bf5749f.patch7.78 KBdpi
#2 gateway-balance-2798161.patch7.74 KBdpi

Comments

dpi created an issue. See original summary.

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Active » Needs review
StatusFileSize
new gateway-balance-2798161.patch7.74 KB

https://github.com/dpi/smsframework/pull/53

This patch changes:

  • Gateway::balance() to Gateway::getCreditsBalance()
  • Changes gateway base class default getCreditsBalance() value from '0' to 'NULL'.
  • Adds 'supports credit query annotation'
  • Adds tests to credit query annotation, and two other existing annotations: schedule_aware and outgoing_message_max_recipients.
  • New testing tools: default capabilities testing gateway, getCreditsBalance to memory gateway.

It does not include usage or testing of Gateway::getCreditsBalance().

almaudoh’s picture

almaudoh commented

Where can we use the balance system in SMS framework?

The following I can think of, amongst others:

  • A more intelligent SMS routing system can use this information to shortlist gateways that are available.
  • A notification system can use this to inform the administrator that the available credits is running out (happens to me often enough)
dpi’s picture

dpi
Location Perth, Australia
commented
Issue summary: View changes

Updated OP with others from #3

dpi’s picture

dpi
Location Perth, Australia
commented
Title: Gateway plugin balance enquiries » Gateway plugin balance query

Updated PR addressing nits

dpi’s picture

dpi
Location Perth, Australia
commented
StatusFileSize
new gateway-balance-2798161-6-bf5749f.patch7.78 KB

Includes nits, merge with base

dpi’s picture

dpi
Location Perth, Australia
commented

Keep this issue around until some spinoff issues are created. I think some use cases in OP are worthy of first party support.

An internal mechanism to cache the current balance would be needed as well.

  • dpi committed f5164e6 on 8.x-1.x 
    Issue #2798161 by dpi: Gateway plugin balance query annotation and...
dpi’s picture

dpi
Location Perth, Australia
commented
Status: Needs review » Active

Committed, leaving open per #7

almaudoh’s picture

almaudoh commented

An interesting problem just came up today: how to represent currencies when specifying the credit balance. I have developed gateway plugins for a number of providers from Europe, America and Asia, and the question of representing the currency of the credit balance keeps coming up. Some are not denominated, while some are.

If we are using getCreditsBalance() for the purpose of display to the site admin, then we have to take question of currency into consideration.

dpi’s picture

dpi
Location Perth, Australia
commented

how to represent currencies

Its funny, because I brought that up earlier in the year :D

The most "correct" way would be to depend on a currency module, and store the currency or credit system code (USD, AUD, Credit, Flooz, Beanz) alongside the balance value.

Or since this issue reformed the currency to always being a float, you can just assume there is only one currency/credit system per plugin, then add appropriate currency code symbols when displaying in UI.

dpi’s picture

dpi
Location Perth, Australia
commented

Balance is difficult because it is a currency: could be an officially traded currency or credit or token system.

Need to figure out what the purpose of the balance system; and why it benefits the Drupal install to know about the balance. Whether the balance is purely informational; or we can take action on available balance.

dpi’s picture

dpi
Location Perth, Australia
commented

Open to suggestions! :)

Then again this problem may be more trouble than we care to address.

almaudoh’s picture

almaudoh commented

Its funny, because I brought that up earlier in the year :D

Sorry, I must have missed that. :)

Any of the options you presented in #11 is good enough - either a getCreditsCurrency/setCreditsCurrency pair or a credit_balance_currency annotation should be enough.

We don't need to overwork it IMHO.

dpi’s picture

dpi
Location Perth, Australia
commented

If we keep it the way it is, with a singular balance per plugin, then the currency unit doesnt really matter.

I think we would need to keep a record of the currency unit if there were multiple currencies per plugin. In which case we need to re-engineer what we have.

dpi’s picture

dpi
Location Perth, Australia
commented

Its been a while,

Wonder if abstracting currency related tasks to something like https://packagist.org/packages/commerceguys/intl makes sense...

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Active » Closed (outdated)
Related issues: +#3510482: [policy] Announcing closure of 3.x issues if they are not applicable for V4