Skip to content

Added search api#4

Open
tigrang wants to merge 2 commits into
prawnsalad:masterfrom
tigrang:search-api
Open

Added search api#4
tigrang wants to merge 2 commits into
prawnsalad:masterfrom
tigrang:search-api

Conversation

@tigrang
Copy link
Copy Markdown

@tigrang tigrang commented May 29, 2012

Made changes to formatting and code style as well (please don't hate me)

New features are in NexmoAccount.php

searchMessage($message_id)
searchMessages($ids)
searchMessagesByRecipient($to, $date)

Do you think the search api should be done like:

'search_messages_ids' => array('method' => 'GET', 'url' => '/search/messages/{k}/{s}?ids={ids}'),
'search_messages_recipient' => array('method' => 'GET', 'url' => '/search/messages/{k}/{s}?to={to}&date={date}'),

rather than the third paramater to apiCall I added to append GET parameters to the URL?

@prawnsalad
Copy link
Copy Markdown
Owner

Thanks for the additions :)
Could you separate out the new search methods into it's own commit? You have introduced unused methods (object_to_array) and the the style changes is huge in itself. I'd be happy to pull the style + search commits separately.

Adding the 3rd parameter to apiCall() makes sense for these and possibly future methods where they may be optional.

@tigrang
Copy link
Copy Markdown
Author

tigrang commented Jun 9, 2012

No problem. I split them into two commits.

There are still some inconsistencies within the code. Most variables use access modifiers, but some still use var.
Some concatenation is done by $one . 'some_text'; and others are $one.'some_text';
Some arrays are written as 'key' => 'value' while others are 'key'=>'value'.

If you want to set a style, I can go in and fix them.

Nexmo also recently came out with a search rejections api so that needs to be added as well. I don't know if I need that right now so I won't be implementing it myself at the moment.

I don't remember why I added the object_to_array (I don't even remember adding it), but it's not needed - I removed it.

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