Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the configuration for Dragonfly so you can use the before_s… #3451

Merged
merged 6 commits into from Nov 27, 2019

Conversation

@joebutler2
Copy link
Contributor

joebutler2 commented Oct 31, 2019

This fixes an issue with setting the Dragonfly configuration for before_serve. It looked like before it was a simple attr_accessor, but since before_serve is more like a DSL it was expecting to be called with a block. This PR corrects that, and I'm able to confirm it works on my project.

The good news is this also solves another use case, serving private content. If you look at this Google Groups post from a few years ago, it appears my client isn't alone in wanting this feature. https://groups.google.com/forum/#!searchin/refinery-cms/dragonfly$20before_serve%7Csort:date/refinery-cms/lRBOsddSzHM/1tXNjzaHcBUJ

@joebutler2 joebutler2 force-pushed the joebutler2:dragonfly-before-serve branch from f492b66 to 5f3d233 Nov 1, 2019
@joebutler2

This comment has been minimized.

Copy link
Contributor Author

joebutler2 commented Nov 1, 2019

The build is failing due to the upgrade to Rails 6. Some of the functionality of this gem requires us to use the relatively new credentials feature.

@anitagraham

This comment has been minimized.

Copy link
Contributor

anitagraham commented Nov 6, 2019

Must be all the rage. I just submitted a similar PR as the same problem was reported on the refinerycms google group. I'll pull my PR.

@parndt
parndt approved these changes Nov 6, 2019
@@ -32,7 +32,7 @@ def configure!(extension)

# These options require a name and block
define_url extension.dragonfly_define_url if extension.dragonfly_define_url.present?
before_serve extension.dragonfly_before_serve if extension.dragonfly_before_serve.present?
before_serve(&extension.dragonfly_before_serve) if extension.dragonfly_before_serve.present?

This comment has been minimized.

Copy link
@parndt

parndt Nov 6, 2019

Member

makes sense, otherwise it'd get immediately executed

This comment has been minimized.

Copy link
@joebutler2

joebutler2 Nov 8, 2019

Author Contributor

Since you're passing in a block, it would be a Proc object at this point. So it would only execute if you invoked the call method on it. The & is converting the Proc object back into a block since the before_serve method is expecting a block.

I'm pretty sure that is the case, let me know if I mistook something.

@parndt parndt mentioned this pull request Nov 6, 2019
@parndt

This comment has been minimized.

Copy link
Member

parndt commented Nov 6, 2019

I've fixed the build failure in #3453

@parndt parndt requested a review from anitagraham Nov 6, 2019
@joebutler2

This comment has been minimized.

Copy link
Contributor Author

joebutler2 commented Nov 8, 2019

@parndt is this good to merge in? Do you want me to do anything else?

@parndt

This comment has been minimized.

Copy link
Member

parndt commented Nov 8, 2019

@joebutler2 I would just like a review from @anitagraham 😄

@parndt parndt changed the title Fix the configuration for Dragonfly so you can use the before_serve DSL. Fix the configuration for Dragonfly so you can use the before_s… Nov 27, 2019
@parndt parndt merged commit c387993 into refinery:master Nov 27, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
codeclimate All good!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.