Shindig
  1. Shindig
  2. SHINDIG-1762

Add code to handle OpenSocial Embedded Experience Container Context extension per http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5.0-beta1, 2.5.0
    • Fix Version/s: 2.5.0-beta2
    • Component/s: Java, Javascript
    • Labels:
      None

      Activity

      Hide
      jiraposter@reviews.apache.org added a comment -

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

      Review request for shindig and Ryan Baxter.

      Summary
      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:
      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model
      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:
      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java
      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

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

      Diffs


      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION
      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227
      trunk/content/sampledata/canonicaldb.json 1333227
      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227
      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227
      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227
      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227
      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227
      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227

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

      Testing
      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227 trunk/content/sampledata/canonicaldb.json 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      Henry here is my first pass.

      I think we should also add support to the sample EE container for display type of image, what do you think?

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml
      <https://reviews.apache.org/r/4976/#comment16596>

      is there any reason why you are calling the associated context id source?

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml
      <https://reviews.apache.org/r/4976/#comment16595>

      clean up white space

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js
      <https://reviews.apache.org/r/4976/#comment16598>

      Shouldn't you verify that the display type is link and not image?

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js
      <https://reviews.apache.org/r/4976/#comment16597>

      why would only targets of type gadget return a title? A URL EE could also have a display title right?

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js
      <https://reviews.apache.org/r/4976/#comment16599>

      what about the associated context fields?

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js
      <https://reviews.apache.org/r/4976/#comment16603>

      I would prefer we be consistent and use single quotes for strings

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js
      <https://reviews.apache.org/r/4976/#comment16600>

      why are we passing opt_containerContext for URL embedded experiences?

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js
      <https://reviews.apache.org/r/4976/#comment16604>

      The preferred experience now allows the service to suggest what experience, gadget or url, they want the container to show, we should probably honor that by default in this function, but provide a way for containers to override that functionality if they want.

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js
      <https://reviews.apache.org/r/4976/#comment16605>

      I think we need more definition around what this function should return. We will need to make sure we clarify this for the container spec as well.

      • Ryan

      On 2012-05-02 23:24:37, Henry Saputra wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4976/

      -----------------------------------------------------------

      (Updated 2012-05-02 23:24:37)

      Review request for shindig and Ryan Baxter.

      Summary

      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:

      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:

      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java

      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

      This addresses bug SHINDIG-1762.

      https://issues.apache.org/jira/browse/SHINDIG-1762

      Diffs

      -----

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227

      trunk/content/sampledata/canonicaldb.json 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227

      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227

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

      Testing

      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7498 ----------------------------------------------------------- Henry here is my first pass. I think we should also add support to the sample EE container for display type of image, what do you think? trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml < https://reviews.apache.org/r/4976/#comment16596 > is there any reason why you are calling the associated context id source? trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml < https://reviews.apache.org/r/4976/#comment16595 > clean up white space trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js < https://reviews.apache.org/r/4976/#comment16598 > Shouldn't you verify that the display type is link and not image? trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js < https://reviews.apache.org/r/4976/#comment16597 > why would only targets of type gadget return a title? A URL EE could also have a display title right? trunk/features/src/main/javascript/features/embeddedexperiences/constant.js < https://reviews.apache.org/r/4976/#comment16599 > what about the associated context fields? trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js < https://reviews.apache.org/r/4976/#comment16603 > I would prefer we be consistent and use single quotes for strings trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js < https://reviews.apache.org/r/4976/#comment16600 > why are we passing opt_containerContext for URL embedded experiences? trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js < https://reviews.apache.org/r/4976/#comment16604 > The preferred experience now allows the service to suggest what experience, gadget or url, they want the container to show, we should probably honor that by default in this function, but provide a way for containers to override that functionality if they want. trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js < https://reviews.apache.org/r/4976/#comment16605 > I think we need more definition around what this function should return. We will need to make sure we clarify this for the container spec as well. Ryan On 2012-05-02 23:24:37, Henry Saputra wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-02 23:24:37) Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs ----- trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227 trunk/content/sampledata/canonicaldb.json 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      I think Ryan covered a lot of my other comments.

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js
      <https://reviews.apache.org/r/4976/#comment16602>

      Can you utilize the constants you define in the EE constant.js here in this code?

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js
      <https://reviews.apache.org/r/4976/#comment16606>

      Can you use gadgets.log here instead? In fact, I see other places where console.log is used and I'd like to avoid it if at all possible.

      • Stanton

      On 2012-05-02 23:24:37, Henry Saputra wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4976/

      -----------------------------------------------------------

      (Updated 2012-05-02 23:24:37)

      Review request for shindig and Ryan Baxter.

      Summary

      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:

      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:

      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java

      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

      This addresses bug SHINDIG-1762.

      https://issues.apache.org/jira/browse/SHINDIG-1762

      Diffs

      -----

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227

      trunk/content/sampledata/canonicaldb.json 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227

      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227

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

      Testing

      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7502 ----------------------------------------------------------- I think Ryan covered a lot of my other comments. trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js < https://reviews.apache.org/r/4976/#comment16602 > Can you utilize the constants you define in the EE constant.js here in this code? trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js < https://reviews.apache.org/r/4976/#comment16606 > Can you use gadgets.log here instead? In fact, I see other places where console.log is used and I'd like to avoid it if at all possible. Stanton On 2012-05-02 23:24:37, Henry Saputra wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-02 23:24:37) Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs ----- trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227 trunk/content/sampledata/canonicaldb.json 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml, line 47

      > <https://reviews.apache.org/r/4976/diff/2/?file=106081#file106081line47>

      >

      > clean up white space

      Will do =)

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js, line 169

      > <https://reviews.apache.org/r/4976/diff/2/?file=106082#file106082line169>

      >

      > Shouldn't you verify that the display type is link and not image?

      Yeah, you are right. Will add check for link type.

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml, line 39

      > <https://reviews.apache.org/r/4976/diff/2/?file=106081#file106081line39>

      >

      > is there any reason why you are calling the associated context id source?

      Just checking if the id is passed. This is just initial gadget for testing if all params are passed correctly. I will keep adding updates to this sample gadget later on.

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js, line 177

      > <https://reviews.apache.org/r/4976/diff/2/?file=106082#file106082line177>

      >

      > why would only targets of type gadget return a title? A URL EE could also have a display title right?

      This is just helper function to get the link text to update the UI. The only intention is to just show how container code could leverage the preferredExperience data structure.

      Later I will add more code to the sample gadget to make it better.

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/embeddedexperiences/constant.js, line 71

      > <https://reviews.apache.org/r/4976/diff/2/?file=106084#file106084line71>

      >

      > what about the associated context fields?

      Will add those. Originally I thought its still pending so I didnt add them

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 53

      > <https://reviews.apache.org/r/4976/diff/2/?file=106085#file106085line53>

      >

      > I would prefer we be consistent and use single quotes for strings

      Yeah, will use ' than "

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 133

      > <https://reviews.apache.org/r/4976/diff/2/?file=106085#file106085line133>

      >

      > why are we passing opt_containerContext for URL embedded experiences?

      No we shouldnt, it was my bad copy-paste coding effort =(

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 170

      > <https://reviews.apache.org/r/4976/diff/2/?file=106085#file106085line170>

      >

      > The preferred experience now allows the service to suggest what experience, gadget or url, they want the container to show, we should probably honor that by default in this function, but provide a way for containers to override that functionality if they want.

      I am not following this one. Are you suggesting if preferredExperience is not set then Shindig should set some default values? The values for preferredExperience will be used mostly by container before calling the navigate function. Other than context I dont think the rest of preferredExperience values are useful when actually rendering the gadget.

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js, line 177

      > <https://reviews.apache.org/r/4976/diff/2/?file=106087#file106087line177>

      >

      > I think we need more definition around what this function should return. We will need to make sure we clarify this for the container spec as well.

      This is just helper function to let container to have additional context before we open the gadget. Not sure if we need to add it in the container spec for the view.

      • Henry

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

      On 2012-05-02 23:24:37, Henry Saputra wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4976/

      -----------------------------------------------------------

      (Updated 2012-05-02 23:24:37)

      Review request for shindig and Ryan Baxter.

      Summary

      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:

      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:

      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java

      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

      This addresses bug SHINDIG-1762.

      https://issues.apache.org/jira/browse/SHINDIG-1762

      Diffs

      -----

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227

      trunk/content/sampledata/canonicaldb.json 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227

      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227

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

      Testing

      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml, line 47 > < https://reviews.apache.org/r/4976/diff/2/?file=106081#file106081line47 > > > clean up white space Will do =) On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js, line 169 > < https://reviews.apache.org/r/4976/diff/2/?file=106082#file106082line169 > > > Shouldn't you verify that the display type is link and not image? Yeah, you are right. Will add check for link type. On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml, line 39 > < https://reviews.apache.org/r/4976/diff/2/?file=106081#file106081line39 > > > is there any reason why you are calling the associated context id source? Just checking if the id is passed. This is just initial gadget for testing if all params are passed correctly. I will keep adding updates to this sample gadget later on. On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js, line 177 > < https://reviews.apache.org/r/4976/diff/2/?file=106082#file106082line177 > > > why would only targets of type gadget return a title? A URL EE could also have a display title right? This is just helper function to get the link text to update the UI. The only intention is to just show how container code could leverage the preferredExperience data structure. Later I will add more code to the sample gadget to make it better. On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/embeddedexperiences/constant.js, line 71 > < https://reviews.apache.org/r/4976/diff/2/?file=106084#file106084line71 > > > what about the associated context fields? Will add those. Originally I thought its still pending so I didnt add them On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 53 > < https://reviews.apache.org/r/4976/diff/2/?file=106085#file106085line53 > > > I would prefer we be consistent and use single quotes for strings Yeah, will use ' than " On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 133 > < https://reviews.apache.org/r/4976/diff/2/?file=106085#file106085line133 > > > why are we passing opt_containerContext for URL embedded experiences? No we shouldnt, it was my bad copy-paste coding effort =( On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 170 > < https://reviews.apache.org/r/4976/diff/2/?file=106085#file106085line170 > > > The preferred experience now allows the service to suggest what experience, gadget or url, they want the container to show, we should probably honor that by default in this function, but provide a way for containers to override that functionality if they want. I am not following this one. Are you suggesting if preferredExperience is not set then Shindig should set some default values? The values for preferredExperience will be used mostly by container before calling the navigate function. Other than context I dont think the rest of preferredExperience values are useful when actually rendering the gadget. On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js, line 177 > < https://reviews.apache.org/r/4976/diff/2/?file=106087#file106087line177 > > > I think we need more definition around what this function should return. We will need to make sure we clarify this for the container spec as well. This is just helper function to let container to have additional context before we open the gadget. Not sure if we need to add it in the container spec for the view. Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7498 ----------------------------------------------------------- On 2012-05-02 23:24:37, Henry Saputra wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-02 23:24:37) Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs ----- trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227 trunk/content/sampledata/canonicaldb.json 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-03 13:47:50, Stanton Sievers wrote:

      > trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js, line 162

      > <https://reviews.apache.org/r/4976/diff/2/?file=106082#file106082line162>

      >

      > Can you utilize the constants you define in the EE constant.js here in this code?

      Will do, thanks.

      On 2012-05-03 13:47:50, Stanton Sievers wrote:

      > trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js, line 178

      > <https://reviews.apache.org/r/4976/diff/2/?file=106087#file106087line178>

      >

      > Can you use gadgets.log here instead? In fact, I see other places where console.log is used and I'd like to avoid it if at all possible.

      I will create separate patch request to change both console logging.

      • Henry

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

      On 2012-05-02 23:24:37, Henry Saputra wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4976/

      -----------------------------------------------------------

      (Updated 2012-05-02 23:24:37)

      Review request for shindig and Ryan Baxter.

      Summary

      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:

      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:

      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java

      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

      This addresses bug SHINDIG-1762.

      https://issues.apache.org/jira/browse/SHINDIG-1762

      Diffs

      -----

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227

      trunk/content/sampledata/canonicaldb.json 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227

      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227

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

      Testing

      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-03 13:47:50, Stanton Sievers wrote: > trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js, line 162 > < https://reviews.apache.org/r/4976/diff/2/?file=106082#file106082line162 > > > Can you utilize the constants you define in the EE constant.js here in this code? Will do, thanks. On 2012-05-03 13:47:50, Stanton Sievers wrote: > trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js, line 178 > < https://reviews.apache.org/r/4976/diff/2/?file=106087#file106087line178 > > > Can you use gadgets.log here instead? In fact, I see other places where console.log is used and I'd like to avoid it if at all possible. I will create separate patch request to change both console logging. Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7502 ----------------------------------------------------------- On 2012-05-02 23:24:37, Henry Saputra wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-02 23:24:37) Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs ----- trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227 trunk/content/sampledata/canonicaldb.json 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js, line 177

      > <https://reviews.apache.org/r/4976/diff/2/?file=106087#file106087line177>

      >

      > I think we need more definition around what this function should return. We will need to make sure we clarify this for the container spec as well.

      Henry Saputra wrote:

      This is just helper function to let container to have additional context before we open the gadget. Not sure if we need to add it in the container spec for the view.

      Ah I missed the comment about possible return value. Will add it.

      • Henry

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

      On 2012-05-02 23:24:37, Henry Saputra wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4976/

      -----------------------------------------------------------

      (Updated 2012-05-02 23:24:37)

      Review request for shindig and Ryan Baxter.

      Summary

      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:

      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:

      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java

      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

      This addresses bug SHINDIG-1762.

      https://issues.apache.org/jira/browse/SHINDIG-1762

      Diffs

      -----

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227

      trunk/content/sampledata/canonicaldb.json 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227

      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227

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

      Testing

      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js, line 177 > < https://reviews.apache.org/r/4976/diff/2/?file=106087#file106087line177 > > > I think we need more definition around what this function should return. We will need to make sure we clarify this for the container spec as well. Henry Saputra wrote: This is just helper function to let container to have additional context before we open the gadget. Not sure if we need to add it in the container spec for the view. Ah I missed the comment about possible return value. Will add it. Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7498 ----------------------------------------------------------- On 2012-05-02 23:24:37, Henry Saputra wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-02 23:24:37) Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs ----- trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227 trunk/content/sampledata/canonicaldb.json 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-05-03 19:13:28.283280)

      Review request for shindig and Ryan Baxter.

      Changes
      -------

      Update the patch to reflect the reviews

      Summary
      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:
      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model
      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:
      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java
      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

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

      Diffs (updated)


      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION
      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227
      trunk/content/sampledata/canonicaldb.json 1333227
      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227
      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227
      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227
      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227
      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227
      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227

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

      Testing
      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-03 19:13:28.283280) Review request for shindig and Ryan Baxter. Changes ------- Update the patch to reflect the reviews Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs (updated) trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227 trunk/content/sampledata/canonicaldb.json 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 170

      > <https://reviews.apache.org/r/4976/diff/2/?file=106085#file106085line170>

      >

      > The preferred experience now allows the service to suggest what experience, gadget or url, they want the container to show, we should probably honor that by default in this function, but provide a way for containers to override that functionality if they want.

      Henry Saputra wrote:

      I am not following this one. Are you suggesting if preferredExperience is not set then Shindig should set some default values? The values for preferredExperience will be used mostly by container before calling the navigate function. Other than context I dont think the rest of preferredExperience values are useful when actually rendering the gadget.

      Nope that is not what I am saying, sorry for not being clear

      Say we had this embedded experience

      {
      "gadget" : "http://www.example.com/embedded/gadget.xml",
      "url" : "http://www.example.com/foo/bar.html",
      "context" :

      { "title" : "Hello World", "id" : 123 }

      ,
      "previewImage" : "http://www.example.com/embedded/123.png",
      "preferredExperience" : {
      "target" :

      { "type" : "url" }

      ,
      "display" :

      { "type" : "link" }

      }

      In this example there is both a gadget and url embedded experience provided by the service but the service has told us via the preferred experience that it prefers we render the url embedded experience over the gadget. I assume this logic for which ee we are going to render in this case should go in osapi.container.ee.navigate. By default if the service provides a target in the preferred experience I would expect the common container to honor that, but I image we should provide some way for containers to override this, so if containers want to always render the gadget no matter what, they can.

      • Ryan

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

      On 2012-05-03 19:13:28, Henry Saputra wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4976/

      -----------------------------------------------------------

      (Updated 2012-05-03 19:13:28)

      Review request for shindig and Ryan Baxter.

      Summary

      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:

      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:

      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java

      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

      This addresses bug SHINDIG-1762.

      https://issues.apache.org/jira/browse/SHINDIG-1762

      Diffs

      -----

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227

      trunk/content/sampledata/canonicaldb.json 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227

      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227

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

      Testing

      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 170 > < https://reviews.apache.org/r/4976/diff/2/?file=106085#file106085line170 > > > The preferred experience now allows the service to suggest what experience, gadget or url, they want the container to show, we should probably honor that by default in this function, but provide a way for containers to override that functionality if they want. Henry Saputra wrote: I am not following this one. Are you suggesting if preferredExperience is not set then Shindig should set some default values? The values for preferredExperience will be used mostly by container before calling the navigate function. Other than context I dont think the rest of preferredExperience values are useful when actually rendering the gadget. Nope that is not what I am saying, sorry for not being clear Say we had this embedded experience { "gadget" : "http://www.example.com/embedded/gadget.xml", "url" : "http://www.example.com/foo/bar.html", "context" : { "title" : "Hello World", "id" : 123 } , "previewImage" : "http://www.example.com/embedded/123.png", "preferredExperience" : { "target" : { "type" : "url" } , "display" : { "type" : "link" } } In this example there is both a gadget and url embedded experience provided by the service but the service has told us via the preferred experience that it prefers we render the url embedded experience over the gadget. I assume this logic for which ee we are going to render in this case should go in osapi.container.ee.navigate. By default if the service provides a target in the preferred experience I would expect the common container to honor that, but I image we should provide some way for containers to override this, so if containers want to always render the gadget no matter what, they can. Ryan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7498 ----------------------------------------------------------- On 2012-05-03 19:13:28, Henry Saputra wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-03 19:13:28) Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs ----- trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227 trunk/content/sampledata/canonicaldb.json 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-05-03 23:12:29.246735)

      Review request for shindig and Ryan Baxter.

      Changes
      -------

      Updated the patch based on Ryan's suggestion to check preferred experience type for determining the path for EE navigation.

      Summary
      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:
      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model
      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:
      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java
      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

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

      Diffs (updated)


      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION
      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592
      trunk/content/sampledata/canonicaldb.json 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592
      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592
      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592
      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592
      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592

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

      Testing
      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-03 23:12:29.246735) Review request for shindig and Ryan Baxter. Changes ------- Updated the patch based on Ryan's suggestion to check preferred experience type for determining the path for EE navigation. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs (updated) trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592 trunk/content/sampledata/canonicaldb.json 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-03 13:12:56, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 170

      > <https://reviews.apache.org/r/4976/diff/2/?file=106085#file106085line170>

      >

      > The preferred experience now allows the service to suggest what experience, gadget or url, they want the container to show, we should probably honor that by default in this function, but provide a way for containers to override that functionality if they want.

      Henry Saputra wrote:

      I am not following this one. Are you suggesting if preferredExperience is not set then Shindig should set some default values? The values for preferredExperience will be used mostly by container before calling the navigate function. Other than context I dont think the rest of preferredExperience values are useful when actually rendering the gadget.

      Ryan Baxter wrote:

      Nope that is not what I am saying, sorry for not being clear

      Say we had this embedded experience

      {

      "gadget" : "http://www.example.com/embedded/gadget.xml",

      "url" : "http://www.example.com/foo/bar.html",

      "context" : { bq. "title" : "Hello World", bq. "id" : 123 bq. },

      "previewImage" : "http://www.example.com/embedded/123.png",

      "preferredExperience" : {

      "target" : { bq. "type" : "url" bq. },

      "display" : { bq. "type" : "link" bq. }

      }

      In this example there is both a gadget and url embedded experience provided by the service but the service has told us via the preferred experience that it prefers we render the url embedded experience over the gadget. I assume this logic for which ee we are going to render in this case should go in osapi.container.ee.navigate. By default if the service provides a target in the preferred experience I would expect the common container to honor that, but I image we should provide some way for containers to override this, so if containers want to always render the gadget no matter what, they can.

      Ah ok, let me add check for it.

      • Henry

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

      On 2012-05-03 19:13:28, Henry Saputra wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4976/

      -----------------------------------------------------------

      (Updated 2012-05-03 19:13:28)

      Review request for shindig and Ryan Baxter.

      Summary

      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:

      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:

      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java

      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

      This addresses bug SHINDIG-1762.

      https://issues.apache.org/jira/browse/SHINDIG-1762

      Diffs

      -----

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227

      trunk/content/sampledata/canonicaldb.json 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227

      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227

      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227

      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227

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

      Testing

      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-03 13:12:56, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 170 > < https://reviews.apache.org/r/4976/diff/2/?file=106085#file106085line170 > > > The preferred experience now allows the service to suggest what experience, gadget or url, they want the container to show, we should probably honor that by default in this function, but provide a way for containers to override that functionality if they want. Henry Saputra wrote: I am not following this one. Are you suggesting if preferredExperience is not set then Shindig should set some default values? The values for preferredExperience will be used mostly by container before calling the navigate function. Other than context I dont think the rest of preferredExperience values are useful when actually rendering the gadget. Ryan Baxter wrote: Nope that is not what I am saying, sorry for not being clear Say we had this embedded experience { "gadget" : "http://www.example.com/embedded/gadget.xml", "url" : "http://www.example.com/foo/bar.html", "context" : { bq. "title" : "Hello World", bq. "id" : 123 bq. }, "previewImage" : "http://www.example.com/embedded/123.png", "preferredExperience" : { "target" : { bq. "type" : "url" bq. }, "display" : { bq. "type" : "link" bq. } } In this example there is both a gadget and url embedded experience provided by the service but the service has told us via the preferred experience that it prefers we render the url embedded experience over the gadget. I assume this logic for which ee we are going to render in this case should go in osapi.container.ee.navigate. By default if the service provides a target in the preferred experience I would expect the common container to honor that, but I image we should provide some way for containers to override this, so if containers want to always render the gadget no matter what, they can. Ah ok, let me add check for it. Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7498 ----------------------------------------------------------- On 2012-05-03 19:13:28, Henry Saputra wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-03 19:13:28) Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs ----- trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333227 trunk/content/sampledata/canonicaldb.json 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333227 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333227 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333227 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333227 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333227 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333227 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-05-04 07:25:24.161034)

      Review request for shindig and Ryan Baxter.

      Changes
      -------

      Fix missing function parameter for the container overridable EE type and added unit test case for it.

      Summary
      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:
      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model
      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:
      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java
      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

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

      Diffs (updated)


      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION
      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592
      trunk/content/sampledata/canonicaldb.json 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592
      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592
      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592
      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592
      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592

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

      Testing
      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-04 07:25:24.161034) Review request for shindig and Ryan Baxter. Changes ------- Fix missing function parameter for the container overridable EE type and added unit test case for it. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs (updated) trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592 trunk/content/sampledata/canonicaldb.json 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-05-04 07:28:05.434413)

      Review request for shindig and Ryan Baxter.

      Changes
      -------

      Added missing semicolon in one of the line in testGadgetNavigateWithCustomEENavigation function

      Summary
      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:
      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model
      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:
      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java
      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

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

      Diffs (updated)


      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION
      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592
      trunk/content/sampledata/canonicaldb.json 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592
      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592
      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592
      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592
      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592

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

      Testing
      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-04 07:28:05.434413) Review request for shindig and Ryan Baxter. Changes ------- Added missing semicolon in one of the line in testGadgetNavigateWithCustomEENavigation function Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs (updated) trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592 trunk/content/sampledata/canonicaldb.json 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js
      <https://reviews.apache.org/r/4976/#comment16735>

      Did you mean 'or' instead of 'and'? I think it would also be good to create constants for target type and display type that the container can use.

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js
      <https://reviews.apache.org/r/4976/#comment16736>

      single quotes

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js
      <https://reviews.apache.org/r/4976/#comment16737>

      single quotes

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js
      <https://reviews.apache.org/r/4976/#comment16738>

      single quotes

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js
      <https://reviews.apache.org/r/4976/#comment16739>

      is there a reason why you separate out associatedContext? You check for the associatedContext in opt_containerContext and set the value to associatedContext then after you loops is done you just add it to openSocialContext. Why not remove that special check for the associated context and just have the "else" part take care of it for you?

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js
      <https://reviews.apache.org/r/4976/#comment16741>

      If this function is supposed to be private you should move it outside of the osapi.container.Container.addMixin. Actually the other "private" functions in here should probably be moved there as well.

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js
      <https://reviews.apache.org/r/4976/#comment16744>

      you should just return null, or you can let the function return undefined. Right now the function could return a string, null, or undefined.

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js
      <https://reviews.apache.org/r/4976/#comment16745>

      spacing is a little off here, I think you need to move this line in a bit, being picky

      • Ryan

      On 2012-05-04 07:28:05, Henry Saputra wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4976/

      -----------------------------------------------------------

      (Updated 2012-05-04 07:28:05)

      Review request for shindig and Ryan Baxter.

      Summary

      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:

      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:

      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java

      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

      This addresses bug SHINDIG-1762.

      https://issues.apache.org/jira/browse/SHINDIG-1762

      Diffs

      -----

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592

      trunk/content/sampledata/canonicaldb.json 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592

      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592

      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592

      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592

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

      Testing

      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7557 ----------------------------------------------------------- trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js < https://reviews.apache.org/r/4976/#comment16735 > Did you mean 'or' instead of 'and'? I think it would also be good to create constants for target type and display type that the container can use. trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js < https://reviews.apache.org/r/4976/#comment16736 > single quotes trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js < https://reviews.apache.org/r/4976/#comment16737 > single quotes trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js < https://reviews.apache.org/r/4976/#comment16738 > single quotes trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js < https://reviews.apache.org/r/4976/#comment16739 > is there a reason why you separate out associatedContext? You check for the associatedContext in opt_containerContext and set the value to associatedContext then after you loops is done you just add it to openSocialContext. Why not remove that special check for the associated context and just have the "else" part take care of it for you? trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js < https://reviews.apache.org/r/4976/#comment16741 > If this function is supposed to be private you should move it outside of the osapi.container.Container.addMixin. Actually the other "private" functions in here should probably be moved there as well. trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js < https://reviews.apache.org/r/4976/#comment16744 > you should just return null, or you can let the function return undefined. Right now the function could return a string, null, or undefined. trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js < https://reviews.apache.org/r/4976/#comment16745 > spacing is a little off here, I think you need to move this line in a bit, being picky Ryan On 2012-05-04 07:28:05, Henry Saputra wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-04 07:28:05) Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs ----- trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592 trunk/content/sampledata/canonicaldb.json 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-04 15:10:34, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 160

      > <https://reviews.apache.org/r/4976/diff/6/?file=106670#file106670line160>

      >

      > you should just return null, or you can let the function return undefined. Right now the function could return a string, null, or undefined.

      Good point.

      On 2012-05-04 15:10:34, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 150

      > <https://reviews.apache.org/r/4976/diff/6/?file=106670#file106670line150>

      >

      > If this function is supposed to be private you should move it outside of the osapi.container.Container.addMixin. Actually the other "private" functions in here should probably be moved there as well.

      It should be ok for the purpose of this CR. I wont change this now so if need to refactor it later it will be in one separate patch request.

      On 2012-05-04 15:10:34, Ryan Baxter wrote:

      > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 79

      > <https://reviews.apache.org/r/4976/diff/6/?file=106670#file106670line79>

      >

      > is there a reason why you separate out associatedContext? You check for the associatedContext in opt_containerContext and set the value to associatedContext then after you loops is done you just add it to openSocialContext. Why not remove that special check for the associated context and just have the "else" part take care of it for you?

      The idea is if container has extra context, the openSocial namespace should have at least associatedContext attribute per spec. I should probably set the empty associatedContext early and let the rest iteration take care of it.

      On 2012-05-04 15:10:34, Ryan Baxter wrote:

      > trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js, line 179

      > <https://reviews.apache.org/r/4976/diff/6/?file=106667#file106667line179>

      >

      > Did you mean 'or' instead of 'and'? I think it would also be good to create constants for target type and display type that the container can use.

      No it was intended to check if EE is gadget but I realize that the EEContainer.js also should be able to render URL based EE. I will just remove the check for gadget type.

      • Henry

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

      On 2012-05-04 07:28:05, Henry Saputra wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4976/

      -----------------------------------------------------------

      (Updated 2012-05-04 07:28:05)

      Review request for shindig and Ryan Baxter.

      Summary

      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:

      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:

      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java

      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

      This addresses bug SHINDIG-1762.

      https://issues.apache.org/jira/browse/SHINDIG-1762

      Diffs

      -----

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592

      trunk/content/sampledata/canonicaldb.json 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592

      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592

      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592

      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592

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

      Testing

      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-04 15:10:34, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 160 > < https://reviews.apache.org/r/4976/diff/6/?file=106670#file106670line160 > > > you should just return null, or you can let the function return undefined. Right now the function could return a string, null, or undefined. Good point. On 2012-05-04 15:10:34, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 150 > < https://reviews.apache.org/r/4976/diff/6/?file=106670#file106670line150 > > > If this function is supposed to be private you should move it outside of the osapi.container.Container.addMixin. Actually the other "private" functions in here should probably be moved there as well. It should be ok for the purpose of this CR. I wont change this now so if need to refactor it later it will be in one separate patch request. On 2012-05-04 15:10:34, Ryan Baxter wrote: > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js, line 79 > < https://reviews.apache.org/r/4976/diff/6/?file=106670#file106670line79 > > > is there a reason why you separate out associatedContext? You check for the associatedContext in opt_containerContext and set the value to associatedContext then after you loops is done you just add it to openSocialContext. Why not remove that special check for the associated context and just have the "else" part take care of it for you? The idea is if container has extra context, the openSocial namespace should have at least associatedContext attribute per spec. I should probably set the empty associatedContext early and let the rest iteration take care of it. On 2012-05-04 15:10:34, Ryan Baxter wrote: > trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js, line 179 > < https://reviews.apache.org/r/4976/diff/6/?file=106667#file106667line179 > > > Did you mean 'or' instead of 'and'? I think it would also be good to create constants for target type and display type that the container can use. No it was intended to check if EE is gadget but I realize that the EEContainer.js also should be able to render URL based EE. I will just remove the check for gadget type. Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7557 ----------------------------------------------------------- On 2012-05-04 07:28:05, Henry Saputra wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-04 07:28:05) Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs ----- trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592 trunk/content/sampledata/canonicaldb.json 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-05-04 16:29:28.042006)

      Review request for shindig and Ryan Baxter.

      Changes
      -------

      Updated based latest comments.

      Summary
      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:
      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model
      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:
      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java
      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

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

      Diffs (updated)


      trunk/content/sampledata/canonicaldb.json 1333592
      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592
      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION
      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592
      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592
      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592
      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592
      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592

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

      Testing
      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-04 16:29:28.042006) Review request for shindig and Ryan Baxter. Changes ------- Updated based latest comments. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs (updated) trunk/content/sampledata/canonicaldb.json 1333592 trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592 trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      LGTM, besides the JS doc. I applied the patch build, and tested everything out and things seem to be working.

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js
      <https://reviews.apache.org/r/4976/#comment16755>

      Add some JS doc for these

      • Ryan

      On 2012-05-04 16:29:28, Henry Saputra wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4976/

      -----------------------------------------------------------

      (Updated 2012-05-04 16:29:28)

      Review request for shindig and Ryan Baxter.

      Summary

      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:

      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model

      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:

      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java

      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

      This addresses bug SHINDIG-1762.

      https://issues.apache.org/jira/browse/SHINDIG-1762

      Diffs

      -----

      trunk/content/sampledata/canonicaldb.json 1333592

      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592

      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION

      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592

      trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592

      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592

      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592

      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592

      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592

      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592

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

      Testing

      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7571 ----------------------------------------------------------- LGTM, besides the JS doc. I applied the patch build, and tested everything out and things seem to be working. trunk/features/src/main/javascript/features/embeddedexperiences/constant.js < https://reviews.apache.org/r/4976/#comment16755 > Add some JS doc for these Ryan On 2012-05-04 16:29:28, Henry Saputra wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-04 16:29:28) Review request for shindig and Ryan Baxter. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs ----- trunk/content/sampledata/canonicaldb.json 1333592 trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592 trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-05-04 17:50:16.817783)

      Review request for shindig and Ryan Baxter.

      Changes
      -------

      Add JS doc for the EE constants params.

      Summary
      -------

      Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec:
      http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model
      http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences

      I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class:
      java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java
      java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java

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

      Diffs (updated)


      trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION
      trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592
      trunk/content/sampledata/canonicaldb.json 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592
      trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592
      trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592
      trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592
      trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592
      trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592
      trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592

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

      Testing
      -------

      Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text.

      Thanks,

      Henry

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/ ----------------------------------------------------------- (Updated 2012-05-04 17:50:16.817783) Review request for shindig and Ryan Baxter. Changes ------- Add JS doc for the EE constants params. Summary ------- Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences I remove the unneeded model for EmbeddedExperience because ActivityEntry uses ExtendableBean, which act like a Map, to store opensocial and its extension so there is no need to explicitly map embed or preferredExperience with physical class: java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java This addresses bug SHINDIG-1762 . https://issues.apache.org/jira/browse/SHINDIG-1762 Diffs (updated) trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml PRE-CREATION trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js 1333592 trunk/content/sampledata/canonicaldb.json 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/constant.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js 1333592 trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml 1333592 trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js 1333592 trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java 1333592 trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json 1333592 trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java 1333592 Diff: https://reviews.apache.org/r/4976/diff Testing ------- Updated the unit tests and modify EE sample gadget to test passing additional context and respect preferredExperience display link text. Thanks, Henry
      Hide
      Henry Saputra added a comment -

      Committed revision 1334122.

      Show
      Henry Saputra added a comment - Committed revision 1334122.
      Hide
      Dan Dumont added a comment -

      Henry, sorry for the late review, but it looks like the code you added to the open-views ee feature doesn't account for async element creation.

      The review site is down now, but it looks like in the code if the element is created async, the augmented context will never get passed to the callback.

      I don't see why it ever needs to be passed to the callback, why can't you init the context before you define the callback and use the outer scope reference instead of passing it in?

      Show
      Dan Dumont added a comment - Henry, sorry for the late review, but it looks like the code you added to the open-views ee feature doesn't account for async element creation. The review site is down now, but it looks like in the code if the element is created async, the augmented context will never get passed to the callback. I don't see why it ever needs to be passed to the callback, why can't you init the context before you define the callback and use the outer scope reference instead of passing it in?
      Hide
      Henry Saputra added a comment -

      Dan, lets create new issue to track the problem with open view. This JIRA already has too many coments its hard to keep track.

      Show
      Henry Saputra added a comment - Dan, lets create new issue to track the problem with open view. This JIRA already has too many coments its hard to keep track.

        People

        • Assignee:
          Henry Saputra
          Reporter:
          Henry Saputra
        • Votes:
          0 Vote for this issue
          Watchers:
          2 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development