[asterisk-dev] [Code Review] 3117: ARI: mailboxes resource - use external MWI with ARI

[Matt Jordan]

> On Jan. 9, 2014, 7:38 p.m., Paul Belanger wrote:
> > /branches/12/rest-api/api-docs/mailboxes.json, line 52
> > <https://reviewboard.asterisk.org/r/3117/diff/2/?file=51373#file51373line52>
> >
> >     Think it would be good to return the mailbox here, in case the user wanted to confirm the settings.
> 
> Jonathan Rose wrote:
>     The precedent set by device states would suggest that I shouldn't do that.  On the other hand, when POST is used to create a channel, the channel object is returned. Personally, I think it's a bit of a tossup. I could go either way on this... but if we do want to change PUT to return the mailbox here, we should probably do the same for device state.

Returning the object that was just changed shouldn't be necessary here.

If you told it to change the state, and ARI returned a 2xx, then it better have changed the state. If ARI returned any other response code, it better not have changed the state.

I'm not sure that returning the object here adds much value, if any.


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3117/#review10556
-----------------------------------------------------------


On Jan. 9, 2014, 5:38 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3117/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2014, 5:38 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch adds the ability to interface with res_external_mwi via AMI.  The following commands are implemented:
> 
> PUT mailboxes/mailboxName
>     modifies mailbox state and implicitly creates new mailboxes
> 
> GET mailboxes/mailboxName
>     retrieves a JSON representation of a single mailbox if it exists
> 
> GET mailboxes
>     retrieves a JSON array of all mailboxes
> 
> DELETE mailbox/mailboxName
>     deletes a mailbox
> 
> Thanks to Richard's external MWI work, this is actually a very simple system.
> 
> 
> Diffs
> -----
> 
>   /branches/12/rest-api/resources.json 405265 
>   /branches/12/rest-api/api-docs/mailboxes.json PRE-CREATION 
>   /branches/12/res/res_stasis_mailbox.exports.in PRE-CREATION 
>   /branches/12/res/res_stasis_mailbox.c PRE-CREATION 
>   /branches/12/res/res_ari_mailboxes.c PRE-CREATION 
>   /branches/12/res/ari/resource_mailboxes.c PRE-CREATION 
>   /branches/12/res/ari/resource_mailboxes.h PRE-CREATION 
>   /branches/12/res/ari/ari_model_validators.c 405265 
>   /branches/12/res/ari/ari_model_validators.h 405265 
>   /branches/12/res/ari.make 405265 
>   /branches/12/include/asterisk/stasis_app_mailbox.h PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3117/diff/
> 
> 
> Testing
> -------
> 
> Tested unloads and reloads of the res_stasis_mailboxes module with res_mwi_external loaded and unloaded
> Tested how commands respond from resource_mailboxes when res_stasis_mailboxes isn't loaded
> Tested each of the commands with numerous parameters
> Created testsuite test in https://reviewboard.asterisk.org/r/3118/ which confirms basic operation of all the new ARI functions.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140110/793c3a1d/attachment-0001.html>