GravityKit / GravityView

The best and easiest way to display Gravity Forms entries on your website.
https://www.gravitykit.com/products/gravityview/
245 stars 63 forks source link

Add Export Link widget #1997

Closed doekenorg closed 6 months ago

doekenorg commented 7 months ago

This PR addresses #1891.

It adds a CSV (or TSV) download link widget with a label and option to wrap inside a <p> tag.

Scherm­afbeelding 2024-02-26 om 16 19 01 Scherm­afbeelding 2024-02-26 om 16 19 15 Scherm­afbeelding 2024-02-26 om 16 19 23

disabled

zackkatz commented 7 months ago

@doekenorg Thanks, this is a good start!

A few things I noticed upon review:

A couple of questions:

zackkatz commented 7 months ago

This is related to #1560 as well, since we want to integrate an icon picker.

doekenorg commented 7 months ago

@zackkatz Ehlers and I noted that the GravityExport support doesn't really make sense here. I created a different issue on that repo to make a separate widget. I feel like it doesn't belong in the gravityview repo.

I'll look into the rest.

doekenorg commented 7 months ago

@doekenorg Thanks, this is a good start!

A few things I noticed upon review:

  • [ ] Add a setting: "Include all entries" or "Include visible entries". If all entries, it would strip search args. Otherwise, it would keep the search args, but add &csv=1 or &tsv=1 to them.
  • [ ] Make sure the link works when Prevent Direct Access is checked
  • [ ] When "Allow export" is disabled, show a message in the widget that exporting isn't enabled for the View and provide a JS-connected button to enables it.

A couple of questions:

  • How will this work with DataTables layout?
  • ~How do you suggest implementing the GravityExport functionality as described in the ticket "Detect if there are GravityExport feeds. For each feed: Enable/disable, Allow customizing link text"~
  • Can you see if using add_query_arg()/remove_query_arg() would be a more compatible with the items above than get_permalink()?
  • There's a View setting "Show all in file"—how should we handle this in the widget UI?

@zackkatz I think I'm a little lost here.

This issue is to create a link to the regular view csv / tsv, because people forget to add the correct link. So I'm not sure where these settings are coming from. There are no search parameters. If Prevent Direct Access is checked or if you are on an embeded view, you cannot use that link. There is also no way around this using the REST API for example, because you are not allowed to view the entries if the direct access is denied.

As for datatables, that looks like a whole different ballgame. I don't think this issue and that feature relate to each other.

I can fix the javascript you mentioned, and make sure it also looks for prevent direct access, but other than that; I'm not sure this is the correct issue.

zackkatz commented 7 months ago

@doekenorg Nice! Good start. Cool to see the nonce working.

On the View URL https://dev.test/view/training-plan/?filter_99=.co.uk&mode=any (searching field id 99), the CSV link is currently https://dev.test/wp-json/gravityview/v1/views/45/entries.csv?_nonce=63953260d4. It should be passing along the search parameters (https://dev.test/wp-json/gravityview/v1/views/45/entries.csv?_nonce=63953260d4&filter_99=.co.uk&mode=any).

With Export disabled:

Export enabled:

What do you think I'm missing?

doekenorg commented 7 months ago

@zackkatz I've fixed the issues at hand (and removed a bunch of warnings from unit tests). As for All entries are being downloaded. I kinda did that on purpose. So I can remove that, and make it follow the logic that it needs Show all in file enabled as well. I'm just not sure if that makes sense for this feature. Personally, I would assume it downloads all results. Not just the ones currently in view.

So we can either make that explicit with a notice on the widget. Or we can make it follow the same logic as /csv. I'm fine with either.

zackkatz commented 6 months ago

This should be released in concert with a DataTables change: https://github.com/GravityKit/DataTables/commit/c95170578e57920c87ec22f200e6a66a357174b1

zackkatz commented 6 months ago

@doekenorg I'm not clear on what "An entry link was added to the CSV output." means in the changelog. Can you elaborate?

doekenorg commented 6 months ago

@doekenorg I'm not clear on what "An entry link was added to the CSV output." means in the changelog. Can you elaborate?

@zackkatz oh sorry. When downloading a CSV with the REST API, there was an <a> tag around the value of the field that linked to the entry.

crbdev commented 6 months ago

Works well for me 👍