Fix fake callback

[sastels]

Summary | Résumé

The INTERNAL_TEST_NUMBER (which is used by the smoke test) was causing an error after we switched on Pinpoint: the fake callback was being processed by the sns callback task rather than the pinpoint callback task. This PR ensures that the correct task (ie based on the provider being used) is called.

Related Issues | Cartes liées

• https://app.zenhub.com/workspaces/notify-planning-core-6411dfb7c95fb80014e0cab0/issues/gh/cds-snc/notification-planning-core/360

Test instructions | Instructions pour tester la modification

To test locally:

Set env vars (from staging):

AWS_PINPOINT_SC_POOL_ID = "pool-b20333ce1e4e49309ba1db3bf94a3f57"
AWS_PINPOINT_DEFAULT_POOL_ID = "pool-8885654b47d6466f9aaee14c62494ff2"

Send an SMS to 16135550123. It should immediately register as sent. and there should be no celery errors.

Release Instructions | Instructions pour le déploiement

None.

Reviewer checklist | Liste de vérification du réviseur

• This PR does not break existing functionality.
• This PR does not violate GCNotify's privacy policies.
• This PR does not raise new security concerns. Refer to our GC Notify Risk Register document on our Google drive.
• This PR does not significantly alter performance.
• Additional required documentation resulting of these changes is covered (such as the README, setup instructions, a related ADR or the technical documentation).

⚠ If boxes cannot be checked off before merging the PR, they should be moved to the "Release Instructions" section with appropriate steps required to verify before release. For example, changes to celery code may require tests on staging to verify that performance has not been affected.

[jimleroyer]

It looks like only the last line is different, could we merge the logic on top into one common function, which would differ on that last function call?

[sastels]

🤔 instead of

def aws_pinpoint_callback(notification_id, to):
    now = datetime.now()
    timestamp = now.strftime("%Y-%m-%d %H:%M:%S.%f")[:-3]

    if to.strip().endswith(perm_fail):
        return pinpoint_failed_callback(
            "Phone is currently unreachable/unavailable", notification_id, destination=to, timestamp=timestamp
        )
    elif to.strip().endswith(temp_fail):
        return pinpoint_failed_callback(
            "Phone carrier is currently unreachable/unavailable", notification_id, destination=to, timestamp=timestamp
        )
    else:
        return pinpoint_delivered_callback(notification_id, destination=to, timestamp=timestamp)

What about

def aws_pinpoint_callback(notification_id, to):
    now = datetime.now()
    timestamp = now.strftime("%Y-%m-%d %H:%M:%S.%f")[:-3]
    using_test_perm_fail_number = to.strip().endswith(perm_fail)
    using_test_temp_fail_number = to.strip().endswith(temp_fail)

    if using_test_perm_fail_number or using_test_temp_fail_number:
        return pinpoint_failed_callback(
            "Phone is currently unreachable/unavailable" if using_test_perm_fail_number else "Phone carrier is currently unreachable/unavailable", 
            notification_id, 
            destination=to, 
            timestamp=timestamp
        )
    else:
        return pinpoint_delivered_callback(notification_id, destination=to, timestamp=timestamp)

[jimleroyer]

Let's ship it as is for now. I think what I was thinking of is a proper abstraction that would send the callback based on the implementation, which would simplify the call sites code, but would likely repeat in the abstractions (potentially objects and with some template patterns).

[jimleroyer]

🥳

[jimleroyer]

sir