Skip to content

Fixed bug#17708

Closed
JaxomCS wants to merge 1 commit intohome-assistant:masterfrom
JaxomCS:patch-1
Closed

Fixed bug#17708
JaxomCS wants to merge 1 commit intohome-assistant:masterfrom
JaxomCS:patch-1

Conversation

@JaxomCS
Copy link
Copy Markdown
Contributor

@JaxomCS JaxomCS commented Oct 23, 2018

From was sending the from number in an array which cannot be handled by the ClickSend system and causes a 500 error. Changed from to "hass" as from number/name can only be up to 11 characters with no special characters or spaces.
Bug was introduced 6 months ago when support for multiple recipients was added.

Description:

Single line change removing a bug so that the From number in the sms sent is set to "hass" to correct sender array 500 error issue.

Related issue (if applicable): fixes #

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>

Example entry for configuration.yaml (if applicable):

notify:
  - platform: clicksend
    name: ClickSend
    username: xxxx
    api_key: xxxxx
    recipient: +61451111111

Checklist:

  • [✓] The code change is tested and works locally.
  • [✓] Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [✓] New dependencies have been added to the REQUIREMENTS variable (example).
  • [✓] New dependencies are only imported inside functions that use them (example).
  • [✓] New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • [✓] New files were added to .coveragerc.

If the code does not interact with devices:

  • [✓] Tests have been added to verify that the new code works.

From was sending the from number in an array which cannot be handled by the ClickSend system and causes a 500 error. Changed from to "hass" as from number/name can only be up to 11 characters with no special characters or spaces.
Bug was introduced 6 months ago when support for multiple recipients was added.
@homeassistant
Copy link
Copy Markdown
Contributor

Hi @JaxomCS,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@homeassistant homeassistant added cla-needed integration: notify core merging-to-master This PR is merging into the master branch and should probably change the branch to `dev`. small-pr PRs with less than 30 lines. labels Oct 23, 2018
@ghost ghost added the in progress label Oct 23, 2018
@JaxomCS
Copy link
Copy Markdown
Contributor Author

JaxomCS commented Oct 23, 2018

I am a developer at ClickSend and am trying to fix the code after another contributed added a bug that prevents the integration working. Is there a way to get this fixed into production as continuous-integration/travis-ci/pr is rejecting my merge and is failing for an unrelated reason.

@Danielhiversen
Copy link
Copy Markdown
Member

You should use to the dev branch

@Danielhiversen
Copy link
Copy Markdown
Member

#17713

@ghost ghost removed the in progress label Oct 23, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed core integration: notify merging-to-master This PR is merging into the master branch and should probably change the branch to `dev`. small-pr PRs with less than 30 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants