Basically, this is saying that the Interfaces for Entity\SmsMessageInterface and Message\SmsMessageInterface should be the same as much as possible for ease of use. Only the internal implementation / storage should differ.

Two comments on SmsProviderInterface stand out.

  /**
   * Queue a standard SMS message for receiving.
   *
   * @todo Remove if standard message gets a direction property.
   *
   * @param \Drupal\sms\Message\SmsMessageInterface $sms_message
   *   A standard SMS message.
   */
  public function queueIn(SmsMessageInterface $sms_message);

  /**
   * Queue a standard SMS message for sending.
   *
   * @todo Remove if standard message gets a direction property.
   *
   * @param \Drupal\sms\Message\SmsMessageInterface $sms_message
   *   A standard SMS message.
   */
  public function queueOut(SmsMessageInterface $sms_message);

We should tidy up that @todo

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

almaudoh created an issue. See original summary.

dpi’s picture

This would involve promoting getDirection/setDirection up from the entity to be a common method.

I have no issue with this. What is your opinion?

almaudoh’s picture

What is your opinion?

No issues...

dpi’s picture

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

  • Removing queueIn
  • Removing queueOut
  • Allowing standard messages on queue(), not just entity.
  • Promoted set/getDirection from message entity to common method.
  • Promoted direction tests to message trait, which enables coverage on both message classes.

Status: Needs review » Needs work

The last submitted patch, 4: direction-method-promote-2776751.patch, failed testing.

The last submitted patch, 4: direction-method-promote-2776751.patch, failed testing.

The last submitted patch, 4: direction-method-promote-2776751.patch, failed testing.

The last submitted patch, 4: direction-method-promote-2776751.patch, failed testing.

dpi’s picture

Status: Needs review » Needs work

The last submitted patch, 9: direction-method-promote-2776751-9.patch, failed testing.

The last submitted patch, 9: direction-method-promote-2776751-9.patch, failed testing.

  • dpi committed 31507d8 on 8.x-1.x
    Issue #2776751 by dpi: Harmonise APIs for Entity\SmsMessageInterface and...
dpi’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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