Initial passwords can now be exported in Xlsx

liqd / aula

An online platform for political participation in schools in Germany (not in active development)

https://liqd.net

Other

27 stars 5 forks source link

Initial passwords can now be exported in Xlsx #1028

Closed np closed 7 years ago

np commented 7 years ago

Fixes #1005.

Things to consider:

The end points names for csv and xlsx are not ideal (the csv endpoint changed)
I propose to change the behavior of the export of students which no longer have an initial password. Currently they are not in the list at all and I propose to include them with an empty (or constant) value in the password column. The rational is to avoid getting an empty file when all students have their hashed password.

fisx commented 7 years ago

I propose to change the behavior of the export of students which no longer have an initial password.

good idea. i'll get a decision on that.

fisx commented 7 years ago

[x] users with hashed passwords are already listed, let's keep it like that. we should just say "<Passwort geschützt>" instead of "" in the password column.
[x] filename should not be "xlsx". Ideally it would be "Passwoerter .xlsx".
[x] email column should be removed (it makes the entire table too wide when printing).
[x] sheet name isn't set when i open downloaded document in libreoffice.

I'll also get a decision on whether we want to keep the csv button. It's nice we still have it, but it may be more distracting than helpful for the target audience.

fisx commented 7 years ago

Otherwise LGTM, thanks! (-: I am happy about the xlsx-templater pull request.

np commented 7 years ago

Your points in the same order:

Users with a hashed password are not currently listed in master (https://github.com/liqd/aula/blob/master/src/Frontend/Page/Admin.hs#L1142), however they are in my commit since this easier to test since I have most users with hashed passwords. I'll add the "<Passwort geschützt>".
About the filename, I try with a Content-Disposition: inline; filename="Passwoerter.xlsx", if I can't make it work, I'll simply change the end point name.
We can remove the email column simply by removing it from the template file, I'll do it.
I've only tested with libreoffice and the sheet name shows up at the usual location of the UI.

fisx commented 7 years ago

cool.

weird about the missing sheet name. please ignore this point for now and i'll investigate if i have the PR back.

np commented 7 years ago

The filename suggestion does not work for me. Additionally using curl, I don't see the headers being added. I think one of these two is happening:

For some reason these response headers are not added
They are added but only show up under some circumstances (query headers?) AND chrome fails to interpret the header

Does this issue rings a bell? I can investigate more.

fisx commented 7 years ago

Does this issue rings a bell?

very faintly. i could look into this, but i think i like your idea of just renaming the end-point best at this point

np commented 7 years ago

I've fixed the remaining issues. Total time tracking on this issue 13 hours.

fisx commented 7 years ago

I'm not getting headers in the xlsx file that show the class name; neither in the sheet name in libreoffice nor in the PDF export. I wonder what's different from what you're doing? I will let you know if we need to fix this, perhaps it's good enough as it is.

fisx commented 7 years ago

Thanks!