Skip to content

Add Zone#states and Zone#countries associations#529

Merged
cbrunsdon merged 3 commits intomasterfrom
unknown repository
Nov 24, 2015
Merged

Add Zone#states and Zone#countries associations#529
cbrunsdon merged 3 commits intomasterfrom
unknown repository

Conversation

@mamhoff
Copy link
Copy Markdown
Contributor

@mamhoff mamhoff commented Nov 23, 2015

This PR also adds a :with_country factory to ease writing tests for taxation.

@cbrunsdon
Copy link
Copy Markdown
Contributor

👍 from me. I don't like zones or members or how we handle any of that, but I don't have a better plan than what we have now and am okay supporting those associations on Zones.

@cbrunsdon cbrunsdon mentioned this pull request Nov 24, 2015
@mamhoff
Copy link
Copy Markdown
Contributor Author

mamhoff commented Nov 24, 2015

Will do.

@mamhoff mamhoff closed this Nov 24, 2015
@mamhoff
Copy link
Copy Markdown
Contributor Author

mamhoff commented Nov 24, 2015

Uh, wrong one. sorry.

@mamhoff mamhoff reopened this Nov 24, 2015
Comment thread core/spec/models/spree/zone_spec.rb Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could this be expect(zone.countries.to_a).to eq([country])? Just to make the expectation a little stricter?

@jordan-brough
Copy link
Copy Markdown
Contributor

Nice update, thanks. 👍 regardless of my one question in the spec.

@mamhoff
Copy link
Copy Markdown
Contributor Author

mamhoff commented Nov 24, 2015

The build failure is due to some CSS not being found. I'm reasonably sure it's nothing to do with stricter expecations on zone associations :)

cbrunsdon added a commit that referenced this pull request Nov 24, 2015
Add Zone#states and Zone#countries associations
@cbrunsdon cbrunsdon merged commit 09bc177 into solidusio:master Nov 24, 2015
@mamhoff mamhoff mentioned this pull request Mar 16, 2016
23 tasks
@mamhoff mamhoff deleted the zone-associations branch May 24, 2016 19:42
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