Metadata cascade policy by carycheng · Pull Request #603 · box/box-java-sdk

carycheng · 2018-05-08T17:42:44Z

No description provided.

boxcla · 2018-05-08T17:42:47Z

Hi @carycheng, thanks for the pull request. Before we can merge it, we need you to sign our Contributor License Agreement. You can do so electronically here: http://opensource.box.com/cla

Once you have signed, just add a comment to this pull request saying, "CLA signed". Thanks!

mattwiller · 2018-08-07T18:46:56Z

+BoxMetadataCascadePolicy.Info metadataCascadePolicyInfo = BoxMetadataCascadePolicy.create(api, 'folder-id', 'metadata-scope', 'example-template-key');
+```
+
+[create-policy]: 


Remember to add the actual link URLs here!

mattwiller · 2018-08-07T18:49:01Z

+with the api connection, scope, template key of the metadata template to be cascaded, and the ID of the folder to apply the policy to.
+
+```java
+BoxMetadataCascadePolicy.Info metadataCascadePolicyInfo = BoxMetadataCascadePolicy.create(api, 'folder-id', 'metadata-scope', 'example-template-key');


Can we use more realistic values for the parameter values in these examples? One way might be to do something like:

String folderID = "22222"; String scope = "enterprise"; String templateKey = "myTemplate"; BoxMetadataCascadePolicy.Info metadataCascadePolicyInfo = BoxMetadataCascadePolicy.create(api, folderID, scope, templateKey);

Also, remember that strings in Java are double-quoted — the example you have here wouldn't compile.

mattwiller · 2018-08-07T20:18:21Z

+You can also set the `owner_enterprise_id` option to retrieve only cascade policies owned by a specific enterprise(defaults to the current enterprise).
+
+```java
+BoxMetadataCascadePolicy.getAll(api, 'folder-id', 'owner-enterprise-id', 100);


What's the last parameter here? The full signature of this method should be documented here somewhere.

mattwiller · 2018-08-07T20:18:53Z

+will overwrite values on items in the folder with the metadata value from the folder.
+
+```java
+BoxMetadataCascadePolicy.forceApply(api, 'none', 'cascade-policy-id');


Remember that "none" needs to be double-quoted here

mattwiller · 2018-08-07T20:20:43Z


    /**
-     * Deletes this collaboration.
+     * Deletes this collanboration.


Typo: "collanboration"

mattwiller · 2018-08-07T20:24:50Z

+/**
+ * Represents a Metadata Cascade Policy.
+ */
+@BoxResourceType("")


Does this need to have a value filled in?

mattwiller · 2018-08-07T20:38:38Z

+     * @param conflictResolution the desired behavior for conflict-resolution. Set to either none or overwrite.
+     * @param cascadePolicyID    the ID of the metadata cascade policy.
+     */
+    public static void forceApply(final BoxAPIConnection api, String conflictResolution,


This should probably be an instance method, not static, since it uses the ID of a specific cascade policy. That way it wouldn't need to take in the ID as a parameter

Remember to make this non-static!

mattwiller · 2018-08-07T20:42:03Z

+    public static void forceApply(final BoxAPIConnection api, String conflictResolution,
+                                  String cascadePolicyID) {
+        QueryStringBuilder builder = new QueryStringBuilder();
+        builder.appendParam("id", cascadePolicyID);


The ID is a path param, not a query parameter

mattwiller · 2018-08-08T19:09:40Z

+```java
+String folderID = "2222";
+String enterpriseID = "1234";
+BoxMetadataCascadePolicy.getAll(api, folderID, enterpriseID, 100);


You should make this more like a real-world use case by assigning this to some variable to show what type comes out of this method

mattwiller · 2018-08-08T19:13:54Z

+     * @param conflictResolution the desired behavior for conflict-resolution. Set to either none or overwrite.
+     * @param cascadePolicyID    the ID of the metadata cascade policy.
+     */
+    public static void forceApply(final BoxAPIConnection api, String conflictResolution,


Remember to make this non-static!

mattwiller · 2018-08-08T19:15:09Z

+/**
+ * Represents a Metadata Cascade Policy.
+ */
+public class BoxMetadataCascadePolicy extends BoxResource {


Since we're waiting to ship until the API has a type field, you can probably just put the resource decorator back here and use the expected value of "metadata_cascade_policy"

…Apply() to nonstatic

coveralls · 2018-08-14T23:51:53Z

Pull Request Test Coverage Report for Build 1725

82 of 93 (88.17%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.4%) to 58.554%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/main/java/com/box/sdk/BoxMetadataCascadePolicy.java	75	86	87.21%

Totals
Change from base Build 1713:	0.4%
Covered Lines:	4576
Relevant Lines:	7815

💛 - Coveralls

mattwiller · 2018-08-17T00:28:08Z

+BoxMetadataCascadePolicy.Info metadataCascadePolicyInfo = BoxMetadataCascadePolicy.create(api, folderID, scope, templateKey);
+```
+
+[create-policy]: http://opensource.box.com/box-java-sdk/javadoc/com/box/sdk/BoxMetadataCascadePolicy.html#create-com.box.sdk.BoxAPIConnection-java.lang.String-java.lang.String-java.lang.String


These URLs should always have a trailing -

mattwiller · 2018-08-17T00:28:53Z

+String folderID = "22222";
+String scope = "enterprise";
+String templateKey = "myTemplate";
+BoxMetadataCascadePolicy.Info metadataCascadePolicyInfo = BoxMetadataCascadePolicy.create(api, folderID, scope, templateKey);


Can we update this example to use the new BoxFolder method?

mattwiller · 2018-08-17T00:29:35Z

+
+[create-policy]: http://opensource.box.com/box-java-sdk/javadoc/com/box/sdk/BoxMetadataCascadePolicy.html#create-com.box.sdk.BoxAPIConnection-java.lang.String-java.lang.String-java.lang.String
+
+Get a Cascade Policies Information


Should use Policy's here

mattwiller · 2018-08-17T00:31:35Z

+```
+
+You can also call [`getAll(BoxAPIConnection api, String folderID, String enterpriseID, int limit)`][get-all-with-limit] 
+and set the `enterpriseID` option to retrieve only cascade policies owned by a specific enterprise(defaults to the current enterprise).


We might want to clarify that the method call above only gets cascade policies from the current enterprise; this overload allows getting those from another enterprise

mattwiller · 2018-08-17T00:32:15Z

+```java
+String folderID = "2222";
+String enterpriseID = "1234";
+Iterator<BoxMetadataCascadePolicy.Info> metadataCascadePolicies = BoxMetadataCascadePolicy.getAll(api, folderID, enterpriseID, 100);


Can you double-check the return type here? I think it might be Iterable instead of Iterator

mattwiller · 2018-08-17T00:33:17Z

+     * @param templateKey   the key of the template.
+     * @return  information about the Metadata Cascade Policy.
+     */
+    public BoxMetadataCascadePolicy.Info createCascadePolicyOnFolder(String scope, String templateKey) {


The OnFolder part of this method name is redundant — this is a method on BoxFolder, after all

mattwiller · 2018-08-17T00:34:43Z

+     * @param fields            optional fields to retrieve for cascade policies.
+     * @return the Iterable of Box Metadata Cascade Policies in your enterprise.
+     */
+    public static Iterable<BoxMetadataCascadePolicy.Info> getAll(final BoxAPIConnection api,


This static method should also be aliased as an instance method on BoxFolder

mattwiller · 2018-08-17T00:36:26Z

+
+    @Test
+    @Category(UnitTest.class)
+    public void testCreateMetadataCascadePolicySucceedsSendsCorrectJson() throws IOException {


We should also add a test for the BoxFolder instance method version of this call

mattwiller · 2018-08-17T00:37:02Z

+
+    @Test
+    @Category(UnitTest.class)
+    public void testGetAllMetadataCascadePoliciesSucceeds() throws IOException {


After you add an instance method version of this call on BoxFolder, add a test for that method as well (should mostly be able to copy/paste this test)

mattwiller · 2018-08-17T00:37:48Z

+                        .withHeader("Content-Type", "application/json")
+                        .withBody(result)));
+
+        Iterator<BoxMetadataCascadePolicy.Info> metadataCascadePolicies =


This doesn't exercise the optional params — those should also have a test to make sure they get passed correctly

mattwiller · 2018-08-17T23:06:40Z

 To set a metadata policy, which applies metadata values on a folder to new items in the folder, call 
-[`create(BoxAPIConnection api, String folderID, String scope, String templateKey)`][create-policy]
-with the api connection, scope, template key of the metadata template to be cascaded, and the ID of the folder to apply the policy to.
+[`createCascadePolicy(String scope, String template)`][create-cascade-policy].


We might want to document what object this method is called on

mattwiller · 2018-08-17T23:07:04Z

+String templateKey = "template";
+String folderId = "12345";
+BoxFolder folder = new BoxFolder(api, folderId);
+BoxMetadataCascadePolicy.Info cascadePolicyInfo = folder.createCascadePolicyOnFolder(scope, template);


The method name here should match the one documented above

mattwiller · 2018-08-17T23:08:09Z

 String folderID = "2222";
-BoxMetadataCascadePolicy.getAll(api, folderID);
+BoxFolder folder = new BoxFolder(api, folderID);
+Iterator<BoxMetadataCascadePolicy.Info> metadataCascadePolicies = folder.getCascadePolicies().iterator();


Most users aren't going to manually call .iterator() here — it might be more illustrative to show how to use this with a for-in loop

mattwiller · 2018-08-17T23:08:36Z


 You can also call [`getAll(BoxAPIConnection api, String folderID, String enterpriseID, int limit)`][get-all-with-limit] 
-and set the `enterpriseID` option to retrieve only cascade policies owned by a specific enterprise(defaults to the current enterprise).
+and set the `enterpriseID` option set to retrieve metadata cascade policies from another enterprise.


"set the enterpriseID option set" should be reworded

mattwiller · 2018-08-17T23:09:00Z

 String folderID = "2222";
 String enterpriseID = "1234";
-Iterator<BoxMetadataCascadePolicy.Info> metadataCascadePolicies = BoxMetadataCascadePolicy.getAll(api, folderID, enterpriseID, 100);
+Iterator<BoxMetadataCascadePolicy.Info> metadataCascadePolicies = BoxMetadataCascadePolicy.getAll(api, folderID, enterpriseID, 100).iterator();


This example should probably also use a for-in loop to capture the common use case

mattwiller · 2018-08-17T23:09:50Z


-If you already have a Box folder object, you can add a Metadata Cascade Policy by using,
-[`createCascadePolicyOnFolder()`][create-cascade-policy-on-folder].
+You can also set a metadata policy on a folder by calling 


I don't think we really need to document both of these

mattwiller · 2018-08-17T23:10:55Z


+    @Test
+    @Category(UnitTest.class)
+    public void testAddMetadataCascadePolicySucceedsSendsCorrectJson() throws IOException {


This test should probably live in the BoxFolderTest.java file

mattwiller · 2018-08-17T23:11:12Z


+    @Test
+    @Category(UnitTest.class)
+    public void testGetAllMetadataCascadePoliciesOnFolderSucceeds() throws IOException {


This test should also probably live in BoxFolderTest.java

mattwiller · 2018-08-23T00:06:28Z

+     * @param fields            optional fields to retrieve for cascade policies.
+     * @return  the Iterable of Box Metadata Cascade Policies in your enterprise.
+     */
+    public Iterable<BoxMetadataCascadePolicy.Info> getCascadePolicies(String... fields) {


We should have an overload of this that allows passing in the owner enterprise ID

mattwiller · 2018-08-23T00:14:39Z

+    public void testForceApplyMetadataCascadePolicySucceedsAndSendsCorrectJson() {
+        final String conflictResolution = "none";
+        final String cascadePolicyID = "12345";
+        final String forceApplyURL = "/metadata_cascade_policies/" + cascadePolicyID;


This URL needs a /apply at the end

mattwiller · 2018-08-23T21:08:50Z

+        BoxMetadataCascadePolicy.Info policyInfo = BoxMetadataCascadePolicy
+                .create(this.getAPI(), this.getID(), scope, templateKey);
+
+        return policyInfo;


You can condense this onto one line with the above if you want

mattwiller · 2018-08-23T21:17:08Z

+     * @param templateKey   the key of the template.
+     * @return  information about the Metadata Cascade Policy.
+     */
+    public BoxMetadataCascadePolicy.Info createCascadePolicy(String scope, String templateKey) {


We should probably add "Metadata" to the name here, e.g. addMetadataCascadePolicy()

Major changes have been addressed

…Folder

carycheng added 5 commits May 7, 2018 10:52

updates to metadata cascade policy

6c25dc5

first implementation of metadata cascade policy

27ac7af

updated tests for metadata cascade policy

2baa8fa

adding docs for metadata cascade policy

f82e32c

added docs for cascade policies

f0b3fe8

carycheng requested a review from mattwiller May 8, 2018 17:42

carycheng removed the request for review from mattwiller May 8, 2018 17:43

carycheng added 2 commits May 8, 2018 10:45

completed metadata cascade policies

8a8fa8a

fixed lint errors

9475cf9

mattwiller suggested changes Aug 7, 2018

View reviewed changes

fixed from feedback

c59df1b

mattwiller previously requested changes Aug 8, 2018

View reviewed changes

carycheng added 3 commits August 14, 2018 12:33

Merge branch 'master' into metadata_cascade_policy

3405786

added return type for getAll() for cascade policies and changed force…

29d0473

…Apply() to nonstatic

added resource type for BoxMetadataCascadePolicy

63f1920

added convenience function on on folder for metadata cascade policy

05ed085

mattwiller reviewed Aug 17, 2018

View reviewed changes

fixed from feedback

2e5c833

mattwiller reviewed Aug 17, 2018

View reviewed changes

fixed docs from feedback and moved tests

eee67d0

mattwiller reviewed Aug 23, 2018

View reviewed changes

fixed from feedback

7d8d42d

mattwiller reviewed Aug 23, 2018

View reviewed changes

carycheng added 3 commits August 23, 2018 14:34

fixed from feedback

ab7fb99

fixed docs naming for create cascade policy

caf64f8

renaming metadata casacade policies in folder to be more clear

eedbb8e

fixed error in test with wrong function name

3c90d83

mattwiller approved these changes Aug 23, 2018

View reviewed changes

updated example to use the getMetadataCascadePolicies function in Box…

868628d

…Folder

carycheng merged commit 8c72d75 into master Aug 23, 2018

carycheng deleted the metadata_cascade_policy branch August 23, 2018 22:17


		[create-policy]: http://opensource.box.com/box-java-sdk/javadoc/com/box/sdk/BoxMetadataCascadePolicy.html#create-com.box.sdk.BoxAPIConnection-java.lang.String-java.lang.String-java.lang.String

		Get a Cascade Policies Information

Uh oh!

Conversation

carycheng commented May 8, 2018

Uh oh!

boxcla commented May 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Aug 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 1725

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

coveralls commented Aug 14, 2018 •

edited

Loading