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

[RFC] Raise exception when there is no default store #1232

Closed

Conversation

jhawthorn
Copy link
Contributor

@jhawthorn jhawthorn commented Jun 8, 2016

We wanted to investigate what it would look like to raise an exception on Store.current/Store.default if there was no default store in the database.

As you can see there are a lot of spec changes needed to accomplish this, but not an obscene amount. We should think that it will be similar for users upgrading.

Some of the required changes were to avoid the "General Settings" page of the admin (which is actually a page to edit the default store). It would probably be helpful if we could make that page not error as part of #1189

@jhawthorn jhawthorn force-pushed the raise_on_no_default_store branch 2 times, most recently from 4e79375 to 4b7ce4b Compare June 8, 2016 23:07
@jhawthorn jhawthorn added the WIP label Jun 8, 2016
@jhawthorn jhawthorn force-pushed the raise_on_no_default_store branch 2 times, most recently from ba27242 to 771fe7c Compare June 16, 2016 20:13
@jhawthorn jhawthorn force-pushed the raise_on_no_default_store branch from 771fe7c to e85ed8e Compare June 29, 2016 23:04
@@ -818,7 +813,8 @@ def merge!(other_order, user = nil)

# Regression test for https://github.com/spree/spree/issues/4923
context "locking" do
let(:order) { Spree::Order.create } # need a persisted in order to test locking
let(:store) { create(:store) } # need a persisted in order to test locking
let(:order) { Spree::Order.create(store: store) } # need a persisted in order to test locking
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment is missing a word, maybe store?

@mamhoff
Copy link
Contributor

mamhoff commented Aug 5, 2016

I'm a fan. 💯

@mamhoff
Copy link
Contributor

mamhoff commented Jun 19, 2017

I'm still a fan of this. @jhawthorn whenever you have a minute would you mind rebasing this on the current master branch?

@tvdeyen
Copy link
Member

tvdeyen commented Oct 4, 2017

@jhawthorn any chance to look into this again?

@jhawthorn jhawthorn force-pushed the raise_on_no_default_store branch from e85ed8e to cfbb3a3 Compare October 6, 2017 00:09
@jhawthorn jhawthorn closed this May 17, 2018
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.

3 participants