Sanitize filenames and handle member issues in attendance.py example script
#155
Conversation
|
Hi, and thanks for your effort. Ideally, this should have been two PRs, but I totally understand you. The filename-sanitising is fine. Even if you found it somewhere, it looks so generic that I can't see any copyright problems, even if you did not invent the exact code yourself, It could also be that somene else here have a few comments, and maybe also have other changes they might want to add to it, so I'll leave that for now. I'll let others also have a look at it, but at first glance, it looks fine to me. |
attendance.py
Outdated
| @@ -38,8 +39,11 @@ async def main(): | |||
| os.makedirs("./exports") | |||
|
|
|||
| for e in events: | |||
| filename = f"{e['startTimestamp']}-{e['heading']}.csv" | |||
There was a problem hiding this comment.
Would be nice to refactor out the filename-derivation code into a self-documenting function, e.g. sanitised_filename(event).
attendance.py
Outdated
| except KeyError: | ||
| full_name = o["id"] | ||
| else: | ||
| full_name = person["firstName"] + " " + person["lastName"] |
There was a problem hiding this comment.
Recommend refactoring out this repeated line to a function e.g. full_name(person).
attendance.py
Outdated
| @@ -64,8 +72,12 @@ async def main(): | |||
| ) | |||
| if args.a is True: | |||
| for r in e["responses"]["acceptedIds"]: | |||
| person = await s.get_person(r) | |||
| full_name = person["firstName"] + " " + person["lastName"] | |||
| try: | |||
There was a problem hiding this comment.
..and this repeated block too.
|
Hi - thanks for the PR! I have made a couple of comments on the code and have a couple more general points:
Edit: I've gone ahead and reverted the |
This reverts commit d8c67a8.
elliot-100
changed the title
attendance.py example script
|
Hey @alsp80 - I hope I didn't put you off? I'm happy to assist or resolve any of these points. Let us know... thanks again! |
|
Thanks again @alsp80, merged! I did do a couple of simple refactors. (I did notice some pre-existing issues here, e.g. 'organiser' column actually contains organisers or members if you use the Could you let me know if all is OK with the changes and raise a new Bug issue if not? Thanks. |
Hi,
I came across two issues when using your project. One is about sanitising filenames based on events (for attendance) where certain special characters allowed by Spond in event names (e.g. a forward slash) would lead to unexpected behaviour or problems. I've added some code to handle these (however this might need to be reviewed for copyright as I found it somewhere but it was mentioned that it might have been taken from Django).
The other change is to handle the situation when member information is not available which lead to a crash. I'm not sure what is wrong in the account on Spond but I've included some handling for the situation when the member based on ID cannot be found. In that case the output will simply be the member guid.
The last (or actually first) commit (adding a folder into .gitignore) can probably be ignored, however I did not manage to exclude it from the Pull Request. Seems I need to beef up my git skills.
Hope some of it is useful, thanks for sharing your project, it has helped me already and I might add additional functionality and send more pull requests if ok for you.
Thanks,
Alex