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

Add contacts CSV export capability #238

Merged
merged 8 commits into from
Oct 1, 2023
Merged

Conversation

katafrakt
Copy link
Contributor

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
Copy link
Contributor

wmnnd commented Sep 24, 2023

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

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
Copy link
Contributor Author

@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
Copy link
Contributor

wmnnd commented Oct 1, 2023

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

@katafrakt
Copy link
Contributor Author

@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
Copy link
Contributor

wmnnd commented Oct 1, 2023

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

@wmnnd wmnnd merged commit 05a8bd1 into pentacent:main Oct 1, 2023
2 checks passed
@wmnnd
Copy link
Contributor

wmnnd commented Oct 1, 2023

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

@katafrakt katafrakt deleted the csv-export branch October 3, 2023 16:51
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.

2 participants