You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Dan Dumont <dd...@us.ibm.com> on 2012/08/14 15:46:28 UTC

Review Request: Container should not call showAction API for actions with an invalid type.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6606/
-----------------------------------------------------------

Review request for shindig and Henry Saputra.


Description
-------

Container should not call showAction API for actions with an invalid type.


This addresses bug SHINDIG-1854.
    https://issues.apache.org/jira/browse/SHINDIG-1854


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1372400 

Diff: https://reviews.apache.org/r/6606/diff/


Testing
-------


Thanks,

Dan Dumont


Re: Review Request: Container should not call showAction API for actions with an invalid type.

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6606/#review10302
-----------------------------------------------------------

Ship it!


Thanks for the tests!

- Ryan Baxter


On Aug. 14, 2012, 7:13 p.m., Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6606/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2012, 7:13 p.m.)
> 
> 
> Review request for shindig and Henry Saputra.
> 
> 
> Description
> -------
> 
> Container should not call showAction API for actions with an invalid type.
> 
> 
> This addresses bug SHINDIG-1854.
>     https://issues.apache.org/jira/browse/SHINDIG-1854
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1372888 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/actions/actions_test.js 1372888 
> 
> Diff: https://reviews.apache.org/r/6606/diff/
> 
> 
> Testing
> -------
> 
> Added test for showactions with invalid object.
> 
> Added mechanism to mock rpc endpoints and use the associated callbacks (we should make more broad use of this in our feature code).
> 
> 
> Thanks,
> 
> Dan Dumont
> 
>


Re: Review Request: Container should not call showAction API for actions with an invalid type.

Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6606/#review10301
-----------------------------------------------------------

Ship it!


+1

Awesome!

- Henry Saputra


On Aug. 14, 2012, 7:13 p.m., Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6606/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2012, 7:13 p.m.)
> 
> 
> Review request for shindig and Henry Saputra.
> 
> 
> Description
> -------
> 
> Container should not call showAction API for actions with an invalid type.
> 
> 
> This addresses bug SHINDIG-1854.
>     https://issues.apache.org/jira/browse/SHINDIG-1854
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1372888 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/actions/actions_test.js 1372888 
> 
> Diff: https://reviews.apache.org/r/6606/diff/
> 
> 
> Testing
> -------
> 
> Added test for showactions with invalid object.
> 
> Added mechanism to mock rpc endpoints and use the associated callbacks (we should make more broad use of this in our feature code).
> 
> 
> Thanks,
> 
> Dan Dumont
> 
>


Re: Review Request: Container should not call showAction API for actions with an invalid type.

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6606/
-----------------------------------------------------------

(Updated Aug. 14, 2012, 7:13 p.m.)


Review request for shindig and Henry Saputra.


Changes
-------

Added testing.


Description
-------

Container should not call showAction API for actions with an invalid type.


This addresses bug SHINDIG-1854.
    https://issues.apache.org/jira/browse/SHINDIG-1854


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1372888 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/actions/actions_test.js 1372888 

Diff: https://reviews.apache.org/r/6606/diff/


Testing (updated)
-------

Added test for showactions with invalid object.

Added mechanism to mock rpc endpoints and use the associated callbacks (we should make more broad use of this in our feature code).


Thanks,

Dan Dumont


Re: Review Request: Container should not call showAction API for actions with an invalid type.

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6606/
-----------------------------------------------------------

(Updated Aug. 14, 2012, 7:11 p.m.)


Review request for shindig and Henry Saputra.


Description
-------

Container should not call showAction API for actions with an invalid type.


This addresses bug SHINDIG-1854.
    https://issues.apache.org/jira/browse/SHINDIG-1854


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1372888 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/actions/actions_test.js 1372888 

Diff: https://reviews.apache.org/r/6606/diff/


Testing
-------


Thanks,

Dan Dumont


Re: Review Request: Container should not call showAction API for actions with an invalid type.

Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6606/#review10278
-----------------------------------------------------------

Ship it!


Code changes looks good. +1

Would be helpful to add test case in the actions_test.js file to verify the fix.

- Henry Saputra


On Aug. 14, 2012, 1:46 p.m., Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6606/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2012, 1:46 p.m.)
> 
> 
> Review request for shindig and Henry Saputra.
> 
> 
> Description
> -------
> 
> Container should not call showAction API for actions with an invalid type.
> 
> 
> This addresses bug SHINDIG-1854.
>     https://issues.apache.org/jira/browse/SHINDIG-1854
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1372400 
> 
> Diff: https://reviews.apache.org/r/6606/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Dumont
> 
>


Re: Review Request: Container should not call showAction API for actions with an invalid type.

Posted by Henry Saputra <hs...@apache.org>.

> On Aug. 14, 2012, 2:02 p.m., Ryan Baxter wrote:
> > LGTM.  Could we add some unit tests for this?  I know the JSUnit tests can sometime be pointless but it seems like we already have tests that call addAction in the container and then checks to see if the action is in the registry.  It looks like some of the more meaningful tests are commented out....


Hmm which tests were you referring to?


- Henry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6606/#review10271
-----------------------------------------------------------


On Aug. 14, 2012, 1:46 p.m., Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6606/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2012, 1:46 p.m.)
> 
> 
> Review request for shindig and Henry Saputra.
> 
> 
> Description
> -------
> 
> Container should not call showAction API for actions with an invalid type.
> 
> 
> This addresses bug SHINDIG-1854.
>     https://issues.apache.org/jira/browse/SHINDIG-1854
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1372400 
> 
> Diff: https://reviews.apache.org/r/6606/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Dumont
> 
>


Re: Review Request: Container should not call showAction API for actions with an invalid type.

Posted by Ryan Baxter <rb...@gmail.com>.

> On Aug. 14, 2012, 2:02 p.m., Ryan Baxter wrote:
> > LGTM.  Could we add some unit tests for this?  I know the JSUnit tests can sometime be pointless but it seems like we already have tests that call addAction in the container and then checks to see if the action is in the registry.  It looks like some of the more meaningful tests are commented out....
> 
> Henry Saputra wrote:
>     
>     Hmm which tests were you referring to?

The tests here http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/actions/actions_test.js


- Ryan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6606/#review10271
-----------------------------------------------------------


On Aug. 14, 2012, 1:46 p.m., Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6606/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2012, 1:46 p.m.)
> 
> 
> Review request for shindig and Henry Saputra.
> 
> 
> Description
> -------
> 
> Container should not call showAction API for actions with an invalid type.
> 
> 
> This addresses bug SHINDIG-1854.
>     https://issues.apache.org/jira/browse/SHINDIG-1854
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1372400 
> 
> Diff: https://reviews.apache.org/r/6606/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Dumont
> 
>


Re: Review Request: Container should not call showAction API for actions with an invalid type.

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6606/#review10271
-----------------------------------------------------------


LGTM.  Could we add some unit tests for this?  I know the JSUnit tests can sometime be pointless but it seems like we already have tests that call addAction in the container and then checks to see if the action is in the registry.  It looks like some of the more meaningful tests are commented out....

- Ryan Baxter


On Aug. 14, 2012, 1:46 p.m., Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6606/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2012, 1:46 p.m.)
> 
> 
> Review request for shindig and Henry Saputra.
> 
> 
> Description
> -------
> 
> Container should not call showAction API for actions with an invalid type.
> 
> 
> This addresses bug SHINDIG-1854.
>     https://issues.apache.org/jira/browse/SHINDIG-1854
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1372400 
> 
> Diff: https://reviews.apache.org/r/6606/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Dumont
> 
>


Re: Review Request: Container should not call showAction API for actions with an invalid type.

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6606/
-----------------------------------------------------------

(Updated Aug. 14, 2012, 1:46 p.m.)


Review request for shindig and Henry Saputra.


Description
-------

Container should not call showAction API for actions with an invalid type.


This addresses bug SHINDIG-1854.
    https://issues.apache.org/jira/browse/SHINDIG-1854


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1372400 

Diff: https://reviews.apache.org/r/6606/diff/


Testing
-------


Thanks,

Dan Dumont