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

[WIP] Fix "Order filtering by status does not always refresh properly" bug #2902 #2925

Closed
wants to merge 11 commits into from

Conversation

efalayi
Copy link
Contributor

@efalayi efalayi commented Sep 22, 2017

Resolves #2902
After selecting one status then another, sometimes orders that are valid do not show up

query contains filter information sent to the server. From previous implementation, Object.assign is used to update this.state.query in Orders react component. However, this not only updates but also merges query keys in state. Hence a wrong query is parsed on to the server.

let query = OrderHelper.makeQuery(value);
query = Object.assign({}, this.state.query, query);
Fix

To update filter query and search query, current search query is also parsed to state. We can update query with just the current search query (if it exists)

let query = OrderHelper.makeQuery(value);
   if (this.state.query._id) {
     query._id = this.state.query._id;
   }
Test
Add some orders
Set some orders to different states
Select a filter for which no orders should show up ("Captured" or "Approved")
Select a filter for which some orders should show up
Observe orders shown
Search filtered orders

Esther Falayi added 2 commits September 22, 2017 12:54
- add search query to filter query
@efalayi efalayi requested review from brent-hoover, joykare and kieckhafer and removed request for brent-hoover September 22, 2017 12:15
@aaronjudd aaronjudd requested review from spencern and removed request for spencern September 25, 2017 22:35
@spencern
Copy link
Contributor

@kieckhafer @joykare Can you guys review this please

Copy link
Member

@kieckhafer kieckhafer left a comment

Choose a reason for hiding this comment

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

Following the instructions here, I am able to do everything I should be able to.

However, I cannot click on the Shipping Status dropdown... nothing happens when I click. Is this normal? I know we were still building that out.

@efalayi
Copy link
Contributor Author

efalayi commented Sep 26, 2017

@kieckhafer, the Shipping Status dropdown wasn't included for this PR. I think @joykare is working on that.

@joykare
Copy link
Contributor

joykare commented Sep 26, 2017

@kieckhafer wasn't already in marketplace there's a PR for that: #2914

However, LGTM

@kieckhafer
Copy link
Member

kieckhafer commented Sep 26, 2017

@joykare OK, I just checked #2914 so i know it works there. In that case, this also LGTM. There are some linting errors, however they are the same as the errors that were in #2893 and various other PR's and I fixed them in my PR, and I think they are fixed in others, so I'll merge this as is to avoid conflicts on fixes that are already done.

@spencern
Copy link
Contributor

@kieckhafer is this good to go now?

@kieckhafer
Copy link
Member

kieckhafer commented Sep 27, 2017 via email

@kieckhafer kieckhafer changed the title Fix "Order filtering by status does not always refresh properly" bug #2902 [WIP] Fix "Order filtering by status does not always refresh properly" bug #2902 Sep 29, 2017
@kieha
Copy link
Contributor

kieha commented Sep 29, 2017

Fixed by #2972

@kieha kieha closed this Sep 29, 2017
@spencern spencern deleted the esther-fix-issue-2902 branch October 11, 2017 23:38
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.

5 participants