Dialog to set start and end records for export #2626

Closed
wants to merge 6 commits into from

5 participants

@ball-hayden

In response to #2449, I've created a modal dialog for setting start and end records when exporting using download links.

The PR also modifies apply_pagination so that it respects export[:start] and export[:end] parameters.

screenshot from 2013-10-27 22 24 16

Related: #1924

@seanlinsley
Active Admin member

I've added a DSL that wraps jQuery UI modals over in #1815. Maybe this could use that once it's merged.

@ball-hayden

Definitely - that looks great.

Are you happy with the changes to lib/active_admin/resource_controller/data_access.rb to accept start and end parameters?

@abhimediratta
@ball-hayden

Set start to 1 and leave end blank.

@abhimediratta
@seanlinsley
Active Admin member

@abhimediratta if you have a better idea then please discuss it over in #2449, or even open your own PR if you know exactly what you want.

@abhimediratta
@seanlinsley
Active Admin member

Yeah I saw that, but "open source" doesn't mean "ask for features from random people on the internet and just expect them to do it". Right now no such feature exists, and we're working to change that. But if you're going to complain about the work done by @ball-hayden then you should suggest an alternative and/or implement it.

@abhimediratta
@ball-hayden

Perhaps an idea would be to have a config option to either:

  • Export current records (as it is at the moment)
  • Export all records
  • Show a dialog

Do you think it would be worth having a clear "all" option in the dialog (radio button all / select records)?

@macfanatic

@ball-hayden I like the thought of this, but I would like to see an inheritable setting that we could specify for the app, per namespace, or per resource with the options you mentioned above.

I definitely don't want the default behavior to be a modal popup for my projects either :)

@seanlinsley
Active Admin member

While I'm not in love with the thought of a modal popup either, I think in any way promising to the user or developer that Active Admin can handle exporting all records is the wrong way to go. There either needs to be a hard limit like we had before, or some UI to help the user export the data in batches.

@ball-hayden

Slightly messily, I've rebased onto #1815 to use @seanlinsley's dialog DSL.

I'm going to have a look at adding configuration options.

@coveralls

Coverage Status

Coverage decreased (-0.22%) when pulling 704158d on ball-hayden:2449-better-exports into 37048c7 on gregbell:master.

@ball-hayden

I'm going to close this, as #3038 seems to be a much better solution.

@ball-hayden ball-hayden closed this Jul 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment