Problem/Motivation

The results of sms_send() are inconsistent and not well document. In some places boolean (TRUE|FALSE) is expected, but in others an array containing more definitive values are expected.

Proposed resolution

1. Clearly define the return values from sms_send()
2. Document these in the codebase and on drupal.org
3. Write a patch to implement
4. Review patch
5. Commit

Remaining tasks

All

User interface changes

None

API changes

None

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

almaudoh’s picture

almaudoh’s picture

After #2509566: Convert sms_send() into a service, the simple solution to this should be to return SmsMessageResultInterface from SmsProviderInterface::send(), then to maintain BC, sms_send() still returns TRUE or throws an exception to match its existing behavior.

This way, old code doesn't break, while new code that needs detailed information on the message result calls the SmsProviderInterface::send() method directly (which should more often be the case) and interrogates the SmsMessageResultInterface return value.

almaudoh’s picture

So, here's a patch following #2. Let's see what breaks.

Status: Needs review » Needs work

The last submitted patch, 3: clarify_the_api_for-2542866-3.patch, failed testing.

The last submitted patch, 3: clarify_the_api_for-2542866-3.patch, failed testing.

almaudoh’s picture

Minor corrections to fix the test fail.

  • almaudoh committed 9dbf42f on 8.x-1.x
    Issue #2542866: Clarify the API for message results after sms_send()
    
almaudoh’s picture

Status: Needs review » Fixed

Easy fix. Committed and pushed to 8.x-1.x

  • almaudoh committed 5a71029 on 8.x-1.x
    Issue #2542866: Clarify the API for message results after sms_send()....
almaudoh’s picture

Updated the LogGateway to return a value that conforms with the new interface expectations.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.