Skip to content

Conversation

@moliholy
Copy link
Contributor

@moliholy moliholy commented Nov 8, 2024

This PR attempts to include CSV delimiters in the remaining cases through the whole project. It currently includes the following cases:

  • Import registrations form
  • Import invitations form
  • Import event members form
  • Import contributions form
Screenshot 2024-11-08 at 16 42 34

@moliholy
Copy link
Contributor Author

moliholy commented Nov 8, 2024

@ThiefMaster I'd need some help here.

  1. Do you have an exhaustive list of cases? I made my own, but I bet it's not accurate.
  2. There are other cases out of the event scope, like ImportMembersCSVForm. How should we handle the event-specific CSVFieldDelimiter enum? I was thinking to place it in a generic place, but again I think you'd definitely know the best place for it.

@ThiefMaster
Copy link
Member

  1. I think searching the code for accepted_file_types='.csv gives you all the locations.
  2. You mean where to have the CSVFieldDelimiter definition? I've keep it where it is now. Even the category roles stuff is currently in the events module even though it's also reused by the categories module... one of those "could be improved in the future" things. But I think adding a new indico/util/csv.py or whatever would be overkill just to put that enum there.

@ThiefMaster
Copy link
Member

Ugh I just realized that CSVFieldDelimiter isn't just event scoped but event/registration scoped. That makes it a bit uglier...

Maybe indico/util/spreadsheets.py is not a completely bad place for it?

@moliholy moliholy marked this pull request as ready for review November 9, 2024 00:01
@moliholy
Copy link
Contributor Author

moliholy commented Nov 9, 2024

@ThiefMaster thanks, that helped a lot. All yours 😄

@moliholy moliholy changed the title Add CSV separator to registration list import Add CSV delimiter to registration list import Nov 9, 2024
@moliholy moliholy changed the title Add CSV delimiter to registration list import Add CSV delimiter to the remaining places Nov 9, 2024
@ThiefMaster ThiefMaster force-pushed the csv-separator-extended branch from 09f734b to cd52033 Compare November 18, 2024 12:22
@ThiefMaster ThiefMaster added this to the v3.3 milestone Nov 18, 2024
@ThiefMaster ThiefMaster merged commit 5d82caf into indico:master Nov 18, 2024
9 checks passed
AjobK pushed a commit to AjobK/indico that referenced this pull request Dec 19, 2024
AjobK pushed a commit to AjobK/indico that referenced this pull request Jan 7, 2025
AjobK pushed a commit to AjobK/indico that referenced this pull request Jan 7, 2025
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.

3 participants