lml/ost#347: Integrate unconfirm gem.

[navilan]

• Add gem dependency.
• Add configuration file for unconfirm.
• Include JS and HTML.
• Add migrations.
• Modify free response and multiple choice submissions
  to use unconfirm.

[navilan]

@Dantemss / @jpslav : This includes the review of: https://github.com/lml/unconfirm.
Things that need attention:

1. This is more than what I anticipated to be the number of steps for integrating the gem. But, I don't see a way around it. Any suggestions to cut down those 5 steps? https://github.com/lml/unconfirm#usage
2. There are a couple of places where I had to explicitly include helpers. I read mixed information about why this doesn't work as expected. The summary being, this is not needed in later versions of rails. Is there a way to remove those explicit includes?
   https://github.com/lml/unconfirm/blob/master/app/controllers/unconfirm/application_controller.rb#L3
   https://github.com/lml/unconfirm/blob/master/app/controllers/unconfirm/user_settings_controller.rb#L3
3. I was expecting that with this:
   https://github.com/lml/unconfirm/blob/master/lib/unconfirm/engine.rb#L5
   I didn't need to worry about the JS files. But even with that an explicit //= require unconfirm is required.

TODO:

• Figure out other places where confirmation messages are used and change them to use unconfirm.
• Add a site wide settings page to view / change unconfirm settings.
• Enhance the unconfirm_tag view helper to include all options supported by jquery.unconfirm with reasonable defaults.

[Dantemss]

For step 2, you could combine these 2 methods into 1.

Actually, you could probably combine 2 and 5 into 1 method (it doesn't matter if the stuff in 2 is in the body, right? If necessary, you could even set some template variables to prevent the stuff in 2 from being included twice in the page).

If you don't mind not using the asset pipeline, you could also put the javascript in 3 inline with 2 and 5. This would, however, mean the page would load marginally slower, as the browser would not cache the javascript. But it would probably be negligible.

Personally I would probably combine 2 and 5, but keep 3 separate.

[navilan]

@Dantemss - Thanks.

Yeah, for step2 - I kept the methods separate as one inserts html and the other one JS. I could perhaps provide a third method that injects both together.

yeah - I'd really like the loading to be separate from the script tags. It does make the page a lot slower.

Do you know of a better way to deal with the helpers?

[Dantemss]

I prefer to avoid helpers entirely and handle helper methods like this:

https://github.com/openstax/action_interceptor/blob/master/lib/action_interceptor/view.rb

I only have an include Common in there, but you can add any instance method and it will work in all views.

[Dantemss]

If you need something from the controller object and/or if the method should also be available in controllers, I do it like this:

https://github.com/openstax/action_interceptor/blob/master/lib/action_interceptor/controller.rb

The only reason I needed both in action_interceptor is that, unfortunately, url_for works differently in controllers vs views (Rails "magic").

[jpslav]

Need to replace this reference to a dir on your computer with the github repo or instead publish to rubygems

[navilan]

Yeah - I was waiting to see if the gem is good enough to be published. I will change it to the github repo for now.

[navilan]

@jpslav - did you get a chance to take a look at the gem code?

[jpslav]

I did not. @Dantemss, can you take a look sometime in the coming weeks?

My inclination is to take it on faith now, subject to Kim's manual testing, and let us interact with it a bit in OST. Then when we integrate unconfirm into future products I'll definitely take a closer look.

Thanks for adding this functionality.

On Jun 27, 2014, at 8:48 PM, Lakshmi notifications@github.com wrote:

In Gemfile:

@@ -59,6 +59,8 @@ gem 'remotipart', '> 1.0'
gem 'babbler', '> 1.0.0'
gem 'sketchily', '~> 1.5.0'

+gem 'unconfirm', path: '../unconfirm'
@jpslav - did you get a chance to take a look at the gem code?

—
Reply to this email directly or view it on GitHub.