Questionnaires: add archived state, tabs in management page#2587
Questionnaires: add archived state, tabs in management page#2587niklasmohrin merged 7 commits intoe-valuation:mainfrom
Conversation
af5b9f8 to
c69ac77
Compare
niklasmohrin
left a comment
There was a problem hiding this comment.
do you want to get feedback yet? I took only a very quick look over the changes so far
Yes, please. Hopefully, only one test is missing |
|
@hansegucker @fekoch @jooooosef @ybrnr I won't find time to review this before I am there tomorrow, and by then I expect that other things will have come up. Can some of you give a review round or two on this? Maybe if you have some time tomorrow at the meeting :) |
ybrnr
left a comment
There was a problem hiding this comment.
you should also run ./manage.py precommit
c395de3 to
0d09e3b
Compare
niklasmohrin
left a comment
There was a problem hiding this comment.
looks good, but there are some rough corners we should still fix
fekoch
left a comment
There was a problem hiding this comment.
@janno42 and me rethought about the "All" - "Visible" - "Non-Archived" filter-buttons. Can you change them to be the following:
- "All" (the default) should show all non-archived questionnaires
- "Visible": same as before
- "Archived": only archived questionaires
cee7d11 to
b34a6db
Compare
janno42
left a comment
There was a problem hiding this comment.
Please remove the text "There are no matching questionnaires" and instead keep the empty table (with header). We will address the handling of empty tables in a separate issue.
niklasmohrin
left a comment
There was a problem hiding this comment.
Everything seems to work as expected, some small comments on the code
|
Note that we still have an "There are no questionnaires yet." with no table at all if there are no questionnaires in any of the tabs; @janno42 do you want this one to also go away? |
janno42
left a comment
There was a problem hiding this comment.
Looking good - except for this small thing. Then we can merge from my side :)
Co-authored-by: Johannes Wolf <dev@janno42.de>
janno42
left a comment
There was a problem hiding this comment.
Manage all the questionnaires!
| def get_string_from_url_or_session(request: HttpRequest, parameter: str, default: str | None = None) -> str | None: | ||
| result = request.GET.get(parameter, None) | ||
| if result is None: | ||
| result = request.session.get(parameter, default) | ||
| request.session[parameter] = result | ||
| return result |
There was a problem hiding this comment.
The two helpers here are now basically the same thing, and they are named a bit weirdly.
We can do it as a follow-up issue to get this merged quickly, but I guess it would be nice to think about proper names here and how to make one of these use the other one.
fix #2545