Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix constructor to properly retrieve capabilities values #813

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ibnsultan
Copy link

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe below

Error Description

The original constructor incorrectly retrieved the mms and sms values due to case sensitivity in the array keys ('mms' and 'sms' instead of 'MMS' and 'SMS'). This resulted in default values being returned as "false" (a string) rather than false (a boolean), causing:

  • Unexpected behavior in JSON serialization and comparisons.

  • Direct retrieval of class items (e.g., getSMS, getMMS, etc.) always returned true because the methods were enforced to return :bool. Since the allocated value was a string, it was evaluated as true.

  • Also the capabilities response does not include the fax status causing to always default to false

Fixed Issues Description

Updated the PhoneNumberCapabilities constructor to correctly retrieve the values for mms, sms, voice, and fax. The case-sensitive keys for MMS and SMS were changed to uppercase, and default values are now false instead of strings. This resolves the issue where incorrect values were being assigned to the capabilities properties.

Unfixed Issue Description

The fax value will still default to false because the capabilities response, whether retrieved through the SDK or the REST API, does not include the fax status. Therefore, even though the constructor attempts to retrieve fax, it will always be set to false.

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

Related Issue

#783

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

Updated the `PhoneNumberCapabilities` constructor to correctly retrieve the values for `mms`, `sms`, `voice`, and `fax`. The case-sensitive keys for `MMS` and `SMS` were changed to uppercase, and default values are now `false` instead of strings. This resolves the issue where incorrect values were being assigned to the capabilities properties.
@ibnsultan ibnsultan changed the title Fix constructor to properly retrieve capabilities values @v2.3.0 fix: Fix constructor to properly retrieve capabilities values Sep 14, 2024
@Shakerrry
Copy link

Shakerrry commented Sep 25, 2024

I have this issue too. Have you found an workaround until this is solved?

Update: I have implemented this workaround:

$capabilities = (array) $result->capabilities;
echo $capabilities["\x00*\x00sms"];

@ibnsultan ibnsultan changed the title fix: Fix constructor to properly retrieve capabilities values [Fix] Fix constructor to properly retrieve capabilities values @v2.3.0 Sep 25, 2024
@ibnsultan
Copy link
Author

@Shakerrry Not really other than the edit and locking the file from being overwritted from the package changes

@ibnsultan ibnsultan changed the title [Fix] Fix constructor to properly retrieve capabilities values @v2.3.0 Fix: Fix constructor to properly retrieve capabilities values Sep 25, 2024
@ibnsultan ibnsultan changed the title Fix: Fix constructor to properly retrieve capabilities values fix: Fix constructor to properly retrieve capabilities values Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants