Representation support #55
Conversation
| def representations(file_id,representation) | ||
| file_id = ensure_id(file_id) | ||
| uri = "#{FILES_URI}/#{file_id}?fields=representations" | ||
| file, response = get(uri, x_rep_hints: "#{representation}") |
There was a problem hiding this comment.
Is it necessary to interpolate the string here? I don't see anything being added to it
There was a problem hiding this comment.
To be honest it's been a while since I wrote this and don't remember exactly why I implemented this this way. With that in mind which string value are you referencing?
Also, this method doesn't return the actual representation file, it only returns Representation Info
I've seen other SDK's that return the actual file, should we do the same?
There was a problem hiding this comment.
I meant that
x_rep_hints: "#{representation}"
could just be
x_rep_hints: representation
There was a problem hiding this comment.
As for the representation content, I agree we should add it to the library. It should be a separate action where you have to specify the representation type, and we can use the identifiers Box has for these representations.
As for the method you've already implemented, I think it's still good since there's a separate endpoint to fetch a list of the representations.
If we want to fill out the feature, it seems we'll also want a method to hit the info.url in order to manually generate representations that were not automatically generated.
I'm fine breaking down this feature into multiple smaller parts, however.
There was a problem hiding this comment.
yeah... good point. I don't know why I added the additional string. I'll fix the code and also add tests.
We should probable breakdown the method to get the info.url endpoint.
|
Hey @Band-Aid , thanks for the PR. A couple of suggestions and we could get this merged! |
modified: lib/boxr/collaborations.rb modified: lib/boxr/files.rb lib/boxr/intergations.rb
|
I was re-reviewing this code in anticipation for wrapping it up and releasing, but I think this implementation isn't quite ready. First off, the relevant docs: https://developer.box.com/en/guides/representations/ TODO:
|
No description provided.