What exactly is the current status of "carriers" in SMS Framework?

I've also looked at the D7 version, and I'm not clear on their purpose?

Filed as bug report since the admin form is broken. Convert to task if they need to be fixed or removed.

CommentFileSizeAuthor
#7 interdiff.txt401 bytesdpi
#7 carrier_remove_2652212-7.patch18.62 KBdpi
#4 carrier_remove_2652212.patch18.74 KBdpi

Comments

dpi created an issue. See original summary.

almaudoh’s picture

almaudoh commented

Left to me the whole carriers 'thing' would be removed. Their application / functionality is still not clear to me. If the functionality is still needed, it can be provided in a separate module.

dpi’s picture

dpi
Location Perth, Australia
commented
Title: Carriers configuration for D8 » Remove 'carrier' systems
Assigned: Unassigned » dpi
Category: Bug report » Task

Sounds like a vote to remove.

I think the gateway plugin system solves all third party integration needs, even if it is a little barebones for the moment.

dpi’s picture

dpi
Location Perth, Australia
commented
Assigned: dpi » Unassigned
Status: Active » Needs review
StatusFileSize
new carrier_remove_2652212.patch18.74 KB

dpi/smsframework/carrier-remove-2652212

almaudoh’s picture

almaudoh commented
Status: Needs review » Needs work

Quick review:

+++ b/sms.links.action.yml
@@ -1,9 +1,9 @@
-  route_name: sms.carrier_add
+entity.phone_number_settings.add:
+  title: 'Add phone number settings'
+  route_name: entity.phone_number_settings.add
   weight: 1
   appears_on:
-    - sms.carrier_admin
+    - sms.phone_number_settings.list

The entity.phone_number_settings.add part looks out of scope

dpi’s picture

dpi
Location Perth, Australia
commented

hmm weird. looks like my other branched leaked

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Needs work » Needs review
StatusFileSize
new carrier_remove_2652212-7.patch18.62 KB
new interdiff.txt401 bytes

Fixed the free-range fence-jumping code.

almaudoh’s picture

almaudoh commented
Status: Needs review » Fixed

Great cleanup. Also removed one last reference to carriers on commit.

index 8c90372..7c0969e 100644
--- a/modules/sms_user/src/Form/SettingsResetForm.php
+++ b/modules/sms_user/src/Form/SettingsResetForm.php
@@ -42,7 +42,6 @@ class SettingsResetForm extends ConfigFormBase {
       '#type' => 'item',
       '#title' => t('Your mobile phone number'),
       '#markup' => $account->sms_user['number'],
-  //    '#markup' => $account->sms_user['number'] . '@' . $account->sms_user['gateway']['carrier'],
       '#description' => t('Your mobile phone number has been confirmed.'),
     );

Committed / pushed to 8.x-1.x. Thanks.

  • almaudoh committed 7e06cc0 on 8.x-1.x 
    Issue #2652212 by dpi: Remove 'carrier' systems
dpi’s picture

dpi
Location Perth, Australia
commented

Awesome thanks. Just a future note, would be great to get author credit.

Drupal.org helpfully generates a Git command for you. See "Git command" below under "Credit & committing".

almaudoh’s picture

almaudoh commented

Just a future note, would be great to get author credit.

Note taken. Thanks for the contributions to SMS Framework.

Status: Fixed » Closed (fixed)

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