Importer improvements (part 3)#2592
Conversation
niklasmohrin
left a comment
niklasmohrin
left a comment
There was a problem hiding this comment.
makes sense, some remarks:
| "editable": True, | ||
| "preview_html": preview_html, | ||
| "questionnaires_with_answers_per_contributor": {}, | ||
| "show_participant_cms_info": evaluation.exam_type is not None and evaluation.cms_id is not None, |
There was a problem hiding this comment.
Can we keep this decision in the form? Maybe we could set something like self.fields["particpants"].cms_disclaimer = _(...) and pick this up in the view?
There was a problem hiding this comment.
even better would be if such an attribute would be included in our typing somehow, then we could also move the whole show-or-collapsed business to that.
not for this PR though, I am fine if we just keep the field specific things close to the field
There was a problem hiding this comment.
adding attributes to fields is tricky, i now added it to the form. does that work for you?
There was a problem hiding this comment.
Out of curiosity: What makes it tricky? I would have expected
self.fields["participants"].evap_participants_set_by_cms = True
to just work (apart from typechecking -> we can either ignore it, or add the four lines to have the custom field class with the attribute defined visibly for mypy).
I would also very much prefer to have this stored in the field, if possible just as a bool, with the translated string being added in the template.
There was a problem hiding this comment.
i tried setting it for the field directly but field.cms_disclaimer is empty
There was a problem hiding this comment.
Ok, we can merge as-is then from my side, I'd look into why it's not working or if we can find a better solution in the next days.
richardebeling
left a comment
richardebeling
left a comment
There was a problem hiding this comment.
lgtm after the field/cms_disclaimer member discussion is resolved
4 participants
see commit messages for details