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

Fix issue displaying completed orders in admin #749

Merged
merged 2 commits into from
Jan 28, 2016

Conversation

dougjohnston
Copy link
Contributor

Fixes an issue where checking the 'only show complete orders' checkbox
and then unchecking it was no longer displaying completed orders.

A similar fix was merged into Spree after the Solidus fork:
spree/spree@69a20c3

Fixes an issue where checking the 'only show complete orders' checkbox
and then unchecking it was no longer displaying completed orders.

A similar fix was merged into Spree after the Solidus fork:
spree/spree@69a20c3
@jhawthorn
Copy link
Contributor

👍 Thank you. The test case is especially appreciated 😄

This changes the behaviour when "only show complete orders" from displaying only incomplete orders to showing both complete and incomplete orders. We'll have to add this to our changelog.

@dougjohnston
Copy link
Contributor Author

It fixes behavior more than changes it.

When initially loading the order admin page, complete and incomplete orders are shown. That's expected. Clicking the 'only show complete order' checkbox removes the incomplete orders and shows only completed orders. Also expected.

If I then uncheck the box, I would expect it to show complete and incomplete orders again, but it was only showing incomplete orders. That's not expected (to me at least 😄 ) and that's the behavior this PR fixes.

Perhaps it still warrants a note in the changelog. Do you want me to add that? Or, will you add it?

@dougjohnston
Copy link
Contributor Author

I see now that the naming of my test doesn't make sense within the context I put it in. I'll fix that.

@jhawthorn
Copy link
Contributor

Do you want me to add that? Or, will you add it?

I'm happy to fill in the changelog, but help is always appreciated 😄

@cbrunsdon
Copy link
Contributor

Yea, looks good to me, thanks a lot @dougjohnston 👍

jhawthorn added a commit that referenced this pull request Jan 28, 2016
Fix issue displaying completed orders in admin
@jhawthorn jhawthorn merged commit 6b15eae into solidusio:master Jan 28, 2016
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