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

List exporter does not take into account what is in the filter prop #5286

Closed
Hatched-Kyle opened this issue Sep 21, 2020 · 7 comments · Fixed by #5675
Closed

List exporter does not take into account what is in the filter prop #5286

Hatched-Kyle opened this issue Sep 21, 2020 · 7 comments · Fixed by #5675

Comments

@Hatched-Kyle
Copy link

What you were expecting:
I am using the List component like the following.

<List
        perPage={15}
        pagination={orderPagination()}
        actions={<OrderListActions />}
        filter={{ store_id: storeIdFilter }}
        filters={<OrderListFilter />}
        bulkActionButtons={false}
        exporter={exporter}
        className="admin-list"
        classes={otherClasses}
        sort={{ field: "placed_on", order: "DESC" }}
        {...rest}
      >

When using the exporter functionality I expect the request to contain the store_id filter I have defined in the filter prop.

What happened instead:
The GET request does not contain the default filter store_id in the params

Environment

  • React-admin version: 3.7.2
  • Last version that did not exhibit the issue (if applicable): This did work at one point. Don't remember what version
  • React version: 16.12.0
  • Browser: All
@djhi
Copy link
Contributor

djhi commented Sep 24, 2020

Indeed. Thanks for reporting

@djhi djhi added the bug label Sep 24, 2020
@fzaninotto
Copy link
Member

The regression came with commit c7129e2, when <ExportButton> started using data from the ListContext rather than from injected props.

For the fix, the permanent filter must be merged into the filterValues in ExportButton. But since the filter isn't present in the ListContext yet, that implies modifying the useListController and ListContext.

@acrtz
Copy link

acrtz commented Sep 27, 2020

I would like to make a pull request for this issue.

I already made the changes and I am now trying to get the tests to pass correctly. When running make test on the code before my changes are applied there are already problems. Some of the e-2-e tests (integration/edit.js) are failing, and 8 of the unit tests are getting skipped (same tests fail or are skipped with and without my changes).

Should I just ignore this and make the pull request or is this something I need to address?

@fzaninotto
Copy link
Member

The failing tests is something you need to address (master is green), you can ignore the skipped tests

@acrtz
Copy link

acrtz commented Sep 28, 2020

Turns out the failing tests are due to a bug (#5116) that only affects people in a negative timezone such as GMT-700. I proposed a solution to that issue, and will finish the pull request for this issue after the other one is resolved.

@CompleteScone
Copy link

Checking in on this issue. It looks like the filter prop in useListController just needs to be passed to the useListParams() call in addition to the filterDefaultValues prop which already is.

As a note, a work around for this if you don't have a filter component for the fields included in your filter prop, you can just pass the same criteria to the filterDefaultValues prop and it then appropriately filters the data sent to the exporter function.

@jayanth-2
Copy link

I created a pr for this #5666

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants