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

Changing order.token to order.guest_token in accordance with the name change in spree 2.3 #10

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

teeg82
Copy link

@teeg82 teeg82 commented Sep 18, 2014

According to the release notes for spree 2.3, the Spree::Order method token was changed to guest_token, making this gem incompatible with spree 2.3 and up. This fix rectifies that problem.

The original error was:

Showing /Users/paul/.rvm/gems/ruby-2.1.2@spree_testing/gems/spree_backend-2.3.2/app/views/spree/admin/orders/index.html.erb where line #9 raised:

undefined method `token' for #Spree::Order:0x007fd768f78b80

Also note that this is my first pull request, so please let me know if I've missed something.

Edit: I just realized the tests are failing. My bad, fixing....

@bansalakhil
Copy link
Contributor

Hi @teeg82,

We haven't tested this extension against spree 2.3. Thanks for identifying and submitting the pull request.
Looks like a quick fix, is this the only change we need to make it compatible with spree 2.3?

@teeg82
Copy link
Author

teeg82 commented Sep 18, 2014

I had to make a couple of changes to some gem versions in the gemfile and gemspec in order to run the tests successfully. And there is a lingering issue posted here in regards to an asset path issue, though I'm not certain whether that's related to a spree version issue.

I'm presently using spree 2.3, and after implementing the fix in the permissions.rb file, I haven't been able to detect any other issues, so I'm inclined to think this may be the only required change. At least, so far that I can tell.

Edit: And apparently I'm still having difficulty with the tests (gemfile this time). Seems to work locally, but Travis doesn't like it. Please stand by...

@teeg82
Copy link
Author

teeg82 commented Sep 19, 2014

Regarding Travis, I'm not really sure why its failing. As I said, it all bundles and runs properly locallly. It seems almost like Travis is keeping its own gem versions and ignoring the version specifications I wrote in the gemfile. Having never dealt with Travis, I'm not sure whether I'm doing something obviously incorrect. Its failing on Sass, basically, which I had to add in order to bundle with spree_auth_devise, which requires sass.

If you happen to see what I'm doing wrong in regards to Travis, please let me know; I'll be very happy to fix it.

Beyond that, the tests run successfully locally. I'm presently using my spree_2.3_fix branch in my current project, and based on that I'm confident that this is the only change that needs to occur to allow for 2.3 compatibility.

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.

2 participants