Skip to content

Update spree_adjustments.finalized to be non null#973

Merged
gmacdougall merged 1 commit into
solidusio:masterfrom
jordan-brough:disallow-adjustment-finalized-nulls
Mar 8, 2016
Merged

Update spree_adjustments.finalized to be non null#973
gmacdougall merged 1 commit into
solidusio:masterfrom
jordan-brough:disallow-adjustment-finalized-nulls

Conversation

@jordan-brough

Copy link
Copy Markdown
Contributor

When we migrated from state=open/closed to finalized=true/false in
#279 we added finalized as a nullable column. This breaks the
scopes that we added in that commit -- when finalized is null the
adjustments are neither finalized nor not_finalized.

I noticed this while trying to add some code and specs using the
not_finalized scope.

We could try to tweak the scopes but I think it'd be better to just
avoid the ternary logic altogether by disallowing nulls.

In the old code we had state_machine :state, initial: :open which
prevented this problem even though the state column itself was
nullable.

@jordan-brough jordan-brough force-pushed the disallow-adjustment-finalized-nulls branch from 25ee7b9 to 0eed00b Compare March 8, 2016 00:20
When we migrated from state=open/closed to finalized=true/false in
d33254d we added `finalized` as a nullable column.  This breaks the
scopes that we added in that commit -- when finalized is null the
adjustments are neither `finalized` nor `not_finalized`.

I noticed this while trying to add some code and specs using the
`not_finalized` scope.

We could try to tweak the scopes but I think it'd be better to just
avoid the ternary logic altogether by disallowing nulls.

In the old code we had `state_machine :state, initial: :open` which
prevented this problem even though the `state` column itself was
nullable.
@jordan-brough jordan-brough force-pushed the disallow-adjustment-finalized-nulls branch from 0eed00b to 899e386 Compare March 8, 2016 00:25
@jhawthorn

Copy link
Copy Markdown
Contributor

Thanks Jordan, that makes sense 👍

@gmacdougall

Copy link
Copy Markdown
Member

Death to tri-state booleans! 👍

gmacdougall added a commit that referenced this pull request Mar 8, 2016
…zed-nulls

Update spree_adjustments.finalized to be non null
@gmacdougall gmacdougall merged commit f239f8b into solidusio:master Mar 8, 2016
@jordan-brough jordan-brough deleted the disallow-adjustment-finalized-nulls branch March 9, 2016 19:52
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