A rebel can be added to working groups when imported

extinctionrebellion / RebelsManager

MIT License

61 stars 20 forks source link

Open mhulet opened 4 years ago

mhulet commented 4 years ago

Our current import (/rebels/imports/new) doesn't allow adding to working groups. We want to have a CSV column for working groups IDs.

begault commented 4 years ago

Hey @mhulet
To begin working on imports, I'd like to take this issue as well. Do you think it's ok ?

begault commented 4 years ago

Is the following format ok ? 1/2/78/4
Do you have a better proposition for separator ? (depending on your use)

begault commented 4 years ago

Some questions about implementation:

What do you think about using representers to better encapsulate stuff like https://github.com/extinctionrebellion/RebelsManager/blob/master/app/services/rebels/import_service.rb#L43 ? (Example of gem: https://github.com/apotonick/roar-rails) I feel like you'll soon need this kind of feature to manage more complex rebels parameters manipulation. It takes an input and returns a hash (or json) to easily create an object.
What do you think about moving csv_import into a library ? This part is not really related to your business model and some code might be shared between CSV part and Excel part. Not completely sure about this one. We might also put it into a model folder called imports to prepare for Excel version.

begault commented 4 years ago

@mhulet still interested by taking this issue ;)

mhulet commented 4 years ago

Damn, I completely missed your previous comments @begault. I will come back to it once I have some time to review your suggestions.

mhulet commented 4 years ago

Thanks a lot @begault.

About the format, why not using columns? On the user side, it will be easier to write (and read) than slashes. But maybe you suggested slashes for a good reason.
About moving csv_import into a library or a model imports folders: yes, that probably makes sense. I let you decide what's best (my only concern is to keep code readable and easy to maintain).
About representers, I have never used them personally but I have been interested in the past in Trailblazer (roar-rails suggests to switch to Trailblazer). My understanding is that architecture would make the code more complex to understand for new contributors, but I may be wrong. Maybe we should start with something small (only one representer) and I'll have a better understanding of representers themselves.