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

Add preference to allow product_description to be rendered raw. #2874

Closed
wants to merge 5 commits into
base: 1-3-stable
from

Conversation

Projects
None yet
3 participants
@robertmassaioli
Contributor

robertmassaioli commented Apr 11, 2013

There was a bug in the spree editor whereby if the WYSIWYG editor that we used output something that met the regex that the current product description code was checking for that we would get some bad output. Read the issue for more details.

I thought that the correct way to solve this problem would be to add a Spree preference and then we could make use of it in the spree_editor code as you can see in this branch here which will soon be a pull request.

I would like to see this pull request merged into both master and 1-3-stable if at all possible. I made this change off 1-3-stable so I raised the PR against it.

I have also followed the "guidelines for contributing" and have written a single test case for this relatively small change.

Thankyou very much for you time in reviewing this pull request I really do appreciate it and Spree and am hoping to help by continuing to commit bug fixes and other changes that I find along the way. Please let me know if I have done it wrong and I'll fix it up.

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Apr 11, 2013

Why are you unsetting it here? I thought this would be taken care of with a reset_spree_preferences call (which I am assuming is in this file)?

Why are you unsetting it here? I thought this would be taken care of with a reset_spree_preferences call (which I am assuming is in this file)?

This comment has been minimized.

Show comment
Hide comment
@robertmassaioli

robertmassaioli Apr 11, 2013

Owner

Sweet, I did not know you had that. I am very new to Spree. I'll remove that line happily. Thanks!

Edit: Based on the fact that we have this in spec_helper.rb I think it should be cleaned out before every test:

config.before(:each) do
  DatabaseCleaner.start
  reset_spree_preferences
end

I will just remove this one line.

Owner

robertmassaioli replied Apr 11, 2013

Sweet, I did not know you had that. I am very new to Spree. I'll remove that line happily. Thanks!

Edit: Based on the fact that we have this in spec_helper.rb I think it should be cleaned out before every test:

config.before(:each) do
  DatabaseCleaner.start
  reset_spree_preferences
end

I will just remove this one line.

Thanks to reset_spree_preferences we can remove a line.
There is no need to reset preferences manually which is great.
@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Apr 12, 2013

Member

Thanks @robertmassaioli! Patch looks very straightforward. Adding it to 1-2-stable, 1-3-stable and master.

Member

radar commented Apr 12, 2013

Thanks @robertmassaioli! Patch looks very straightforward. Adding it to 1-2-stable, 1-3-stable and master.

@radar radar closed this in 30fdf08 Apr 12, 2013

@stevensouthard

This comment has been minimized.

Show comment
Hide comment
@stevensouthard

stevensouthard Apr 16, 2013

@radar I see the addition to 1-3-stable but I don't see it to 1-2-stable. Am I just overlooking it?

stevensouthard commented Apr 16, 2013

@radar I see the addition to 1-3-stable but I don't see it to 1-2-stable. Am I just overlooking it?

radar added a commit that referenced this pull request Apr 17, 2013

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Apr 17, 2013

Member

My bad. I hadn't pushed it to 1-2-stable yet. Please try again.

Member

radar commented Apr 17, 2013

My bad. I hadn't pushed it to 1-2-stable yet. Please try again.

@stevensouthard

This comment has been minimized.

Show comment
Hide comment
@stevensouthard

stevensouthard Apr 17, 2013

Oh my god. It works! Thank you both.

stevensouthard commented Apr 17, 2013

Oh my god. It works! Thank you both.

@robertmassaioli

This comment has been minimized.

Show comment
Hide comment
@robertmassaioli

robertmassaioli Apr 17, 2013

Contributor

@radar bro five It's the little things that make me happy. :)

Contributor

robertmassaioli commented Apr 17, 2013

@radar bro five It's the little things that make me happy. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment