pentacent / keila

Open Source Newsletter Tool.
https://keila.io
GNU Affero General Public License v3.0
1.45k stars 81 forks source link

Add contacts CSV export capability #238

Closed katafrakt closed 1 year ago

katafrakt commented 1 year ago

This adds possibility to export contacts from a project or a segment to CSV, compatible with CSV import. It addresses #110.

I wasn't really sure where to put the buttons, so please advise for a better place.

Screenshots

screenshot-localhost_4000-2023 09 22-00_30_27

screenshot-localhost_4000-2023 09 22-01_15_06

Screenshot_20230922_011119

Notes

The code in these two controllers look very similar. I wasn't sure if it makes sense to abstract it out and especially - where. This feels like highly controller-level code, so pushing it to a context does not feel right. There could be a shared codebase space for just controllers, but I didn't want to just create it without consultation.

wmnnd commented 1 year ago

This is awesome, thank you for creating this PR, @katafrakt! :partying_face:

Here are some small changes to the code I’d suggest:

  1. In most interfaces in Keila, all action buttons are in the top row, so I suggest you put the export button on the contacts and segment pages up there as well, probably in the left-most position.
  2. Like you’ve already mentioned, the code for both controllers is pretty much the same. I think it makes sense to create a helper module for centralizing this code. My suggestion would be adding KeilaWeb.ContactsCsvExport in lib/keila_web/helpers with a function stream_csv_response/3 that takes as its first argument Plug.Conn and the arguments for Contacts.stream_project_contacts/2 as its second and third argument.
  3. I also recommend you move the chunk configuration away from being directly under the :keila key to something like config :keila, KeilaWeb.Helpers.CsvExport, chunk_size: 100
katafrakt commented 1 year ago

@wmnnd I made the changes you suggested.

screenshot-localhost_4000-2023 09 24-20_49_58

screenshot-localhost_4000-2023 09 24-20_50_27

I'm not 100% sure about the location of the download button in the segment page. It is very detached from the contacts list. And, until the segment will be saved, it will not reflected the changes done by the user. Now I'm thinking that maybe that button should be even placed here?

screenshot-localhost_4000-2023 09 24-20_52_45

Not sure though, it's your project and you have the vision for it, I'm just sharing some thoughts.

wmnnd commented 1 year ago

@katafrakt Can you give me write permissions to your branch so I can adjust the button positions? Then I’ll merge your PR :smiley:

katafrakt commented 1 year ago

@wmnnd you should already have it. At least that's how I always understood this option.

screenshot-github com-2023 10 01-20_44_57

wmnnd commented 1 year ago

Ah, you’re right. Looks like this just didn’t work with Github’s github.dev editor.

wmnnd commented 1 year ago

Thank you for your contribution, @katafrakt! I have just released a new version that includes mainly your new feature :partying_face: