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

Order data fetch #2972

Merged
merged 18 commits into from
Sep 29, 2017
Merged

Order data fetch #2972

merged 18 commits into from
Sep 29, 2017

Conversation

joykare
Copy link
Contributor

@joykare joykare commented Sep 28, 2017

Resolves #2837 #2952 #2768

TEST #2837

Expected behavior

When there's a filter applied to the orders table and select all action performed, only the filtered orders should be selected.

Actual Behavior

Even with a filter applied, select all selects all orders present in the table.

How to test

  • Have a couple of orders in different states/stages
  • Visit the orders dashboard (click on the orders icon in the sidebar)
  • Apply any kind of filter to the orders e.g. filter by status
  • Click Select All checkbox on order table header
  • Change filters observe that only the filtered orders are selected

TEST #2952

Expected behavior

Orders table should have the same number of rows as orders.
For example: If there are two orders, have just two rows displayed.

Actual Behavior

Extra empty rows appear even when there are orders.

How to test

  • Create two.more orders
  • Open order dashboard, observe that table grows/shrinks depending on the number of orders
  • If there are no results the default 3 rows show up

TEST #2768

Problem

Order Dashboard Table was using ReactTable's way of passing publication/collection as opposed to passing it the actual data

How to test

  • Look through code, see that Order Table passes its own data to the React Table implementation.

@joykare joykare changed the title [WIP] Order data fetch Order data fetch Sep 29, 2017
@joykare joykare requested review from kieckhafer and removed request for brent-hoover September 29, 2017 16:44
selectAllOrders={this.selectAllOrders}
multipleSelect={this.state.multipleSelect}
setShippingStatus={this.setShippingStatus}

Copy link
Member

Choose a reason for hiding this comment

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

Remove this space here

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.

Looks good overall, just seeing one error on a translation and then I think it's good to go.

cart_completed

@joykare
Copy link
Contributor Author

joykare commented Sep 29, 2017

@kieckhafer fixed

@kieckhafer
Copy link
Member

Looks good!

cart_completed

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.

Fixes made to translations, looks good to go.

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