search

delivered / chargify-zendesk

Access your Chargify subscription data from within Zendesk.
MIT License
6 stars 5 forks source link

don't return an array of empty domains #5

Closed jmatthewpryor closed 7 years ago

jmatthewpryor commented 7 years ago

don't return an array of empty domains for the users organization if the domain is not set or is blank/empty string Apologies if this pull request if not 100% - first time PR

stevenmaguire commented 7 years ago

Oh no! I just pushed up a fix for this and did not see this PR before hand. Sorry! I love contributions and especially if it was your first.

I think our solutions are similar. Take a look: https://github.com/delivered/chargify-zendesk/commit/1fc4adfb193599e4b7ed03ce313027c14afd36f9

I decided to define a domains array for the entire method and furnish both organization.domains().split(' ').map methods with a closure that only appends non-empty strings to the domains array. Then I removed early returns and updated the final return evaluation to either return the unique values or null.

I like the isEmptyOrBlank method that you proposed.

I hope this hasn't shaken you from contributing again! Please do!

jmatthewpryor commented 7 years ago

No problem - happy to help out and learned stuff on the way Should I just close this PR?