Problem/Motivation

When the test message badge argument is omitted, NULL is converted to 0.

We should fix this and add tests for the form.

Issue fork apns_php-3600669

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

ptmkenny created an issue. See original summary.

ptmkenny’s picture

Here's the full plan from Opus 4.8:

Fix omitted-badge-becomes-zero in ApnsPhpSendTestMessageForm + add tests

 Context

 ApnsPhpSendTestMessageForm lets an admin send a test push notification from
 /admin/config/system/apns_php/test. The badge-count field is optional
 (#required => FALSE). When it is left blank, submitForm() still calls
 intval($badge_count) (line 111), which turns the empty/NULL value into 0.
 A badge of 0 is a meaningful APNs value: it clears the app icon badge.
 So omitting the field unintentionally wipes the device's badge instead of
 leaving it untouched.

 The downstream service already does the right thing: ApnsPhpMessagingApi::createPayload()
 only calls setBadge() when is_int($badge_count) is true
 (src/Service/ApnsPhpMessagingApi.php:126), so passing NULL correctly omits
 the badge. The bug is purely in the form coercing NULL/empty to 0 before it
 reaches the service. This form currently has no tests.

 Goal: an omitted badge must forward NULL (badge left unchanged); the badge
 must be changed only when a value is explicitly entered — including an explicit
 0 to clear it.

 Change 1 — Fix the form

 File: /dev/apns_php/src/Form/ApnsPhpSendTestMessageForm.php

 Replace the badge handling in submitForm() (current lines 103-104 and the
 intval($badge_count) argument on line 111). An empty Drupal number element
 can submit either NULL or ''; both mean "omitted" and must stay NULL.
 Assert::nullOrNumeric('') would throw on the empty string, so handle the
 empty case first, then assert/cast only a real value:

 $badge_count = $form_state->getValue('badge_count');
 // An omitted badge (blank field) must stay NULL so the device's application
 // badge is left unchanged. Only an explicitly entered value sets the badge —
 // including an explicit 0, which clears it. Casting NULL/'' to int would
 // silently clear the badge to 0.
 $badge = NULL;
 if ($badge_count !== NULL && $badge_count !== '') {
   Assert::numeric($badge_count);
   $badge = (int) $badge_count;
 }

 Then pass $badge (not intval($badge_count)) as the 5th argument to
 $this->messagingService->sendMessageSingleDevice(...).

 Change 2 — Add unit tests for the form

 New file: /dev/apns_php/tests/src/Unit/Form/ApnsPhpSendTestMessageFormTest.php

 Follow the existing unit-test pattern in
 tests/src/Unit/Form/ApnsPhpConfigurationFormValidateTest.php: extend
 Drupal\Tests\UnitTestCase, @group apns_php,
 @coversDefaultClass \Drupal\apns_php\Form\ApnsPhpSendTestMessageForm.

 Test strategy — drive submitForm() directly and capture the badge argument
 forwarded to the messaging service:

 - Mock ApnsPhpMessagingApiInterface; stub sendMessageSingleDevice() with a
 willReturnCallback that captures argument index 4 (the badge) and returns [].
 - Build the form: new ApnsPhpSendTestMessageForm($service), then
 setStringTranslation($this->getStringTranslationStub()) and
 setMessenger($this->createMock(MessengerInterface::class)) (the happy path
 calls $this->messenger()->addStatus(); MessengerTrait::setMessenger()
 exists on FormBase).
 - Mock FormStateInterface; getValue() via willReturnCallback returning
 values from a per-test array keyed by field name (device_token, title,
 message, badge_count, silent).
 - Call $form->submitForm($form_array, $form_state) and assert the captured
 badge with assertSame (distinguishes NULL, 0, and 5 — do not use
 with()/equalTo, since 0 == null and would not catch a regression).

 Test cases:
 1. testOmittedBadgeIsForwardedAsNull — badge_count value NULL → captured badge is NULL.
 2. testEmptyStringBadgeIsForwardedAsNull — badge_count value '' → captured badge is NULL.
 3. testExplicitZeroBadgeIsForwardedAsZero — badge_count value '0' → captured badge is 0 (the regression guard: explicit 0
 still clears).
 4. testPositiveBadgeIsForwarded — badge_count value '5' → captured badge is 5.

 (Use string values '0'/'5' since a Drupal number element submits strings;
 this also exercises the (int) cast.)

 Change 3 — Cover the service's badge-omission guard

 File: /dev/apns_php/tests/src/Unit/Service/ApnsPhpMessagingApiTest.php

 The is_int($badge_count) guard in createPayload()
 (src/Service/ApnsPhpMessagingApi.php:126) — the reason a forwarded NULL
 leaves the badge untouched — is currently uncovered. The existing
 testBadgeCountIsSet only proves the positive case. Add a complementary test
 using the same captureNotifications() helper:

 - testNullBadgeCountLeavesBadgeUnset — call
 sendMessageSingleDevice('tok', 'Title', 'Body') (badge defaults to NULL)
 and assert $captured[0]->getPayload()->getBadge() is NULL
 (Pushok\Payload::getBadge() returns null when setBadge() was never
 called). @covers ::sendMessageSingleDevice.

 This pins the service contract the form fix relies on: NULL => no badge,
 int (incl. 0) => badge set.

ptmkenny’s picture

Title: Test message badges » Add tests for badge setting; fix bug where NULL clears badge on test form

  • ptmkenny committed c081e5a2 on 1.0.x
    task: #3600669 Add tests for badge setting; fix bug where NULL clears...
ptmkenny’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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