Skip to content

Conversation

rupurt
Copy link

@rupurt rupurt commented Sep 15, 2015

Howdy,

Thanks for the great work on the gem. It's proved very useful, however we're running into a couple of encapsulation limitations when trying to use policies.

Our use cases for making the above public are:

  • Policy: We have a javascript heavy app that cannot use the filepicker_field helper. We use the Filepicker JS api but don't want to re-implement the work of the Policy class for asset security. We would like to render the policy and signature into the initial html of the page using the Policy class and read those values in JS.
  • FilepickerImageUrl: We have a background job that sends the uploaded file to BoxView for document & presentation HTML5 conversion. We would prefer to use the url generation class directly instead of mixing in Rails helpers.

@maurogeorge
Copy link
Contributor

Hi @rupurt thanks for your PR.

About the Policy we have discussed this before #112 as a public API, so we need to add the RDocs to this and add a simple use case on the README showing why and how to use this API and I think we can do the same for the FilepickerImageUrl.

What you think about it?

@rupurt
Copy link
Author

rupurt commented Sep 17, 2015

@maurogeorge sounds good. How do I update the RDocs?

@maurogeorge
Copy link
Contributor

Hi @rupurt take a look at this example https://github.com/Ink/filepicker-rails/blob/master/app/helpers/filepicker_rails/form_helper.rb#L6-L43 you need to update in the code and later this is released on the Rubydoc.

@rupurt
Copy link
Author

rupurt commented Sep 17, 2015

Great, thanks.

@rupurt
Copy link
Author

rupurt commented Sep 17, 2015

@maurogeorge how does that look?

I wasn't sure how to document multiple arguments so I looked at how the Capybara project does it, they use @param & @options. Let me know if you have other ideas about how that should be done.

@rupurt rupurt force-pushed the public-policy-image-url-generation branch 2 times, most recently from e566243 to a5d8236 Compare September 17, 2015 15:55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please format the HTML? Like

<html>
  <head>
    <meta name="fp-policy" content='<%= FilepickerRails::Policy.apply.to_json.html_safe %>' />
  </head>
  <body>
  </body>
</html>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@maurogeorge
Copy link
Contributor

@rupurt thanks a lot for your PR ❤️
I added some comments.

@rupurt
Copy link
Author

rupurt commented Oct 12, 2015

@maurogeorge sorry I haven't had a chance to add the fixes. I'm aiming to block off some time this week.

@maurogeorge
Copy link
Contributor

@rupurt no problem 😄

@attack attack force-pushed the public-policy-image-url-generation branch from a5d8236 to 11a97d4 Compare August 17, 2016 15:29
@coveralls
Copy link

coveralls commented Aug 17, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 11a97d4 on SchoolKeep:public-policy-image-url-generation into dffdf5e on Ink:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 11a97d4 on SchoolKeep:public-policy-image-url-generation into dffdf5e on Ink:master.

Copy link

@Oshuma Oshuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any word on when this will be merged into master and an updated gem will be released?

@acolchagoff
Copy link

acolchagoff commented Jun 1, 2017

Why didn't this get merged?

Edit I've been maintaining a fork of this gem ever since you guys took the Policy API private, it would be nice to not have to do that anymore.

@staturecrane
Copy link
Contributor

As we are reworking this gem, this PR is unfortunately no longer compatible. However, I will be including this feature in the new release. I am closing this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants